From f3c946278c83ab07ec18b5eb258a54865fc0993f Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Thu, 5 May 2022 15:27:52 +0800 Subject: [PATCH] Limit data usage request per uid Currently, there is no limtation for an app to request data usage callback, which is dangerous if the app fire hundreds of thousands requests and potientially this might cause OOM if the apps don't free them. Test: atest NetworkStatsObserversTest#testRegister_limit Bug: 229103088 Change-Id: I8299f46fd47a82ec9b25ba2e0d3c95db5512c331 --- .../server/net/NetworkStatsObservers.java | 12 ++++- .../android/server/ConnectivityService.java | 1 + .../server/net/NetworkStatsObserversTest.java | 47 +++++++++++++++++++ 3 files changed, 59 insertions(+), 1 deletion(-) diff --git a/service-t/src/com/android/server/net/NetworkStatsObservers.java b/service-t/src/com/android/server/net/NetworkStatsObservers.java index c51a886dcc..df4e7f5e65 100644 --- a/service-t/src/com/android/server/net/NetworkStatsObservers.java +++ b/service-t/src/com/android/server/net/NetworkStatsObservers.java @@ -44,6 +44,7 @@ import android.util.Log; import android.util.SparseArray; import com.android.internal.annotations.VisibleForTesting; +import com.android.net.module.util.PerUidCounter; import java.util.concurrent.atomic.AtomicInteger; @@ -63,10 +64,17 @@ class NetworkStatsObservers { private static final int DUMP_USAGE_REQUESTS_COUNT = 200; + // The maximum number of request allowed per uid before an exception is thrown. + @VisibleForTesting + static final int MAX_REQUESTS_PER_UID = 100; + // All access to this map must be done from the handler thread. // indexed by DataUsageRequest#requestId private final SparseArray mDataUsageRequests = new SparseArray<>(); + // Request counters per uid, this is thread safe. + private final PerUidCounter mDataUsageRequestsPerUid = new PerUidCounter(MAX_REQUESTS_PER_UID); + // Sequence number of DataUsageRequests private final AtomicInteger mNextDataUsageRequestId = new AtomicInteger(); @@ -89,8 +97,9 @@ class NetworkStatsObservers { DataUsageRequest request = buildRequest(context, inputRequest, callingUid); RequestInfo requestInfo = buildRequestInfo(request, callback, callingPid, callingUid, callingPackage, accessLevel); - if (LOG) Log.d(TAG, "Registering observer for " + requestInfo); + mDataUsageRequestsPerUid.incrementCountOrThrow(callingUid); + getHandler().sendMessage(mHandler.obtainMessage(MSG_REGISTER, requestInfo)); return request; } @@ -189,6 +198,7 @@ class NetworkStatsObservers { if (LOG) Log.d(TAG, "Unregistering " + requestInfo); mDataUsageRequests.remove(request.requestId); + mDataUsageRequestsPerUid.decrementCountOrThrow(callingUid); requestInfo.unlinkDeathRecipient(); requestInfo.callCallback(NetworkStatsManager.CALLBACK_RELEASED); } diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 078fff0bfb..bc390aff76 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -1187,6 +1187,7 @@ public class ConnectivityService extends IConnectivityManager.Stub /** * Keeps track of the number of requests made under different uids. */ + // TODO: Remove the hack and use com.android.net.module.util.PerUidCounter instead. public static class PerUidCounter { private final int mMaxCountPerUid; diff --git a/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java b/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java index 13a85e875d..e8c9637fa6 100644 --- a/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java +++ b/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java @@ -32,6 +32,7 @@ import static android.text.format.DateUtils.MINUTE_IN_MILLIS; import static com.android.testutils.DevSdkIgnoreRuleKt.SC_V2; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; @@ -64,6 +65,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; +import java.util.ArrayList; import java.util.Objects; /** @@ -184,6 +186,51 @@ public class NetworkStatsObserversTest { assertEquals(THRESHOLD_BYTES, request2.thresholdInBytes); } + @Test + public void testRegister_limit() throws Exception { + final DataUsageRequest inputRequest = new DataUsageRequest( + DataUsageRequest.REQUEST_ID_UNSET, sTemplateWifi, THRESHOLD_BYTES); + + // Register maximum requests for red. + final ArrayList redRequests = new ArrayList<>(); + for (int i = 0; i < NetworkStatsObservers.MAX_REQUESTS_PER_UID; i++) { + final DataUsageRequest returnedRequest = + mStatsObservers.register(mContext, inputRequest, mUsageCallback, + PID_RED, UID_RED, PACKAGE_RED, NetworkStatsAccess.Level.DEVICE); + redRequests.add(returnedRequest); + assertTrue(returnedRequest.requestId > 0); + } + + // Verify request exceeds the limit throws. + assertThrows(IllegalStateException.class, () -> + mStatsObservers.register(mContext, inputRequest, mUsageCallback, + PID_RED, UID_RED, PACKAGE_RED, NetworkStatsAccess.Level.DEVICE)); + + // Verify another uid is not affected. + final ArrayList blueRequests = new ArrayList<>(); + for (int i = 0; i < NetworkStatsObservers.MAX_REQUESTS_PER_UID; i++) { + final DataUsageRequest returnedRequest = + mStatsObservers.register(mContext, inputRequest, mUsageCallback, + PID_BLUE, UID_BLUE, PACKAGE_BLUE, NetworkStatsAccess.Level.DEVICE); + blueRequests.add(returnedRequest); + assertTrue(returnedRequest.requestId > 0); + } + + // Again, verify request exceeds the limit throws for the 2nd uid. + assertThrows(IllegalStateException.class, () -> + mStatsObservers.register(mContext, inputRequest, mUsageCallback, + PID_RED, UID_RED, PACKAGE_RED, NetworkStatsAccess.Level.DEVICE)); + + // Unregister all registered requests. Note that exceptions cannot be tested since + // unregister is handled in the handler thread. + for (final DataUsageRequest request : redRequests) { + mStatsObservers.unregister(request, UID_RED); + } + for (final DataUsageRequest request : blueRequests) { + mStatsObservers.unregister(request, UID_BLUE); + } + } + @Test public void testUnregister_unknownRequest_noop() throws Exception { DataUsageRequest unknownRequest = new DataUsageRequest(