From eb199dd7481e7d1f1f548844c8e7f68cbfbfeaf8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= <maze@google.com>
Date: Fri, 1 Jul 2022 03:21:40 -0700
Subject: [PATCH] use mapRetrieveRO() instead of bpf_obj_get()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

bpf_obj_get(path) is entirely equivalent to mapRetrieveRW(path)

See implementation in frameworks/libs/net common/native/bpf_syscall_wrappers/include/BpfSyscallWrappers.h:

inline int bpfFdGet(const char* pathname, uint32_t flag) {
    return bpf(BPF_OBJ_GET, {
                                    .pathname = ptr_to_u64(pathname),
                                    .file_flags = flag,
                            });
}

inline int mapRetrieve(const char* pathname, uint32_t flag) { return bpfFdGet(pathname, flag); }
inline int mapRetrieveRW(const char* pathname) { return mapRetrieve(pathname, 0); }
inline int mapRetrieveRO(const char* pathname) { return mapRetrieve(pathname, BPF_F_RDONLY); }
inline int mapRetrieveWO(const char* pathname) { return mapRetrieve(pathname, BPF_F_WRONLY); }
inline int retrieveProgram(const char* pathname) { return bpfFdGet(pathname, BPF_F_RDONLY); }

However, this requires selinux file:write access which bpfloader
currently lacks, ie. we would need:

system/sepolicy private/bpfloader.te:
  -allow bpfloader bpffs_type:file { create read rename setattr };
  +allow bpfloader bpffs_type:file { create read rename setattr write };

Switching from mapRetrieveRW() to mapRetrieveRO() eliminates this problem.

BpfLoader itself does not need to write to the maps, while bpf program write
access is controlled by a different bit (it is independent of the r/w access
bits of the file descriptor itself).

Verified by re-opening the maps after pinning them.

Bug: 218408035
Bug: 237716689
Test: TreeHugger
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Ica7ac8ee48d4a73e5f92dbf47cd441c3bfba38cf
---
 libbpf_android/Loader.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libbpf_android/Loader.cpp b/libbpf_android/Loader.cpp
index 57e5303..2983c76 100644
--- a/libbpf_android/Loader.cpp
+++ b/libbpf_android/Loader.cpp
@@ -30,9 +30,9 @@
 #include <sys/wait.h>
 #include <unistd.h>
 
-// This is BpfLoader v0.19
+// This is BpfLoader v0.20
 #define BPFLOADER_VERSION_MAJOR 0u
-#define BPFLOADER_VERSION_MINOR 19u
+#define BPFLOADER_VERSION_MINOR 20u
 #define BPFLOADER_VERSION ((BPFLOADER_VERSION_MAJOR << 16) | BPFLOADER_VERSION_MINOR)
 
 #include "bpf/BpfUtils.h"
@@ -822,7 +822,7 @@ static int createMaps(const char* elfPath, ifstream& elfFile, vector<unique_fd>&
         int saved_errno;
 
         if (access(mapPinLoc.c_str(), F_OK) == 0) {
-            fd.reset(bpf_obj_get(mapPinLoc.c_str()));
+            fd.reset(mapRetrieveRO(mapPinLoc.c_str()));
             saved_errno = errno;
             ALOGD("bpf_create_map reusing map %s, ret: %d", mapNames[i].c_str(), fd.get());
             reuse = true;