commit 0ea4c6b242168dc7798bc0019cfcb73e4655281d
author Connor O'Brien <connoro@google.com> Fri Jan 14 00:18:11 2022 -0800
committer Connor O'Brien <connoro@google.com> Mon Jan 24 21:32:09 2022 -0800
tree b780ec5c10701ac5e23cbe273fa81a142c3f0f77
parent 0056fdce27095a8207e600c6cf45a0fdcfefe1ac

bpfloader: don't depend on symtab ordering to find function name

readCodeSections implicitly assumes that a function's name will be the
first symbol in its section to appear in the symbol table, but this is
not guaranteed. In particular, when compiling with -g the section name
can appear before it, resulting in a failure to find the bpf_prog_def
information for that section's program.

Add a check for the STT_FUNC symbol type to narrow our search to the
function name symbol we actually want.

Also, add a test to check that a program with a minimum kernel version
of infinity will not be loaded, to verify that bpfloader successfully
finds bpf_prog_def information.

Test: boot & confirm progs all have correct owners & permissions
Test: build time_in_state.c with -g and confirm owner & permissions
set correctly
Test: libbpf_load_test now passes iff this fix is applied
Signed-off-by: Connor O'Brien <connoro@google.com>
Change-Id: I31db0c50d3adeed86ee5810b1a0fb972f56fdb47

libbpf_android/BpfLoadTest.cpp

diff --git a/libbpf_android/BpfLoadTest.cpp b/libbpf_android/BpfLoadTest.cpp
index 715ae95..635d877 100644
--- a/libbpf_android/BpfLoadTest.cpp
+++ b/libbpf_android/BpfLoadTest.cpp
@@ -34,6 +34,7 @@
     BpfLoadTest() {}
     int mProgFd;
     std::string mTpProgPath;
+    std::string mTpNeverLoadProgPath;
     std::string mTpMapPath;;
 
     void SetUp() {
@@ -42,6 +43,9 @@
         mTpProgPath = "/sys/fs/bpf/prog_" + progName + "_tracepoint_sched_sched_switch";
         unlink(mTpProgPath.c_str());
 
+        mTpNeverLoadProgPath = "/sys/fs/bpf/prog_" + progName + "_tracepoint_sched_sched_wakeup";
+        unlink(mTpNeverLoadProgPath.c_str());
+
         mTpMapPath = "/sys/fs/bpf/map_" + progName + "_cpu_pid_map";
         unlink(mTpMapPath.c_str());
 
@@ -96,6 +100,11 @@
         EXPECT_EQ(android::base::ReadFileToString(mTpMapPath, &str), haveBtf);
         if (haveBtf) EXPECT_FALSE(str.empty());
     }
+
+    void checkKernelVersionEnforced() {
+        EXPECT_EQ(bpf_obj_get(mTpNeverLoadProgPath.c_str()), -1);
+        EXPECT_EQ(errno, ENOENT);
+    }
 };
 
 INSTANTIATE_TEST_SUITE_P(BpfLoadTests, BpfLoadTest,
@@ -110,5 +119,9 @@
     checkMapBtf();
 }
 
+TEST_P(BpfLoadTest, bpfCheckMinKernelVersionEnforced) {
+    checkKernelVersionEnforced();
+}
+
 }  // namespace bpf
 }  // namespace android

libbpf_android/Loader.cpp

diff --git a/libbpf_android/Loader.cpp b/libbpf_android/Loader.cpp
index cad5950..eb48cd0 100644
--- a/libbpf_android/Loader.cpp
+++ b/libbpf_android/Loader.cpp
@@ -356,7 +356,8 @@
     return 0;
 }
 
-static int getSectionSymNames(ifstream& elfFile, const string& sectionName, vector<string>& names) {
+static int getSectionSymNames(ifstream& elfFile, const string& sectionName, vector<string>& names,
+                              optional<unsigned> symbolType = std::nullopt) {
     int ret;
     string name;
     vector<Elf64_Sym> symtab;
@@ -387,6 +388,8 @@
     }
 
     for (int i = 0; i < (int)symtab.size(); i++) {
+        if (symbolType.has_value() && ELF_ST_TYPE(symtab[i].st_info) != symbolType) continue;
+
         if (symtab[i].st_shndx == sec_idx) {
             string s;
             ret = getSymName(elfFile, symtab[i].st_name, s);
@@ -437,7 +440,7 @@
             ALOGD("Loaded code section %d (%s)\n", i, name.c_str());
 
             vector<string> csSymNames;
-            ret = getSectionSymNames(elfFile, oldName, csSymNames);
+            ret = getSectionSymNames(elfFile, oldName, csSymNames, STT_FUNC);
             if (ret || !csSymNames.size()) return ret;
             for (size_t i = 0; i < progDefNames.size(); ++i) {
                 if (!progDefNames[i].compare(csSymNames[0] + "_def")) {