diff options
3 files changed, 84 insertions, 21 deletions
diff --git a/services/core/java/com/android/server/VcnManagementService.java b/services/core/java/com/android/server/VcnManagementService.java index f652cb050cbd..78d4708e70a2 100644 --- a/services/core/java/com/android/server/VcnManagementService.java +++ b/services/core/java/com/android/server/VcnManagementService.java @@ -1104,7 +1104,7 @@ public class VcnManagementService extends IVcnManagementService.Stub { final NetworkCapabilities result = ncBuilder.build(); final VcnUnderlyingNetworkPolicy policy = new VcnUnderlyingNetworkPolicy( mTrackingNetworkCallback - .requiresRestartForImmutableCapabilityChanges(result), + .requiresRestartForImmutableCapabilityChanges(result, linkProperties), result); logVdbg("getUnderlyingNetworkPolicy() called for caps: " + networkCapabilities @@ -1354,19 +1354,29 @@ public class VcnManagementService extends IVcnManagementService.Stub { * without requiring a Network restart. */ private class TrackingNetworkCallback extends ConnectivityManager.NetworkCallback { + private final Object mLockObject = new Object(); private final Map<Network, NetworkCapabilities> mCaps = new ArrayMap<>(); + private final Map<Network, LinkProperties> mLinkProperties = new ArrayMap<>(); @Override public void onCapabilitiesChanged(Network network, NetworkCapabilities caps) { - synchronized (mCaps) { + synchronized (mLockObject) { mCaps.put(network, caps); } } @Override + public void onLinkPropertiesChanged(Network network, LinkProperties lp) { + synchronized (mLockObject) { + mLinkProperties.put(network, lp); + } + } + + @Override public void onLost(Network network) { - synchronized (mCaps) { + synchronized (mLockObject) { mCaps.remove(network); + mLinkProperties.remove(network); } } @@ -1393,22 +1403,28 @@ public class VcnManagementService extends IVcnManagementService.Stub { return true; } - private boolean requiresRestartForImmutableCapabilityChanges(NetworkCapabilities caps) { + private boolean requiresRestartForImmutableCapabilityChanges( + NetworkCapabilities caps, LinkProperties lp) { if (caps.getSubscriptionIds() == null) { return false; } - synchronized (mCaps) { - for (NetworkCapabilities existing : mCaps.values()) { - if (caps.getSubscriptionIds().equals(existing.getSubscriptionIds()) - && hasSameTransportsAndCapabilities(caps, existing)) { - // Restart if any immutable capabilities have changed - return existing.hasCapability(NET_CAPABILITY_NOT_RESTRICTED) + synchronized (mLockObject) { + // Search for an existing network (using interfce names) + // TODO: Get network from NetworkFactory (if exists) for this match. + for (Entry<Network, LinkProperties> lpEntry : mLinkProperties.entrySet()) { + if (lp.getInterfaceName() != null + && !lp.getInterfaceName().isEmpty() + && Objects.equals( + lp.getInterfaceName(), lpEntry.getValue().getInterfaceName())) { + return mCaps.get(lpEntry.getKey()) + .hasCapability(NET_CAPABILITY_NOT_RESTRICTED) != caps.hasCapability(NET_CAPABILITY_NOT_RESTRICTED); } } } + // If no network found, by definition does not need restart. return false; } diff --git a/services/core/java/com/android/server/am/ProcessList.java b/services/core/java/com/android/server/am/ProcessList.java index abddc4366460..256df98ae760 100644 --- a/services/core/java/com/android/server/am/ProcessList.java +++ b/services/core/java/com/android/server/am/ProcessList.java @@ -2573,7 +2573,10 @@ public final class ProcessList { + ", " + reason); app.setPendingStart(false); killProcessQuiet(pid); - Process.killProcessGroup(app.uid, app.getPid()); + final int appPid = app.getPid(); + if (appPid != 0) { + Process.killProcessGroup(app.uid, appPid); + } noteAppKill(app, ApplicationExitInfo.REASON_OTHER, ApplicationExitInfo.SUBREASON_INVALID_START, reason); return false; diff --git a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java index 075bc5e5214e..4123f8070e36 100644 --- a/tests/vcn/java/com/android/server/VcnManagementServiceTest.java +++ b/tests/vcn/java/com/android/server/VcnManagementServiceTest.java @@ -17,7 +17,9 @@ package com.android.server; import static android.net.ConnectivityManager.NetworkCallback; +import static android.net.NetworkCapabilities.NET_CAPABILITY_ENTERPRISE; import static android.net.NetworkCapabilities.NET_CAPABILITY_IMS; +import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VCN_MANAGED; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; @@ -67,7 +69,6 @@ import android.net.LinkProperties; import android.net.Network; import android.net.NetworkCapabilities; import android.net.NetworkRequest; -import android.net.TelephonyNetworkSpecifier; import android.net.Uri; import android.net.vcn.IVcnStatusCallback; import android.net.vcn.IVcnUnderlyingNetworkPolicyListener; @@ -128,6 +129,15 @@ public class VcnManagementServiceTest { private static final VcnConfig TEST_VCN_CONFIG; private static final VcnConfig TEST_VCN_CONFIG_PKG_2; private static final int TEST_UID = Process.FIRST_APPLICATION_UID; + private static final String TEST_IFACE_NAME = "TEST_IFACE"; + private static final String TEST_IFACE_NAME_2 = "TEST_IFACE2"; + private static final LinkProperties TEST_LP_1 = new LinkProperties(); + private static final LinkProperties TEST_LP_2 = new LinkProperties(); + + static { + TEST_LP_1.setInterfaceName(TEST_IFACE_NAME); + TEST_LP_2.setInterfaceName(TEST_IFACE_NAME_2); + } static { final Context mockConfigContext = mock(Context.class); @@ -1034,8 +1044,7 @@ public class VcnManagementServiceTest { setupSubscriptionAndStartVcn(subId, subGrp, isVcnActive); return mVcnMgmtSvc.getUnderlyingNetworkPolicy( - getNetworkCapabilitiesBuilderForTransport(subId, transport).build(), - new LinkProperties()); + getNetworkCapabilitiesBuilderForTransport(subId, transport).build(), TEST_LP_1); } private void checkGetRestrictedTransportsFromCarrierConfig( @@ -1260,7 +1269,7 @@ public class VcnManagementServiceTest { false /* expectRestricted */); } - private void setupTrackedCarrierWifiNetwork(NetworkCapabilities caps) { + private void setupTrackedNetwork(NetworkCapabilities caps, LinkProperties lp) { mVcnMgmtSvc.systemReady(); final ArgumentCaptor<NetworkCallback> captor = @@ -1269,7 +1278,10 @@ public class VcnManagementServiceTest { .registerNetworkCallback( eq(new NetworkRequest.Builder().clearCapabilities().build()), captor.capture()); - captor.getValue().onCapabilitiesChanged(mock(Network.class, CALLS_REAL_METHODS), caps); + + Network mockNetwork = mock(Network.class, CALLS_REAL_METHODS); + captor.getValue().onCapabilitiesChanged(mockNetwork, caps); + captor.getValue().onLinkPropertiesChanged(mockNetwork, lp); } @Test @@ -1279,7 +1291,7 @@ public class VcnManagementServiceTest { getNetworkCapabilitiesBuilderForTransport(TEST_SUBSCRIPTION_ID, TRANSPORT_WIFI) .removeCapability(NET_CAPABILITY_NOT_RESTRICTED) .build(); - setupTrackedCarrierWifiNetwork(existingNetworkCaps); + setupTrackedNetwork(existingNetworkCaps, TEST_LP_1); // Trigger test without VCN instance alive; expect restart due to change of NOT_RESTRICTED // immutable capability @@ -1288,7 +1300,7 @@ public class VcnManagementServiceTest { getNetworkCapabilitiesBuilderForTransport( TEST_SUBSCRIPTION_ID, TRANSPORT_WIFI) .build(), - new LinkProperties()); + TEST_LP_1); assertTrue(policy.isTeardownRequested()); } @@ -1298,7 +1310,7 @@ public class VcnManagementServiceTest { final NetworkCapabilities existingNetworkCaps = getNetworkCapabilitiesBuilderForTransport(TEST_SUBSCRIPTION_ID, TRANSPORT_WIFI) .build(); - setupTrackedCarrierWifiNetwork(existingNetworkCaps); + setupTrackedNetwork(existingNetworkCaps, TEST_LP_1); final VcnUnderlyingNetworkPolicy policy = startVcnAndGetPolicyForTransport( @@ -1315,7 +1327,7 @@ public class VcnManagementServiceTest { .addCapability(NET_CAPABILITY_NOT_RESTRICTED) .removeCapability(NET_CAPABILITY_IMS) .build(); - setupTrackedCarrierWifiNetwork(existingNetworkCaps); + setupTrackedNetwork(existingNetworkCaps, TEST_LP_1); final VcnUnderlyingNetworkPolicy policy = mVcnMgmtSvc.getUnderlyingNetworkPolicy( @@ -1336,7 +1348,7 @@ public class VcnManagementServiceTest { new NetworkCapabilities.Builder() .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) .addCapability(NET_CAPABILITY_NOT_VCN_MANAGED) - .setNetworkSpecifier(new TelephonyNetworkSpecifier(TEST_SUBSCRIPTION_ID_2)) + .setSubscriptionIds(Collections.singleton(TEST_SUBSCRIPTION_ID_2)) .build(); VcnUnderlyingNetworkPolicy policy = @@ -1346,6 +1358,38 @@ public class VcnManagementServiceTest { assertEquals(nc, policy.getMergedNetworkCapabilities()); } + /** + * Checks that networks with similar capabilities do not clobber each other. + * + * <p>In previous iterations, the VcnMgmtSvc used capability-matching to check if a network + * undergoing policy checks were the same as an existing networks. However, this meant that if + * there were newly added capabilities that the VCN did not check, two networks differing only + * by that capability would restart each other constantly. + */ + @Test + public void testGetUnderlyingNetworkPolicySimilarNetworks() throws Exception { + NetworkCapabilities nc1 = + new NetworkCapabilities.Builder() + .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) + .addCapability(NET_CAPABILITY_NOT_VCN_MANAGED) + .addCapability(NET_CAPABILITY_INTERNET) + .setSubscriptionIds(Collections.singleton(TEST_SUBSCRIPTION_ID_2)) + .build(); + + NetworkCapabilities nc2 = + new NetworkCapabilities.Builder(nc1) + .addCapability(NET_CAPABILITY_ENTERPRISE) + .removeCapability(NET_CAPABILITY_NOT_RESTRICTED) + .build(); + + setupTrackedNetwork(nc1, TEST_LP_1); + + VcnUnderlyingNetworkPolicy policy = mVcnMgmtSvc.getUnderlyingNetworkPolicy(nc2, TEST_LP_2); + + assertFalse(policy.isTeardownRequested()); + assertEquals(nc2, policy.getMergedNetworkCapabilities()); + } + @Test(expected = SecurityException.class) public void testGetUnderlyingNetworkPolicyInvalidPermission() { doReturn(PackageManager.PERMISSION_DENIED) |