commit 6bab21216444918fe6cdb0fea23bfa45dc468ddc
author Patrick Rohr <prohr@google.com> Wed Aug 17 21:21:07 2022 -0700
committer Patrick Rohr <prohr@google.com> Wed Aug 17 21:22:59 2022 -0700
tree 5ce9515de0b9710c5ca7dfb2f76d2fdc386e244d
parent 91f7d6d9062a85ef14d700df585d6652153fe3d2

TestNetworkService: do not set IFF_MULTICAST when bringUp is false

Setting IFF_MULTICAST in TestNetworkService when the interface is being
brought up elsewhere creates a race condition that might clear the
IFF_UP flag (if TestNetworkService loses the race).

Test: atest EthernetManagerTest --iterations 10
Bug: 242343156
Change-Id: I86e1c8afdb82beed5202f027d906bcce8f963c82

service/jni/com_android_server_TestNetworkService.cpp

diff --git a/service/jni/com_android_server_TestNetworkService.cpp b/service/jni/com_android_server_TestNetworkService.cpp
index a1d0310..bd74d54 100644
--- a/service/jni/com_android_server_TestNetworkService.cpp
+++ b/service/jni/com_android_server_TestNetworkService.cpp
@@ -59,7 +59,8 @@
     }
 }
 
-static int createTunTapImpl(JNIEnv* env, bool isTun, bool hasCarrier, const char* iface) {
+static int createTunTapImpl(JNIEnv* env, bool isTun, bool hasCarrier, bool setIffMulticast,
+                            const char* iface) {
     base::unique_fd tun(open("/dev/tun", O_RDWR | O_NONBLOCK));
     ifreq ifr{};
 
@@ -76,8 +77,8 @@
         setTunTapCarrierEnabledImpl(env, iface, tun.get(), hasCarrier);
     }
 
-    // Mark TAP interfaces as supporting multicast
-    if (!isTun) {
+    // Mark some TAP interfaces as supporting multicast
+    if (setIffMulticast && !isTun) {
         base::unique_fd inet6CtrlSock(socket(AF_INET6, SOCK_DGRAM, 0));
         ifr.ifr_flags = IFF_MULTICAST;
 
@@ -122,14 +123,14 @@
 }
 
 static jint createTunTap(JNIEnv* env, jclass /* clazz */, jboolean isTun,
-                             jboolean hasCarrier, jstring jIface) {
+                             jboolean hasCarrier, jboolean setIffMulticast, jstring jIface) {
     ScopedUtfChars iface(env, jIface);
     if (!iface.c_str()) {
         jniThrowNullPointerException(env, "iface");
         return -1;
     }
 
-    return createTunTapImpl(env, isTun, hasCarrier, iface.c_str());
+    return createTunTapImpl(env, isTun, hasCarrier, setIffMulticast, iface.c_str());
 }
 
 static void bringUpInterface(JNIEnv* env, jclass /* clazz */, jstring jIface) {
@@ -145,7 +146,7 @@
 
 static const JNINativeMethod gMethods[] = {
     {"nativeSetTunTapCarrierEnabled", "(Ljava/lang/String;IZ)V", (void*)setTunTapCarrierEnabled},
-    {"nativeCreateTunTap", "(ZZLjava/lang/String;)I", (void*)createTunTap},
+    {"nativeCreateTunTap", "(ZZZLjava/lang/String;)I", (void*)createTunTap},
     {"nativeBringUpInterface", "(Ljava/lang/String;)V", (void*)bringUpInterface},
 };
 

service/src/com/android/server/TestNetworkService.java

diff --git a/service/src/com/android/server/TestNetworkService.java b/service/src/com/android/server/TestNetworkService.java
index 15d9f13..5549fbe 100644
--- a/service/src/com/android/server/TestNetworkService.java
+++ b/service/src/com/android/server/TestNetworkService.java
@@ -77,7 +77,7 @@
 
     // Native method stubs
     private static native int nativeCreateTunTap(boolean isTun, boolean hasCarrier,
-            @NonNull String iface);
+            boolean setIffMulticast, @NonNull String iface);
 
     private static native void nativeSetTunTapCarrierEnabled(@NonNull String iface, int tunFd,
             boolean enabled);
@@ -136,8 +136,14 @@
 
         final long token = Binder.clearCallingIdentity();
         try {
+            // Note: if the interface is brought up by ethernet, setting IFF_MULTICAST
+            // races NetUtils#setInterfaceUp(). This flag is not necessary for ethernet
+            // tests, so let's not set it when bringUp is false. See also b/242343156.
+            // In the future, we could use RTM_SETLINK with ifi_change set to set the
+            // flags atomically.
+            final boolean setIffMulticast = bringUp;
             ParcelFileDescriptor tunIntf = ParcelFileDescriptor.adoptFd(
-                    nativeCreateTunTap(isTun, hasCarrier, interfaceName));
+                    nativeCreateTunTap(isTun, hasCarrier, setIffMulticast, interfaceName));
 
             // Disable DAD and remove router_solicitation_delay before assigning link addresses.
             if (disableIpv6ProvisioningDelay) {