From b4f8d8e13050ab6c587dd694be2327a8d4435f0f Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Tue, 30 Mar 2021 19:29:00 +0900 Subject: [PATCH] Allow the system to register 250 NetworkCallbacks. Give anyone with PERMISSION_MAINLINE_NETWORK_STACK (i.e., either the system or the networkstack process) a separate limit of 250 callbacks per UID. Bug: 183921387 Test: new unit tests Change-Id: I24580ea48e3ad502ef584efc5fde0b5d22e392b4 --- .../android/server/ConnectivityService.java | 31 +++++++++++++++---- .../server/ConnectivityServiceTest.java | 28 +++++++++++++++++ 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 0c4258561f..77a2d79f30 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -318,6 +318,9 @@ public class ConnectivityService extends IConnectivityManager.Stub // The maximum number of network request allowed per uid before an exception is thrown. private static final int MAX_NETWORK_REQUESTS_PER_UID = 100; + // The maximum number of network request allowed for system UIDs before an exception is thrown. + private static final int MAX_NETWORK_REQUESTS_PER_SYSTEM_UID = 250; + @VisibleForTesting protected int mLingerDelayMs; // Can't be final, or test subclass constructors can't change it. @VisibleForTesting @@ -333,6 +336,7 @@ public class ConnectivityService extends IConnectivityManager.Stub protected final PermissionMonitor mPermissionMonitor; private final PerUidCounter mNetworkRequestCounter; + private final PerUidCounter mSystemNetworkRequestCounter; private volatile boolean mLockdownEnabled; @@ -1201,6 +1205,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mContext = Objects.requireNonNull(context, "missing Context"); mResources = deps.getResources(mContext); mNetworkRequestCounter = new PerUidCounter(MAX_NETWORK_REQUESTS_PER_UID); + mSystemNetworkRequestCounter = new PerUidCounter(MAX_NETWORK_REQUESTS_PER_SYSTEM_UID); mMetricsLog = logger; mNetworkRanker = new NetworkRanker(); @@ -4005,7 +4010,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } } } - mNetworkRequestCounter.decrementCount(nri.mUid); + decrementRequestCount(nri); mNetworkRequestInfoLogs.log("RELEASE " + nri); if (null != nri.getActiveRequest()) { @@ -4116,6 +4121,20 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + private PerUidCounter getRequestCounter(NetworkRequestInfo nri) { + return checkAnyPermissionOf( + nri.mPid, nri.mUid, NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK) + ? mSystemNetworkRequestCounter : mNetworkRequestCounter; + } + + private void incrementRequestCountOrThrow(NetworkRequestInfo nri) { + getRequestCounter(nri).incrementCountOrThrow(nri.mUid); + } + + private void decrementRequestCount(NetworkRequestInfo nri) { + getRequestCounter(nri).decrementCount(nri.mUid); + } + @Override public void setAcceptUnvalidated(Network network, boolean accept, boolean always) { enforceNetworkStackSettingsOrSetup(); @@ -5464,7 +5483,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mPid = getCallingPid(); mUid = mDeps.getCallingUid(); mAsUid = asUid; - mNetworkRequestCounter.incrementCountOrThrow(mUid); + incrementRequestCountOrThrow(this); /** * Location sensitive data not included in pending intent. Only included in * {@link NetworkCallback}. @@ -5496,7 +5515,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mUid = mDeps.getCallingUid(); mAsUid = asUid; mPendingIntent = null; - mNetworkRequestCounter.incrementCountOrThrow(mUid); + incrementRequestCountOrThrow(this); mCallbackFlags = callbackFlags; mCallingAttributionTag = callingAttributionTag; @@ -5539,7 +5558,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mUid = nri.mUid; mAsUid = nri.mAsUid; mPendingIntent = nri.mPendingIntent; - mNetworkRequestCounter.incrementCountOrThrow(mUid); + incrementRequestCountOrThrow(this); mCallbackFlags = nri.mCallbackFlags; mCallingAttributionTag = nri.mCallingAttributionTag; } @@ -8724,7 +8743,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // Decrement the reference count for this NetworkRequestInfo. The reference count is // incremented when the NetworkRequestInfo is created as part of // enforceRequestCountLimit(). - mNetworkRequestCounter.decrementCount(nri.mUid); + decrementRequestCount(nri); return; } @@ -8790,7 +8809,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // Decrement the reference count for this NetworkRequestInfo. The reference count is // incremented when the NetworkRequestInfo is created as part of // enforceRequestCountLimit(). - mNetworkRequestCounter.decrementCount(nri.mUid); + decrementRequestCount(nri); iCb.unlinkToDeath(cbInfo, 0); } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index e3e42ed5db..e38773f548 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -5391,6 +5391,7 @@ public class ConnectivityServiceTest { final int MAX_REQUESTS = 100; final int CALLBACKS = 89; final int INTENTS = 11; + final int SYSTEM_ONLY_MAX_REQUESTS = 250; assertEquals(MAX_REQUESTS, CALLBACKS + INTENTS); NetworkRequest networkRequest = new NetworkRequest.Builder().build(); @@ -5439,6 +5440,33 @@ public class ConnectivityServiceTest { new Intent("d"), FLAG_IMMUTABLE)) ); + // The system gets another SYSTEM_ONLY_MAX_REQUESTS slots. + final Handler handler = new Handler(ConnectivityThread.getInstanceLooper()); + withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, () -> { + ArrayList systemRegistered = new ArrayList<>(); + for (int i = 0; i < SYSTEM_ONLY_MAX_REQUESTS - 1; i++) { + NetworkCallback cb = new NetworkCallback(); + if (i % 2 == 0) { + mCm.registerDefaultNetworkCallbackAsUid(1000000 + i, cb, handler); + } else { + mCm.registerNetworkCallback(networkRequest, cb); + } + systemRegistered.add(cb); + } + waitForIdle(); + + assertThrows(TooManyRequestsException.class, () -> + mCm.registerDefaultNetworkCallbackAsUid(1001042, new NetworkCallback(), + handler)); + assertThrows(TooManyRequestsException.class, () -> + mCm.registerNetworkCallback(networkRequest, new NetworkCallback())); + + for (NetworkCallback callback : systemRegistered) { + mCm.unregisterNetworkCallback(callback); + } + waitForIdle(); // Wait for requests to be unregistered before giving up the permission. + }); + for (Object o : registered) { if (o instanceof NetworkCallback) { mCm.unregisterNetworkCallback((NetworkCallback)o);