Fix NPE or RTE handling for sockets in dump
- Fix to prevent crash when socket is null
- Modified to handle exception for the socket
- Added thread-safe guide to dump
- Add gating permission
Bug: 335688353
Test: atest FrameworksIkeTests & atest CtsIkeTestCases
Test: tested with VoWiFi call and gen bugreport
Change-Id: I5f53b59d9d65de04c76f28bd860cdb6d132c6369
diff --git a/src/java/android/net/ipsec/ike/IkeSession.java b/src/java/android/net/ipsec/ike/IkeSession.java
index ce6b6e8..14facd4 100644
--- a/src/java/android/net/ipsec/ike/IkeSession.java
+++ b/src/java/android/net/ipsec/ike/IkeSession.java
@@ -470,10 +470,16 @@
*/
@FlaggedApi("com.android.ipsec.flags.dumpsys_api")
public void dump(@NonNull PrintWriter pw) {
+ // TODO(b/336409878): Add @RequiresPermission annotation.
+ mContext.enforceCallingOrSelfPermission(
+ android.Manifest.permission.DUMP, mContext.getAttributionTag());
+
+ // Please make sure that the dump is thread-safe
+ // so the client won't get a crash or exception when adding codes to the dump.
pw.println();
pw.println("IkeSession:");
pw.println("------------------------------");
- // Dump ike state machine
+ // Dump ike state machine.
if (mIkeSessionStateMachine != null) {
pw.println();
mIkeSessionStateMachine.dump(pw);
diff --git a/src/java/android/net/ipsec/ike/IkeSessionParams.java b/src/java/android/net/ipsec/ike/IkeSessionParams.java
index c723ed2..433cb23 100644
--- a/src/java/android/net/ipsec/ike/IkeSessionParams.java
+++ b/src/java/android/net/ipsec/ike/IkeSessionParams.java
@@ -2368,9 +2368,13 @@
* Dumps the state of {@link IkeSessionParams}
*
* @param pw {@link PrintWriter} to write the state of the object.
+ * @param prefix prefix for indentation
* @hide
*/
public void dump(PrintWriter pw, String prefix) {
+ // Please make sure that the dump is thread-safe
+ // so the client won't get a crash or exception when adding codes to the dump.
+
pw.println("------------------------------");
pw.println("IkeSessionParams:");
pw.println(prefix + "Caller configured network: " + mCallerConfiguredNetwork);
diff --git a/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java b/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java
index adc8f49..c08b421 100644
--- a/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java
+++ b/src/java/com/android/internal/net/ipsec/ike/IkeSessionStateMachine.java
@@ -6015,16 +6015,18 @@
*/
public void dump(PrintWriter pw) {
super.dump(new FileDescriptor(), pw, new String[0]);
+ // Please make sure that the dump is thread-safe
+ // so the client won't get a crash or exception when adding codes to the dump.
// TODO(b/310058405): To use IndentingPrintWriter Utility Class for Indentation purpose
String prefix = " ";
- // dump ike session params data
+ // Dump ike session params data.
if (mIkeSessionParams != null) {
mIkeSessionParams.dump(pw, prefix);
}
- // dump ike connection controller data
+ // Dump ike connection controller data.
if (mIkeConnectionCtrl != null) {
mIkeConnectionCtrl.dump(pw, prefix);
}
diff --git a/src/java/com/android/internal/net/ipsec/ike/net/IkeConnectionController.java b/src/java/com/android/internal/net/ipsec/ike/net/IkeConnectionController.java
index 78140ce..9f6ebd2 100644
--- a/src/java/com/android/internal/net/ipsec/ike/net/IkeConnectionController.java
+++ b/src/java/com/android/internal/net/ipsec/ike/net/IkeConnectionController.java
@@ -1275,8 +1275,12 @@
* Dumps the state of {@link IkeConnectionController}
*
* @param pw {@link PrintWriter} to write the state of the object.
+ * @param prefix prefix for indentation
*/
public void dump(PrintWriter pw, String prefix) {
+ // Please make sure that the dump is thread-safe
+ // so the client won't get a crash or exception when adding codes to the dump.
+
pw.println("------------------------------");
pw.println("IkeConnectionController:");
pw.println(prefix + "Network: " + mNetwork);
@@ -1284,8 +1288,7 @@
pw.println(prefix + "Local address: " + mLocalAddress);
pw.println(prefix + "Remote(Server) address: " + mRemoteAddress);
pw.println(prefix + "Mobility status: " + mMobilityEnabled);
- pw.println(prefix + "Local port: " + getLocalPort());
- pw.println(prefix + "Remote(server) port: " + getRemotePort());
+ printPortInfo(pw, prefix);
pw.println(
prefix + "Esp ip version: " + IkeSessionParams.IP_VERSION_TO_STR.get(mIpVersion));
pw.println(
@@ -1294,6 +1297,30 @@
pw.println();
}
+ /**
+ * Port information may sometimes cause exceptions such as NPE or RTE, Dumps ports including the
+ * exception.
+ *
+ * @param pw {@link PrintWriter} to write the state of the object.
+ * @param prefix prefix for indentation
+ */
+ private void printPortInfo(PrintWriter pw, String prefix) {
+ // Make it thread-safe. Since this method may be accessed simultaneously from
+ // multiple threads, The socket is assigned locally and then printed.
+ IkeSocket socket = mIkeSocket;
+ if (socket == null) {
+ pw.println(prefix + "Local port: null socket");
+ pw.println(prefix + "Remote(server) port: null socket");
+ } else {
+ try {
+ pw.println(prefix + "Local port: " + socket.getLocalPort());
+ } catch (ErrnoException e) {
+ pw.println(prefix + "Local port: failed to get port");
+ }
+ pw.println(prefix + "Remote(server) port: " + socket.getIkeServerPort());
+ }
+ }
+
@Override
public void onUnderlyingNetworkUpdated(
Network network,
diff --git a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/net/IkeConnectionControllerTest.java b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/net/IkeConnectionControllerTest.java
index a60e081..e221f64 100644
--- a/tests/iketests/src/java/com/android/internal/net/ipsec/ike/net/IkeConnectionControllerTest.java
+++ b/tests/iketests/src/java/com/android/internal/net/ipsec/ike/net/IkeConnectionControllerTest.java
@@ -38,6 +38,7 @@
import static com.android.internal.net.ipsec.test.ike.net.IkeConnectionController.NAT_TRAVERSAL_UNSUPPORTED;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;
@@ -96,6 +97,8 @@
import org.mockito.ArgumentCaptor;
import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
import java.net.Inet4Address;
import java.net.Inet6Address;
import java.net.InetAddress;
@@ -1634,4 +1637,19 @@
public void testOnUnderlyingNetworkUpdatedWithNat64V6Address() throws Exception {
verifyDnsLookupWithCachedIpv6Address(true /* isNat64 */, true /* dnsExpected */);
}
+
+ @Test
+ @IgnoreUpTo(VERSION_CODES.UPSIDE_DOWN_CAKE)
+ public void testDump() throws Exception {
+ mIkeConnectionCtrl.tearDown();
+ // Clear the network callback registration call in #setUp()
+ resetMockConnectManager();
+
+ setupRemoteAddressForNetwork(mMockDefaultNetwork, new InetAddress[0]);
+ mIkeConnectionCtrl = buildIkeConnectionCtrl();
+
+ final StringWriter stringWriter = new StringWriter();
+ mIkeConnectionCtrl.dump(new PrintWriter(stringWriter), "");
+ assertFalse(stringWriter.toString().isEmpty());
+ }
}
