BpfNetMaps.java - uid U32 -> S32
This converts the key, which is a uid, of sUidOwnerMap & sUidPermissionMap from U32 to S32.
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.
It's worth pointing out that these functions already accept int's
as arguments, so when converting them to U32 (and thus long) they
technically previously did the wrong thing (though it's unlikely
it ever mattered).
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I7b389a1cf822c56134b20df5ded269963f5bf69f
diff --git a/service/src/com/android/server/BpfNetMaps.java b/service/src/com/android/server/BpfNetMaps.java
index 2796dbe..cb1f38a 100644
--- a/service/src/com/android/server/BpfNetMaps.java
+++ b/service/src/com/android/server/BpfNetMaps.java
@@ -54,6 +54,7 @@
import com.android.net.module.util.DeviceConfigUtils;
import com.android.net.module.util.IBpfMap;
import com.android.net.module.util.Struct;
+import com.android.net.module.util.Struct.S32;
import com.android.net.module.util.Struct.U32;
import com.android.net.module.util.Struct.U8;
import com.android.net.module.util.bpf.CookieTagMapKey;
@@ -113,8 +114,8 @@
private static IBpfMap<U32, U32> sConfigurationMap = null;
// BpfMap for UID_OWNER_MAP_PATH. This map is not accessed by others.
- private static IBpfMap<U32, UidOwnerValue> sUidOwnerMap = null;
- private static IBpfMap<U32, U8> sUidPermissionMap = null;
+ private static IBpfMap<S32, UidOwnerValue> sUidOwnerMap = null;
+ private static IBpfMap<S32, U8> sUidPermissionMap = null;
private static IBpfMap<CookieTagMapKey, CookieTagMapValue> sCookieTagMap = null;
// LINT.IfChange(match_type)
@@ -153,7 +154,7 @@
* Set uidOwnerMap for test.
*/
@VisibleForTesting
- public static void setUidOwnerMapForTest(IBpfMap<U32, UidOwnerValue> uidOwnerMap) {
+ public static void setUidOwnerMapForTest(IBpfMap<S32, UidOwnerValue> uidOwnerMap) {
sUidOwnerMap = uidOwnerMap;
}
@@ -161,7 +162,7 @@
* Set uidPermissionMap for test.
*/
@VisibleForTesting
- public static void setUidPermissionMapForTest(IBpfMap<U32, U8> uidPermissionMap) {
+ public static void setUidPermissionMapForTest(IBpfMap<S32, U8> uidPermissionMap) {
sUidPermissionMap = uidPermissionMap;
}
@@ -183,19 +184,19 @@
}
}
- private static IBpfMap<U32, UidOwnerValue> getUidOwnerMap() {
+ private static IBpfMap<S32, UidOwnerValue> getUidOwnerMap() {
try {
return new BpfMap<>(
- UID_OWNER_MAP_PATH, BpfMap.BPF_F_RDWR, U32.class, UidOwnerValue.class);
+ UID_OWNER_MAP_PATH, BpfMap.BPF_F_RDWR, S32.class, UidOwnerValue.class);
} catch (ErrnoException e) {
throw new IllegalStateException("Cannot open uid owner map", e);
}
}
- private static IBpfMap<U32, U8> getUidPermissionMap() {
+ private static IBpfMap<S32, U8> getUidPermissionMap() {
try {
return new BpfMap<>(
- UID_PERMISSION_MAP_PATH, BpfMap.BPF_F_RDWR, U32.class, U8.class);
+ UID_PERMISSION_MAP_PATH, BpfMap.BPF_F_RDWR, S32.class, U8.class);
} catch (ErrnoException e) {
throw new IllegalStateException("Cannot open uid permission map", e);
}
@@ -389,7 +390,7 @@
private void removeRule(final int uid, final long match, final String caller) {
try {
synchronized (sUidOwnerMap) {
- final UidOwnerValue oldMatch = sUidOwnerMap.getValue(new U32(uid));
+ final UidOwnerValue oldMatch = sUidOwnerMap.getValue(new S32(uid));
if (oldMatch == null) {
throw new ServiceSpecificException(ENOENT,
@@ -402,9 +403,9 @@
);
if (newMatch.rule == 0) {
- sUidOwnerMap.deleteEntry(new U32(uid));
+ sUidOwnerMap.deleteEntry(new S32(uid));
} else {
- sUidOwnerMap.updateEntry(new U32(uid), newMatch);
+ sUidOwnerMap.updateEntry(new S32(uid), newMatch);
}
}
} catch (ErrnoException e) {
@@ -421,7 +422,7 @@
try {
synchronized (sUidOwnerMap) {
- final UidOwnerValue oldMatch = sUidOwnerMap.getValue(new U32(uid));
+ final UidOwnerValue oldMatch = sUidOwnerMap.getValue(new S32(uid));
final UidOwnerValue newMatch;
if (oldMatch != null) {
@@ -435,7 +436,7 @@
match
);
}
- sUidOwnerMap.updateEntry(new U32(uid), newMatch);
+ sUidOwnerMap.updateEntry(new S32(uid), newMatch);
}
} catch (ErrnoException e) {
throw new ServiceSpecificException(e.errno,
@@ -855,7 +856,7 @@
if (permissions == PERMISSION_UNINSTALLED || permissions == PERMISSION_INTERNET) {
for (final int uid : uids) {
try {
- sUidPermissionMap.deleteEntry(new U32(uid));
+ sUidPermissionMap.deleteEntry(new S32(uid));
} catch (ErrnoException e) {
Log.e(TAG, "Failed to remove uid " + uid + " from permission map: " + e);
}
@@ -865,7 +866,7 @@
for (final int uid : uids) {
try {
- sUidPermissionMap.updateEntry(new U32(uid), new U8((short) permissions));
+ sUidPermissionMap.updateEntry(new S32(uid), new U8((short) permissions));
} catch (ErrnoException e) {
Log.e(TAG, "Failed to set permission "
+ permissions + " to uid " + uid + ": " + e);
diff --git a/tests/unit/java/com/android/server/BpfNetMapsTest.java b/tests/unit/java/com/android/server/BpfNetMapsTest.java
index 8450c2b..38d2cd2 100644
--- a/tests/unit/java/com/android/server/BpfNetMapsTest.java
+++ b/tests/unit/java/com/android/server/BpfNetMapsTest.java
@@ -66,6 +66,7 @@
import com.android.modules.utils.build.SdkLevel;
import com.android.net.module.util.IBpfMap;
+import com.android.net.module.util.Struct.S32;
import com.android.net.module.util.Struct.U32;
import com.android.net.module.util.Struct.U8;
import com.android.net.module.util.bpf.CookieTagMapKey;
@@ -124,9 +125,9 @@
@Mock BpfNetMaps.Dependencies mDeps;
@Mock Context mContext;
private final IBpfMap<U32, U32> mConfigurationMap = new TestBpfMap<>(U32.class, U32.class);
- private final IBpfMap<U32, UidOwnerValue> mUidOwnerMap =
- new TestBpfMap<>(U32.class, UidOwnerValue.class);
- private final IBpfMap<U32, U8> mUidPermissionMap = new TestBpfMap<>(U32.class, U8.class);
+ private final IBpfMap<S32, UidOwnerValue> mUidOwnerMap =
+ new TestBpfMap<>(S32.class, UidOwnerValue.class);
+ private final IBpfMap<S32, U8> mUidPermissionMap = new TestBpfMap<>(S32.class, U8.class);
private final IBpfMap<CookieTagMapKey, CookieTagMapValue> mCookieTagMap =
spy(new TestBpfMap<>(CookieTagMapKey.class, CookieTagMapValue.class));
@@ -292,9 +293,9 @@
() -> mBpfNetMaps.setChildChain(FIREWALL_CHAIN_DOZABLE, true /* enable */));
}
- private void checkUidOwnerValue(final long uid, final int expectedIif,
+ private void checkUidOwnerValue(final int uid, final int expectedIif,
final long expectedMatch) throws Exception {
- final UidOwnerValue config = mUidOwnerMap.getValue(new U32(uid));
+ final UidOwnerValue config = mUidOwnerMap.getValue(new S32(uid));
if (expectedMatch == 0) {
assertNull(config);
} else {
@@ -304,7 +305,7 @@
}
private void doTestRemoveNaughtyApp(final int iif, final long match) throws Exception {
- mUidOwnerMap.updateEntry(new U32(TEST_UID), new UidOwnerValue(iif, match));
+ mUidOwnerMap.updateEntry(new S32(TEST_UID), new UidOwnerValue(iif, match));
mBpfNetMaps.removeNaughtyApp(TEST_UID);
@@ -343,7 +344,7 @@
private void doTestAddNaughtyApp(final int iif, final long match) throws Exception {
if (match != NO_MATCH) {
- mUidOwnerMap.updateEntry(new U32(TEST_UID), new UidOwnerValue(iif, match));
+ mUidOwnerMap.updateEntry(new S32(TEST_UID), new UidOwnerValue(iif, match));
}
mBpfNetMaps.addNaughtyApp(TEST_UID);
@@ -374,7 +375,7 @@
}
private void doTestRemoveNiceApp(final int iif, final long match) throws Exception {
- mUidOwnerMap.updateEntry(new U32(TEST_UID), new UidOwnerValue(iif, match));
+ mUidOwnerMap.updateEntry(new S32(TEST_UID), new UidOwnerValue(iif, match));
mBpfNetMaps.removeNiceApp(TEST_UID);
@@ -413,7 +414,7 @@
private void doTestAddNiceApp(final int iif, final long match) throws Exception {
if (match != NO_MATCH) {
- mUidOwnerMap.updateEntry(new U32(TEST_UID), new UidOwnerValue(iif, match));
+ mUidOwnerMap.updateEntry(new S32(TEST_UID), new UidOwnerValue(iif, match));
}
mBpfNetMaps.addNiceApp(TEST_UID);
@@ -446,7 +447,7 @@
private void doTestUpdateUidLockdownRule(final int iif, final long match, final boolean add)
throws Exception {
if (match != NO_MATCH) {
- mUidOwnerMap.updateEntry(new U32(TEST_UID), new UidOwnerValue(iif, match));
+ mUidOwnerMap.updateEntry(new S32(TEST_UID), new UidOwnerValue(iif, match));
}
mBpfNetMaps.updateUidLockdownRule(TEST_UID, add);
@@ -516,8 +517,8 @@
final int uid1 = TEST_UIDS[1];
final long match0 = DOZABLE_MATCH;
final long match1 = DOZABLE_MATCH | POWERSAVE_MATCH | RESTRICTED_MATCH;
- mUidOwnerMap.updateEntry(new U32(uid0), new UidOwnerValue(NO_IIF, match0));
- mUidOwnerMap.updateEntry(new U32(uid1), new UidOwnerValue(NO_IIF, match1));
+ mUidOwnerMap.updateEntry(new S32(uid0), new UidOwnerValue(NO_IIF, match0));
+ mUidOwnerMap.updateEntry(new S32(uid1), new UidOwnerValue(NO_IIF, match1));
mBpfNetMaps.addUidInterfaceRules(TEST_IF_NAME, TEST_UIDS);
@@ -532,8 +533,8 @@
final int uid1 = TEST_UIDS[1];
final long match0 = IIF_MATCH;
final long match1 = IIF_MATCH | DOZABLE_MATCH | POWERSAVE_MATCH | RESTRICTED_MATCH;
- mUidOwnerMap.updateEntry(new U32(uid0), new UidOwnerValue(TEST_IF_INDEX + 1, match0));
- mUidOwnerMap.updateEntry(new U32(uid1), new UidOwnerValue(NULL_IIF, match1));
+ mUidOwnerMap.updateEntry(new S32(uid0), new UidOwnerValue(TEST_IF_INDEX + 1, match0));
+ mUidOwnerMap.updateEntry(new S32(uid1), new UidOwnerValue(NULL_IIF, match1));
mBpfNetMaps.addUidInterfaceRules(TEST_IF_NAME, TEST_UIDS);
@@ -556,8 +557,8 @@
final int uid1 = TEST_UIDS[1];
final long match0 = IIF_MATCH;
final long match1 = IIF_MATCH | DOZABLE_MATCH | POWERSAVE_MATCH | RESTRICTED_MATCH;
- mUidOwnerMap.updateEntry(new U32(uid0), new UidOwnerValue(TEST_IF_INDEX, match0));
- mUidOwnerMap.updateEntry(new U32(uid1), new UidOwnerValue(NULL_IIF, match1));
+ mUidOwnerMap.updateEntry(new S32(uid0), new UidOwnerValue(TEST_IF_INDEX, match0));
+ mUidOwnerMap.updateEntry(new S32(uid1), new UidOwnerValue(NULL_IIF, match1));
mBpfNetMaps.addUidInterfaceRules(null /* ifName */, TEST_UIDS);
@@ -569,8 +570,8 @@
final int iif1, final long match1) throws Exception {
final int uid0 = TEST_UIDS[0];
final int uid1 = TEST_UIDS[1];
- mUidOwnerMap.updateEntry(new U32(uid0), new UidOwnerValue(iif0, match0));
- mUidOwnerMap.updateEntry(new U32(uid1), new UidOwnerValue(iif1, match1));
+ mUidOwnerMap.updateEntry(new S32(uid0), new UidOwnerValue(iif0, match0));
+ mUidOwnerMap.updateEntry(new S32(uid1), new UidOwnerValue(iif1, match1));
mBpfNetMaps.removeUidInterfaceRules(TEST_UIDS);
@@ -593,7 +594,7 @@
}
private void doTestSetUidRule(final List<Integer> testChains) throws Exception {
- mUidOwnerMap.updateEntry(new U32(TEST_UID), new UidOwnerValue(TEST_IF_INDEX, IIF_MATCH));
+ mUidOwnerMap.updateEntry(new S32(TEST_UID), new UidOwnerValue(TEST_IF_INDEX, IIF_MATCH));
for (final int chain: testChains) {
final int ruleToAddMatch = mBpfNetMaps.isFirewallAllowList(chain)
@@ -697,8 +698,8 @@
final int uid1 = TEST_UIDS[1];
final long match0 = POWERSAVE_MATCH;
final long match1 = POWERSAVE_MATCH | RESTRICTED_MATCH;
- mUidOwnerMap.updateEntry(new U32(uid0), new UidOwnerValue(NO_IIF, match0));
- mUidOwnerMap.updateEntry(new U32(uid1), new UidOwnerValue(NO_IIF, match1));
+ mUidOwnerMap.updateEntry(new S32(uid0), new UidOwnerValue(NO_IIF, match0));
+ mUidOwnerMap.updateEntry(new S32(uid1), new UidOwnerValue(NO_IIF, match1));
mBpfNetMaps.replaceUidChain(FIREWALL_CHAIN_DOZABLE, new int[]{uid1});
@@ -713,8 +714,8 @@
final int uid1 = TEST_UIDS[1];
final long match0 = IIF_MATCH;
final long match1 = IIF_MATCH | POWERSAVE_MATCH | RESTRICTED_MATCH;
- mUidOwnerMap.updateEntry(new U32(uid0), new UidOwnerValue(TEST_IF_INDEX, match0));
- mUidOwnerMap.updateEntry(new U32(uid1), new UidOwnerValue(NULL_IIF, match1));
+ mUidOwnerMap.updateEntry(new S32(uid0), new UidOwnerValue(TEST_IF_INDEX, match0));
+ mUidOwnerMap.updateEntry(new S32(uid1), new UidOwnerValue(NULL_IIF, match1));
mBpfNetMaps.replaceUidChain(FIREWALL_CHAIN_DOZABLE, TEST_UIDS);
@@ -729,8 +730,8 @@
final int uid1 = TEST_UIDS[1];
final long match0 = IIF_MATCH | DOZABLE_MATCH;
final long match1 = IIF_MATCH | POWERSAVE_MATCH | RESTRICTED_MATCH;
- mUidOwnerMap.updateEntry(new U32(uid0), new UidOwnerValue(TEST_IF_INDEX, match0));
- mUidOwnerMap.updateEntry(new U32(uid1), new UidOwnerValue(NULL_IIF, match1));
+ mUidOwnerMap.updateEntry(new S32(uid0), new UidOwnerValue(TEST_IF_INDEX, match0));
+ mUidOwnerMap.updateEntry(new S32(uid1), new UidOwnerValue(NULL_IIF, match1));
mBpfNetMaps.replaceUidChain(FIREWALL_CHAIN_DOZABLE, new int[]{uid1});
@@ -768,8 +769,8 @@
final int uid0 = TEST_UIDS[0];
final int uid1 = TEST_UIDS[1];
- assertEquals(PERMISSION_UPDATE_DEVICE_STATS, mUidPermissionMap.getValue(new U32(uid0)).val);
- assertEquals(PERMISSION_UPDATE_DEVICE_STATS, mUidPermissionMap.getValue(new U32(uid1)).val);
+ assertEquals(PERMISSION_UPDATE_DEVICE_STATS, mUidPermissionMap.getValue(new S32(uid0)).val);
+ assertEquals(PERMISSION_UPDATE_DEVICE_STATS, mUidPermissionMap.getValue(new S32(uid1)).val);
}
@Test
@@ -780,8 +781,8 @@
final int uid0 = TEST_UIDS[0];
final int uid1 = TEST_UIDS[1];
- assertEquals(permission, mUidPermissionMap.getValue(new U32(uid0)).val);
- assertEquals(permission, mUidPermissionMap.getValue(new U32(uid1)).val);
+ assertEquals(permission, mUidPermissionMap.getValue(new S32(uid0)).val);
+ assertEquals(permission, mUidPermissionMap.getValue(new S32(uid1)).val);
}
@Test
@@ -792,8 +793,8 @@
mBpfNetMaps.setNetPermForUids(PERMISSION_INTERNET, TEST_UIDS);
mBpfNetMaps.setNetPermForUids(PERMISSION_NONE, new int[]{uid0});
- assertEquals(PERMISSION_NONE, mUidPermissionMap.getValue(new U32(uid0)).val);
- assertNull(mUidPermissionMap.getValue(new U32(uid1)));
+ assertEquals(PERMISSION_NONE, mUidPermissionMap.getValue(new S32(uid0)).val);
+ assertNull(mUidPermissionMap.getValue(new S32(uid1)));
}
@Test
@@ -804,8 +805,8 @@
mBpfNetMaps.setNetPermForUids(PERMISSION_UPDATE_DEVICE_STATS, TEST_UIDS);
mBpfNetMaps.setNetPermForUids(PERMISSION_NONE, new int[]{uid0});
- assertEquals(PERMISSION_NONE, mUidPermissionMap.getValue(new U32(uid0)).val);
- assertEquals(PERMISSION_UPDATE_DEVICE_STATS, mUidPermissionMap.getValue(new U32(uid1)).val);
+ assertEquals(PERMISSION_NONE, mUidPermissionMap.getValue(new S32(uid0)).val);
+ assertEquals(PERMISSION_UPDATE_DEVICE_STATS, mUidPermissionMap.getValue(new S32(uid1)).val);
}
@Test
@@ -817,8 +818,8 @@
mBpfNetMaps.setNetPermForUids(permission, TEST_UIDS);
mBpfNetMaps.setNetPermForUids(PERMISSION_NONE, new int[]{uid0});
- assertEquals(PERMISSION_NONE, mUidPermissionMap.getValue(new U32(uid0)).val);
- assertEquals(permission, mUidPermissionMap.getValue(new U32(uid1)).val);
+ assertEquals(PERMISSION_NONE, mUidPermissionMap.getValue(new S32(uid0)).val);
+ assertEquals(permission, mUidPermissionMap.getValue(new S32(uid1)).val);
}
@Test
@@ -830,8 +831,8 @@
mBpfNetMaps.setNetPermForUids(permission, TEST_UIDS);
mBpfNetMaps.setNetPermForUids(PERMISSION_UNINSTALLED, new int[]{uid0});
- assertNull(mUidPermissionMap.getValue(new U32(uid0)));
- assertEquals(permission, mUidPermissionMap.getValue(new U32(uid1)).val);
+ assertNull(mUidPermissionMap.getValue(new S32(uid0)));
+ assertEquals(permission, mUidPermissionMap.getValue(new S32(uid1)).val);
}
@Test
@@ -842,28 +843,28 @@
final int permission = PERMISSION_INTERNET | PERMISSION_UPDATE_DEVICE_STATS;
mBpfNetMaps.setNetPermForUids(permission, TEST_UIDS);
- assertEquals(permission, mUidPermissionMap.getValue(new U32(uid0)).val);
- assertEquals(permission, mUidPermissionMap.getValue(new U32(uid1)).val);
+ assertEquals(permission, mUidPermissionMap.getValue(new S32(uid0)).val);
+ assertEquals(permission, mUidPermissionMap.getValue(new S32(uid1)).val);
mBpfNetMaps.setNetPermForUids(permission, TEST_UIDS);
- assertEquals(permission, mUidPermissionMap.getValue(new U32(uid0)).val);
- assertEquals(permission, mUidPermissionMap.getValue(new U32(uid1)).val);
+ assertEquals(permission, mUidPermissionMap.getValue(new S32(uid0)).val);
+ assertEquals(permission, mUidPermissionMap.getValue(new S32(uid1)).val);
mBpfNetMaps.setNetPermForUids(PERMISSION_NONE, TEST_UIDS);
- assertEquals(PERMISSION_NONE, mUidPermissionMap.getValue(new U32(uid0)).val);
- assertEquals(PERMISSION_NONE, mUidPermissionMap.getValue(new U32(uid1)).val);
+ assertEquals(PERMISSION_NONE, mUidPermissionMap.getValue(new S32(uid0)).val);
+ assertEquals(PERMISSION_NONE, mUidPermissionMap.getValue(new S32(uid1)).val);
mBpfNetMaps.setNetPermForUids(PERMISSION_NONE, TEST_UIDS);
- assertEquals(PERMISSION_NONE, mUidPermissionMap.getValue(new U32(uid0)).val);
- assertEquals(PERMISSION_NONE, mUidPermissionMap.getValue(new U32(uid1)).val);
+ assertEquals(PERMISSION_NONE, mUidPermissionMap.getValue(new S32(uid0)).val);
+ assertEquals(PERMISSION_NONE, mUidPermissionMap.getValue(new S32(uid1)).val);
mBpfNetMaps.setNetPermForUids(PERMISSION_UNINSTALLED, TEST_UIDS);
- assertNull(mUidPermissionMap.getValue(new U32(uid0)));
- assertNull(mUidPermissionMap.getValue(new U32(uid1)));
+ assertNull(mUidPermissionMap.getValue(new S32(uid0)));
+ assertNull(mUidPermissionMap.getValue(new S32(uid1)));
mBpfNetMaps.setNetPermForUids(PERMISSION_UNINSTALLED, TEST_UIDS);
- assertNull(mUidPermissionMap.getValue(new U32(uid0)));
- assertNull(mUidPermissionMap.getValue(new U32(uid1)));
+ assertNull(mUidPermissionMap.getValue(new S32(uid0)));
+ assertNull(mUidPermissionMap.getValue(new S32(uid1)));
}
@Test
@@ -898,13 +899,13 @@
mCookieTagMap.updateEntry(new CookieTagMapKey(0), new CookieTagMapValue(0, 0));
// mUidOwnerMap has 2 entries
- mUidOwnerMap.updateEntry(new U32(0), new UidOwnerValue(0, 0));
- mUidOwnerMap.updateEntry(new U32(1), new UidOwnerValue(0, 0));
+ mUidOwnerMap.updateEntry(new S32(0), new UidOwnerValue(0, 0));
+ mUidOwnerMap.updateEntry(new S32(1), new UidOwnerValue(0, 0));
// mUidPermissionMap has 3 entries
- mUidPermissionMap.updateEntry(new U32(0), new U8((short) 0));
- mUidPermissionMap.updateEntry(new U32(1), new U8((short) 0));
- mUidPermissionMap.updateEntry(new U32(2), new U8((short) 0));
+ mUidPermissionMap.updateEntry(new S32(0), new U8((short) 0));
+ mUidPermissionMap.updateEntry(new S32(1), new U8((short) 0));
+ mUidPermissionMap.updateEntry(new S32(2), new U8((short) 0));
final int ret = mBpfNetMaps.pullBpfMapInfoAtom(NETWORK_BPF_MAP_INFO, new ArrayList<>());
assertEquals(StatsManager.PULL_SUCCESS, ret);