diff --git a/service/jni/com_android_server_BpfNetMaps.cpp b/service/jni/com_android_server_BpfNetMaps.cpp index 2aaa4c3b8a..c29eb2bc7d 100644 --- a/service/jni/com_android_server_BpfNetMaps.cpp +++ b/service/jni/com_android_server_BpfNetMaps.cpp @@ -202,24 +202,6 @@ static void native_setPermissionForUids(JNIEnv* env, jobject clazz, jint permiss mTc.setPermissionForUids(permission, data); } -static jint native_setCounterSet(JNIEnv* env, jobject clazz, jint setNum, jint uid) { - uid_t callingUid = getuid(); - int res = mTc.setCounterSet(setNum, (uid_t)uid, callingUid); - if (res) { - ALOGE("%s failed, error code = %d", __func__, res); - } - return (jint)res; -} - -static jint native_deleteTagData(JNIEnv* env, jobject clazz, jint tagNum, jint uid) { - uid_t callingUid = getuid(); - int res = mTc.deleteTagData(tagNum, (uid_t)uid, callingUid); - if (res) { - ALOGE("%s failed, error code = %d", __func__, res); - } - return (jint)res; -} - /* * JNI registration. */ @@ -250,10 +232,6 @@ static const JNINativeMethod gMethods[] = { (void*)native_swapActiveStatsMap}, {"native_setPermissionForUids", "(I[I)V", (void*)native_setPermissionForUids}, - {"native_setCounterSet", "(II)I", - (void*)native_setCounterSet}, - {"native_deleteTagData", "(II)I", - (void*)native_deleteTagData}, }; // clang-format on diff --git a/service/native/TrafficController.cpp b/service/native/TrafficController.cpp index 5981906fc3..1cbfd947b6 100644 --- a/service/native/TrafficController.cpp +++ b/service/native/TrafficController.cpp @@ -54,7 +54,6 @@ namespace net { using base::StringPrintf; using base::unique_fd; using bpf::BpfMap; -using bpf::OVERFLOW_COUNTERSET; using bpf::synchronizeKernelRCU; using netdutils::DumpWriter; using netdutils::getIfaceList; @@ -239,99 +238,6 @@ Status TrafficController::start() { return netdutils::status::ok; } -int TrafficController::setCounterSet(int counterSetNum, uid_t uid, uid_t callingUid) { - if (counterSetNum < 0 || counterSetNum >= OVERFLOW_COUNTERSET) return -EINVAL; - - std::lock_guard guard(mMutex); - if (!hasUpdateDeviceStatsPermission(callingUid)) return -EPERM; - - // The default counter set for all uid is 0, so deleting the current counterset for that uid - // will automatically set it to 0. - if (counterSetNum == 0) { - Status res = mUidCounterSetMap.deleteValue(uid); - if (isOk(res) || (!isOk(res) && res.code() == ENOENT)) { - return 0; - } else { - ALOGE("Failed to delete the counterSet: %s\n", strerror(res.code())); - return -res.code(); - } - } - uint8_t tmpCounterSetNum = (uint8_t)counterSetNum; - Status res = mUidCounterSetMap.writeValue(uid, tmpCounterSetNum, BPF_ANY); - if (!isOk(res)) { - ALOGE("Failed to set the counterSet: %s, fd: %d", strerror(res.code()), - mUidCounterSetMap.getMap().get()); - return -res.code(); - } - return 0; -} - -// This method only get called by system_server when an app get uinstalled, it -// is called inside removeUidsLocked() while holding mStatsLock. So it is safe -// to iterate and modify the stats maps. -int TrafficController::deleteTagData(uint32_t tag, uid_t uid, uid_t callingUid) { - std::lock_guard guard(mMutex); - if (!hasUpdateDeviceStatsPermission(callingUid)) return -EPERM; - - // First we go through the cookieTagMap to delete the target uid tag combination. Or delete all - // the tags related to the uid if the tag is 0. - const auto deleteMatchedCookieEntries = [uid, tag](const uint64_t& key, - const UidTagValue& value, - BpfMap& map) { - if (value.uid == uid && (value.tag == tag || tag == 0)) { - auto res = map.deleteValue(key); - if (res.ok() || (res.error().code() == ENOENT)) { - return base::Result(); - } - ALOGE("Failed to delete data(cookie = %" PRIu64 "): %s\n", key, - strerror(res.error().code())); - } - // Move forward to next cookie in the map. - return base::Result(); - }; - mCookieTagMap.iterateWithValue(deleteMatchedCookieEntries); - // Now we go through the Tag stats map and delete the data entry with correct uid and tag - // combination. Or all tag stats under that uid if the target tag is 0. - const auto deleteMatchedUidTagEntries = [uid, tag](const StatsKey& key, - BpfMap& map) { - if (key.uid == uid && (key.tag == tag || tag == 0)) { - auto res = map.deleteValue(key); - if (res.ok() || (res.error().code() == ENOENT)) { - //Entry is deleted, use the current key to get a new nextKey; - return base::Result(); - } - ALOGE("Failed to delete data(uid=%u, tag=%u): %s\n", key.uid, key.tag, - strerror(res.error().code())); - } - return base::Result(); - }; - mStatsMapB.iterate(deleteMatchedUidTagEntries); - mStatsMapA.iterate(deleteMatchedUidTagEntries); - // If the tag is not zero, we already deleted all the data entry required. If tag is 0, we also - // need to delete the stats stored in uidStatsMap and counterSet map. - if (tag != 0) return 0; - - auto res = mUidCounterSetMap.deleteValue(uid); - if (!res.ok() && res.error().code() != ENOENT) { - ALOGE("Failed to delete counterSet data(uid=%u, tag=%u): %s\n", uid, tag, - strerror(res.error().code())); - } - - auto deleteAppUidStatsEntry = [uid](const uint32_t& key, - BpfMap& map) -> base::Result { - if (key == uid) { - auto res = map.deleteValue(key); - if (res.ok() || (res.error().code() == ENOENT)) { - return {}; - } - ALOGE("Failed to delete data(uid=%u): %s", key, strerror(res.error().code())); - } - return {}; - }; - mAppUidStatsMap.iterate(deleteAppUidStatsEntry); - return 0; -} - int TrafficController::addInterface(const char* name, uint32_t ifaceIndex) { IfaceValue iface; if (ifaceIndex == 0) { diff --git a/service/native/TrafficControllerTest.cpp b/service/native/TrafficControllerTest.cpp index d0eca34e24..9529caee48 100644 --- a/service/native/TrafficControllerTest.cpp +++ b/service/native/TrafficControllerTest.cpp @@ -55,7 +55,6 @@ constexpr uid_t TEST_UID2 = 54321; constexpr uid_t TEST_UID3 = 98765; constexpr uint32_t TEST_TAG = 42; constexpr uint32_t TEST_COUNTERSET = 1; -constexpr uint32_t DEFAULT_COUNTERSET = 0; #define ASSERT_VALID(x) ASSERT_TRUE((x).isValid()) @@ -64,7 +63,6 @@ class TrafficControllerTest : public ::testing::Test { TrafficControllerTest() {} TrafficController mTc; BpfMap mFakeCookieTagMap; - BpfMap mFakeUidCounterSetMap; BpfMap mFakeAppUidStatsMap; BpfMap mFakeStatsMapA; BpfMap mFakeConfigurationMap; @@ -79,10 +77,6 @@ class TrafficControllerTest : public ::testing::Test { TEST_MAP_SIZE, 0)); ASSERT_VALID(mFakeCookieTagMap); - mFakeUidCounterSetMap.reset( - createMap(BPF_MAP_TYPE_HASH, sizeof(uint32_t), sizeof(uint8_t), TEST_MAP_SIZE, 0)); - ASSERT_VALID(mFakeUidCounterSetMap); - mFakeAppUidStatsMap.reset(createMap(BPF_MAP_TYPE_HASH, sizeof(uint32_t), sizeof(StatsValue), TEST_MAP_SIZE, 0)); ASSERT_VALID(mFakeAppUidStatsMap); @@ -104,8 +98,6 @@ class TrafficControllerTest : public ::testing::Test { mTc.mCookieTagMap.reset(dupFd(mFakeCookieTagMap.getMap())); ASSERT_VALID(mTc.mCookieTagMap); - mTc.mUidCounterSetMap.reset(dupFd(mFakeUidCounterSetMap.getMap())); - ASSERT_VALID(mTc.mUidCounterSetMap); mTc.mAppUidStatsMap.reset(dupFd(mFakeAppUidStatsMap.getMap())); ASSERT_VALID(mTc.mAppUidStatsMap); mTc.mStatsMapA.reset(dupFd(mFakeStatsMapA.getMap())); @@ -132,8 +124,6 @@ class TrafficControllerTest : public ::testing::Test { EXPECT_RESULT_OK(mFakeCookieTagMap.writeValue(cookie, cookieMapkey, BPF_ANY)); *key = {.uid = uid, .tag = tag, .counterSet = TEST_COUNTERSET, .ifaceIndex = 1}; StatsValue statsMapValue = {.rxPackets = 1, .rxBytes = 100}; - uint8_t counterSet = TEST_COUNTERSET; - EXPECT_RESULT_OK(mFakeUidCounterSetMap.writeValue(uid, counterSet, BPF_ANY)); EXPECT_RESULT_OK(mFakeStatsMapA.writeValue(*key, statsMapValue, BPF_ANY)); key->tag = 0; EXPECT_RESULT_OK(mFakeStatsMapA.writeValue(*key, statsMapValue, BPF_ANY)); @@ -259,9 +249,6 @@ class TrafficControllerTest : public ::testing::Test { EXPECT_RESULT_OK(cookieMapResult); EXPECT_EQ(uid, cookieMapResult.value().uid); EXPECT_EQ(tag, cookieMapResult.value().tag); - Result counterSetResult = mFakeUidCounterSetMap.readValue(uid); - EXPECT_RESULT_OK(counterSetResult); - EXPECT_EQ(TEST_COUNTERSET, counterSetResult.value()); Result statsMapResult = mFakeStatsMapA.readValue(tagStatsMapKey); EXPECT_RESULT_OK(statsMapResult); EXPECT_EQ((uint64_t)1, statsMapResult.value().rxPackets); @@ -289,157 +276,6 @@ class TrafficControllerTest : public ::testing::Test { }; -TEST_F(TrafficControllerTest, TestSetCounterSet) { - uid_t callingUid = TEST_UID2; - addPrivilegedUid(callingUid); - ASSERT_EQ(0, mTc.setCounterSet(TEST_COUNTERSET, TEST_UID, callingUid)); - uid_t uid = TEST_UID; - Result counterSetResult = mFakeUidCounterSetMap.readValue(uid); - ASSERT_RESULT_OK(counterSetResult); - ASSERT_EQ(TEST_COUNTERSET, counterSetResult.value()); - ASSERT_EQ(0, mTc.setCounterSet(DEFAULT_COUNTERSET, TEST_UID, callingUid)); - ASSERT_FALSE(mFakeUidCounterSetMap.readValue(uid).ok()); - expectMapEmpty(mFakeUidCounterSetMap); -} - -TEST_F(TrafficControllerTest, TestSetCounterSetWithoutPermission) { - ASSERT_EQ(-EPERM, mTc.setCounterSet(TEST_COUNTERSET, TEST_UID, TEST_UID2)); - uid_t uid = TEST_UID; - ASSERT_FALSE(mFakeUidCounterSetMap.readValue(uid).ok()); - expectMapEmpty(mFakeUidCounterSetMap); -} - -TEST_F(TrafficControllerTest, TestSetInvalidCounterSet) { - uid_t callingUid = TEST_UID2; - addPrivilegedUid(callingUid); - ASSERT_GT(0, mTc.setCounterSet(OVERFLOW_COUNTERSET, TEST_UID, callingUid)); - uid_t uid = TEST_UID; - ASSERT_FALSE(mFakeUidCounterSetMap.readValue(uid).ok()); - expectMapEmpty(mFakeUidCounterSetMap); -} - -TEST_F(TrafficControllerTest, TestDeleteTagDataWithoutPermission) { - uint64_t cookie = 1; - uid_t uid = TEST_UID; - uint32_t tag = TEST_TAG; - StatsKey tagStatsMapKey; - populateFakeStats(cookie, uid, tag, &tagStatsMapKey); - ASSERT_EQ(-EPERM, mTc.deleteTagData(0, TEST_UID, TEST_UID2)); - - expectFakeStatsUnchanged(cookie, tag, uid, tagStatsMapKey); -} - -TEST_F(TrafficControllerTest, TestDeleteTagData) { - uid_t callingUid = TEST_UID2; - addPrivilegedUid(callingUid); - uint64_t cookie = 1; - uid_t uid = TEST_UID; - uint32_t tag = TEST_TAG; - StatsKey tagStatsMapKey; - populateFakeStats(cookie, uid, tag, &tagStatsMapKey); - ASSERT_EQ(0, mTc.deleteTagData(TEST_TAG, TEST_UID, callingUid)); - ASSERT_FALSE(mFakeCookieTagMap.readValue(cookie).ok()); - Result counterSetResult = mFakeUidCounterSetMap.readValue(uid); - ASSERT_RESULT_OK(counterSetResult); - ASSERT_EQ(TEST_COUNTERSET, counterSetResult.value()); - ASSERT_FALSE(mFakeStatsMapA.readValue(tagStatsMapKey).ok()); - tagStatsMapKey.tag = 0; - Result statsMapResult = mFakeStatsMapA.readValue(tagStatsMapKey); - ASSERT_RESULT_OK(statsMapResult); - ASSERT_EQ((uint64_t)1, statsMapResult.value().rxPackets); - ASSERT_EQ((uint64_t)100, statsMapResult.value().rxBytes); - auto appStatsResult = mFakeAppUidStatsMap.readValue(TEST_UID); - ASSERT_RESULT_OK(appStatsResult); - ASSERT_EQ((uint64_t)1, appStatsResult.value().rxPackets); - ASSERT_EQ((uint64_t)100, appStatsResult.value().rxBytes); -} - -TEST_F(TrafficControllerTest, TestDeleteAllUidData) { - uid_t callingUid = TEST_UID2; - addPrivilegedUid(callingUid); - uint64_t cookie = 1; - uid_t uid = TEST_UID; - uint32_t tag = TEST_TAG; - StatsKey tagStatsMapKey; - populateFakeStats(cookie, uid, tag, &tagStatsMapKey); - ASSERT_EQ(0, mTc.deleteTagData(0, TEST_UID, callingUid)); - ASSERT_FALSE(mFakeCookieTagMap.readValue(cookie).ok()); - ASSERT_FALSE(mFakeUidCounterSetMap.readValue(uid).ok()); - ASSERT_FALSE(mFakeStatsMapA.readValue(tagStatsMapKey).ok()); - tagStatsMapKey.tag = 0; - ASSERT_FALSE(mFakeStatsMapA.readValue(tagStatsMapKey).ok()); - ASSERT_FALSE(mFakeAppUidStatsMap.readValue(TEST_UID).ok()); -} - -TEST_F(TrafficControllerTest, TestDeleteDataWithTwoTags) { - uid_t callingUid = TEST_UID2; - addPrivilegedUid(callingUid); - uint64_t cookie1 = 1; - uint64_t cookie2 = 2; - uid_t uid = TEST_UID; - uint32_t tag1 = TEST_TAG; - uint32_t tag2 = TEST_TAG + 1; - StatsKey tagStatsMapKey1; - StatsKey tagStatsMapKey2; - populateFakeStats(cookie1, uid, tag1, &tagStatsMapKey1); - populateFakeStats(cookie2, uid, tag2, &tagStatsMapKey2); - ASSERT_EQ(0, mTc.deleteTagData(TEST_TAG, TEST_UID, callingUid)); - ASSERT_FALSE(mFakeCookieTagMap.readValue(cookie1).ok()); - Result cookieMapResult = mFakeCookieTagMap.readValue(cookie2); - ASSERT_RESULT_OK(cookieMapResult); - ASSERT_EQ(TEST_UID, cookieMapResult.value().uid); - ASSERT_EQ(TEST_TAG + 1, cookieMapResult.value().tag); - Result counterSetResult = mFakeUidCounterSetMap.readValue(uid); - ASSERT_RESULT_OK(counterSetResult); - ASSERT_EQ(TEST_COUNTERSET, counterSetResult.value()); - ASSERT_FALSE(mFakeStatsMapA.readValue(tagStatsMapKey1).ok()); - Result statsMapResult = mFakeStatsMapA.readValue(tagStatsMapKey2); - ASSERT_RESULT_OK(statsMapResult); - ASSERT_EQ((uint64_t)1, statsMapResult.value().rxPackets); - ASSERT_EQ((uint64_t)100, statsMapResult.value().rxBytes); -} - -TEST_F(TrafficControllerTest, TestDeleteDataWithTwoUids) { - uid_t callingUid = TEST_UID2; - addPrivilegedUid(callingUid); - uint64_t cookie1 = 1; - uint64_t cookie2 = 2; - uid_t uid1 = TEST_UID; - uid_t uid2 = TEST_UID + 1; - uint32_t tag = TEST_TAG; - StatsKey tagStatsMapKey1; - StatsKey tagStatsMapKey2; - populateFakeStats(cookie1, uid1, tag, &tagStatsMapKey1); - populateFakeStats(cookie2, uid2, tag, &tagStatsMapKey2); - - // Delete the stats of one of the uid. Check if it is properly collected by - // removedStats. - ASSERT_EQ(0, mTc.deleteTagData(0, uid2, callingUid)); - ASSERT_FALSE(mFakeCookieTagMap.readValue(cookie2).ok()); - Result counterSetResult = mFakeUidCounterSetMap.readValue(uid1); - ASSERT_RESULT_OK(counterSetResult); - ASSERT_EQ(TEST_COUNTERSET, counterSetResult.value()); - ASSERT_FALSE(mFakeUidCounterSetMap.readValue(uid2).ok()); - ASSERT_FALSE(mFakeStatsMapA.readValue(tagStatsMapKey2).ok()); - tagStatsMapKey2.tag = 0; - ASSERT_FALSE(mFakeStatsMapA.readValue(tagStatsMapKey2).ok()); - ASSERT_FALSE(mFakeAppUidStatsMap.readValue(uid2).ok()); - tagStatsMapKey1.tag = 0; - Result statsMapResult = mFakeStatsMapA.readValue(tagStatsMapKey1); - ASSERT_RESULT_OK(statsMapResult); - ASSERT_EQ((uint64_t)1, statsMapResult.value().rxPackets); - ASSERT_EQ((uint64_t)100, statsMapResult.value().rxBytes); - auto appStatsResult = mFakeAppUidStatsMap.readValue(uid1); - ASSERT_RESULT_OK(appStatsResult); - ASSERT_EQ((uint64_t)1, appStatsResult.value().rxPackets); - ASSERT_EQ((uint64_t)100, appStatsResult.value().rxBytes); - - // Delete the stats of the other uid. - ASSERT_EQ(0, mTc.deleteTagData(0, uid1, callingUid)); - ASSERT_FALSE(mFakeStatsMapA.readValue(tagStatsMapKey1).ok()); - ASSERT_FALSE(mFakeAppUidStatsMap.readValue(uid1).ok()); -} - TEST_F(TrafficControllerTest, TestUpdateOwnerMapEntry) { uint32_t uid = TEST_UID; ASSERT_TRUE(isOk(mTc.updateOwnerMapEntry(STANDBY_MATCH, uid, DENY, DENYLIST))); diff --git a/service/native/include/TrafficController.h b/service/native/include/TrafficController.h index e741dd6f22..6fe117f578 100644 --- a/service/native/include/TrafficController.h +++ b/service/native/include/TrafficController.h @@ -40,18 +40,6 @@ class TrafficController { */ netdutils::Status start(); - int setCounterSet(int counterSetNum, uid_t uid, uid_t callingUid) EXCLUDES(mMutex); - - /* - * When deleting a tag data, the qtaguid module will grab the spinlock of each - * related rb_tree one by one and delete the tag information, counterSet - * information, iface stats information and uid stats information one by one. - * The new eBPF implementation is done similiarly by removing the entry on - * each map one by one. And deleting processes are also protected by the - * spinlock of the map. So no additional lock is required. - */ - int deleteTagData(uint32_t tag, uid_t uid, uid_t callingUid) EXCLUDES(mMutex); - /* * Swap the stats map config from current active stats map to the idle one. */ diff --git a/service/src/com/android/server/BpfNetMaps.java b/service/src/com/android/server/BpfNetMaps.java index 7a3bab3719..ddee275d7a 100644 --- a/service/src/com/android/server/BpfNetMaps.java +++ b/service/src/com/android/server/BpfNetMaps.java @@ -273,32 +273,6 @@ public class BpfNetMaps { native_setPermissionForUids(permissions, uids); } - /** - * Set counter set for uid - * - * @param counterSet either SET_DEFAULT or SET_FOREGROUND - * @param uid uid to foreground/background - * @throws ServiceSpecificException in case of failure, with an error code indicating the - * cause of the failure. - */ - public void setCounterSet(final int counterSet, final int uid) { - final int err = native_setCounterSet(counterSet, uid); - maybeThrow(err, "setCounterSet failed"); - } - - /** - * Reset Uid stats - * - * @param tag default 0 - * @param uid given uid to be clear - * @throws ServiceSpecificException in case of failure, with an error code indicating the - * cause of the failure. - */ - public void deleteTagData(final int tag, final int uid) { - final int err = native_deleteTagData(tag, uid); - maybeThrow(err, "deleteTagData failed"); - } - private static native void native_init(); private native int native_addNaughtyApp(int uid); private native int native_removeNaughtyApp(int uid); @@ -311,6 +285,4 @@ public class BpfNetMaps { private native int native_removeUidInterfaceRules(int[] uids); private native int native_swapActiveStatsMap(); private native void native_setPermissionForUids(int permissions, int[] uids); - private native int native_setCounterSet(int counterSet, int uid); - private native int native_deleteTagData(int tag, int uid); }