Fix race when starting NetworkMonitor

NetworkMonitor obtained LinkProperties and NetworkCapabilities via
synchronous calls to ConnectivityManager after receiving an asynchronous
notification, which is prone to races: the network could be gone before
the LinkProperties/NetworkCapabilities can be fetched.

Fix the race by passing LinkProperties/NetworkCapabilities directly to
NetworkMonitor in the asynchronous notifications.

Test: atest FrameworksNetTests NetworkStackTests
Test: booted, WiFi works
Bug: 129375892
Change-Id: I200ac7ca6ff79590b11c9be705f650c92fd3cb63

services/core/java/com/android/server/ConnectivityService.java

@@ -5382,7 +5382,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 mContext, mTrackerHandler, new NetworkMisc(networkMisc), this, mNetd, mNMS,
                 factorySerialNumber);
         // Make sure the network capabilities reflect what the agent info says.
-        nai.networkCapabilities = mixInCapabilities(nai, nc);
+        nai.setNetworkCapabilities(mixInCapabilities(nai, nc));
         final String extraInfo = networkInfo.getExtraInfo();
         final String name = TextUtils.isEmpty(extraInfo)
                 ? nai.networkCapabilities.getSSID() : extraInfo;
@@ -5475,12 +5475,12 @@ public class ConnectivityService extends IConnectivityManager.Stub
             // Start or stop DNS64 detection and 464xlat according to network state.
             networkAgent.clatd.update();
             notifyIfacesChangedForNetworkStats();
+            try {
+                networkAgent.networkMonitor().notifyLinkPropertiesChanged(newLp);
+            } catch (RemoteException e) {
+                e.rethrowFromSystemServer();
+            }
             if (networkAgent.everConnected) {
-                try {
-                    networkAgent.networkMonitor().notifyLinkPropertiesChanged();
-                } catch (RemoteException e) {
-                    e.rethrowFromSystemServer();
-                }
                 notifyNetworkCallbacks(networkAgent, ConnectivityManager.CALLBACK_IP_CHANGED);
             }
         }
@@ -5708,7 +5708,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         final NetworkCapabilities prevNc;
         synchronized (nai) {
             prevNc = nai.networkCapabilities;
-            nai.networkCapabilities = newNc;
+            nai.setNetworkCapabilities(newNc);
         }
 
         updateUids(nai, prevNc, newNc);
@@ -5723,11 +5723,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
             // If the requestable capabilities have changed or the score changed, we can't have been
             // called by rematchNetworkAndRequests, so it's safe to start a rematch.
             rematchAllNetworksAndRequests(nai, oldScore);
-            try {
-                nai.networkMonitor().notifyNetworkCapabilitiesChanged();
-            } catch (RemoteException e) {
-                e.rethrowFromSystemServer();
-            }
             notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_CAP_CHANGED);
         }
 
@@ -5986,11 +5981,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
         }
 
         if (capabilitiesChanged) {
-            try {
-                nai.networkMonitor().notifyNetworkCapabilitiesChanged();
-            } catch (RemoteException e) {
-                e.rethrowFromSystemServer();
-            }
             notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_CAP_CHANGED);
         }
 
@@ -6399,7 +6389,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 if (networkAgent.networkMisc.acceptPartialConnectivity) {
                     networkAgent.networkMonitor().setAcceptPartialConnectivity();
                 }
-                networkAgent.networkMonitor().notifyNetworkConnected();
+                networkAgent.networkMonitor().notifyNetworkConnected(
+                        networkAgent.linkProperties, networkAgent.networkCapabilities);
             } catch (RemoteException e) {
                 e.rethrowFromSystemServer();
             }

services/core/java/com/android/server/connectivity/NetworkAgentInfo.java

@@ -29,6 +29,7 @@ import android.net.NetworkState;
 import android.os.Handler;
 import android.os.INetworkManagementService;
 import android.os.Messenger;
+import android.os.RemoteException;
 import android.os.SystemClock;
 import android.util.Log;
 import android.util.SparseArray;
@@ -120,7 +121,8 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
     // This Network object is always valid.
     public final Network network;
     public LinkProperties linkProperties;
-    // This should only be modified via ConnectivityService.updateCapabilities().
+    // This should only be modified by ConnectivityService, via setNetworkCapabilities().
+    // TODO: make this private with a getter.
     public NetworkCapabilities networkCapabilities;
     public final NetworkMisc networkMisc;
     // Indicates if netd has been told to create this Network. From this point on the appropriate
@@ -278,6 +280,25 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
         mNetworkMonitor = networkMonitor;
     }
 
+    /**
+     * Set the NetworkCapabilities on this NetworkAgentInfo. Also attempts to notify NetworkMonitor
+     * of the new capabilities, if NetworkMonitor has been created.
+     *
+     * <p>If {@link NetworkMonitor#notifyNetworkCapabilitiesChanged(NetworkCapabilities)} fails,
+     * the exception is logged but not reported to callers.
+     */
+    public void setNetworkCapabilities(NetworkCapabilities nc) {
+        networkCapabilities = nc;
+        final INetworkMonitor nm = mNetworkMonitor;
+        if (nm != null) {
+            try {
+                nm.notifyNetworkCapabilitiesChanged(nc);
+            } catch (RemoteException e) {
+                Log.e(TAG, "Error notifying NetworkMonitor of updated NetworkCapabilities", e);
+            }
+        }
+    }
+
     public ConnectivityService connService() {
         return mConnService;
     }

tests/net/java/com/android/server/ConnectivityServiceTest.java

@@ -496,7 +496,7 @@ public class ConnectivityServiceTest {
             };
 
             try {
-                doAnswer(validateAnswer).when(mNetworkMonitor).notifyNetworkConnected();
+                doAnswer(validateAnswer).when(mNetworkMonitor).notifyNetworkConnected(any(), any());
                 doAnswer(validateAnswer).when(mNetworkMonitor).forceReevaluation(anyInt());
             } catch (RemoteException e) {
                 fail(e.getMessage());