diff --git a/netd/BpfHandler.cpp b/netd/BpfHandler.cpp index 994db1d516..3f7ed2a8b1 100644 --- a/netd/BpfHandler.cpp +++ b/netd/BpfHandler.cpp @@ -110,12 +110,12 @@ Status BpfHandler::init(const char* cg2_path) { } Status BpfHandler::initMaps() { - std::lock_guard guard(mMutex); - RETURN_IF_NOT_OK(mCookieTagMap.init(COOKIE_TAG_MAP_PATH)); RETURN_IF_NOT_OK(mStatsMapA.init(STATS_MAP_A_PATH)); RETURN_IF_NOT_OK(mStatsMapB.init(STATS_MAP_B_PATH)); RETURN_IF_NOT_OK(mConfigurationMap.init(CONFIGURATION_MAP_PATH)); RETURN_IF_NOT_OK(mUidPermissionMap.init(UID_PERMISSION_MAP_PATH)); + // initialized last so mCookieTagMap.isValid() implies everything else is valid too + RETURN_IF_NOT_OK(mCookieTagMap.init(COOKIE_TAG_MAP_PATH)); ALOGI("%s successfully", __func__); return netdutils::status::ok; @@ -133,7 +133,6 @@ bool BpfHandler::hasUpdateDeviceStatsPermission(uid_t uid) { } int BpfHandler::tagSocket(int sockFd, uint32_t tag, uid_t chargeUid, uid_t realUid) { - std::lock_guard guard(mMutex); if (!mCookieTagMap.isValid()) return -EPERM; if (chargeUid != realUid && !hasUpdateDeviceStatsPermission(realUid)) return -EPERM; @@ -185,9 +184,9 @@ int BpfHandler::tagSocket(int sockFd, uint32_t tag, uid_t chargeUid, uid_t realU uint32_t perUidEntryCount = 0; // Now we go through the stats map and count how many entries are associated // with chargeUid. If the uid entry hit the limit for each chargeUid, we block - // the request to prevent the map from overflow. It is safe here to iterate - // over the map since when mMutex is hold, system server cannot toggle - // the live stats map and clean it. So nobody can delete entries from the map. + // the request to prevent the map from overflow. Note though that it isn't really + // safe here to iterate over the map since it might be modified by the system server, + // which might toggle the live stats map and clean it. const auto countUidStatsEntries = [chargeUid, &totalEntryCount, &perUidEntryCount]( const StatsKey& key, const BpfMap&) { @@ -227,9 +226,9 @@ int BpfHandler::tagSocket(int sockFd, uint32_t tag, uid_t chargeUid, uid_t realU } // Update the tag information of a socket to the cookieUidMap. Use BPF_ANY // flag so it will insert a new entry to the map if that value doesn't exist - // yet. And update the tag if there is already a tag stored. Since the eBPF + // yet and update the tag if there is already a tag stored. Since the eBPF // program in kernel only read this map, and is protected by rcu read lock. It - // should be fine to cocurrently update the map while eBPF program is running. + // should be fine to concurrently update the map while eBPF program is running. res = mCookieTagMap.writeValue(sock_cookie, newKey, BPF_ANY); if (!res.ok()) { ALOGE("Failed to tag the socket: %s, fd: %d", strerror(res.error().code()), @@ -240,8 +239,6 @@ int BpfHandler::tagSocket(int sockFd, uint32_t tag, uid_t chargeUid, uid_t realU } int BpfHandler::untagSocket(int sockFd) { - std::lock_guard guard(mMutex); - uint64_t sock_cookie = getSocketCookie(sockFd); if (sock_cookie == NONEXISTENT_COOKIE) return -errno; diff --git a/netd/BpfHandler.h b/netd/BpfHandler.h index 5ee04d1a53..925a725ac7 100644 --- a/netd/BpfHandler.h +++ b/netd/BpfHandler.h @@ -16,8 +16,6 @@ #pragma once -#include - #include #include "bpf/BpfMap.h" #include "bpf_shared.h" @@ -66,8 +64,6 @@ class BpfHandler { BpfMapRO mConfigurationMap; BpfMap mUidPermissionMap; - std::mutex mMutex; - // The limit on the number of stats entries a uid can have in the per uid stats map. BpfHandler // will block that specific uid from tagging new sockets after the limit is reached. const uint32_t mPerUidStatsEntriesLimit; diff --git a/netd/BpfHandlerTest.cpp b/netd/BpfHandlerTest.cpp index 99160daca1..f5c9a68b2c 100644 --- a/netd/BpfHandlerTest.cpp +++ b/netd/BpfHandlerTest.cpp @@ -53,7 +53,6 @@ class BpfHandlerTest : public ::testing::Test { BpfMap mFakeUidPermissionMap; void SetUp() { - std::lock_guard guard(mBh.mMutex); ASSERT_EQ(0, setrlimitForTest()); mFakeCookieTagMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE);