Don't unpack kernel headers or look in tmp

Looking in shared writeable locations for kernel headers is inherently risky even bpftrace does the unpacking. Remove this functionality and let the user specify the path to these headers if we can't find them in known locations. References: #3033 #3154
bpftrace · May 13, 2024 · 6d57bf7 · 6d57bf7
1 parent 5d861ee
commit 6d57bf7
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 161 deletions.
diff --git a/src/clang_parser.cpp b/src/clang_parser.cpp
@@ -660,9 +660,6 @@ bool ClangParser::parse(ast::Program *program,
   StderrSilencer silencer;
   silencer.silence();
 #endif
-  if (program->c_definitions.empty() && bpftrace.btf_set_.empty())
-    return true;
-
   input = "#include </bpftrace/include/__btf_generated_header.h>\n" +
           program->c_definitions;
 

diff --git a/src/fuzz_main.cpp b/src/fuzz_main.cpp
@@ -131,7 +131,7 @@ int fuzz_main(const char* data, size_t sz)
     struct utsname utsname;
     uname(&utsname);
     std::string ksrc, kobj;
-    auto kdirs = get_kernel_dirs(utsname, !bpftrace.feature_->has_btf());
+    auto kdirs = get_kernel_dirs(utsname);
     ksrc = std::get<0>(kdirs);
     kobj = std::get<1>(kdirs);
 

diff --git a/src/main.cpp b/src/main.cpp
@@ -387,41 +387,47 @@ static void parse_env(BPFtrace& bpftrace)
   if (TracepointFormatParser::parse(driver.root.get(), bpftrace) == false)
     return nullptr;
 
-  ClangParser clang;
-  std::vector<std::string> extra_flags;
-  {
-    struct utsname utsname;
-    uname(&utsname);
-    std::string ksrc, kobj;
-    auto kdirs = get_kernel_dirs(utsname);
-    ksrc = std::get<0>(kdirs);
-    kobj = std::get<1>(kdirs);
-
-    if (ksrc != "")
-      extra_flags = get_kernel_cflags(
-          utsname.machine, ksrc, kobj, bpftrace.kconfig);
-  }
-  extra_flags.push_back("-include");
-  extra_flags.push_back("/bpftrace/include/" CLANG_WORKAROUNDS_H);
-
-  for (auto dir : include_dirs) {
-    extra_flags.push_back("-I");
-    extra_flags.push_back(dir);
-  }
-  for (auto file : include_files) {
-    extra_flags.push_back("-include");
-    extra_flags.push_back(file);
-  }
-
   // NOTE(mmarchini): if there are no C definitions, clang parser won't run to
   // avoid issues in some versions. Since we're including files in the command
   // line, we want to force parsing, so we make sure C definitions are not
   // empty before going to clang parser stage.
   if (!include_files.empty() && driver.root->c_definitions.empty())
     driver.root->c_definitions = "#define __BPFTRACE_DUMMY__";
 
-  if (!clang.parse(driver.root.get(), bpftrace, extra_flags))
-    return nullptr;
+  bool should_clang_parse = !(driver.root.get()->c_definitions.empty() &&
+                              bpftrace.btf_set_.empty());
+
+  if (should_clang_parse) {
+    ClangParser clang;
+    std::vector<std::string> extra_flags;
+    {
+      struct utsname utsname;
+      uname(&utsname);
+      std::string ksrc, kobj;
+      auto kdirs = get_kernel_dirs(utsname);
+      ksrc = std::get<0>(kdirs);
+      kobj = std::get<1>(kdirs);
+
+      if (ksrc != "") {
+        extra_flags = get_kernel_cflags(
+            utsname.machine, ksrc, kobj, bpftrace.kconfig);
+      }
+    }
+    extra_flags.push_back("-include");
+    extra_flags.push_back(CLANG_WORKAROUNDS_H);
+
+    for (auto dir : include_dirs) {
+      extra_flags.push_back("-I");
+      extra_flags.push_back(dir);
+    }
+    for (auto file : include_files) {
+      extra_flags.push_back("-include");
+      extra_flags.push_back(file);
+    }
+
+    if (!clang.parse(driver.root.get(), bpftrace, extra_flags))
+      return nullptr;
+  }
 
   err = driver.parse();
   if (err)

diff --git a/src/utils.cpp b/src/utils.cpp
@@ -108,8 +108,6 @@ const struct vmlinux_location vmlinux_locs[] = {
   { nullptr, false },
 };
 
-constexpr std::string_view PROC_KHEADERS_PATH = "/sys/kernel/kheaders.tar.xz";
-
 static bool pid_in_different_mountns(int pid);
 static std::vector<std::string> resolve_binary_path(const std::string &cmd,
                                                     const char *env_paths,
@@ -683,103 +681,6 @@ bool is_dir(const std::string &path)
   return std_filesystem::is_directory(buf, ec);
 }
 
-bool file_exists_and_ownedby_root(const char *f)
-{
-  struct stat st;
-  if (stat(f, &st) == 0) {
-    if (st.st_uid != 0) {
-      LOG(ERROR) << "header file ownership expected to be root: "
-                 << std::string(f);
-      return false;
-    }
-    return true;
-  }
-  return false;
-}
-
-namespace {
-struct KernelHeaderTmpDir {
-  KernelHeaderTmpDir(const std::string &prefix) : path{ prefix + "XXXXXX" }
-  {
-    if (::mkdtemp(&path[0]) == nullptr) {
-      LOG(FATAL) << "creating temporary path for kheaders.tar.xz failed";
-    }
-  }
-
-  ~KernelHeaderTmpDir()
-  {
-    if (path.size() > 0) {
-      // move_to either did not succeed or did not run, so clean up after
-      // ourselves
-      exec_system(("rm -rf " + path).c_str());
-    }
-  }
-
-  void move_to(const std::string &new_path)
-  {
-    int err = ::rename(path.c_str(), new_path.c_str());
-    if (err == 0) {
-      path = "";
-    }
-  }
-
-  std::string path;
-};
-
-std::string unpack_kheaders_tar_xz(const struct utsname &utsname)
-{
-  std::error_code ec;
-#if defined(__ANDROID__)
-  std_filesystem::path path_prefix{ "/data/local/tmp" };
-#else
-  std_filesystem::path path_prefix{ "/tmp" };
-#endif
-  std_filesystem::path path_kheaders{ PROC_KHEADERS_PATH };
-  if (const char *tmpdir = ::getenv("TMPDIR")) {
-    path_prefix = tmpdir;
-  }
-  path_prefix /= "kheaders-";
-  std_filesystem::path shared_path{ path_prefix.string() + utsname.release };
-
-  if (file_exists_and_ownedby_root(shared_path.c_str())) {
-    // already unpacked
-    return shared_path.string();
-  }
-
-  if (!std_filesystem::exists(path_kheaders, ec)) {
-    StderrSilencer silencer;
-    silencer.silence();
-
-    FILE *modprobe = ::popen("modprobe kheaders", "w");
-    if (modprobe == nullptr || pclose(modprobe) != 0) {
-      return "";
-    }
-
-    if (!std_filesystem::exists(path_kheaders, ec)) {
-      return "";
-    }
-  }
-
-  KernelHeaderTmpDir tmpdir{ path_prefix };
-
-  FILE *tar = ::popen(("tar xf " + std::string(PROC_KHEADERS_PATH) + " -C " +
-                       tmpdir.path)
-                          .c_str(),
-                      "w");
-  if (!tar) {
-    return "";
-  }
-
-  int rc = ::pclose(tar);
-  if (rc == 0) {
-    tmpdir.move_to(shared_path);
-    return shared_path;
-  }
-
-  return "";
-}
-} // namespace
-
 // get_kernel_dirs returns {ksrc, kobj} - directories for pristine and
 // generated kernel sources.
 //
@@ -798,8 +699,7 @@ std::string unpack_kheaders_tar_xz(const struct utsname &utsname)
 // Both ksrc and kobj are guaranteed to be != "", if at least some trace of
 // kernel sources was found.
 std::tuple<std::string, std::string> get_kernel_dirs(
-    const struct utsname &utsname,
-    bool unpack_kheaders)
+    const struct utsname &utsname)
 {
 #ifdef KERNEL_HEADERS_DIR
   return { KERNEL_HEADERS_DIR, KERNEL_HEADERS_DIR };
@@ -827,11 +727,14 @@ std::tuple<std::string, std::string> get_kernel_dirs(
     kobj = "";
   }
   if (ksrc.empty() && kobj.empty()) {
-    if (unpack_kheaders) {
-      const auto kheaders_tar_xz_path = unpack_kheaders_tar_xz(utsname);
-      if (kheaders_tar_xz_path.size() > 0)
-        return std::make_tuple(kheaders_tar_xz_path, kheaders_tar_xz_path);
-    }
+    LOG(WARNING) << "Could not find kernel headers in " << ksrc << " or "
+                 << kobj
+                 << ". To specify a particular path to kernel headers, set the "
+                    "env variables BPFTRACE_KERNEL_SOURCE and, optionally, "
+                    "BPFTRACE_KERNEL_BUILD if the kernel was built in a "
+                    "different directory than its source. To create kernel "
+                    "headers run 'modprobe kheaders', which will create a tar "
+                    "file at /sys/kernel/kheaders.tar.xz";
     return std::make_tuple("", "");
   }
   if (ksrc.empty()) {

diff --git a/src/utils.h b/src/utils.h
@@ -160,8 +160,7 @@ std::vector<int> get_possible_cpus();
 bool is_dir(const std::string &path);
 bool file_exists_and_ownedby_root(const char *f);
 std::tuple<std::string, std::string> get_kernel_dirs(
-    const struct utsname &utsname,
-    bool unpack_kheaders = true);
+    const struct utsname &utsname);
 std::vector<std::string> get_kernel_cflags(const char *uname_machine,
                                            const std::string &ksrc,
                                            const std::string &kobj,

diff --git a/tests/utils.cpp b/tests/utils.cpp
@@ -358,27 +358,6 @@ TEST(utils, get_pids_for_program)
   ASSERT_EQ(pids.size(), 0);
 }
 
-TEST(utils, file_exists_and_ownedby_root)
-{
-  std::string tmpdir = "/tmp/bpftrace-test-utils-XXXXXX";
-  std::string file1 = "/ownedby-user";
-  std::string file2 = "/no-exists";
-  if (::mkdtemp(tmpdir.data()) == nullptr) {
-    throw std::runtime_error("creating temporary path for tests failed");
-  }
-
-  int fd;
-  fd = open((tmpdir + file1).c_str(), O_CREAT, S_IRUSR);
-  close(fd);
-  ASSERT_GE(fd, 0);
-
-  EXPECT_FALSE(file_exists_and_ownedby_root((tmpdir + file1).c_str()));
-  EXPECT_FALSE(file_exists_and_ownedby_root((tmpdir + file2).c_str()));
-  EXPECT_TRUE(file_exists_and_ownedby_root("/proc/1/maps"));
-
-  EXPECT_GT(std_filesystem::remove_all(tmpdir), 0);
-}
-
 } // namespace utils
 } // namespace test
 } // namespace bpftrace