Use PerUidCounter in the connectivity static library

This is a no-op refactoring which uses common PerUidCounter
from the static library and delete the private implementation
inside ConnectivityService. This refactoring includes:
  1. Make a private implementation that inherite PerUidCounter
     to convert and rethrow exception to maintain backward
     compatibility.
  2. Adjust per-uid max limit argument in the counter constructor
     since the private implementation is already buggy.
  3. Use the getter in PerUidCounter for the existing tests.

Test: testNetworkCallbackMaximum
      testProfileNetworkPrefCountsRequestsCorrectlyOnSet
      testRequestCountLimits
Bug: 235771502
Change-Id: I5c49edab18774acf819828201041c9931fabccc4
This commit is contained in:
Junyu Lai
2022-07-05 11:01:52 +08:00
parent 31d38bffdf
commit 00d92df645
3 changed files with 52 additions and 85 deletions

View File

@@ -257,6 +257,7 @@ import com.android.net.module.util.LinkPropertiesUtils.CompareOrUpdateResult;
import com.android.net.module.util.LinkPropertiesUtils.CompareResult;
import com.android.net.module.util.LocationPermissionChecker;
import com.android.net.module.util.NetworkCapabilitiesUtils;
import com.android.net.module.util.PerUidCounter;
import com.android.net.module.util.PermissionUtils;
import com.android.net.module.util.TcUtils;
import com.android.net.module.util.netlink.InetDiagMessage;
@@ -386,9 +387,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
protected final PermissionMonitor mPermissionMonitor;
@VisibleForTesting
final PerUidCounter mNetworkRequestCounter;
final RequestInfoPerUidCounter mNetworkRequestCounter;
@VisibleForTesting
final PerUidCounter mSystemNetworkRequestCounter;
final RequestInfoPerUidCounter mSystemNetworkRequestCounter;
private volatile boolean mLockdownEnabled;
@@ -1185,71 +1186,6 @@ 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;
// Map from UID to number of NetworkRequests that UID has filed.
@VisibleForTesting
@GuardedBy("mUidToNetworkRequestCount")
final SparseIntArray mUidToNetworkRequestCount = new SparseIntArray();
/**
* Constructor
*
* @param maxCountPerUid the maximum count per uid allowed
*/
public PerUidCounter(final int maxCountPerUid) {
mMaxCountPerUid = maxCountPerUid;
}
/**
* Increments the request count of the given uid. Throws an exception if the number
* of open requests for the uid exceeds the value of maxCounterPerUid which is the value
* passed into the constructor. see: {@link #PerUidCounter(int)}.
*
* @throws ServiceSpecificException with
* {@link ConnectivityManager.Errors.TOO_MANY_REQUESTS} if the number of requests for
* the uid exceed the allowed number.
*
* @param uid the uid that the request was made under
*/
public void incrementCountOrThrow(final int uid) {
synchronized (mUidToNetworkRequestCount) {
final int newRequestCount = mUidToNetworkRequestCount.get(uid, 0) + 1;
if (newRequestCount >= mMaxCountPerUid) {
throw new ServiceSpecificException(
ConnectivityManager.Errors.TOO_MANY_REQUESTS,
"Uid " + uid + " exceeded its allotted requests limit");
}
mUidToNetworkRequestCount.put(uid, newRequestCount);
}
}
/**
* Decrements the request count of the given uid.
*
* @param uid the uid that the request was made under
*/
public void decrementCount(final int uid) {
synchronized (mUidToNetworkRequestCount) {
/* numToDecrement */
final int newRequestCount = mUidToNetworkRequestCount.get(uid, 0) - 1;
if (newRequestCount < 0) {
logwtf("BUG: too small request count " + newRequestCount + " for UID " + uid);
} else if (newRequestCount == 0) {
mUidToNetworkRequestCount.delete(uid);
} else {
mUidToNetworkRequestCount.put(uid, newRequestCount);
}
}
}
}
/**
* Dependencies of ConnectivityService, for injection in tests.
*/
@@ -1464,8 +1400,13 @@ public class ConnectivityService extends IConnectivityManager.Stub
mNetIdManager = mDeps.makeNetIdManager();
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);
// The legacy PerUidCounter is buggy and throwing exception at count == limit.
// Pass limit - 1 to maintain backward compatibility.
// TODO: Remove the workaround.
mNetworkRequestCounter =
new RequestInfoPerUidCounter(MAX_NETWORK_REQUESTS_PER_UID - 1);
mSystemNetworkRequestCounter =
new RequestInfoPerUidCounter(MAX_NETWORK_REQUESTS_PER_SYSTEM_UID - 1);
mMetricsLog = logger;
mNetworkRanker = new NetworkRanker();
@@ -4763,7 +4704,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
}
}
}
nri.decrementRequestCount();
nri.mPerUidCounter.decrementCount(nri.mUid);
mNetworkRequestInfoLogs.log("RELEASE " + nri);
checkNrisConsistency(nri);
@@ -4866,7 +4807,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
}
}
private PerUidCounter getRequestCounter(NetworkRequestInfo nri) {
private RequestInfoPerUidCounter getRequestCounter(NetworkRequestInfo nri) {
return checkAnyPermissionOf(
nri.mPid, nri.mUid, NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK)
? mSystemNetworkRequestCounter : mNetworkRequestCounter;
@@ -6230,7 +6171,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
final String mCallingAttributionTag;
// Counter keeping track of this NRI.
final PerUidCounter mPerUidCounter;
final RequestInfoPerUidCounter mPerUidCounter;
// Effective UID of this request. This is different from mUid when a privileged process
// files a request on behalf of another UID. This UID is used to determine blocked status,
@@ -6396,10 +6337,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
return Collections.unmodifiableList(tempRequests);
}
void decrementRequestCount() {
mPerUidCounter.decrementCount(mUid);
}
void linkDeathRecipient() {
if (null != mBinder) {
try {
@@ -6461,6 +6398,38 @@ public class ConnectivityService extends IConnectivityManager.Stub
}
}
// Keep backward compatibility since the ServiceSpecificException is used by
// the API surface, see {@link ConnectivityManager#convertServiceException}.
public static class RequestInfoPerUidCounter extends PerUidCounter {
RequestInfoPerUidCounter(int maxCountPerUid) {
super(maxCountPerUid);
}
@Override
public synchronized void incrementCountOrThrow(int uid) {
try {
super.incrementCountOrThrow(uid);
} catch (IllegalStateException e) {
throw new ServiceSpecificException(
ConnectivityManager.Errors.TOO_MANY_REQUESTS,
"Uid " + uid + " exceeded its allotted requests limit");
}
}
@Override
public synchronized void decrementCountOrThrow(int uid) {
throw new UnsupportedOperationException("Use decrementCount instead.");
}
public synchronized void decrementCount(int uid) {
try {
super.decrementCountOrThrow(uid);
} catch (IllegalStateException e) {
logwtf("Exception when decrement per uid request count: ", e);
}
}
}
// This checks that the passed capabilities either do not request a
// specific SSID/SignalStrength, or the calling app has permission to do so.
private void ensureSufficientPermissionsForRequest(NetworkCapabilities nc,
@@ -9949,7 +9918,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().
nri.decrementRequestCount();
nri.mPerUidCounter.decrementCount(nri.mUid);
return;
}
@@ -10015,7 +9984,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().
nri.decrementRequestCount();
nri.mPerUidCounter.decrementCount(nri.mUid);
iCb.unlinkToDeath(cbInfo, 0);
}