From 7e2f53e7620c3725436ed8fc601e8d33c8e54c85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Thu, 28 Sep 2023 01:08:28 +0000 Subject: [PATCH] reverse BpfMap & BpfMapRO inheritence to make sense MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: 235590615 Bug: 235907076 Bug: 286003437 Test: TreeHugger, m droid gpuservice_unittest libtimeinstate_test bpf_benchmark bpf_module_test libbpf_load_test && mma Signed-off-by: Maciej Żenczykowski Change-Id: I020a898fe8b257040a100f146654d4a04b19f843 --- bpf_progs/netd.c | 4 +- netd/BpfHandler.cpp | 5 +- netd/BpfHandler.h | 4 +- netd/BpfHandlerTest.cpp | 2 +- .../libs/libnetworkstats/BpfNetworkStats.cpp | 22 +- .../libnetworkstats/BpfNetworkStatsTest.cpp | 10 +- .../include/netdbpf/BpfNetworkStats.h | 23 +- staticlibs/native/bpf_headers/BpfMapTest.cpp | 27 +- .../native/bpf_headers/include/bpf/BpfMap.h | 315 ++++++++++-------- 9 files changed, 235 insertions(+), 177 deletions(-) diff --git a/bpf_progs/netd.c b/bpf_progs/netd.c index 0054d4aea6..f86d8fd3c6 100644 --- a/bpf_progs/netd.c +++ b/bpf_progs/netd.c @@ -89,11 +89,11 @@ DEFINE_BPF_MAP_RO_NETD(configuration_map, ARRAY, uint32_t, uint32_t, CONFIGURATI DEFINE_BPF_MAP_RW_NETD(cookie_tag_map, HASH, uint64_t, UidTagValue, COOKIE_UID_MAP_SIZE) DEFINE_BPF_MAP_NO_NETD(uid_counterset_map, HASH, uint32_t, uint8_t, UID_COUNTERSET_MAP_SIZE) DEFINE_BPF_MAP_NO_NETD(app_uid_stats_map, HASH, uint32_t, StatsValue, APP_STATS_MAP_SIZE) -DEFINE_BPF_MAP_RW_NETD(stats_map_A, HASH, StatsKey, StatsValue, STATS_MAP_SIZE) +DEFINE_BPF_MAP_RO_NETD(stats_map_A, HASH, StatsKey, StatsValue, STATS_MAP_SIZE) DEFINE_BPF_MAP_RO_NETD(stats_map_B, HASH, StatsKey, StatsValue, STATS_MAP_SIZE) DEFINE_BPF_MAP_NO_NETD(iface_stats_map, HASH, uint32_t, StatsValue, IFACE_STATS_MAP_SIZE) DEFINE_BPF_MAP_NO_NETD(uid_owner_map, HASH, uint32_t, UidOwnerValue, UID_OWNER_MAP_SIZE) -DEFINE_BPF_MAP_RW_NETD(uid_permission_map, HASH, uint32_t, uint8_t, UID_OWNER_MAP_SIZE) +DEFINE_BPF_MAP_RO_NETD(uid_permission_map, HASH, uint32_t, uint8_t, UID_OWNER_MAP_SIZE) DEFINE_BPF_MAP_NO_NETD(ingress_discard_map, HASH, IngressDiscardKey, IngressDiscardValue, INGRESS_DISCARD_MAP_SIZE) diff --git a/netd/BpfHandler.cpp b/netd/BpfHandler.cpp index fc680d9712..fb6d5b8f27 100644 --- a/netd/BpfHandler.cpp +++ b/netd/BpfHandler.cpp @@ -214,7 +214,7 @@ int BpfHandler::tagSocket(int sockFd, uint32_t tag, uid_t chargeUid, uid_t realU // which might toggle the live stats map and clean it. const auto countUidStatsEntries = [chargeUid, &totalEntryCount, &perUidEntryCount]( const StatsKey& key, - const BpfMap&) { + const BpfMapRO&) { if (key.uid == chargeUid) { perUidEntryCount++; } @@ -232,9 +232,8 @@ int BpfHandler::tagSocket(int sockFd, uint32_t tag, uid_t chargeUid, uid_t realU return -EINVAL; } - BpfMap& currentMap = + BpfMapRO& currentMap = (configuration.value() == SELECT_MAP_A) ? mStatsMapA : mStatsMapB; - // HACK: mStatsMapB becomes RW BpfMap here, but countUidStatsEntries doesn't modify so it works base::Result res = currentMap.iterate(countUidStatsEntries); if (!res.ok()) { ALOGE("Failed to count the stats entry in map: %s", diff --git a/netd/BpfHandler.h b/netd/BpfHandler.h index a6da4eb712..9e69efc42f 100644 --- a/netd/BpfHandler.h +++ b/netd/BpfHandler.h @@ -59,10 +59,10 @@ class BpfHandler { bool hasUpdateDeviceStatsPermission(uid_t uid); BpfMap mCookieTagMap; - BpfMap mStatsMapA; + BpfMapRO mStatsMapA; BpfMapRO mStatsMapB; BpfMapRO mConfigurationMap; - BpfMap mUidPermissionMap; + BpfMapRO mUidPermissionMap; // 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. diff --git a/netd/BpfHandlerTest.cpp b/netd/BpfHandlerTest.cpp index f5c9a68b2c..b38fa16210 100644 --- a/netd/BpfHandlerTest.cpp +++ b/netd/BpfHandlerTest.cpp @@ -49,7 +49,7 @@ class BpfHandlerTest : public ::testing::Test { BpfHandler mBh; BpfMap mFakeCookieTagMap; BpfMap mFakeStatsMapA; - BpfMapRO mFakeConfigurationMap; + BpfMap mFakeConfigurationMap; BpfMap mFakeUidPermissionMap; void SetUp() { diff --git a/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp b/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp index fed297919d..31013972b8 100644 --- a/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp +++ b/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp @@ -41,7 +41,7 @@ namespace bpf { using base::Result; int bpfGetUidStatsInternal(uid_t uid, StatsValue* stats, - const BpfMap& appUidStatsMap) { + const BpfMapRO& appUidStatsMap) { auto statsEntry = appUidStatsMap.readValue(uid); if (!statsEntry.ok()) { *stats = {}; @@ -57,14 +57,14 @@ int bpfGetUidStats(uid_t uid, StatsValue* stats) { } int bpfGetIfaceStatsInternal(const char* iface, StatsValue* stats, - const BpfMap& ifaceStatsMap, - const BpfMap& ifaceNameMap) { + const BpfMapRO& ifaceStatsMap, + const BpfMapRO& ifaceNameMap) { *stats = {}; int64_t unknownIfaceBytesTotal = 0; const auto processIfaceStats = [iface, stats, &ifaceNameMap, &unknownIfaceBytesTotal]( const uint32_t& key, - const BpfMap& ifaceStatsMap) -> Result { + const BpfMapRO& ifaceStatsMap) -> Result { char ifname[IFNAMSIZ]; if (getIfaceNameFromMap(ifaceNameMap, ifaceStatsMap, key, ifname, key, &unknownIfaceBytesTotal)) { @@ -90,7 +90,7 @@ int bpfGetIfaceStats(const char* iface, StatsValue* stats) { } int bpfGetIfIndexStatsInternal(uint32_t ifindex, StatsValue* stats, - const BpfMap& ifaceStatsMap) { + const BpfMapRO& ifaceStatsMap) { auto statsEntry = ifaceStatsMap.readValue(ifindex); if (!statsEntry.ok()) { *stats = {}; @@ -120,13 +120,13 @@ stats_line populateStatsEntry(const StatsKey& statsKey, const StatsValue& statsE } int parseBpfNetworkStatsDetailInternal(std::vector& lines, - const BpfMap& statsMap, - const BpfMap& ifaceMap) { + const BpfMapRO& statsMap, + const BpfMapRO& ifaceMap) { int64_t unknownIfaceBytesTotal = 0; const auto processDetailUidStats = [&lines, &unknownIfaceBytesTotal, &ifaceMap]( const StatsKey& key, - const BpfMap& statsMap) -> Result { + const BpfMapRO& statsMap) -> Result { char ifname[IFNAMSIZ]; if (getIfaceNameFromMap(ifaceMap, statsMap, key.ifaceIndex, ifname, key, &unknownIfaceBytesTotal)) { @@ -212,12 +212,12 @@ int parseBpfNetworkStatsDetail(std::vector* lines) { } int parseBpfNetworkStatsDevInternal(std::vector& lines, - const BpfMap& statsMap, - const BpfMap& ifaceMap) { + const BpfMapRO& statsMap, + const BpfMapRO& ifaceMap) { int64_t unknownIfaceBytesTotal = 0; const auto processDetailIfaceStats = [&lines, &unknownIfaceBytesTotal, &ifaceMap, &statsMap]( const uint32_t& key, const StatsValue& value, - const BpfMap&) { + const BpfMapRO&) { char ifname[IFNAMSIZ]; if (getIfaceNameFromMap(ifaceMap, statsMap, key, ifname, key, &unknownIfaceBytesTotal)) { return Result(); diff --git a/service-t/native/libs/libnetworkstats/BpfNetworkStatsTest.cpp b/service-t/native/libs/libnetworkstats/BpfNetworkStatsTest.cpp index 76c56eb5ff..bcc455069c 100644 --- a/service-t/native/libs/libnetworkstats/BpfNetworkStatsTest.cpp +++ b/service-t/native/libs/libnetworkstats/BpfNetworkStatsTest.cpp @@ -80,19 +80,19 @@ class BpfNetworkStatsHelperTest : public testing::Test { void SetUp() { ASSERT_EQ(0, setrlimitForTest()); - mFakeCookieTagMap = BpfMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, 0); + mFakeCookieTagMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE); ASSERT_TRUE(mFakeCookieTagMap.isValid()); - mFakeAppUidStatsMap = BpfMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, 0); + mFakeAppUidStatsMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE); ASSERT_TRUE(mFakeAppUidStatsMap.isValid()); - mFakeStatsMap = BpfMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, 0); + mFakeStatsMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE); ASSERT_TRUE(mFakeStatsMap.isValid()); - mFakeIfaceIndexNameMap = BpfMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, 0); + mFakeIfaceIndexNameMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE); ASSERT_TRUE(mFakeIfaceIndexNameMap.isValid()); - mFakeIfaceStatsMap = BpfMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, 0); + mFakeIfaceStatsMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE); ASSERT_TRUE(mFakeIfaceStatsMap.isValid()); } diff --git a/service-t/native/libs/libnetworkstats/include/netdbpf/BpfNetworkStats.h b/service-t/native/libs/libnetworkstats/include/netdbpf/BpfNetworkStats.h index ea068fcdf2..8058d0580a 100644 --- a/service-t/native/libs/libnetworkstats/include/netdbpf/BpfNetworkStats.h +++ b/service-t/native/libs/libnetworkstats/include/netdbpf/BpfNetworkStats.h @@ -57,24 +57,25 @@ bool operator<(const stats_line& lhs, const stats_line& rhs); // For test only int bpfGetUidStatsInternal(uid_t uid, StatsValue* stats, - const BpfMap& appUidStatsMap); + const BpfMapRO& appUidStatsMap); // For test only int bpfGetIfaceStatsInternal(const char* iface, StatsValue* stats, - const BpfMap& ifaceStatsMap, - const BpfMap& ifaceNameMap); + const BpfMapRO& ifaceStatsMap, + const BpfMapRO& ifaceNameMap); // For test only int bpfGetIfIndexStatsInternal(uint32_t ifindex, StatsValue* stats, - const BpfMap& ifaceStatsMap); + const BpfMapRO& ifaceStatsMap); // For test only int parseBpfNetworkStatsDetailInternal(std::vector& lines, - const BpfMap& statsMap, - const BpfMap& ifaceMap); + const BpfMapRO& statsMap, + const BpfMapRO& ifaceMap); // For test only int cleanStatsMapInternal(const base::unique_fd& cookieTagMap, const base::unique_fd& tagStatsMap); // For test only template -int getIfaceNameFromMap(const BpfMap& ifaceMap, - const BpfMap& statsMap, uint32_t ifaceIndex, char* ifname, +int getIfaceNameFromMap(const BpfMapRO& ifaceMap, + const BpfMapRO& statsMap, + uint32_t ifaceIndex, char* ifname, const Key& curKey, int64_t* unknownIfaceBytesTotal) { auto iface = ifaceMap.readValue(ifaceIndex); if (!iface.ok()) { @@ -86,7 +87,7 @@ int getIfaceNameFromMap(const BpfMap& ifaceMap, } template -void maybeLogUnknownIface(int ifaceIndex, const BpfMap& statsMap, +void maybeLogUnknownIface(int ifaceIndex, const BpfMapRO& statsMap, const Key& curKey, int64_t* unknownIfaceBytesTotal) { // Have we already logged an error? if (*unknownIfaceBytesTotal == -1) { @@ -110,8 +111,8 @@ void maybeLogUnknownIface(int ifaceIndex, const BpfMap& statsMa // For test only int parseBpfNetworkStatsDevInternal(std::vector& lines, - const BpfMap& statsMap, - const BpfMap& ifaceMap); + const BpfMapRO& statsMap, + const BpfMapRO& ifaceMap); int bpfGetUidStats(uid_t uid, StatsValue* stats); int bpfGetIfaceStats(const char* iface, StatsValue* stats); diff --git a/staticlibs/native/bpf_headers/BpfMapTest.cpp b/staticlibs/native/bpf_headers/BpfMapTest.cpp index 10afe8603f..862114d11d 100644 --- a/staticlibs/native/bpf_headers/BpfMapTest.cpp +++ b/staticlibs/native/bpf_headers/BpfMapTest.cpp @@ -107,12 +107,14 @@ TEST_F(BpfMapTest, constructor) { BpfMap testMap1; checkMapInvalid(testMap1); - BpfMap testMap2(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC); + BpfMap testMap2; + ASSERT_RESULT_OK(testMap2.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC)); checkMapValid(testMap2); } TEST_F(BpfMapTest, basicHelpers) { - BpfMap testMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC); + BpfMap testMap; + ASSERT_RESULT_OK(testMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC)); uint32_t key = TEST_KEY1; uint32_t value_write = TEST_VALUE1; writeToMapAndCheck(testMap, key, value_write); @@ -126,7 +128,8 @@ TEST_F(BpfMapTest, basicHelpers) { } TEST_F(BpfMapTest, reset) { - BpfMap testMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC); + BpfMap testMap; + ASSERT_RESULT_OK(testMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC)); uint32_t key = TEST_KEY1; uint32_t value_write = TEST_VALUE1; writeToMapAndCheck(testMap, key, value_write); @@ -138,7 +141,8 @@ TEST_F(BpfMapTest, reset) { } TEST_F(BpfMapTest, moveConstructor) { - BpfMap testMap1(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC); + BpfMap testMap1; + ASSERT_RESULT_OK(testMap1.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC)); BpfMap testMap2; testMap2 = std::move(testMap1); uint32_t key = TEST_KEY1; @@ -149,7 +153,8 @@ TEST_F(BpfMapTest, moveConstructor) { TEST_F(BpfMapTest, SetUpMap) { EXPECT_NE(0, access(PINNED_MAP_PATH, R_OK)); - BpfMap testMap1(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC); + BpfMap testMap1; + ASSERT_RESULT_OK(testMap1.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC)); ASSERT_EQ(0, bpfFdPin(testMap1.getMap(), PINNED_MAP_PATH)); EXPECT_EQ(0, access(PINNED_MAP_PATH, R_OK)); checkMapValid(testMap1); @@ -164,7 +169,8 @@ TEST_F(BpfMapTest, SetUpMap) { } TEST_F(BpfMapTest, iterate) { - BpfMap testMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC); + BpfMap testMap; + ASSERT_RESULT_OK(testMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC)); populateMap(TEST_MAP_SIZE, testMap); int totalCount = 0; int totalSum = 0; @@ -182,7 +188,8 @@ TEST_F(BpfMapTest, iterate) { } TEST_F(BpfMapTest, iterateWithValue) { - BpfMap testMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC); + BpfMap testMap; + ASSERT_RESULT_OK(testMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC)); populateMap(TEST_MAP_SIZE, testMap); int totalCount = 0; int totalSum = 0; @@ -202,7 +209,8 @@ TEST_F(BpfMapTest, iterateWithValue) { } TEST_F(BpfMapTest, mapIsEmpty) { - BpfMap testMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC); + BpfMap testMap; + ASSERT_RESULT_OK(testMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC)); expectMapEmpty(testMap); uint32_t key = TEST_KEY1; uint32_t value_write = TEST_VALUE1; @@ -232,7 +240,8 @@ TEST_F(BpfMapTest, mapIsEmpty) { } TEST_F(BpfMapTest, mapClear) { - BpfMap testMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE, BPF_F_NO_PREALLOC); + BpfMap testMap; + ASSERT_RESULT_OK(testMap.resetMap(BPF_MAP_TYPE_HASH, TEST_MAP_SIZE)); populateMap(TEST_MAP_SIZE, testMap); Result isEmpty = testMap.isEmpty(); ASSERT_RESULT_OK(isEmpty); diff --git a/staticlibs/native/bpf_headers/include/bpf/BpfMap.h b/staticlibs/native/bpf_headers/include/bpf/BpfMap.h index 3be7067e8d..5d7eb0d8e1 100644 --- a/staticlibs/native/bpf_headers/include/bpf/BpfMap.h +++ b/staticlibs/native/bpf_headers/include/bpf/BpfMap.h @@ -48,41 +48,32 @@ using std::function; // is not safe to iterate over a map while another thread or process is deleting // from it. In this case the iteration can return duplicate entries. template -class BpfMap { +class BpfMapRO { public: - BpfMap() {}; + BpfMapRO() {}; // explicitly force no copy constructor, since it would need to dup the fd // (later on, for testing, we still make available a copy assignment operator) - BpfMap(const BpfMap&) = delete; + BpfMapRO(const BpfMapRO&) = delete; - private: - void abortOnKeyOrValueSizeMismatch() { + protected: + void abortOnMismatch(bool writable) const { if (!mMapFd.ok()) abort(); if (isAtLeastKernelVersion(4, 14, 0)) { + int flags = bpfGetFdMapFlags(mMapFd); + if (flags < 0) abort(); + if (flags & BPF_F_WRONLY) abort(); + if (writable && (flags & BPF_F_RDONLY)) abort(); if (bpfGetFdKeySize(mMapFd) != sizeof(Key)) abort(); if (bpfGetFdValueSize(mMapFd) != sizeof(Value)) abort(); } } - protected: - // flag must be within BPF_OBJ_FLAG_MASK, ie. 0, BPF_F_RDONLY, BPF_F_WRONLY - BpfMap(const char* pathname, uint32_t flags) { - mMapFd.reset(mapRetrieve(pathname, flags)); - abortOnKeyOrValueSizeMismatch(); - } - public: - explicit BpfMap(const char* pathname) : BpfMap(pathname, 0) {} - -#ifdef BPF_MAP_MAKE_VISIBLE_FOR_TESTING - // All bpf maps should be created by the bpfloader, so this constructor - // is reserved for tests - BpfMap(bpf_map_type map_type, uint32_t max_entries, uint32_t map_flags = 0) { - mMapFd.reset(createMap(map_type, sizeof(Key), sizeof(Value), max_entries, map_flags)); - if (!mMapFd.ok()) abort(); + explicit BpfMapRO(const char* pathname) { + mMapFd.reset(mapRetrieveRO(pathname)); + abortOnMismatch(/* writable */ false); } -#endif Result getFirstKey() const { Key firstKey; @@ -100,13 +91,6 @@ class BpfMap { return nextKey; } - Result writeValue(const Key& key, const Value& value, uint64_t flags) { - if (writeToMapEntry(mMapFd, &key, &value, flags)) { - return ErrnoErrorf("Write to map {} failed", mMapFd.get()); - } - return {}; - } - Result readValue(const Key key) const { Value value; if (findMapEntry(mMapFd, &key, &value)) { @@ -115,6 +99,155 @@ class BpfMap { return value; } + protected: + [[clang::reinitializes]] Result init(const char* path, int fd, bool writable) { + mMapFd.reset(fd); + if (!mMapFd.ok()) { + return ErrnoErrorf("Pinned map not accessible or does not exist: ({})", path); + } + // Normally we should return an error here instead of calling abort, + // but this cannot happen at runtime without a massive code bug (K/V type mismatch) + // and as such it's better to just blow the system up and let the developer fix it. + // Crashes are much more likely to be noticed than logs and missing functionality. + abortOnMismatch(writable); + return {}; + } + + public: + // Function that tries to get map from a pinned path. + [[clang::reinitializes]] Result init(const char* path) { + return init(path, mapRetrieveRO(path), /* writable */ false); + } + + // Iterate through the map and handle each key retrieved based on the filter + // without modification of map content. + Result iterate( + const function(const Key& key, + const BpfMapRO& map)>& filter) const; + + // Iterate through the map and get each pair, handle each pair based on the filter without modification of map content. + Result iterateWithValue( + const function(const Key& key, const Value& value, + const BpfMapRO& map)>& filter) const; + +#ifdef BPF_MAP_MAKE_VISIBLE_FOR_TESTING + const unique_fd& getMap() const { return mMapFd; }; + + // Copy assignment operator - due to need for fd duping, should not be used in non-test code. + BpfMapRO& operator=(const BpfMapRO& other) { + if (this != &other) mMapFd.reset(fcntl(other.mMapFd.get(), F_DUPFD_CLOEXEC, 0)); + return *this; + } +#else + BpfMapRO& operator=(const BpfMapRO&) = delete; +#endif + + // Move assignment operator + BpfMapRO& operator=(BpfMapRO&& other) noexcept { + if (this != &other) { + mMapFd = std::move(other.mMapFd); + other.reset(); + } + return *this; + } + +#ifdef BPF_MAP_MAKE_VISIBLE_FOR_TESTING + // Note that unique_fd.reset() carefully saves and restores the errno, + // and BpfMap.reset() won't touch the errno if passed in fd is negative either, + // hence you can do something like BpfMap.reset(systemcall()) and then + // check BpfMap.isValid() and look at errno and see why systemcall() failed. + [[clang::reinitializes]] void reset(int fd) { + mMapFd.reset(fd); + if (mMapFd.ok()) abortOnMismatch(/* writable */ false); // false isn't ideal + } + + // unique_fd has an implicit int conversion defined, which combined with the above + // reset(int) would result in double ownership of the fd, hence we either need a custom + // implementation of reset(unique_fd), or to delete it and thus cause compile failures + // to catch this and prevent it. + void reset(unique_fd fd) = delete; +#endif + + [[clang::reinitializes]] void reset() { + mMapFd.reset(); + } + + bool isValid() const { return mMapFd.ok(); } + + Result isEmpty() const { + auto key = getFirstKey(); + if (key.ok()) return false; + if (key.error().code() == ENOENT) return true; + return key.error(); + } + + protected: + unique_fd mMapFd; +}; + +template +Result BpfMapRO::iterate( + const function(const Key& key, + const BpfMapRO& map)>& filter) const { + Result curKey = getFirstKey(); + while (curKey.ok()) { + const Result& nextKey = getNextKey(curKey.value()); + Result status = filter(curKey.value(), *this); + if (!status.ok()) return status; + curKey = nextKey; + } + if (curKey.error().code() == ENOENT) return {}; + return curKey.error(); +} + +template +Result BpfMapRO::iterateWithValue( + const function(const Key& key, const Value& value, + const BpfMapRO& map)>& filter) const { + Result curKey = getFirstKey(); + while (curKey.ok()) { + const Result& nextKey = getNextKey(curKey.value()); + Result curValue = readValue(curKey.value()); + if (!curValue.ok()) return curValue.error(); + Result status = filter(curKey.value(), curValue.value(), *this); + if (!status.ok()) return status; + curKey = nextKey; + } + if (curKey.error().code() == ENOENT) return {}; + return curKey.error(); +} + +template +class BpfMap : public BpfMapRO { + protected: + using BpfMapRO::mMapFd; + using BpfMapRO::abortOnMismatch; + + public: + using BpfMapRO::getFirstKey; + using BpfMapRO::getNextKey; + using BpfMapRO::readValue; + + BpfMap() {}; + + explicit BpfMap(const char* pathname) { + mMapFd.reset(mapRetrieveRW(pathname)); + abortOnMismatch(/* writable */ true); + } + + // Function that tries to get map from a pinned path. + [[clang::reinitializes]] Result init(const char* path) { + return BpfMapRO::init(path, mapRetrieveRW(path), /* writable */ true); + } + + Result writeValue(const Key& key, const Value& value, uint64_t flags) { + if (writeToMapEntry(mMapFd, &key, &value, flags)) { + return ErrnoErrorf("Write to map {} failed", mMapFd.get()); + } + return {}; + } + Result deleteValue(const Key& key) { if (deleteMapEntry(mMapFd, &key)) { return ErrnoErrorf("Delete entry from map {} failed", mMapFd.get()); @@ -122,37 +255,33 @@ class BpfMap { return {}; } - protected: - [[clang::reinitializes]] Result init(const char* path, int fd) { - mMapFd.reset(fd); - if (!mMapFd.ok()) { - return ErrnoErrorf("Pinned map not accessible or does not exist: ({})", path); + Result clear() { + while (true) { + auto key = getFirstKey(); + if (!key.ok()) { + if (key.error().code() == ENOENT) return {}; // empty: success + return key.error(); // Anything else is an error + } + auto res = deleteValue(key.value()); + if (!res.ok()) { + // Someone else could have deleted the key, so ignore ENOENT + if (res.error().code() == ENOENT) continue; + ALOGE("Failed to delete data %s", strerror(res.error().code())); + return res.error(); + } } - // Normally we should return an error here instead of calling abort, - // but this cannot happen at runtime without a massive code bug (K/V type mismatch) - // and as such it's better to just blow the system up and let the developer fix it. - // Crashes are much more likely to be noticed than logs and missing functionality. - abortOnKeyOrValueSizeMismatch(); - return {}; } - public: - // Function that tries to get map from a pinned path. - [[clang::reinitializes]] Result init(const char* path) { - return init(path, mapRetrieveRW(path)); - } - - #ifdef BPF_MAP_MAKE_VISIBLE_FOR_TESTING - // due to Android SELinux limitations which prevent map creation by anyone besides the bpfloader - // this should only ever be used by test code, it is equivalent to: - // .reset(createMap(type, keysize, valuesize, max_entries, map_flags) - // TODO: derive map_flags from BpfMap vs BpfMapRO [[clang::reinitializes]] Result resetMap(bpf_map_type map_type, - uint32_t max_entries, - uint32_t map_flags = 0) { - mMapFd.reset(createMap(map_type, sizeof(Key), sizeof(Value), max_entries, map_flags)); + uint32_t max_entries, + uint32_t map_flags = 0) { + if (map_flags & BPF_F_WRONLY) abort(); + if (map_flags & BPF_F_RDONLY) abort(); + mMapFd.reset(createMap(map_type, sizeof(Key), sizeof(Value), max_entries, + map_flags)); if (!mMapFd.ok()) return ErrnoErrorf("Unable to create map."); + abortOnMismatch(/* writable */ true); return {}; } #endif @@ -180,72 +309,6 @@ class BpfMap { const function(const Key& key, const Value& value, BpfMap& map)>& filter); -#ifdef BPF_MAP_MAKE_VISIBLE_FOR_TESTING - const unique_fd& getMap() const { return mMapFd; }; - - // Copy assignment operator - due to need for fd duping, should not be used in non-test code. - BpfMap& operator=(const BpfMap& other) { - if (this != &other) mMapFd.reset(fcntl(other.mMapFd.get(), F_DUPFD_CLOEXEC, 0)); - return *this; - } -#else - BpfMap& operator=(const BpfMap&) = delete; -#endif - - // Move assignment operator - BpfMap& operator=(BpfMap&& other) noexcept { - if (this != &other) { - mMapFd = std::move(other.mMapFd); - other.reset(); - } - return *this; - } - - void reset(unique_fd fd) = delete; - -#ifdef BPF_MAP_MAKE_VISIBLE_FOR_TESTING - // Note that unique_fd.reset() carefully saves and restores the errno, - // and BpfMap.reset() won't touch the errno if passed in fd is negative either, - // hence you can do something like BpfMap.reset(systemcall()) and then - // check BpfMap.isValid() and look at errno and see why systemcall() failed. - [[clang::reinitializes]] void reset(int fd) { - mMapFd.reset(fd); - if (mMapFd.ok()) abortOnKeyOrValueSizeMismatch(); - } -#endif - - [[clang::reinitializes]] void reset() { - mMapFd.reset(); - } - - bool isValid() const { return mMapFd.ok(); } - - Result clear() { - while (true) { - auto key = getFirstKey(); - if (!key.ok()) { - if (key.error().code() == ENOENT) return {}; // empty: success - return key.error(); // Anything else is an error - } - auto res = deleteValue(key.value()); - if (!res.ok()) { - // Someone else could have deleted the key, so ignore ENOENT - if (res.error().code() == ENOENT) continue; - ALOGE("Failed to delete data %s", strerror(res.error().code())); - return res.error(); - } - } - } - - Result isEmpty() const { - auto key = getFirstKey(); - if (key.ok()) return false; - if (key.error().code() == ENOENT) return true; - return key.error(); - } - - private: - unique_fd mMapFd; }; template @@ -312,19 +375,5 @@ Result BpfMap::iterateWithValue( return curKey.error(); } -template -class BpfMapRO : public BpfMap { - public: - BpfMapRO() {}; - - explicit BpfMapRO(const char* pathname) - : BpfMap(pathname, BPF_F_RDONLY) {} - - // Function that tries to get map from a pinned path. - [[clang::reinitializes]] Result init(const char* path) { - return BpfMap::init(path, mapRetrieveRO(path)); - } -}; - } // namespace bpf } // namespace android