LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge "Fix a crash when changing preferences" am: aeb051b962

Browse Source 
Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/1781202

Change-Id: Ifa0249c059488029e5e8902cd7bbece06a6d54f2
This commit is contained in:
Chalard Jean
2021-08-04 12:35:33 +00:00
committed by Automerger Merge Worker
parent aca228c07a aeb051b962
commit 916aeb7b0d
2 changed files with 145 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
service/src/com/android/server/ConnectivityService.java
View File

@@ -324,7 +324,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
private static final int DEFAULT_NASCENT_DELAY_MS = 5_000;
// The maximum number of network request allowed per uid before an exception is thrown.
private static final int MAX_NETWORK_REQUESTS_PER_UID = 100;
@VisibleForTesting
static final int MAX_NETWORK_REQUESTS_PER_UID = 100;
// The maximum number of network request allowed for system UIDs before an exception is thrown.
@VisibleForTesting
@@ -344,7 +345,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
@VisibleForTesting
protected final PermissionMonitor mPermissionMonitor;
private final PerUidCounter mNetworkRequestCounter;
@VisibleForTesting
final PerUidCounter mNetworkRequestCounter;
@VisibleForTesting
final PerUidCounter mSystemNetworkRequestCounter;
@@ -1154,9 +1156,20 @@ public class ConnectivityService extends IConnectivityManager.Stub
private void incrementCountOrThrow(final int uid, final int numToIncrement) {
final int newRequestCount =
mUidToNetworkRequestCount.get(uid, 0) + numToIncrement;
if (newRequestCount >= mMaxCountPerUid) {
if (newRequestCount >= mMaxCountPerUid
// HACK : the system server is allowed to go over the request count limit
// when it is creating requests on behalf of another app (but not itself,
// so it can still detect its own request leaks). This only happens in the
// per-app API flows in which case the old requests for that particular
// UID will be removed soon.
// TODO : instead of this hack, addPerAppDefaultNetworkRequests and other
// users of transact() should unregister the requests to decrease the count
// before they increase it again by creating a new NRI. Then remove the
// transact() method.
&& (Process.myUid() == uid || Process.myUid() != Binder.getCallingUid())) {
throw new ServiceSpecificException(
ConnectivityManager.Errors.TOO_MANY_REQUESTS);
ConnectivityManager.Errors.TOO_MANY_REQUESTS,
"Uid " + uid + " exceeded its allotted requests limit");
}
mUidToNetworkRequestCount.put(uid, newRequestCount);
}
@@ -5845,7 +5858,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
mUid = nri.mUid;
mAsUid = nri.mAsUid;
mPendingIntent = nri.mPendingIntent;
mPerUidCounter = getRequestCounter(this);
mPerUidCounter = nri.mPerUidCounter;
mPerUidCounter.incrementCountOrThrow(mUid);
mCallbackFlags = nri.mCallbackFlags;
mCallingAttributionTag = nri.mCallingAttributionTag;
@@ -10248,7 +10261,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
final NetworkRequestInfo trackingNri =
getDefaultRequestTrackingUid(callbackRequest.mAsUid);
// If this nri is not being tracked, the change it back to an untracked nri.
// If this nri is not being tracked, then change it back to an untracked nri.
if (trackingNri == mDefaultRequest) {
callbackRequestsToRegister.add(new NetworkRequestInfo(
callbackRequest,

178
tests/unit/java/com/android/server/ConnectivityServiceTest.java
View File

@@ -125,6 +125,7 @@ import static android.net.resolv.aidl.IDnsResolverUnsolicitedEventListener.VALID
import static android.os.Process.INVALID_UID;
import static android.system.OsConstants.IPPROTO_TCP;
import static com.android.server.ConnectivityService.MAX_NETWORK_REQUESTS_PER_SYSTEM_UID;
import static com.android.server.ConnectivityService.PREFERENCE_ORDER_MOBILE_DATA_PREFERERRED;
import static com.android.server.ConnectivityService.PREFERENCE_ORDER_OEM;
import static com.android.server.ConnectivityService.PREFERENCE_ORDER_PROFILE;
@@ -540,6 +541,9 @@ public class ConnectivityServiceTest {
private final LinkedBlockingQueue<Intent> mStartedActivities = new LinkedBlockingQueue<>();
// Map of permission name -> PermissionManager.Permission_{GRANTED|DENIED} constant
// For permissions granted across the board, the key is only the permission name.
// For permissions only granted to a combination of uid/pid, the key
// is "<permission name>,<pid>,<uid>". PID+UID permissons have priority over generic ones.
private final HashMap<String, Integer> mMockedPermissions = new HashMap<>();
MockContext(Context base, ContentProvider settingsProvider) {
@@ -641,30 +645,40 @@ public class ConnectivityServiceTest {
return mPackageManager;
}
private int checkMockedPermission(String permission, Supplier<Integer> ifAbsent) {
final Integer granted = mMockedPermissions.get(permission);
return granted != null ? granted : ifAbsent.get();
private int checkMockedPermission(String permission, int pid, int uid,
Supplier<Integer> ifAbsent) {
final Integer granted = mMockedPermissions.get(permission + "," + pid + "," + uid);
if (null != granted) {
return granted;
}
final Integer allGranted = mMockedPermissions.get(permission);
if (null != allGranted) {
return allGranted;
}
return ifAbsent.get();
}
@Override
public int checkPermission(String permission, int pid, int uid) {
return checkMockedPermission(
permission, () -> super.checkPermission(permission, pid, uid));
return checkMockedPermission(permission, pid, uid,
() -> super.checkPermission(permission, pid, uid));
}
@Override
public int checkCallingOrSelfPermission(String permission) {
return checkMockedPermission(
permission, () -> super.checkCallingOrSelfPermission(permission));
return checkMockedPermission(permission, Process.myPid(), Process.myUid(),
() -> super.checkCallingOrSelfPermission(permission));
}
@Override
public void enforceCallingOrSelfPermission(String permission, String message) {
final Integer granted = mMockedPermissions.get(permission);
if (granted == null) {
super.enforceCallingOrSelfPermission(permission, message);
return;
}
final Integer granted = checkMockedPermission(permission,
Process.myPid(), Process.myUid(),
() -> {
super.enforceCallingOrSelfPermission(permission, message);
// enforce will crash if the permission is not granted
return PERMISSION_GRANTED;
});
if (!granted.equals(PERMISSION_GRANTED)) {
throw new SecurityException("[Test] permission denied: " + permission);
@@ -674,6 +688,8 @@ public class ConnectivityServiceTest {
/**
* Mock checks for the specified permission, and have them behave as per {@code granted}.
*
* This will apply across the board no matter what the checked UID and PID are.
*
* <p>Passing null reverts to default behavior, which does a real permission check on the
* test package.
* @param granted One of {@link PackageManager#PERMISSION_GRANTED} or
@@ -683,6 +699,21 @@ public class ConnectivityServiceTest {
mMockedPermissions.put(permission, granted);
}
/**
* Mock checks for the specified permission, and have them behave as per {@code granted}.
*
* This will only apply to the passed UID and PID.
*
* <p>Passing null reverts to default behavior, which does a real permission check on the
* test package.
* @param granted One of {@link PackageManager#PERMISSION_GRANTED} or
* {@link PackageManager#PERMISSION_DENIED}.
*/
public void setPermission(String permission, int pid, int uid, Integer granted) {
final String key = permission + "," + pid + "," + uid;
mMockedPermissions.put(key, granted);
}
@Override
public Intent registerReceiverForAllUsers(@Nullable BroadcastReceiver receiver,
@NonNull IntentFilter filter, @Nullable String broadcastPermission,
@@ -1564,15 +1595,21 @@ public class ConnectivityServiceTest {
}
private void withPermission(String permission, ExceptionalRunnable r) throws Exception {
if (mServiceContext.checkCallingOrSelfPermission(permission) == PERMISSION_GRANTED) {
r.run();
return;
}
try {
mServiceContext.setPermission(permission, PERMISSION_GRANTED);
r.run();
} finally {
mServiceContext.setPermission(permission, PERMISSION_DENIED);
mServiceContext.setPermission(permission, null);
}
}
private void withPermission(String permission, int pid, int uid, ExceptionalRunnable r)
throws Exception {
try {
mServiceContext.setPermission(permission, pid, uid, PERMISSION_GRANTED);
r.run();
} finally {
mServiceContext.setPermission(permission, pid, uid, null);
}
}
@@ -13283,17 +13320,45 @@ public class ConnectivityServiceTest {
@Test
public void testProfileNetworkPrefCountsRequestsCorrectlyOnSet() throws Exception {
final UserHandle testHandle = setupEnterpriseNetwork();
testRequestCountLimits(() -> {
// Set initially to test the limit prior to having existing requests.
final TestOnCompleteListener listener = new TestOnCompleteListener();
mCm.setProfileNetworkPreference(testHandle, PROFILE_NETWORK_PREFERENCE_ENTERPRISE,
Runnable::run, listener);
final TestOnCompleteListener listener = new TestOnCompleteListener();
// Leave one request available so the profile preference can be set.
testRequestCountLimits(1 /* countToLeaveAvailable */, () -> {
withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK,
Process.myPid(), Process.myUid(), () -> {
// Set initially to test the limit prior to having existing requests.
mCm.setProfileNetworkPreference(testHandle,
PROFILE_NETWORK_PREFERENCE_ENTERPRISE,
Runnable::run, listener);
});
listener.expectOnComplete();
// re-set so as to test the limit as part of replacing existing requests.
mCm.setProfileNetworkPreference(testHandle, PROFILE_NETWORK_PREFERENCE_ENTERPRISE,
Runnable::run, listener);
// Simulate filing requests as some app on the work profile
final int otherAppUid = UserHandle.getUid(TEST_WORK_PROFILE_USER_ID,
UserHandle.getAppId(Process.myUid() + 1));
final int remainingCount = ConnectivityService.MAX_NETWORK_REQUESTS_PER_UID
- mService.mNetworkRequestCounter.mUidToNetworkRequestCount.get(otherAppUid)
- 1;
final NetworkCallback[] callbacks = new NetworkCallback[remainingCount];
doAsUid(otherAppUid, () -> {
for (int i = 0; i < remainingCount; ++i) {
callbacks[i] = new TestableNetworkCallback();
mCm.registerDefaultNetworkCallback(callbacks[i]);
}
});
withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK,
Process.myPid(), Process.myUid(), () -> {
// re-set so as to test the limit as part of replacing existing requests.
mCm.setProfileNetworkPreference(testHandle,
PROFILE_NETWORK_PREFERENCE_ENTERPRISE, Runnable::run, listener);
});
listener.expectOnComplete();
doAsUid(otherAppUid, () -> {
for (final NetworkCallback callback : callbacks) {
mCm.unregisterNetworkCallback(callback);
}
});
});
}
@@ -13305,39 +13370,45 @@ public class ConnectivityServiceTest {
mockHasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE, true);
@OemNetworkPreferences.OemNetworkPreference final int networkPref =
OEM_NETWORK_PREFERENCE_OEM_PRIVATE_ONLY;
testRequestCountLimits(() -> {
// Set initially to test the limit prior to having existing requests.
final TestOemListenerCallback listener = new TestOemListenerCallback();
mService.setOemNetworkPreference(
createDefaultOemNetworkPreferences(networkPref), listener);
listener.expectOnComplete();
// Leave one request available so the OEM preference can be set.
testRequestCountLimits(1 /* countToLeaveAvailable */, () ->
withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, () -> {
// Set initially to test the limit prior to having existing requests.
final TestOemListenerCallback listener = new TestOemListenerCallback();
mService.setOemNetworkPreference(
createDefaultOemNetworkPreferences(networkPref), listener);
listener.expectOnComplete();
// re-set so as to test the limit as part of replacing existing requests.
mService.setOemNetworkPreference(
createDefaultOemNetworkPreferences(networkPref), listener);
listener.expectOnComplete();
});
// re-set so as to test the limit as part of replacing existing requests.
mService.setOemNetworkPreference(
createDefaultOemNetworkPreferences(networkPref), listener);
listener.expectOnComplete();
}));
}
private void testRequestCountLimits(@NonNull final Runnable r) throws Exception {
private void testRequestCountLimits(final int countToLeaveAvailable,
@NonNull final ExceptionalRunnable r) throws Exception {
final ArraySet<TestNetworkCallback> callbacks = new ArraySet<>();
try {
final int requestCount = mService.mSystemNetworkRequestCounter
.mUidToNetworkRequestCount.get(Process.myUid());
// The limit is hit when total requests <= limit.
final int maxCount =
ConnectivityService.MAX_NETWORK_REQUESTS_PER_SYSTEM_UID - requestCount;
// The limit is hit when total requests = limit - 1, and exceeded with a crash when
// total requests >= limit.
final int countToFile =
MAX_NETWORK_REQUESTS_PER_SYSTEM_UID - requestCount - countToLeaveAvailable;
// Need permission so registerDefaultNetworkCallback uses mSystemNetworkRequestCounter
withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, () -> {
for (int i = 1; i < maxCount - 1; i++) {
for (int i = 1; i < countToFile; i++) {
final TestNetworkCallback cb = new TestNetworkCallback();
mCm.registerDefaultNetworkCallback(cb);
callbacks.add(cb);
}
// Code to run to check if it triggers a max request count limit error.
r.run();
assertEquals(MAX_NETWORK_REQUESTS_PER_SYSTEM_UID - 1 - countToLeaveAvailable,
mService.mSystemNetworkRequestCounter
.mUidToNetworkRequestCount.get(Process.myUid()));
});
// Code to run to check if it triggers a max request count limit error.
r.run();
} finally {
for (final TestNetworkCallback cb : callbacks) {
mCm.unregisterNetworkCallback(cb);
@@ -13582,15 +13653,18 @@ public class ConnectivityServiceTest {
public void testMobileDataPreferredUidsChangedCountsRequestsCorrectlyOnSet() throws Exception {
ConnectivitySettingsManager.setMobileDataPreferredUids(mServiceContext,
Set.of(PRIMARY_USER_HANDLE.getUid(TEST_PACKAGE_UID)));
testRequestCountLimits(() -> {
// Set initially to test the limit prior to having existing requests.
mService.updateMobileDataPreferredUids();
waitForIdle();
// Leave one request available so MDO preference set up above can be set.
testRequestCountLimits(1 /* countToLeaveAvailable */, () ->
withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK,
Process.myPid(), Process.myUid(), () -> {
// Set initially to test the limit prior to having existing requests.
mService.updateMobileDataPreferredUids();
waitForIdle();
// re-set so as to test the limit as part of replacing existing requests.
mService.updateMobileDataPreferredUids();
waitForIdle();
});
// re-set so as to test the limit as part of replacing existing requests
mService.updateMobileDataPreferredUids();
waitForIdle();
}));
}
@Test