From 47bdfeeb8739bf2458941d634808a6710ce0a1d2 Mon Sep 17 00:00:00 2001 From: Viktor Malik Date: Mon, 6 May 2024 14:11:17 +0200 Subject: [PATCH] Fix security hole checking unpacked kernel headers Resolves: RHEL-28764 CVE: CVE-2024-2313 Signed-off-by: Viktor Malik --- ...k-kernel-headers-or-look-in-tmp-3156.patch | 236 ++++++++++++++++++ ...e-checking-unpacked-kernel-headers-3.patch | 127 ++++++++++ bpftrace.spec | 8 +- 3 files changed, 370 insertions(+), 1 deletion(-) create mode 100644 bpftrace-0.16.0-Don-t-unpack-kernel-headers-or-look-in-tmp-3156.patch create mode 100644 bpftrace-0.16.0-Fix-security-hole-checking-unpacked-kernel-headers-3.patch diff --git a/bpftrace-0.16.0-Don-t-unpack-kernel-headers-or-look-in-tmp-3156.patch b/bpftrace-0.16.0-Don-t-unpack-kernel-headers-or-look-in-tmp-3156.patch new file mode 100644 index 0000000..19e8638 --- /dev/null +++ b/bpftrace-0.16.0-Don-t-unpack-kernel-headers-or-look-in-tmp-3156.patch @@ -0,0 +1,236 @@ +From 84cdf2a78199910642c6f8d78d906eb41e865529 Mon Sep 17 00:00:00 2001 +From: Jordan Rome +Date: Wed, 15 May 2024 10:21:30 -0600 +Subject: [PATCH] Don't unpack kernel headers or look in tmp (#3156) + +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: +https://github.com/bpftrace/bpftrace/pull/3033 +https://github.com/bpftrace/bpftrace/pull/3154 + +Co-authored-by: Jordan Rome +--- + src/fuzz_main.cpp | 2 +- + src/main.cpp | 2 +- + src/utils.cpp | 105 +--------------------------------------------- + src/utils.h | 3 +- + tests/utils.cpp | 21 ---------- + 5 files changed, 4 insertions(+), 129 deletions(-) + +diff --git a/src/fuzz_main.cpp b/src/fuzz_main.cpp +index e168b8d9..a538ece1 100644 +--- a/src/fuzz_main.cpp ++++ b/src/fuzz_main.cpp +@@ -135,7 +135,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 +index 8f543038..61ecc2d4 100644 +--- a/src/main.cpp ++++ b/src/main.cpp +@@ -362,7 +362,7 @@ static std::optional get_boottime() + 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/utils.cpp b/src/utils.cpp +index 872014ad..2bf489a6 100644 +--- a/src/utils.cpp ++++ b/src/utils.cpp +@@ -111,8 +111,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 + resolve_binary_path(const std::string &cmd, const char *env_paths, int pid); +@@ -505,100 +503,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) { +- throw std::runtime_error("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; +- std_filesystem::path path_prefix{ "/tmp" }; +- 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. + // +@@ -616,8 +520,7 @@ namespace { + // {"", ""} is returned if no trace of kernel headers was found at all. + // Both ksrc and kobj are guaranteed to be != "", if at least some trace of kernel sources was found. + std::tuple 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}; +@@ -647,12 +550,6 @@ std::tuple get_kernel_dirs( + } + 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); +- } + return std::make_tuple("", ""); + } + if (ksrc.empty()) +diff --git a/src/utils.h b/src/utils.h +index 103af0d3..c3503676 100644 +--- a/src/utils.h ++++ b/src/utils.h +@@ -158,8 +158,7 @@ std::vector get_possible_cpus(); + bool is_dir(const std::string &path); + bool file_exists_and_ownedby_root(const char *f); + std::tuple get_kernel_dirs( +- const struct utsname &utsname, +- bool unpack_kheaders); ++ const struct utsname &utsname); + std::vector 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 +index 5244874f..9ca4ace5 100644 +--- a/tests/utils.cpp ++++ b/tests/utils.cpp +@@ -222,27 +222,6 @@ TEST(utils, get_cgroup_path_in_hierarchy) + } + } + +-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 +-- +2.45.0 + diff --git a/bpftrace-0.16.0-Fix-security-hole-checking-unpacked-kernel-headers-3.patch b/bpftrace-0.16.0-Fix-security-hole-checking-unpacked-kernel-headers-3.patch new file mode 100644 index 0000000..47d5136 --- /dev/null +++ b/bpftrace-0.16.0-Fix-security-hole-checking-unpacked-kernel-headers-3.patch @@ -0,0 +1,127 @@ +From 6d659a5283da67837e0b0ea81991d71ae068ac1c Mon Sep 17 00:00:00 2001 +From: Jordan Rome +Date: Wed, 6 Mar 2024 13:59:05 -0500 +Subject: [PATCH] Fix security hole checking unpacked kernel headers (#3033) + +Make sure to check that the unpacked kheaders tar +is owned by root to prevent bpftrace from loading +compromised linux headers. + +Co-authored-by: Jordan Rome +--- + src/utils.cpp | 26 ++++++++++++++++++++++---- + src/utils.h | 1 + + tests/utils.cpp | 21 +++++++++++++++++++++ + 3 files changed, 44 insertions(+), 4 deletions(-) + +diff --git a/src/utils.cpp b/src/utils.cpp +index 426644e8..872014ad 100644 +--- a/src/utils.cpp ++++ b/src/utils.cpp +@@ -111,6 +111,8 @@ 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 + resolve_binary_path(const std::string &cmd, const char *env_paths, int pid); +@@ -503,6 +505,20 @@ 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"} +@@ -535,14 +551,14 @@ namespace { + { + std::error_code ec; + std_filesystem::path path_prefix{ "/tmp" }; +- std_filesystem::path path_kheaders{ "/sys/kernel/kheaders.tar.xz" }; ++ 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 (std_filesystem::exists(shared_path, ec)) ++ if (file_exists_and_ownedby_root(shared_path.c_str())) + { + // already unpacked + return shared_path.string(); +@@ -565,8 +581,10 @@ namespace { + } + + KernelHeaderTmpDir tmpdir{path_prefix}; +- +- FILE* tar = ::popen(("tar xf /sys/kernel/kheaders.tar.xz -C " + tmpdir.path).c_str(), "w"); ++ FILE *tar = ::popen(("tar xf " + std::string(PROC_KHEADERS_PATH) + " -C " + ++ tmpdir.path) ++ .c_str(), ++ "w"); + if (!tar) { + return ""; + } +diff --git a/src/utils.h b/src/utils.h +index 9b96be9f..103af0d3 100644 +--- a/src/utils.h ++++ b/src/utils.h +@@ -156,6 +156,7 @@ std::vector get_wildcard_tokens(const std::string &input, + std::vector get_online_cpus(); + std::vector get_possible_cpus(); + bool is_dir(const std::string &path); ++bool file_exists_and_ownedby_root(const char *f); + std::tuple get_kernel_dirs( + const struct utsname &utsname, + bool unpack_kheaders); +diff --git a/tests/utils.cpp b/tests/utils.cpp +index 9ca4ace5..5244874f 100644 +--- a/tests/utils.cpp ++++ b/tests/utils.cpp +@@ -222,6 +222,27 @@ TEST(utils, get_cgroup_path_in_hierarchy) + } + } + ++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 +-- +2.44.0 + diff --git a/bpftrace.spec b/bpftrace.spec index 65213ea..c4057bb 100644 --- a/bpftrace.spec +++ b/bpftrace.spec @@ -2,7 +2,7 @@ Name: bpftrace Version: 0.16.0 -Release: 5%{?dist} +Release: 6%{?dist} Summary: High-level tracing language for Linux eBPF License: ASL 2.0 @@ -21,6 +21,8 @@ Patch2: %{name}-%{version}-tcpdrop-Fix-ERROR-Error-attaching-probe-kprob Patch3: %{name}-%{version}-RHEL8-remove-not-existing-attachpoints-from-tools.patch Patch4: %{name}-%{version}-cmake-Raise-max-llvm-major-version-to-16.patch Patch5: %{name}-%{version}-Adjust-to-build-with-llvm-17.patch +Patch6: %{name}-%{version}-Fix-security-hole-checking-unpacked-kernel-headers-3.patch +Patch7: %{name}-%{version}-Don-t-unpack-kernel-headers-or-look-in-tmp-3156.patch Patch10: %{name}-%{version}-RHEL-8-aarch64-fixes-statsnoop-and-opensnoop.patch # Arches will be included as upstream support is added and dependencies are @@ -112,6 +114,10 @@ cp %{buildroot}/%{_datadir}/%{name}/tools/old/mdflush.bt %{buildroot}/%{_datadir %exclude %{_datadir}/%{name}/tools/old %changelog +* Thu May 30 2024 Viktor Malik - 0.16.0-6 +- Fix security hole checking unpacked kernel headers (CVE-2024-2313) +- Resolves: RHEL-28764 + * Mon Nov 06 2023 - 0.16.0-5 - Rebuild for LLVM17 - Resolves: RHEL-10690