rpms/bpftrace
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Fix security hole checking unpacked kernel headers

Browse Source 
Resolves: RHEL-28764
CVE: CVE-2024-2313

Signed-off-by: Viktor Malik <vmalik@redhat.com>
This commit is contained in:
Viktor Malik 2024-05-06 14:11:17 +02:00
parent a7b48743a9
commit 47bdfeeb87
No known key found for this signature in database
GPG Key ID: AF7A2E1F6EE74FB3
3 changed files with 370 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

236
bpftrace-0.16.0-Don-t-unpack-kernel-headers-or-look-in-tmp-3156.patch Normal file
View File

@ -0,0 +1,236 @@
From 84cdf2a78199910642c6f8d78d906eb41e865529 Mon Sep 17 00:00:00 2001
From: Jordan Rome <jordalgo@meta.com>
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 <jordalgo@fedoraproject.org>
---
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<struct timespec> 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<std::string>
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<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};
@@ -647,12 +550,6 @@ std::tuple<std::string, std::string> 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<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);
+ 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
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

127
bpftrace-0.16.0-Fix-security-hole-checking-unpacked-kernel-headers-3.patch Normal file
View File

@ -0,0 +1,127 @@
From 6d659a5283da67837e0b0ea81991d71ae068ac1c Mon Sep 17 00:00:00 2001
From: Jordan Rome <jordalgo@meta.com>
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 <jordalgo@fedoraproject.org>
---
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<std::string>
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<std::string> get_wildcard_tokens(const std::string &input,
std::vector<int> get_online_cpus();
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);
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

8
bpftrace.spec
View File

@ -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 <vmalik@redhat.com> - 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