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 67a64d564b..d3a267af45 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(