Skip to content

Commit

Permalink
Don't unpack kernel headers or look in tmp
Browse files Browse the repository at this point in the history 
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:
bpftrace#3033
bpftrace#3154
  • Loading branch information
Jordan Rome committed May 13, 2024
1 parent 5d861ee commit 1730a16
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 146 deletions.
3 changes: 0 additions & 3 deletions src/clang_parser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion src/fuzz_main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
40 changes: 27 additions & 13 deletions src/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,18 +389,6 @@ static void parse_env(BPFtrace& bpftrace)

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);

Expand All @@ -420,7 +408,33 @@ static void parse_env(BPFtrace& bpftrace)
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))
bool should_clang_parse = !driver.root.get()->c_definitions.empty() ||
!bpftrace.btf_set_.empty();

{
struct utsname utsname;
uname(&utsname);
std::string ksrc, kobj;
if (should_clang_parse) {
auto kdirs = get_kernel_dirs(utsname);
ksrc = std::get<0>(kdirs);
kobj = std::get<1>(kdirs);
} else {
ksrc = "";
kobj = "";
}

if (ksrc != "") {
std::vector<std::string> kernel_cflags = get_kernel_cflags(
utsname.machine, ksrc, kobj, bpftrace.kconfig);
extra_flags.insert(extra_flags.end(),
std::make_move_iterator(kernel_cflags.begin()),
std::make_move_iterator(kernel_cflags.end()));
}
}

if (should_clang_parse &&
!clang.parse(driver.root.get(), bpftrace, extra_flags))
return nullptr;

err = driver.parse();
Expand Down
115 changes: 9 additions & 106 deletions src/utils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
//
Expand All @@ -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 };
Expand Down Expand Up @@ -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()) {
Expand Down
3 changes: 1 addition & 2 deletions src/utils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
21 changes: 0 additions & 21 deletions tests/utils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 1730a16

Please sign in to comment.