BpfHandler - eliminate mutex
since it doesn't actually work (this runs in netd, but is meant
to protect against system server modifications)
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: I87f268f1a68c559b19caa4d625cb9eb8395c91e2
diff --git a/netd/BpfHandler.cpp b/netd/BpfHandler.cpp
index 994db1d..3f7ed2a 100644
--- a/netd/BpfHandler.cpp
+++ b/netd/BpfHandler.cpp
@@ -110,12 +110,12 @@
}
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 @@
}
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 @@
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<StatsKey, StatsValue>&) {
@@ -227,9 +226,9 @@
}
// 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::untagSocket(int sockFd) {
- std::lock_guard guard(mMutex);
-
uint64_t sock_cookie = getSocketCookie(sockFd);
if (sock_cookie == NONEXISTENT_COOKIE) return -errno;