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

mUidCounterSetMap - key, which is uid, U32 -> S32

Browse Source 
The kernel is actually not consistent in whether uids & gids
are signed or unsigned, and neither is our Java code, which
also commonly uses just 'int' for uid.  In practice values
greater or equal to 2**31 often don't quite work right.
For example icmp sockets are enabled via a sysctl that
takes a minimum and maximum gid - and these are signed int32s.

Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Ia309eb99f71765e30510d6a253c1329c20719f21
This commit is contained in:
Maciej Żenczykowski
2022-09-16 22:21:25 +00:00
parent f4913b1323
commit 2b0a6dce97
2 changed files with 18 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
service-t/src/com/android/server/net/NetworkStatsService.java
View File

@@ -164,7 +164,7 @@ import com.android.net.module.util.NetworkStatsUtils;
import com.android.net.module.util.PermissionUtils; import com.android.net.module.util.PermissionUtils;
import com.android.net.module.util.SharedLog; import com.android.net.module.util.SharedLog;
import com.android.net.module.util.Struct; import com.android.net.module.util.Struct;
import com.android.net.module.util.Struct.U32; import com.android.net.module.util.Struct.S32;
import com.android.net.module.util.Struct.U8; import com.android.net.module.util.Struct.U8;
import com.android.net.module.util.bpf.CookieTagMapKey; import com.android.net.module.util.bpf.CookieTagMapKey;
import com.android.net.module.util.bpf.CookieTagMapValue; import com.android.net.module.util.bpf.CookieTagMapValue;
@@ -408,7 +408,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
* mActiveUidCounterSet to avoid accessing kernel too frequently. * mActiveUidCounterSet to avoid accessing kernel too frequently.
*/ */
private SparseIntArray mActiveUidCounterSet = new SparseIntArray(); private SparseIntArray mActiveUidCounterSet = new SparseIntArray();
private final IBpfMap<U32, U8> mUidCounterSetMap; private final IBpfMap<S32, U8> mUidCounterSetMap;
private final IBpfMap<CookieTagMapKey, CookieTagMapValue> mCookieTagMap; private final IBpfMap<CookieTagMapKey, CookieTagMapValue> mCookieTagMap;
private final IBpfMap<StatsMapKey, StatsMapValue> mStatsMapA; private final IBpfMap<StatsMapKey, StatsMapValue> mStatsMapA;
private final IBpfMap<StatsMapKey, StatsMapValue> mStatsMapB; private final IBpfMap<StatsMapKey, StatsMapValue> mStatsMapB;
@@ -741,10 +741,10 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
} }
/** Get counter sets map for each UID. */ /** Get counter sets map for each UID. */
public IBpfMap<U32, U8> getUidCounterSetMap() { public IBpfMap<S32, U8> getUidCounterSetMap() {
try { try {
return new BpfMap<U32, U8>(UID_COUNTERSET_MAP_PATH, BpfMap.BPF_F_RDWR, return new BpfMap<S32, U8>(UID_COUNTERSET_MAP_PATH, BpfMap.BPF_F_RDWR,
U32.class, U8.class); S32.class, U8.class);
} catch (ErrnoException e) { } catch (ErrnoException e) {
Log.wtf(TAG, "Cannot open uid counter set map: " + e); Log.wtf(TAG, "Cannot open uid counter set map: " + e);
return null; return null;
@@ -1747,7 +1747,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
if (set == SET_DEFAULT) { if (set == SET_DEFAULT) {
try { try {
mUidCounterSetMap.deleteEntry(new U32(uid)); mUidCounterSetMap.deleteEntry(new S32(uid));
} catch (ErrnoException e) { } catch (ErrnoException e) {
Log.w(TAG, "UidCounterSetMap.deleteEntry(" + uid + ") failed with errno: " + e); Log.w(TAG, "UidCounterSetMap.deleteEntry(" + uid + ") failed with errno: " + e);
} }
@@ -1755,7 +1755,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
} }
try { try {
mUidCounterSetMap.updateEntry(new U32(uid), new U8((short) set)); mUidCounterSetMap.updateEntry(new S32(uid), new U8((short) set));
} catch (ErrnoException e) { } catch (ErrnoException e) {
Log.w(TAG, "UidCounterSetMap.updateEntry(" + uid + ", " + set Log.w(TAG, "UidCounterSetMap.updateEntry(" + uid + ", " + set
+ ") failed with errno: " + e); + ") failed with errno: " + e);
@@ -2472,7 +2472,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
deleteStatsMapTagData(mStatsMapB, uid); deleteStatsMapTagData(mStatsMapB, uid);
try { try {
mUidCounterSetMap.deleteEntry(new U32(uid)); mUidCounterSetMap.deleteEntry(new S32(uid));
} catch (ErrnoException e) { } catch (ErrnoException e) {
logErrorIfNotErrNoent(e, "Failed to delete tag data from uid counter set map"); logErrorIfNotErrNoent(e, "Failed to delete tag data from uid counter set map");
} }

20
tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java
View File

@@ -139,7 +139,7 @@ import com.android.net.module.util.BpfDump;
import com.android.net.module.util.IBpfMap; import com.android.net.module.util.IBpfMap;
import com.android.net.module.util.LocationPermissionChecker; import com.android.net.module.util.LocationPermissionChecker;
import com.android.net.module.util.Struct; import com.android.net.module.util.Struct;
import com.android.net.module.util.Struct.U32; import com.android.net.module.util.Struct.S32;
import com.android.net.module.util.Struct.U8; import com.android.net.module.util.Struct.U8;
import com.android.net.module.util.bpf.CookieTagMapKey; import com.android.net.module.util.bpf.CookieTagMapKey;
import com.android.net.module.util.bpf.CookieTagMapValue; import com.android.net.module.util.bpf.CookieTagMapValue;
@@ -249,7 +249,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
private HandlerThread mHandlerThread; private HandlerThread mHandlerThread;
@Mock @Mock
private LocationPermissionChecker mLocationPermissionChecker; private LocationPermissionChecker mLocationPermissionChecker;
private TestBpfMap<U32, U8> mUidCounterSetMap = spy(new TestBpfMap<>(U32.class, U8.class)); private TestBpfMap<S32, U8> mUidCounterSetMap = spy(new TestBpfMap<>(S32.class, U8.class));
@Mock @Mock
private BpfNetMaps mBpfNetMaps; private BpfNetMaps mBpfNetMaps;
@Mock @Mock
@@ -478,7 +478,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
} }
@Override @Override
public IBpfMap<U32, U8> getUidCounterSetMap() { public IBpfMap<S32, U8> getUidCounterSetMap() {
return mUidCounterSetMap; return mUidCounterSetMap;
} }
@@ -646,7 +646,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
mService.incrementOperationCount(UID_RED, 0xFAAD, 4); mService.incrementOperationCount(UID_RED, 0xFAAD, 4);
mService.noteUidForeground(UID_RED, true); mService.noteUidForeground(UID_RED, true);
verify(mUidCounterSetMap).updateEntry( verify(mUidCounterSetMap).updateEntry(
eq(new U32(UID_RED)), eq(new U8((short) SET_FOREGROUND))); eq(new S32(UID_RED)), eq(new U8((short) SET_FOREGROUND)));
mService.incrementOperationCount(UID_RED, 0xFAAD, 6); mService.incrementOperationCount(UID_RED, 0xFAAD, 6);
forcePollAndWaitForIdle(); forcePollAndWaitForIdle();
@@ -1311,7 +1311,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
.insertEntry(TEST_IFACE, UID_RED, SET_FOREGROUND, 0xFAAD, 1L, 1L, 1L, 1L, 0L)); .insertEntry(TEST_IFACE, UID_RED, SET_FOREGROUND, 0xFAAD, 1L, 1L, 1L, 1L, 0L));
mService.noteUidForeground(UID_RED, true); mService.noteUidForeground(UID_RED, true);
verify(mUidCounterSetMap).updateEntry( verify(mUidCounterSetMap).updateEntry(
eq(new U32(UID_RED)), eq(new U8((short) SET_FOREGROUND))); eq(new S32(UID_RED)), eq(new U8((short) SET_FOREGROUND)));
mService.incrementOperationCount(UID_RED, 0xFAAD, 1); mService.incrementOperationCount(UID_RED, 0xFAAD, 1);
forcePollAndWaitForIdle(); forcePollAndWaitForIdle();
@@ -1927,7 +1927,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
mService.incrementOperationCount(UID_RED, 0xFAAD, 4); mService.incrementOperationCount(UID_RED, 0xFAAD, 4);
mService.noteUidForeground(UID_RED, true); mService.noteUidForeground(UID_RED, true);
verify(mUidCounterSetMap).updateEntry( verify(mUidCounterSetMap).updateEntry(
eq(new U32(UID_RED)), eq(new U8((short) SET_FOREGROUND))); eq(new S32(UID_RED)), eq(new U8((short) SET_FOREGROUND)));
mService.incrementOperationCount(UID_RED, 0xFAAD, 6); mService.incrementOperationCount(UID_RED, 0xFAAD, 6);
forcePollAndWaitForIdle(); forcePollAndWaitForIdle();
@@ -2424,13 +2424,13 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
mAppUidStatsMap.insertEntry(new UidStatsMapKey(uid), new StatsMapValue(10, 10000, 6, 6000)); mAppUidStatsMap.insertEntry(new UidStatsMapKey(uid), new StatsMapValue(10, 10000, 6, 6000));
mUidCounterSetMap.insertEntry(new U32(uid), new U8((short) 1)); mUidCounterSetMap.insertEntry(new S32(uid), new U8((short) 1));
assertTrue(cookieTagMapContainsUid(uid)); assertTrue(cookieTagMapContainsUid(uid));
assertTrue(statsMapContainsUid(mStatsMapA, uid)); assertTrue(statsMapContainsUid(mStatsMapA, uid));
assertTrue(statsMapContainsUid(mStatsMapB, uid)); assertTrue(statsMapContainsUid(mStatsMapB, uid));
assertTrue(mAppUidStatsMap.containsKey(new UidStatsMapKey(uid))); assertTrue(mAppUidStatsMap.containsKey(new UidStatsMapKey(uid)));
assertTrue(mUidCounterSetMap.containsKey(new U32(uid))); assertTrue(mUidCounterSetMap.containsKey(new S32(uid)));
} }
@Test @Test
@@ -2447,14 +2447,14 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
assertFalse(statsMapContainsUid(mStatsMapA, UID_BLUE)); assertFalse(statsMapContainsUid(mStatsMapA, UID_BLUE));
assertFalse(statsMapContainsUid(mStatsMapB, UID_BLUE)); assertFalse(statsMapContainsUid(mStatsMapB, UID_BLUE));
assertFalse(mAppUidStatsMap.containsKey(new UidStatsMapKey(UID_BLUE))); assertFalse(mAppUidStatsMap.containsKey(new UidStatsMapKey(UID_BLUE)));
assertFalse(mUidCounterSetMap.containsKey(new U32(UID_BLUE))); assertFalse(mUidCounterSetMap.containsKey(new S32(UID_BLUE)));
// assert that UID_RED related tag data is still in the maps. // assert that UID_RED related tag data is still in the maps.
assertTrue(cookieTagMapContainsUid(UID_RED)); assertTrue(cookieTagMapContainsUid(UID_RED));
assertTrue(statsMapContainsUid(mStatsMapA, UID_RED)); assertTrue(statsMapContainsUid(mStatsMapA, UID_RED));
assertTrue(statsMapContainsUid(mStatsMapB, UID_RED)); assertTrue(statsMapContainsUid(mStatsMapB, UID_RED));
assertTrue(mAppUidStatsMap.containsKey(new UidStatsMapKey(UID_RED))); assertTrue(mAppUidStatsMap.containsKey(new UidStatsMapKey(UID_RED)));
assertTrue(mUidCounterSetMap.containsKey(new U32(UID_RED))); assertTrue(mUidCounterSetMap.containsKey(new S32(UID_RED)));
} }
private void assertDumpContains(final String dump, final String message) { private void assertDumpContains(final String dump, final String message) {