diff --git a/crates/memtrack/build.rs b/crates/memtrack/build.rs index 1c337ebd1..15cdb39a5 100644 --- a/crates/memtrack/build.rs +++ b/crates/memtrack/build.rs @@ -12,18 +12,33 @@ fn build_ebpf() { println!("cargo:rerun-if-changed=src/ebpf/c"); - // Build the BPF program + // The same source (main.bpf.c) is compiled into two skeletons through thin + // wrappers that differ only in whether MEMTRACK_BPF_VARIANT_TOKEN is set: + // the token variant attaches uprobes as uprobe_multi links, which a + // delegated BPF token can authorize, and the legacy variant uses + // perf_event_open for kernels predating uprobe_multi. See + // src/ebpf/c/utils/variant.h. The runtime picks one based on token + // availability. let arch = env::var("CARGO_CFG_TARGET_ARCH") .expect("CARGO_CFG_TARGET_ARCH must be set in build script"); - let memtrack_out = PathBuf::from(env::var("OUT_DIR").unwrap()).join("memtrack.skel.rs"); - SkeletonBuilder::new() - .source("src/ebpf/c/main.bpf.c") - .clang_args([ - "-I", - &vmlinux::include_path_root().join(arch).to_string_lossy(), - ]) - .build_and_generate(&memtrack_out) - .unwrap(); + let vmlinux_inc = vmlinux::include_path_root() + .join(arch) + .to_string_lossy() + .into_owned(); + let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap()); + for (source, skel) in [ + ("src/ebpf/c/memtrack_token.bpf.c", "memtrack_token.skel.rs"), + ( + "src/ebpf/c/memtrack_legacy.bpf.c", + "memtrack_legacy.skel.rs", + ), + ] { + SkeletonBuilder::new() + .source(source) + .clang_args(["-I", &vmlinux_inc]) + .build_and_generate(out_dir.join(skel)) + .unwrap(); + } // Generate bindings for event.h let bindings = bindgen::Builder::default() diff --git a/crates/memtrack/src/bpf_token.rs b/crates/memtrack/src/bpf_token.rs new file mode 100644 index 000000000..390552060 --- /dev/null +++ b/crates/memtrack/src/bpf_token.rs @@ -0,0 +1,9 @@ +/// Whether libbpf has a delegated BPF token to attach to its `bpf()` calls. It +/// reads one from the bpffs mount named by `LIBBPF_BPF_TOKEN_PATH`, which is how +/// eBPF gets loaded without host privileges. +/// +/// +pub fn has_delegated_bpf_token() -> bool { + std::env::var_os("LIBBPF_BPF_TOKEN_PATH") + .is_some_and(|p| !p.is_empty() && std::path::Path::new(&p).is_dir()) +} diff --git a/crates/memtrack/src/ebpf/attach_worker.rs b/crates/memtrack/src/ebpf/attach_worker.rs index 493bf916d..61a49c4ee 100644 --- a/crates/memtrack/src/ebpf/attach_worker.rs +++ b/crates/memtrack/src/ebpf/attach_worker.rs @@ -232,8 +232,38 @@ impl Worker { ), }; - let Ok(lib) = AllocatorLib::from_path_static(&mapping.attach_path) else { - debug!("on-demand: not an allocator: {}", mapping.display); + let candidates = mapping.candidate_paths(); + let mut lib = None; + let mut unreadable = Vec::new(); + for path in &candidates { + match AllocatorLib::from_path_static(path) { + Ok(found) => { + lib = Some(found); + break; + } + // Classification reads the whole file, so a failed open is + // indistinguishable from a readable non-allocator. Probe + // openability to tell them apart. + Err(_) => { + if let Err(e) = std::fs::File::open(path) { + unreadable.push(format!("{}: {e}", path.display())); + } + } + } + } + + let Some(lib) = lib else { + // A mapping no path can reach costs coverage of that one library, + // which is not worth killing the run over. + if unreadable.len() == candidates.len() { + warn!( + "no readable path for mapping {} in pid {}, allocator coverage may be incomplete", + mapping.display, req.pid + ); + debug!("unreadable paths: {}", unreadable.join(", ")); + } else { + debug!("on-demand: not an allocator: {}", mapping.display); + } return Ok(true); }; diff --git a/crates/memtrack/src/ebpf/c/allocator.h b/crates/memtrack/src/ebpf/c/allocator.h index 648786ec4..bf7f7e850 100644 --- a/crates/memtrack/src/ebpf/c/allocator.h +++ b/crates/memtrack/src/ebpf/c/allocator.h @@ -7,9 +7,9 @@ #define UPROBE_ARG_RET(name, arg_expr, submit_block) \ BPF_HASH_MAP(name##_arg, __u64, __u64, 10000); \ - SEC("uprobe") \ + SEC(UPROBE_SEC) \ int uprobe_##name(struct pt_regs* ctx) { return store_param(&name##_arg, arg_expr); } \ - SEC("uretprobe") \ + SEC(URETPROBE_SEC) \ int uretprobe_##name(struct pt_regs* ctx) { \ __u64* arg_ptr = take_param(&name##_arg); \ if (!arg_ptr) { \ @@ -24,7 +24,7 @@ } #define UPROBE_RET(name, arg_expr, submit_block) \ - SEC("uprobe") \ + SEC(UPROBE_SEC) \ int uprobe_##name(struct pt_regs* ctx) { \ __u64 arg0 = arg_expr; \ if (arg0 == 0) { \ @@ -39,12 +39,12 @@ __u64 arg1; \ }; \ BPF_HASH_MAP(name##_args, __u64, struct name##_args_t, 10000); \ - SEC("uprobe") \ + SEC(UPROBE_SEC) \ int uprobe_##name(struct pt_regs* ctx) { \ - __u64 tid = bpf_get_current_pid_tgid(); \ - __u32 pid = tid >> 32; \ + struct task_ids ids = current_task_ids(); \ + __u64 tid = ids.tid; \ \ - if (!is_tracked(pid)) { \ + if (!is_tracked(ids.tgid)) { \ return 0; \ } \ \ @@ -53,9 +53,9 @@ bpf_map_update_elem(&name##_args, &tid, &args, BPF_ANY); \ return 0; \ } \ - SEC("uretprobe") \ + SEC(URETPROBE_SEC) \ int uretprobe_##name(struct pt_regs* ctx) { \ - __u64 tid = bpf_get_current_pid_tgid(); \ + __u64 tid = current_tid(); \ struct name##_args_t* args = bpf_map_lookup_elem(&name##_args, &tid); \ \ if (!args) { \ @@ -105,11 +105,11 @@ struct posix_memalign_args_t { }; BPF_HASH_MAP(posix_memalign_args, __u64, struct posix_memalign_args_t, 10000); -SEC("uprobe") +SEC(UPROBE_SEC) int uprobe_posix_memalign(struct pt_regs* ctx) { - __u64 tid = bpf_get_current_pid_tgid(); - __u32 pid = tid >> 32; - if (!is_tracked(pid)) { + struct task_ids ids = current_task_ids(); + __u64 tid = ids.tid; + if (!is_tracked(ids.tgid)) { return 0; } @@ -118,9 +118,9 @@ int uprobe_posix_memalign(struct pt_regs* ctx) { return 0; } -SEC("uretprobe") +SEC(URETPROBE_SEC) int uretprobe_posix_memalign(struct pt_regs* ctx) { - __u64 tid = bpf_get_current_pid_tgid(); + __u64 tid = current_tid(); struct posix_memalign_args_t* args = bpf_map_lookup_elem(&posix_memalign_args, &tid); if (!args) { return 0; @@ -149,9 +149,9 @@ struct mmap_args { BPF_HASH_MAP(mmap_temp, __u64, struct mmap_args, 10000); static __always_inline void store_mmap_args(__u64 addr, __u64 len) { - __u64 tid = bpf_get_current_pid_tgid(); - __u32 pid = tid >> 32; - if (is_tracked(pid)) { + struct task_ids ids = current_task_ids(); + __u64 tid = ids.tid; + if (is_tracked(ids.tgid)) { struct mmap_args args = {.addr = addr, .len = len}; bpf_map_update_elem(&mmap_temp, &tid, &args, BPF_ANY); } diff --git a/crates/memtrack/src/ebpf/c/attach.h b/crates/memtrack/src/ebpf/c/attach.h index b996e9242..e188c7d5f 100644 --- a/crates/memtrack/src/ebpf/c/attach.h +++ b/crates/memtrack/src/ebpf/c/attach.h @@ -31,8 +31,7 @@ int BPF_PROG(watch_exec_mmap, struct file* file, unsigned long prot, unsigned lo return 0; } - __u64 pid_tgid = bpf_get_current_pid_tgid(); - __u32 tgid = pid_tgid >> 32; + __u32 tgid = current_tgid(); if (!is_tracked(tgid)) { return 0; } diff --git a/crates/memtrack/src/ebpf/c/memtrack_legacy.bpf.c b/crates/memtrack/src/ebpf/c/memtrack_legacy.bpf.c new file mode 100644 index 000000000..5267d3c94 --- /dev/null +++ b/crates/memtrack/src/ebpf/c/memtrack_legacy.bpf.c @@ -0,0 +1,5 @@ +/* Legacy variant: uprobes attach through perf_event_open(), which works on + * kernels predating uprobe_multi (< 6.6) but needs CAP_PERFMON in the init user + * namespace and so cannot be delegated via a BPF token. See utils/variant.h. + * Program bodies live in main.bpf.c. */ +#include "main.bpf.c" diff --git a/crates/memtrack/src/ebpf/c/memtrack_token.bpf.c b/crates/memtrack/src/ebpf/c/memtrack_token.bpf.c new file mode 100644 index 000000000..bb6001726 --- /dev/null +++ b/crates/memtrack/src/ebpf/c/memtrack_token.bpf.c @@ -0,0 +1,7 @@ +/* Token variant: uprobes attach as uprobe_multi links, so every attach goes + * through bpf() and a delegated BPF token can authorize it without host + * privileges. Requires kernel >= 6.6. See utils/variant.h for why classic + * uprobes cannot be delegated. Program bodies live in main.bpf.c; only the + * SEC() annotations differ, keyed on this define. */ +#define MEMTRACK_BPF_VARIANT_TOKEN 1 +#include "main.bpf.c" diff --git a/crates/memtrack/src/ebpf/c/utils/event_helpers.h b/crates/memtrack/src/ebpf/c/utils/event_helpers.h index b5d79b372..89c8be14f 100644 --- a/crates/memtrack/src/ebpf/c/utils/event_helpers.h +++ b/crates/memtrack/src/ebpf/c/utils/event_helpers.h @@ -21,16 +21,18 @@ static __always_inline long wake_flags(void) { } static __always_inline int store_param(void* map, __u64 value) { - __u64 tid = bpf_get_current_pid_tgid(); - __u32 pid = tid >> 32; - if (is_tracked(pid)) { + /* Key by the tid: unique per thread, so it survives the entry/exit pair even + * when several threads are inside the same allocator call. */ + struct task_ids ids = current_task_ids(); + __u64 tid = ids.tid; + if (is_tracked(ids.tgid)) { bpf_map_update_elem(map, &tid, &value, BPF_ANY); } return 0; } static __always_inline __u64* take_param(void* map) { - __u64 tid = bpf_get_current_pid_tgid(); + __u64 tid = current_tid(); __u64* value = bpf_map_lookup_elem(map, &tid); if (value) { bpf_map_delete_elem(map, &tid); @@ -40,10 +42,9 @@ static __always_inline __u64* take_param(void* map) { #define SUBMIT_EVENT(evt_type, fill_data) \ { \ - __u64 tid = bpf_get_current_pid_tgid(); \ - __u32 pid = tid >> 32; \ + struct task_ids ids = current_task_ids(); \ \ - if (!is_tracked(pid) || !is_enabled()) { \ + if (!is_tracked(ids.tgid) || !is_enabled()) { \ return 0; \ } \ \ @@ -58,8 +59,8 @@ static __always_inline __u64* take_param(void* map) { } \ \ e->header.timestamp = bpf_ktime_get_ns(); \ - e->header.pid = pid; \ - e->header.tid = tid & 0xFFFFFFFF; \ + e->header.pid = ids.tgid; \ + e->header.tid = ids.tid; \ e->header.event_type = evt_type; \ \ fill_data; \ diff --git a/crates/memtrack/src/ebpf/c/utils/process_tracking.h b/crates/memtrack/src/ebpf/c/utils/process_tracking.h index 19cff3386..7b147cbc4 100644 --- a/crates/memtrack/src/ebpf/c/utils/process_tracking.h +++ b/crates/memtrack/src/ebpf/c/utils/process_tracking.h @@ -2,6 +2,7 @@ #define __PROCESS_TRACKING_H__ #include "map_helpers.h" +#include "variant.h" BPF_HASH_MAP(tracked_pids, __u32, __u8, 10000); BPF_HASH_MAP(pids_ppid, __u32, __u32, 10000); @@ -43,15 +44,26 @@ static __always_inline void track_child(__u32 child_pid, __u32 parent_pid) { bpf_map_update_elem(&pids_ppid, &child_pid, &parent_pid, BPF_ANY); } -SEC("tracepoint/sched/sched_process_fork") -int tracepoint_sched_fork(struct trace_event_raw_sched_process_fork* ctx) { - __u32 parent_pid = ctx->parent_pid; - __u32 child_pid = ctx->child_pid; - +/* Record a parent→child fork so the child inherits the parent's tracked state. */ +static __always_inline void follow_fork(__u32 parent_pid, __u32 child_pid) { + if (parent_pid == 0 || child_pid == 0) { + return; + } if (is_tracked(parent_pid)) { track_child(child_pid, parent_pid); } +} +/* tp_btf rather than a classic tracepoint on two counts: it attaches with + * BPF_RAW_TRACEPOINT_OPEN, so a token can delegate it (perf_event_open() cannot + * be), and its arguments are task_struct pointers rather than a flattened trace + * event, so PIDs resolve through the tracker's namespace. + * + * https://docs.ebpf.io/linux/program-type/BPF_PROG_TYPE_TRACING/ + */ +SEC("tp_btf/sched_process_fork") +int BPF_PROG(tracepoint_sched_fork, struct task_struct* parent, struct task_struct* child) { + follow_fork(task_ns_tgid(parent), task_ns_tgid(child)); return 0; } diff --git a/crates/memtrack/src/ebpf/c/utils/variant.h b/crates/memtrack/src/ebpf/c/utils/variant.h new file mode 100644 index 000000000..036c63ba4 --- /dev/null +++ b/crates/memtrack/src/ebpf/c/utils/variant.h @@ -0,0 +1,111 @@ +#ifndef __VARIANT_H__ +#define __VARIANT_H__ + +/* Attach mechanism, selected at build time by MEMTRACK_BPF_VARIANT_TOKEN. + * + * A BPF token only relaxes capability checks made by the bpf() syscall itself + * (bpf_token_capable() in kernel/bpf/token.c), so only attach paths that go + * entirely through bpf() can be delegated into an unprivileged user namespace. + * uprobe_multi links qualify: BPF_LINK_CREATE with BPF_TRACE_UPROBE_MULTI does + * no capability check of its own, the CAP_BPF/CAP_PERFMON check happens at + * BPF_PROG_LOAD, where the token applies. + * + * Classic uprobes do not: libbpf attaches them with perf_event_open(), which is + * not a bpf() command and checks capabilities against the init user namespace + * (capable(CAP_SYS_ADMIN) in perf_uprobe_event_init(), or CAP_PERFMON plus + * tracefs write access on the legacy path). No token can grant those. + * + * uprobe_multi requires kernel >= 6.6. + * + * https://docs.ebpf.io/linux/concepts/token/ + * https://lwn.net/Articles/959350/ + */ +#ifdef MEMTRACK_BPF_VARIANT_TOKEN +#define UPROBE_SEC "uprobe.multi" +#define URETPROBE_SEC "uretprobe.multi" +#else +#define UPROBE_SEC "uprobe" +#define URETPROBE_SEC "uretprobe" +#endif + +/* PID namespace the userspace tracker resolves PIDs in, as the (dev, ino) pair + * bpf_get_ns_current_pid_tgid() expects. Set from userspace; ino == 0 means + * "report global PIDs", which is what these helpers resolve to in the init + * namespace anyway. + * + * eBPF observes PIDs from the init namespace, so when the tracker runs inside a + * PID namespace the PIDs it registers are namespace-local and would never match + * what the probes see. Resolving in the tracker's namespace keeps both agreeing. + * + * https://docs.ebpf.io/linux/helper-function/bpf_get_ns_current_pid_tgid/ + */ +const volatile __u64 target_pidns_dev = 0; +const volatile __u64 target_pidns_ino = 0; + +/* Identity of the current task in the configured namespace. + * + * tgid is the thread-group id (what userspace calls the pid); tid is the thread + * id (what the kernel calls the pid), unique per thread, so it keys the + * uprobe/uretprobe argument hand-off maps. + */ +struct task_ids { + __u32 tgid; + __u32 tid; +}; + +/* The current task's (tgid << 32) | pid in the configured namespace, packed the + * way bpf_get_current_pid_tgid() returns it. + * + * https://docs.ebpf.io/linux/helper-function/bpf_get_current_pid_tgid/ + */ +static __always_inline __u64 memtrack_current_pid_tgid(void) { + if (target_pidns_ino == 0) { + return bpf_get_current_pid_tgid(); + } + struct bpf_pidns_info nsinfo = {}; + if (bpf_get_ns_current_pid_tgid(target_pidns_dev, target_pidns_ino, &nsinfo, sizeof(nsinfo)) != + 0) { + return 0; + } + return ((__u64)nsinfo.tgid << 32) | nsinfo.pid; +} + +/* Both ids from a single helper call, for paths that need them together. */ +static __always_inline struct task_ids current_task_ids(void) { + __u64 pid_tgid = memtrack_current_pid_tgid(); + struct task_ids ids = {.tgid = pid_tgid >> 32, .tid = (__u32)pid_tgid}; + return ids; +} + +static __always_inline __u32 current_tgid(void) { return current_task_ids().tgid; } + +static __always_inline __u32 current_tid(void) { return current_task_ids().tid; } + +/* Thread-group id of an arbitrary task in the configured namespace. + * + * struct pid holds one upid per namespace level the task is visible in, with + * numbers[level] the innermost. A task whose innermost namespace is not the + * target lives in a sibling or deeper namespace, so it has no PID the tracker + * could match, hence 0. + */ +static __always_inline __u32 task_ns_tgid(struct task_struct* task) { + if (target_pidns_ino == 0) { + return BPF_CORE_READ(task, tgid); + } + struct pid* thread_pid = BPF_CORE_READ(task, thread_pid); + if (!thread_pid) { + return 0; + } + unsigned int level = BPF_CORE_READ(thread_pid, level); + /* Bounded so the verifier can prove the numbers[] access is in range. */ + if (level >= 4) { + return 0; + } + struct upid* up = &thread_pid->numbers[level]; + if (BPF_CORE_READ(up, ns, ns.inum) != target_pidns_ino) { + return 0; + } + return BPF_CORE_READ(up, nr); +} + +#endif /* __VARIANT_H__ */ diff --git a/crates/memtrack/src/ebpf/memtrack/allocator.rs b/crates/memtrack/src/ebpf/memtrack/allocator.rs index e858fc584..061116908 100644 --- a/crates/memtrack/src/ebpf/memtrack/allocator.rs +++ b/crates/memtrack/src/ebpf/memtrack/allocator.rs @@ -1,5 +1,5 @@ use crate::prelude::*; -use libbpf_rs::UprobeOpts; +use libbpf_rs::{UprobeMultiOpts, UprobeOpts}; use paste::paste; use std::path::Path; diff --git a/crates/memtrack/src/ebpf/memtrack/macros.rs b/crates/memtrack/src/ebpf/memtrack/macros.rs index 88da64a6a..d7b7847cf 100644 --- a/crates/memtrack/src/ebpf/memtrack/macros.rs +++ b/crates/memtrack/src/ebpf/memtrack/macros.rs @@ -1,3 +1,54 @@ +/// Run `$body` with `$skel` bound to the loaded skeleton. The two variants are +/// distinct generated types, so this is a match rather than a function, but they +/// share field and method names for all maps and auto-attached programs. +macro_rules! with_skel { + ($self:expr, $skel:ident => $body:expr) => { + match &$self.skel { + crate::ebpf::memtrack::Skel::Token($skel) => $body, + crate::ebpf::memtrack::Skel::Legacy($skel) => $body, + } + }; + (mut $self:expr, $skel:ident => $body:expr) => { + match &mut $self.skel { + crate::ebpf::memtrack::Skel::Token($skel) => $body, + crate::ebpf::memtrack::Skel::Legacy($skel) => $body, + } + }; +} + +/// Attach one program (entry or return) at `offset` in `lib_path`, through the +/// mechanism the loaded variant was built for. +macro_rules! attach_one { + ($self:expr, $prog:ident, $lib_path:expr, $offset:expr, $retprobe:expr) => {{ + let lib_path = $lib_path; + let offset = $offset; + let retprobe = $retprobe; + match &mut $self.skel { + crate::ebpf::memtrack::Skel::Token(skel) => { + skel.progs.$prog.attach_uprobe_multi_with_opts( + -1, + lib_path, + "", + UprobeMultiOpts { + offsets: vec![offset], + retprobe, + ..Default::default() + }, + ) + } + crate::ebpf::memtrack::Skel::Legacy(skel) => skel.progs.$prog.attach_uprobe_with_opts( + -1, + lib_path, + offset, + UprobeOpts { + retprobe, + ..Default::default() + }, + ), + } + }}; +} + /// Macro to attach a function with both entry and return probes at a resolved /// file offset. Also generates an `attach_*_if_found` variant that skips /// symbols absent from the offset table (returning whether it attached) and @@ -6,19 +57,7 @@ macro_rules! attach_uprobe_uretprobe { ($name:ident, $prog_entry:ident, $prog_return:ident) => { paste! { fn [](&mut self, lib_path: &Path, offset: usize) -> Result<()> { - let link = self - .skel - .progs - .$prog_entry - .attach_uprobe_with_opts( - -1, - lib_path, - offset, - UprobeOpts { - retprobe: false, - ..Default::default() - }, - ) + let link = attach_one!(self, $prog_entry, lib_path, offset, false) .context(format!( "Failed to attach uprobe at offset {:#x} in {}", offset, @@ -26,19 +65,7 @@ macro_rules! attach_uprobe_uretprobe { ))?; self.probes.push(link); - let link = self - .skel - .progs - .$prog_return - .attach_uprobe_with_opts( - -1, - lib_path, - offset, - UprobeOpts { - retprobe: true, - ..Default::default() - }, - ) + let link = attach_one!(self, $prog_return, lib_path, offset, true) .context(format!( "Failed to attach uretprobe at offset {:#x} in {}", offset, @@ -75,19 +102,7 @@ macro_rules! attach_uprobe { ($name:ident, $prog:ident) => { paste! { fn [](&mut self, lib_path: &Path, offset: usize) -> Result<()> { - let link = self - .skel - .progs - .$prog - .attach_uprobe_with_opts( - -1, - lib_path, - offset, - UprobeOpts { - retprobe: false, - ..Default::default() - }, - ) + let link = attach_one!(self, $prog, lib_path, offset, false) .context(format!( "Failed to attach uprobe at offset {:#x} in {}", offset, @@ -118,11 +133,7 @@ macro_rules! attach_uprobe { macro_rules! attach_tracepoint { ($func:ident, $prog:ident) => { fn $func(&mut self) -> Result<()> { - let link = self - .skel - .progs - .$prog - .attach() + let link = with_skel!(mut self, skel => skel.progs.$prog.attach()) .context(format!("Failed to attach {} tracepoint", stringify!($prog)))?; self.probes.push(link); Ok(()) diff --git a/crates/memtrack/src/ebpf/memtrack/maps.rs b/crates/memtrack/src/ebpf/memtrack/maps.rs index 81dabfe1b..42d2dd2e7 100644 --- a/crates/memtrack/src/ebpf/memtrack/maps.rs +++ b/crates/memtrack/src/ebpf/memtrack/maps.rs @@ -4,15 +4,12 @@ use libbpf_rs::MapCore; impl MemtrackBpf { pub fn add_tracked_pid(&mut self, pid: i32) -> Result<()> { - self.skel - .maps - .tracked_pids - .update( - &pid.to_le_bytes(), - &1u8.to_le_bytes(), - libbpf_rs::MapFlags::ANY, - ) - .context("Failed to add PID to uprobes tracked set")?; + with_skel!(self, skel => skel.maps.tracked_pids.update( + &pid.to_le_bytes(), + &1u8.to_le_bytes(), + libbpf_rs::MapFlags::ANY, + )) + .context("Failed to add PID to uprobes tracked set")?; Ok(()) } @@ -20,30 +17,24 @@ impl MemtrackBpf { pub fn enable_tracking(&mut self) -> Result<()> { let key = 0u32; let value = true as u8; - self.skel - .maps - .tracking_enabled - .update( - &key.to_le_bytes(), - &value.to_le_bytes(), - libbpf_rs::MapFlags::ANY, - ) - .context("Failed to enable tracking")?; + with_skel!(self, skel => skel.maps.tracking_enabled.update( + &key.to_le_bytes(), + &value.to_le_bytes(), + libbpf_rs::MapFlags::ANY, + )) + .context("Failed to enable tracking")?; Ok(()) } pub fn disable_tracking(&mut self) -> Result<()> { let key = 0u32; let value = false as u8; - self.skel - .maps - .tracking_enabled - .update( - &key.to_le_bytes(), - &value.to_le_bytes(), - libbpf_rs::MapFlags::ANY, - ) - .context("Failed to disable tracking")?; + with_skel!(self, skel => skel.maps.tracking_enabled.update( + &key.to_le_bytes(), + &value.to_le_bytes(), + libbpf_rs::MapFlags::ANY, + )) + .context("Failed to disable tracking")?; Ok(()) } @@ -53,46 +44,42 @@ impl MemtrackBpf { let mut key = [0u8; 16]; key[..8].copy_from_slice(&dev.to_le_bytes()); key[8..].copy_from_slice(&ino.to_le_bytes()); - self.skel - .maps - .known_inodes - .update(&key, &1u8.to_le_bytes(), libbpf_rs::MapFlags::ANY) - .context("Failed to insert known inode")?; + with_skel!(self, skel => skel.maps.known_inodes.update( + &key, + &1u8.to_le_bytes(), + libbpf_rs::MapFlags::ANY, + )) + .context("Failed to insert known inode")?; Ok(()) } /// Number of exec-mapping requests dropped because the request ring buffer was full. pub fn attach_request_dropped_count(&self) -> Result { - let key = 0u32; - let value = self - .skel - .maps - .attach_request_dropped - .lookup(&key.to_le_bytes(), libbpf_rs::MapFlags::ANY) - .context("Failed to read attach_request_dropped counter")? - .ok_or_else(|| anyhow!("attach_request_dropped slot 0 missing"))?; - - let bytes: [u8; 8] = value - .as_slice() - .try_into() - .map_err(|_| anyhow!("attach_request_dropped value has unexpected size"))?; - Ok(u64::from_le_bytes(bytes)) + read_counter( + with_skel!(self, skel => &skel.maps.attach_request_dropped), + "attach_request_dropped", + ) } pub fn dropped_events_count(&self) -> Result { - let key = 0u32; - let value = self - .skel - .maps - .dropped_events - .lookup(&key.to_le_bytes(), libbpf_rs::MapFlags::ANY) - .context("Failed to read dropped_events counter")? - .ok_or_else(|| anyhow!("dropped_events slot 0 missing"))?; - - let bytes: [u8; 8] = value - .as_slice() - .try_into() - .map_err(|_| anyhow!("dropped_events value has unexpected size"))?; - Ok(u64::from_le_bytes(bytes)) + read_counter( + with_skel!(self, skel => &skel.maps.dropped_events), + "dropped_events", + ) } } + +/// Read slot 0 of a single-entry `__u64` array map. +fn read_counter(map: &impl MapCore, name: &str) -> Result { + let key = 0u32; + let value = map + .lookup(&key.to_le_bytes(), libbpf_rs::MapFlags::ANY) + .with_context(|| format!("Failed to read {name} counter"))? + .ok_or_else(|| anyhow!("{name} slot 0 missing"))?; + + let bytes: [u8; 8] = value + .as_slice() + .try_into() + .map_err(|_| anyhow!("{name} value has unexpected size"))?; + Ok(u64::from_le_bytes(bytes)) +} diff --git a/crates/memtrack/src/ebpf/memtrack/mod.rs b/crates/memtrack/src/ebpf/memtrack/mod.rs index 2faba0ec0..f0b80b797 100644 --- a/crates/memtrack/src/ebpf/memtrack/mod.rs +++ b/crates/memtrack/src/ebpf/memtrack/mod.rs @@ -8,10 +8,12 @@ use std::path::Path; use crate::ebpf::poller::RingBufferPoller; -pub mod memtrack_skel { - include!(concat!(env!("OUT_DIR"), "/memtrack.skel.rs")); +mod token { + include!(concat!(env!("OUT_DIR"), "/memtrack_token.skel.rs")); +} +mod legacy { + include!(concat!(env!("OUT_DIR"), "/memtrack_legacy.skel.rs")); } -pub use memtrack_skel::*; #[macro_use] mod macros; @@ -19,6 +21,36 @@ mod allocator; mod maps; mod tracking; +use crate::bpf_token::has_delegated_bpf_token; + +/// Which attach mechanism a loaded skeleton uses for its uprobes. See +/// `src/ebpf/c/utils/variant.h` for why only one of them is delegatable. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BpfVariant { + /// `uprobe_multi` links, attached through `bpf()` so a BPF token can + /// authorize them. Requires kernel >= 6.6. + Token, + /// `perf_event_open`-based uprobes, for kernels predating `uprobe_multi`. + /// Needs `CAP_PERFMON` in the init user namespace. + Legacy, +} + +/// The loaded skeleton. Both variants come from the same BPF source and expose +/// identical maps and program names; only the uprobe attach mechanism differs. +pub(super) enum Skel { + Token(Box>), + Legacy(Box>), +} + +/// Device and inode of our PID namespace, in the form +/// `bpf_get_ns_current_pid_tgid` takes them. `None` if unreadable, which leaves +/// the programs reporting global PIDs. +fn current_pidns_ids() -> Option<(u64, u64)> { + use std::os::unix::fs::MetadataExt; + let meta = std::fs::metadata("/proc/self/ns/pid").ok()?; + Some((meta.dev(), meta.ino())) +} + /// Resolve libbpf attach targets for every defined symbol in `lib_path`. pub fn resolve_symbol_offsets(lib_path: &Path) -> Result { use object::{Object, ObjectSymbol}; @@ -74,23 +106,56 @@ impl ResolvedSymbols { } pub struct MemtrackBpf { - skel: Box>, - probes: Vec, + pub(super) skel: Skel, + pub(super) probes: Vec, } impl MemtrackBpf { + /// Load the skeleton, picking the variant a BPF token is available for. pub fn new() -> Result { - let builder = MainSkelBuilder::default(); - let open_object = Box::leak(Box::new(MaybeUninit::uninit())); - let open_skel = builder - .open(open_object) - .context("Failed to open syscalls BPF skeleton")?; - - let skel = Box::new( - open_skel - .load() - .context("Failed to load syscalls BPF skeleton")?, - ); + let variant = if has_delegated_bpf_token() { + BpfVariant::Token + } else { + BpfVariant::Legacy + }; + Self::with_variant(variant) + } + + /// Load a specific variant rather than the one [`Self::new`] would detect. + /// Either attaches given host privileges; the token only matters when + /// `bpf()` is called from an unprivileged user namespace. + pub fn with_variant(variant: BpfVariant) -> Result { + // Both variants expose `rodata_data` under the same field names, but as + // distinct generated types, so this can't be a function over the two. + macro_rules! open_and_load { + ($builder:expr, $skel:path) => {{ + let open_object = Box::leak(Box::new(MaybeUninit::uninit())); + let mut open_skel = $builder + .open(open_object) + .context("Failed to open memtrack BPF skeleton")?; + if let (Some((dev, ino)), Some(rodata)) = ( + current_pidns_ids(), + open_skel.maps.rodata_data.as_deref_mut(), + ) { + rodata.target_pidns_dev = dev; + rodata.target_pidns_ino = ino; + } + $skel(Box::new( + open_skel + .load() + .context("Failed to load memtrack BPF skeleton")?, + )) + }}; + } + + let skel = match variant { + BpfVariant::Token => { + open_and_load!(token::MemtrackTokenSkelBuilder::default(), Skel::Token) + } + BpfVariant::Legacy => { + open_and_load!(legacy::MemtrackLegacySkelBuilder::default(), Skel::Legacy) + } + }; Ok(Self { skel, @@ -105,12 +170,12 @@ impl MemtrackBpf { poll_interval_ms: u64, tx: std::sync::mpsc::Sender, ) -> Result { - RingBufferPoller::new( - &self.skel.maps.events, + with_skel!(self, skel => RingBufferPoller::new( + &skel.maps.events, crate::ebpf::events::parse_event, tx, poll_interval_ms, - ) + )) } /// Poll the exec-mapping request ring buffer into `tx`. Same contract as @@ -120,12 +185,12 @@ impl MemtrackBpf { poll_interval_ms: u64, tx: std::sync::mpsc::Sender, ) -> Result { - RingBufferPoller::new( - &self.skel.maps.attach_requests, + with_skel!(self, skel => RingBufferPoller::new( + &skel.maps.attach_requests, crate::ebpf::events::AttachRequest::parse, tx, poll_interval_ms, - ) + )) } /// Number of currently-attached probes/links. diff --git a/crates/memtrack/src/ebpf/memtrack/tracking.rs b/crates/memtrack/src/ebpf/memtrack/tracking.rs index dd2a56044..9b3935b65 100644 --- a/crates/memtrack/src/ebpf/memtrack/tracking.rs +++ b/crates/memtrack/src/ebpf/memtrack/tracking.rs @@ -13,11 +13,7 @@ impl MemtrackBpf { /// Attach the exec-mapping watcher (fentry/security_mmap_file). Only used in /// on-demand mode; the program is loaded and verified in all modes. pub fn attach_exec_watcher(&mut self) -> Result<()> { - let link = self - .skel - .progs - .watch_exec_mmap - .attach() + let link = with_skel!(mut self, skel => skel.progs.watch_exec_mmap.attach()) .context("Failed to attach exec-mapping watcher")?; self.probes.push(link); Ok(()) diff --git a/crates/memtrack/src/ebpf/mod.rs b/crates/memtrack/src/ebpf/mod.rs index f05153d7f..cb6632e10 100644 --- a/crates/memtrack/src/ebpf/mod.rs +++ b/crates/memtrack/src/ebpf/mod.rs @@ -6,5 +6,5 @@ mod proc_fs; mod spawn; mod tracker; -pub use memtrack::{MemtrackBpf, ResolvedSymbols, resolve_symbol_offsets}; +pub use memtrack::{BpfVariant, MemtrackBpf, ResolvedSymbols, resolve_symbol_offsets}; pub use tracker::Tracker; diff --git a/crates/memtrack/src/ebpf/proc_fs.rs b/crates/memtrack/src/ebpf/proc_fs.rs index 164568d19..510fb9504 100644 --- a/crates/memtrack/src/ebpf/proc_fs.rs +++ b/crates/memtrack/src/ebpf/proc_fs.rs @@ -5,13 +5,36 @@ use std::time::{Duration, Instant}; /// A mapping resolved from `/proc//maps` back to an attachable path. #[derive(Debug)] pub(super) struct ResolvedMapping { - /// `/proc//map_files/-` — resolves uniformly for deleted - /// files and requires root (which memtrack already has for uprobes). + /// `/proc//map_files/-` — resolves uniformly for replaced + /// and deleted files, but opening it requires `CAP_CHECKPOINT_RESTORE` or + /// `CAP_SYS_ADMIN` *in the init user namespace* (`proc_map_files_get_link` + /// calls `checkpoint_restore_ns_capable(&init_user_ns)`). A process in a + /// nested user namespace cannot satisfy that, however privileged it is there. pub attach_path: PathBuf, - /// The pathname column, for logs. + /// The pathname column, for logs and as the fallback attach path. pub display: String, } +impl ResolvedMapping { + /// Paths to try opening the mapped file through, in preference order. + /// + /// [`Self::attach_path`] comes first because it references the mapped inode, + /// so it stays correct even when the path has been replaced or unlinked. The + /// pathname column is the fallback: it needs no privilege beyond read access, + /// but being a name rather than an inode reference it can resolve to + /// different contents than the process actually mapped. + /// + /// A `[deleted]`-suffixed or non-absolute pathname (`[heap]`, `[vdso]`, an + /// anonymous mapping) names no openable file and is skipped. + pub fn candidate_paths(&self) -> Vec { + let mut paths = vec![self.attach_path.clone()]; + if self.display.starts_with('/') && !self.display.ends_with(" (deleted)") { + paths.push(PathBuf::from(&self.display)); + } + paths + } +} + /// Block until every thread of `pid` is group-stopped. /// /// The process state is the first non-space char after the LAST `)` in @@ -258,6 +281,46 @@ mod tests { } } + /// A tracker that cannot open `map_files` reaches the mapped file only + /// through the pathname column, so it must be offered. + #[test] + fn candidate_paths_falls_back_to_the_pathname_column() { + let mapping = ResolvedMapping { + attach_path: PathBuf::from("/proc/1/map_files/400000-452000"), + display: "/lib/x86_64-linux-gnu/libc.so.6".to_string(), + }; + assert_eq!( + mapping.candidate_paths(), + vec![ + PathBuf::from("/proc/1/map_files/400000-452000"), + PathBuf::from("/lib/x86_64-linux-gnu/libc.so.6"), + ], + "map_files must be preferred, with the pathname column as fallback" + ); + } + + /// A pathname naming no openable file must not be offered: opening it either + /// fails or, for a reused path, reads the wrong file. + #[test] + fn candidate_paths_skips_unopenable_pathnames() { + for display in [ + "/tmp/libfoo.so (deleted)", + "[heap]", + "[vdso]", + "", + ] { + let mapping = ResolvedMapping { + attach_path: PathBuf::from("/proc/1/map_files/400000-452000"), + display: display.to_string(), + }; + assert_eq!( + mapping.candidate_paths(), + vec![PathBuf::from("/proc/1/map_files/400000-452000")], + "{display} should not be offered as an attach path" + ); + } + } + #[test] fn resolve_mapping_unresolved_for_wrong_inode_on_live_process() { // Our own maps is readable (process alive), but no row carries this diff --git a/crates/memtrack/src/ebpf/tracker.rs b/crates/memtrack/src/ebpf/tracker.rs index 11ecaf04e..094da2eb4 100644 --- a/crates/memtrack/src/ebpf/tracker.rs +++ b/crates/memtrack/src/ebpf/tracker.rs @@ -1,6 +1,6 @@ -use crate::ebpf::MemtrackBpf; use crate::ebpf::attach_worker::AttachWorker; use crate::ebpf::spawn::{resume, spawn_stopped, wrap_stopped}; +use crate::ebpf::{BpfVariant, MemtrackBpf}; use crate::prelude::*; use crate::session::Session; use parking_lot::Mutex; @@ -18,9 +18,18 @@ impl Tracker { /// Create a new tracker. The exec-mapping watcher discovers and attaches /// allocator probes as the tracked process tree maps executable files. pub fn new() -> Result { + Self::with_bpf(MemtrackBpf::new()?) + } + + /// Like [`Tracker::new`], but pinned to a specific BPF variant instead of + /// the detected one. + pub fn with_variant(variant: BpfVariant) -> Result { + Self::with_bpf(MemtrackBpf::with_variant(variant)?) + } + + fn with_bpf(mut bpf: MemtrackBpf) -> Result { Self::bump_memlock_rlimit()?; - let mut bpf = MemtrackBpf::new()?; bpf.attach_tracepoints()?; bpf.attach_exec_watcher()?; diff --git a/crates/memtrack/src/lib.rs b/crates/memtrack/src/lib.rs index 1186ef2d6..20b6b10d1 100644 --- a/crates/memtrack/src/lib.rs +++ b/crates/memtrack/src/lib.rs @@ -1,4 +1,5 @@ mod allocators; +mod bpf_token; #[cfg(feature = "ebpf")] mod ebpf; mod ipc; @@ -7,6 +8,7 @@ pub mod prelude; mod session; pub use allocators::{AllocatorKind, AllocatorLib}; +pub use bpf_token::has_delegated_bpf_token; pub use ipc::{ IpcCommand as MemtrackIpcCommand, IpcMessage as MemtrackIpcMessage, IpcResponse as MemtrackIpcResponse, MemtrackIpcClient, MemtrackIpcServer, diff --git a/crates/memtrack/tests/c_tests.rs b/crates/memtrack/tests/c_tests.rs index d9952a778..17fb0a3ad 100644 --- a/crates/memtrack/tests/c_tests.rs +++ b/crates/memtrack/tests/c_tests.rs @@ -98,11 +98,7 @@ fn test_allocation_tracking( let temp_dir = TempDir::new()?; let binary = compile_c_source(test_case.source, test_case.name, temp_dir.path())?; - let (events, thread_handle) = shared::track_binary(&binary)?; - - assert_events_snapshot!(test_case.name, events); - - thread_handle.join().unwrap(); + assert_events_snapshot_for_each_variant!(test_case.name, || Command::new(&binary))?; Ok(()) } diff --git a/crates/memtrack/tests/cpp_tests.rs b/crates/memtrack/tests/cpp_tests.rs index b7c5b61c7..3eac700dc 100644 --- a/crates/memtrack/tests/cpp_tests.rs +++ b/crates/memtrack/tests/cpp_tests.rs @@ -75,9 +75,7 @@ fn test_cpp_alloc_tracking(#[case] target: &str) -> Result<(), Box Result<(), Box> { &["-ldl"], ); - let mut cmd = Command::new(&exe); - cmd.arg(&lib); - let (events, thread_handle) = shared::track_command(cmd)?; + shared::for_each_variant( + || { + let mut cmd = Command::new(&exe); + cmd.arg(&lib); + cmd + }, + |events| { + let malloc_addrs: HashSet = events + .iter() + .filter_map(|e| match e.kind { + MemtrackEventKind::Malloc { size: 4242 } => Some(e.addr), + _ => None, + }) + .collect(); + let malloc_count = events + .iter() + .filter(|e| matches!(e.kind, MemtrackEventKind::Malloc { size: 4242 })) + .count(); + let free_count = events + .iter() + .filter(|e| { + matches!(e.kind, MemtrackEventKind::Free) && malloc_addrs.contains(&e.addr) + }) + .count(); - let malloc_addrs: HashSet = events - .iter() - .filter_map(|e| match e.kind { - MemtrackEventKind::Malloc { size: 4242 } => Some(e.addr), - _ => None, - }) - .collect(); - let malloc_count = events - .iter() - .filter(|e| matches!(e.kind, MemtrackEventKind::Malloc { size: 4242 })) - .count(); - let free_count = events - .iter() - .filter(|e| matches!(e.kind, MemtrackEventKind::Free) && malloc_addrs.contains(&e.addr)) - .count(); + assert_eq!(malloc_count, 100, "expected 100 mi_malloc(4242) events"); + assert_eq!( + free_count, 100, + "expected 100 mi_free events for tracked addrs" + ); + }, + )?; - assert_eq!(malloc_count, 100, "expected 100 mi_malloc(4242) events"); - assert_eq!( - free_count, 100, - "expected 100 mi_free events for tracked addrs" - ); - - thread_handle.join().unwrap(); Ok(()) } @@ -110,22 +116,26 @@ fn test_thread_dlopen() -> Result<(), Box> { &["-ldl", "-lpthread"], ); - let mut cmd = Command::new(&exe); - cmd.arg(&mimalloc).arg(&jemalloc); - let (events, thread_handle) = shared::track_command(cmd)?; - - let m4242 = events - .iter() - .filter(|e| matches!(e.kind, MemtrackEventKind::Malloc { size: 4242 })) - .count(); - let m4243 = events - .iter() - .filter(|e| matches!(e.kind, MemtrackEventKind::Malloc { size: 4243 })) - .count(); + shared::for_each_variant( + || { + let mut cmd = Command::new(&exe); + cmd.arg(&mimalloc).arg(&jemalloc); + cmd + }, + |events| { + let m4242 = events + .iter() + .filter(|e| matches!(e.kind, MemtrackEventKind::Malloc { size: 4242 })) + .count(); + let m4243 = events + .iter() + .filter(|e| matches!(e.kind, MemtrackEventKind::Malloc { size: 4243 })) + .count(); - assert_eq!(m4242, 100, "expected 100 mi_malloc(4242) events"); - assert_eq!(m4243, 100, "expected 100 je_malloc(4243) events"); + assert_eq!(m4242, 100, "expected 100 mi_malloc(4242) events"); + assert_eq!(m4243, 100, "expected 100 je_malloc(4243) events"); + }, + )?; - thread_handle.join().unwrap(); Ok(()) } diff --git a/crates/memtrack/tests/rust_tests.rs b/crates/memtrack/tests/rust_tests.rs index ce1857aa8..467631815 100644 --- a/crates/memtrack/tests/rust_tests.rs +++ b/crates/memtrack/tests/rust_tests.rs @@ -3,6 +3,7 @@ mod shared; use rstest::rstest; use std::path::Path; +use std::process::Command; #[test_with::env(GITHUB_ACTIONS)] #[rstest] @@ -18,9 +19,7 @@ fn test_rust_alloc_tracking( let binary = shared::compile_rust_binary(crate_path, "alloc_rust", features)?; // No extra allocators: the watcher must discover the static allocator itself. - let (events, thread_handle) = shared::track_binary(&binary)?; - assert_events_with_marker!(name, &events); + assert_events_with_marker_for_each_variant!(name, || Command::new(&binary))?; - thread_handle.join().unwrap(); Ok(()) } diff --git a/crates/memtrack/tests/shared.rs b/crates/memtrack/tests/shared.rs index d31bce093..f55cff49b 100644 --- a/crates/memtrack/tests/shared.rs +++ b/crates/memtrack/tests/shared.rs @@ -1,19 +1,16 @@ #![allow(dead_code, unused)] -use memtrack::Tracker; use memtrack::prelude::*; +use memtrack::{BpfVariant, Tracker}; use runner_shared::artifacts::{MemtrackEvent as Event, MemtrackEventKind}; use std::path::Path; use std::process::Command; type TrackResult = anyhow::Result<(Vec, std::thread::JoinHandle<()>)>; -/// Asserts memory events using snapshot testing without marker filtering. +/// Snapshot every tracked event, ordered by timestamp and deduplicated by +/// `(addr, kind)` so repeated tracking of one allocation counts once. /// -/// Formats and snapshots all memory events for regression testing. -/// Deduplicates by address and type to remove duplicate tracking. -/// -/// # Example /// ```no_run /// let (events, _handle) = track_binary(&binary)?; /// assert_events_snapshot!("test_name", events); @@ -21,87 +18,96 @@ type TrackResult = anyhow::Result<(Vec, std::thread::JoinHandle<()>)>; macro_rules! assert_events_snapshot { ($name:expr, $events:expr) => {{ use itertools::Itertools; - use runner_shared::artifacts::MemtrackEventKind; use std::mem::discriminant; - // Dedup events by address and type to remove duplicates - let events = $events + let formatted_events: Vec = $events .iter() .sorted_by_key(|e| e.timestamp) .dedup_by(|a, b| a.addr == b.addr && discriminant(&a.kind) == discriminant(&b.kind)) - .collect::>(); - - let formatted_events: Vec = events - .iter() - .map(|e| match e.kind { - // Exclude address in snapshots: - MemtrackEventKind::Realloc { size, .. } => format!("Realloc {{ size: {} }}", size), - _ => format!("{:?}", e.kind), - }) + .map(|e| shared::describe_kind(&e.kind)) .collect(); insta::assert_debug_snapshot!($name, formatted_events); }}; } -/// Asserts events, filtered by a 0xC0D59EED allocation marker to -/// exclude noise. -/// -/// Processes events by: -/// 1. Deduplicating by address and event type -/// 2. Filtering to events between 0xC0D59EED marker allocations -/// 3. Creating an insta snapshot for regression testing -/// -/// Binary must alloc `0xC0D59EED` memory before and after. -/// -/// # Example +/// [`assert_events_snapshot`] over only the events the workload bracketed with +/// `malloc(0xC0D59EED)`, which keeps libc and runtime noise out of the snapshot. +/// The workload must allocate the marker before and after the region of interest: /// -/// Do the following in the test: /// ```no_run -/// malloc(0xC0D59EED) -/// // do you allocations/frees here -/// malloc(0xC0D59EED) +/// malloc(0xC0D59EED); +/// // allocations under test +/// malloc(0xC0D59EED); /// ``` /// -/// Then in Rust: /// ```no_run /// let (events, _handle) = track_binary(&binary)?; /// assert_events_with_marker!("test_name", events); /// ``` macro_rules! assert_events_with_marker { ($name:expr, $events:expr) => {{ - use itertools::Itertools; - use runner_shared::artifacts::MemtrackEventKind; - use std::mem::discriminant; - - // Remove events outside our 0xC0D59EED marker allocations - let filtered_events = $events - .iter() - .sorted_by_key(|e| e.timestamp) - .dedup_by(|a, b| a.addr == b.addr && discriminant(&a.kind) == discriminant(&b.kind)) - .skip_while(|e| { - let MemtrackEventKind::Malloc { size } = e.kind else { - return true; - }; - size != 0xC0D59EED - }) - .skip(2) // Skip the marker allocation and free - .take_while(|e| { - let MemtrackEventKind::Malloc { size } = e.kind else { - return true; - }; - size != 0xC0D59EED - }) - .cloned() - .collect::>(); - - assert_events_snapshot!($name, filtered_events); + let filtered_events = shared::between_markers($events); + assert_events_snapshot!($name, &filtered_events); }}; } -/// Compile a Rust binary from a test crate directory. +/// [`assert_events_snapshot`] run under each BPF variant. `$workload` is called +/// once per variant, so it must return a fresh [`Command`] every time. /// -/// Each feature set builds into its own target dir: parallel test cases would -/// otherwise overwrite the same output binary and race against each other. +/// ```no_run +/// assert_events_snapshot_for_each_variant!("test_name", || Command::new(&binary)); +/// ``` +macro_rules! assert_events_snapshot_for_each_variant { + ($name:expr, $workload:expr) => { + shared::for_each_variant($workload, |events| { + assert_events_snapshot!($name, events); + }) + }; +} + +/// [`assert_events_with_marker`] run under each BPF variant. `$workload` is +/// called once per variant, so it must return a fresh [`Command`] every time. +macro_rules! assert_events_with_marker_for_each_variant { + ($name:expr, $workload:expr) => { + shared::for_each_variant($workload, |events| { + assert_events_with_marker!($name, events); + }) + }; +} + +/// An event's kind and size, without the addresses that differ between runs of +/// the same workload. `Realloc` needs spelling out since its `Debug` includes the +/// old address. +pub fn describe_kind(kind: &MemtrackEventKind) -> String { + match kind { + MemtrackEventKind::Realloc { size, .. } => format!("Realloc {{ size: {size} }}"), + other => format!("{other:?}"), + } +} + +/// The events between the workload's `malloc(0xC0D59EED)` markers, ordered by +/// timestamp and deduplicated by `(addr, kind)`. +pub fn between_markers(events: &[Event]) -> Vec { + use itertools::Itertools; + use std::mem::discriminant; + + const MARKER: u64 = 0xC0D5_9EED; + let is_marker = + |e: &&Event| matches!(e.kind, MemtrackEventKind::Malloc { size } if size == MARKER); + + events + .iter() + .sorted_by_key(|e| e.timestamp) + .dedup_by(|a, b| a.addr == b.addr && discriminant(&a.kind) == discriminant(&b.kind)) + .skip_while(|e| !is_marker(e)) + .skip(2) // the opening marker malloc and its free + .take_while(|e| !is_marker(e)) + .cloned() + .collect() +} + +/// Compile a Rust binary from a test crate directory. Each feature set gets its +/// own target dir, otherwise parallel test cases race to overwrite one binary. pub fn compile_rust_binary( crate_dir: &Path, name: &str, @@ -141,12 +147,72 @@ pub fn track_binary(binary: &Path) -> TrackResult { track_command(Command::new(binary)) } -/// Track a command, collecting all memory events. -/// -/// No allocators are pre-attached: the exec-mapping watcher discovers and -/// attaches them as the tracked tree maps executable files. +/// Track a command, collecting all memory events. No allocators are pre-attached: +/// the exec-mapping watcher discovers them as the tracked tree maps executables. pub fn track_command(command: Command) -> TrackResult { - let tracker = Tracker::new()?; + track_command_with_tracker(command, Tracker::new()?) +} + +/// Track a command under a specific BPF variant rather than the detected one. +pub fn track_command_with_variant(command: Command, variant: BpfVariant) -> TrackResult { + track_command_with_tracker(command, Tracker::with_variant(variant)?) +} + +/// How many events of each kind-and-size a run saw. Addresses, timestamps, pids +/// and event order all differ legitimately between runs of the same workload, so +/// none of them can be compared across variants. +type EventProfile = std::collections::BTreeMap; + +fn event_profile(events: &[Event]) -> EventProfile { + let mut profile = EventProfile::new(); + for event in events { + *profile.entry(describe_kind(&event.kind)).or_default() += 1; + } + profile +} + +/// Run `workload` under each BPF variant, pass every run's events to +/// `assert_events`, and require the variants to have observed the same +/// allocations: they differ only in how probes attach, so what one sees the other +/// must see too. +/// +/// Variants that cannot attach on this host are skipped, since the token variant +/// needs `uprobe_multi` (kernel >= 6.6). Panics if none attach. +pub fn for_each_variant( + mut workload: impl FnMut() -> Command, + mut assert_events: impl FnMut(&[Event]), +) -> anyhow::Result<()> { + let mut profiles: Vec<(BpfVariant, EventProfile)> = Vec::new(); + + for variant in [BpfVariant::Legacy, BpfVariant::Token] { + let tracker = match Tracker::with_variant(variant) { + Ok(tracker) => tracker, + Err(err) => { + eprintln!("skipping {variant:?} variant, cannot attach here: {err:#}"); + continue; + } + }; + + let (events, thread_handle) = track_command_with_tracker(workload(), tracker)?; + assert_events(&events); + profiles.push((variant, event_profile(&events))); + thread_handle.join().unwrap(); + } + + let Some(((first_variant, first), rest)) = profiles.split_first() else { + panic!("no BPF variant could attach"); + }; + for (variant, profile) in rest { + assert_eq!( + first, profile, + "{first_variant:?} and {variant:?} variants disagree on tracked allocations" + ); + } + + Ok(()) +} + +fn track_command_with_tracker(command: Command, tracker: Tracker) -> TrackResult { tracker.enable_tracking()?; let mut session = tracker.spawn(&command, None)?; @@ -160,7 +226,8 @@ pub fn track_command(command: Command) -> TrackResult { tracker.finish()?; - // Drop the tracker in a new thread to not block the test. + // Detaching the probes blocks on RCU grace periods; let the caller decide + // when to wait for it. let thread_handle = std::thread::spawn(move || drop(tracker)); info!("Tracked {} events", events.len()); diff --git a/crates/memtrack/tests/spawn_tests.rs b/crates/memtrack/tests/spawn_tests.rs index 5511a752f..25b2cb0c5 100644 --- a/crates/memtrack/tests/spawn_tests.rs +++ b/crates/memtrack/tests/spawn_tests.rs @@ -17,12 +17,11 @@ fn test_spawn_static_allocator_discovery() -> Result<(), Box Result<()> { - if is_root_user() || has_memtrack_capabilities() { + if is_root_user() || has_delegated_bpf_token() || has_memtrack_capabilities() { return Ok(()); } @@ -114,6 +115,11 @@ impl Executor for MemoryExecutor { detail: "running as root".to_string(), }); } + if has_delegated_bpf_token() { + return Some(PrivilegeStatus::Satisfied { + detail: "delegated BPF token".to_string(), + }); + } if has_memtrack_capabilities() { return Some(PrivilegeStatus::Satisfied { detail: "capabilities granted".to_string(), @@ -140,6 +146,11 @@ impl Executor for MemoryExecutor { } fn grant_privileges(&self) -> Result<()> { + // A delegated BPF token already provides the privilege memtrack needs; + // there is nothing to grant (and no sudo to prompt for) in that case. + if has_delegated_bpf_token() { + return Ok(()); + } ensure_memtrack_capabilities() } diff --git a/src/executor/tests.rs b/src/executor/tests.rs index d562c6c90..1432c92ee 100644 --- a/src/executor/tests.rs +++ b/src/executor/tests.rs @@ -127,6 +127,7 @@ fi let path = escargot::CargoBuild::new() .package(package) .bin(bin) + .release() .current_target() .run() .unwrap_or_else(|e| panic!("failed to build the {bin} binary: {e}")) diff --git a/src/executor/wall_time/executor.rs b/src/executor/wall_time/executor.rs index a15a96008..810049ebd 100644 --- a/src/executor/wall_time/executor.rs +++ b/src/executor/wall_time/executor.rs @@ -1,5 +1,5 @@ use super::helpers::validate_walltime_results; -use super::isolation::{requires_isolation, wrap_isolation_scope}; +use super::isolation::Isolation; use super::profiler::Profiler; use super::profiler::perf::PerfProfiler; use super::profiler::samply::SamplyProfiler; @@ -29,58 +29,8 @@ use std::cell::OnceCell; use std::fs::canonicalize; use std::io::Write; use std::path::Path; -use std::path::PathBuf; -use std::process::Command; use tempfile::NamedTempFile; -struct HookScriptsGuard { - post_bench_script: PathBuf, -} - -impl HookScriptsGuard { - fn execute_script_from_path>(path: P) -> anyhow::Result<()> { - let path = path.as_ref(); - if !path.exists() || !path.is_file() { - debug!("Script not found or not a file: {}", path.display()); - return Ok(()); - } - - let output = Command::new("bash").args([&path]).output()?; - if !output.status.success() { - debug!("stdout: {}", String::from_utf8_lossy(&output.stdout)); - debug!("stderr: {}", String::from_utf8_lossy(&output.stderr)); - bail!("Failed to execute script: {}", path.display()); - } - - Ok(()) - } - - pub fn setup_with_scripts>(pre_bench_script: P, post_bench_script: P) -> Self { - if let Err(e) = Self::execute_script_from_path(pre_bench_script.as_ref()) { - debug!("Failed to execute pre-bench script: {e}"); - } - - Self { - post_bench_script: post_bench_script.as_ref().to_path_buf(), - } - } - - pub fn setup() -> Self { - Self::setup_with_scripts( - "/usr/local/bin/codspeed-pre-bench", - "/usr/local/bin/codspeed-post-bench", - ) - } -} - -impl Drop for HookScriptsGuard { - fn drop(&mut self) { - if let Err(e) = Self::execute_script_from_path(&self.post_bench_script) { - debug!("Failed to execute post-bench script: {e}"); - } - } -} - pub struct WallTimeExecutor { profiler: Option>, @@ -168,19 +118,16 @@ impl Executor for WallTimeExecutor { execution_context: &ExecutionContext, _mongo_tracer: &Option, ) -> Result<()> { - let _guard = HookScriptsGuard::setup(); + // Resolve isolation once: this runs the pre-bench setup, wraps the bench + // leaf, and (for hook mode) runs post-bench on drop. Held for the whole run + // so its teardown fires after the benchmark completes. `requires_sudo` is + // read off it for the privilege wrapping below (or in the profiler). + let isolation = Isolation::resolve(); + let requires_sudo = isolation.requires_sudo(); let (_env_file, _script_file, cmd_builder) = WallTimeExecutor::walltime_bench_cmd(&execution_context.config, execution_context)?; - - // Resolve the isolation decision once and reuse it for both the scope - // wrapping here and the privilege wrapping below (or in the profiler). - let isolate = requires_isolation(); - let cmd_builder = if isolate { - wrap_isolation_scope(cmd_builder)? - } else { - cmd_builder - }; + let cmd_builder = isolation.wrap_bench(cmd_builder)?; // Split-borrow `self` so the closure inside `run_with_profiler` can // capture `benchmark_state` while we hold `&mut profiler`. @@ -196,16 +143,13 @@ impl Executor for WallTimeExecutor { cmd_builder, &execution_context.config, &execution_context.profile_folder, - isolate, + requires_sudo, benchmark_state, ) .await } _ => { - // No profiler: still honor the isolation decision, since the - // scope (and the sudo it requires) is a property of the run, not - // of the profiler. - let cmd_builder = if isolate { + let cmd_builder = if requires_sudo { wrap_with_sudo(cmd_builder)? } else { cmd_builder @@ -254,11 +198,11 @@ async fn run_with_profiler( cmd_builder: CommandBuilder, config: &ExecutorConfig, profile_folder: &Path, - isolate: bool, + requires_sudo: bool, benchmark_state: &OnceCell<(FifoBenchmarkData, ExecutionTimestamps)>, ) -> Result { let wrapped = profiler - .wrap_command(cmd_builder, config, profile_folder, isolate) + .wrap_command(cmd_builder, config, profile_folder, requires_sudo) .await?; let cmd = wrapped.build(); debug!("cmd: {cmd:?}"); @@ -296,51 +240,3 @@ async fn run_with_profiler( }) .await } - -#[cfg(test)] -mod tests { - use tempfile::NamedTempFile; - - use super::*; - use std::{ - io::{Read, Write}, - os::unix::fs::PermissionsExt, - }; - - #[test] - fn test_env_guard_no_crash() { - fn create_run_script(content: &str) -> anyhow::Result { - let rwx = std::fs::Permissions::from_mode(0o777); - let mut script_file = tempfile::Builder::new() - .suffix(".sh") - .permissions(rwx) - .disable_cleanup(true) - .tempfile()?; - script_file.write_all(content.as_bytes())?; - - Ok(script_file) - } - - let mut tmp_dst = tempfile::NamedTempFile::new().unwrap(); - - let pre_script = create_run_script(&format!( - "#!/usr/bin/env bash\necho \"pre\" >> {}", - tmp_dst.path().display() - )) - .unwrap(); - let post_script = create_run_script(&format!( - "#!/usr/bin/env bash\necho \"post\" >> {}", - tmp_dst.path().display() - )) - .unwrap(); - - { - let _guard = - HookScriptsGuard::setup_with_scripts(pre_script.path(), post_script.path()); - } - - let mut result = String::new(); - tmp_dst.read_to_string(&mut result).unwrap(); - assert_eq!(result, "pre\npost\n"); - } -} diff --git a/src/executor/wall_time/isolation.rs b/src/executor/wall_time/isolation.rs index cc6911908..33ac04f48 100644 --- a/src/executor/wall_time/isolation.rs +++ b/src/executor/wall_time/isolation.rs @@ -1,60 +1,171 @@ +use std::process::Command; + use crate::executor::helpers::command::CommandBuilder; use crate::prelude::*; -/// Whether the benchmark must run inside the privileged systemd scope, which in -/// turn requires the profiler itself to record under elevated privileges. -/// -/// When isolated, the scope reparents the benchmark out of the profiler's -/// process subtree, so the profiler can only observe it by recording -/// system-wide — which needs `sudo`. When not isolated, the profiler records its -/// own descendant tree and runs unprivileged (relying on a permissive -/// `perf_event_paranoid`). -/// -/// `CODSPEED_ISOLATION` overrides the decision; otherwise we isolate only when we -/// can elevate without prompting (root or passwordless sudo, as on CI), so a -/// local run never blocks on a password. +/// Paths of the machine's CPU-isolation hooks. The machine image installs them +/// and owns all cpuset logic behind them; the runner only invokes them. +const PRE_BENCH_HOOK: &str = "/usr/local/bin/codspeed-pre-bench"; +const WRAP_BENCH_HOOK: &str = "/usr/local/bin/codspeed-wrap-bench"; +const POST_BENCH_HOOK: &str = "/usr/local/bin/codspeed-post-bench"; + +/// Environment variable selecting how walltime runs isolate the benchmark. +const ISOLATION_ENV: &str = "CODSPEED_ISOLATION"; + +/// How the benchmark is pinned to dedicated cores, away from the rest of the +/// system, for the lifetime of one walltime run. /// -/// Isolation relies on `systemd-run`, so it is Linux-only; other platforms always -/// record their own descendant tree unprivileged. -pub fn requires_isolation() -> bool { - if !cfg!(target_os = "linux") { - return false; +/// This owns the whole isolation lifecycle: [`resolve`](Self::resolve) runs the +/// machine's pre-bench setup, [`wrap_bench`](Self::wrap_bench) pins the benchmark +/// leaf, and [`Drop`] runs the post-bench teardown. The runner holds one of these +/// for the duration of the run and is otherwise oblivious to *how* cores are +/// attributed — that lives entirely on the machine (its hooks) or in the systemd +/// fallback below. +#[derive(Debug)] +pub enum Isolation { + /// The benchmark runs unpinned, in the runner's own process subtree. + None, + /// The machine's hooks own core attribution. The image built a static cgroup + /// skeleton and ships `codspeed-{pre,wrap,post}-bench`. `pre-bench` (already + /// run) evacuated the bench cores: every process of the workload tree — the + /// runner included, hence also the profiler it later spawns — moved onto the + /// system cores; `wrap-bench` moves the benchmark onto the bench cores; + /// `post-bench` restores the default placement on drop. The benchmark stays a + /// descendant of the profiler, so the profiler records it unprivileged. + Hooks, + /// Gen-1 fallback, used when the machine ships no `codspeed-wrap-bench` hook. + /// `systemd-run --scope --slice=codspeed.slice` reparents the benchmark out of + /// the profiler's subtree, so the profiler needs elevated privileges (sudo) to + /// observe it. + Systemd, +} + +impl Isolation { + /// Resolve how this run isolates the benchmark, running any required setup + /// (the pre-bench hook) as a side effect. Decision, from [`ISOLATION_ENV`]: + /// + /// - non-Linux: [`None`](Self::None), no mechanism is available; + /// - `false`: [`None`](Self::None); + /// - otherwise, on Linux: + /// - if the machine ships an executable `codspeed-wrap-bench`: [`Hooks`](Self::Hooks) + /// (the forward path — the machine owns core attribution); + /// - else [`Systemd`](Self::Systemd) if `true` was set explicitly, which opts + /// into a password prompt, or if we can elevate without one (root or + /// passwordless sudo, as on CI / gen-1 images); + /// - else [`None`](Self::None), so an unattended local run never blocks on a prompt. + pub fn resolve() -> Self { + if !cfg!(target_os = "linux") { + return Isolation::None; + } + + let forced = match std::env::var(ISOLATION_ENV).as_deref() { + Ok("false") => return Isolation::None, + Ok("true") => true, + _ => false, + }; + + if hook_is_executable(WRAP_BENCH_HOOK) { + run_pre_bench(); + return Isolation::Hooks; + } + + if forced || crate::executor::helpers::run_with_sudo::can_elevate_without_prompt() { + return Isolation::Systemd; + } + + info!( + "Running without process isolation: no {WRAP_BENCH_HOOK} hook, and elevating \ + would require a password prompt. Set {ISOLATION_ENV}=true to force it." + ); + Isolation::None } - match std::env::var("CODSPEED_ISOLATION").as_deref() { - Ok("true") => true, - Ok("false") => false, - _ => { - let can_isolate = crate::executor::helpers::run_with_sudo::can_elevate_without_prompt(); - if !can_isolate { - info!( - "Running without process isolation: elevating privileges would require a \ - password prompt. Set CODSPEED_ISOLATION=true to force it." - ); - } - can_isolate + + /// Whether the profiler must run under sudo. True only for [`Systemd`](Self::Systemd), + /// where the benchmark is reparented out of the profiler's subtree. + pub fn requires_sudo(&self) -> bool { + matches!(self, Isolation::Systemd) + } + + /// Wrap the benchmark leaf so it runs pinned according to this mode. The + /// profiler wraps its recorder around the result, so this must only touch the + /// leaf and never move the profiler onto the bench cores. + pub fn wrap_bench(&self, bench_cmd: CommandBuilder) -> Result { + match self { + Isolation::None => Ok(bench_cmd), + Isolation::Hooks => wrap_with_hook(bench_cmd), + Isolation::Systemd => wrap_isolation_scope(bench_cmd), } } } -/// Run the benchmark command in an isolated process scope. -/// -/// On Linux, the command is wrapped with `systemd-run --scope` so it runs inside the -/// `codspeed.slice` cgroup (predefined on CodSpeed CI runners to pin and isolate the -/// benchmark). Only applied when [`requires_isolation`] is true. +impl Drop for Isolation { + fn drop(&mut self) { + if matches!(self, Isolation::Hooks) { + run_hook(POST_BENCH_HOOK, &[]); + } + } +} + +/// Whether `path` is an executable file. Used to detect machine hooks. +fn hook_is_executable(path: &str) -> bool { + #[cfg(target_os = "linux")] + { + use std::os::unix::fs::PermissionsExt; + std::fs::metadata(path) + .map(|m| m.is_file() && m.permissions().mode() & 0o111 != 0) + .unwrap_or(false) + } + #[cfg(not(target_os = "linux"))] + { + let _ = path; + false + } +} + +/// Run a machine hook, logging (never failing the run) on error. Isolation is +/// best-effort: a missing or failing hook leaves the benchmark less isolated, but +/// must not abort the run. +fn run_hook(hook: &str, args: &[String]) { + let output = Command::new(hook).args(args).output(); + match output { + Ok(o) if o.status.success() => {} + Ok(o) => debug!( + "{hook} exited {}: {}", + o.status, + String::from_utf8_lossy(&o.stderr) + ), + Err(e) => debug!("failed to run {hook}: {e}"), + } +} + +/// Run the pre-bench hook, which moves the workload tree — the runner included, +/// hence the profiler it later spawns — onto the system cores. The runner's own +/// PID is passed for hooks that only relocate their caller. +fn run_pre_bench() { + run_hook(PRE_BENCH_HOOK, &[std::process::id().to_string()]); +} + +/// Wrap the benchmark with `codspeed-wrap-bench`, which moves itself onto the +/// bench cores and `exec`s the benchmark — so the benchmark inherits the pinning +/// while staying the same PID and a descendant of the profiler. The hook is a +/// command wrapper: it takes the program and its arguments and `exec`s them. +fn wrap_with_hook(mut bench_cmd: CommandBuilder) -> Result { + bench_cmd.wrap(WRAP_BENCH_HOOK, [] as [&str; 0]); + Ok(bench_cmd) +} + +/// Wrap the benchmark leaf so it runs inside the systemd `codspeed.slice` scope. /// -/// Remarks: -/// - We're using `--scope` so that the profiler is able to capture the events of the benchmark -/// process. -/// - We can't use `--user` here because we need to run in `codspeed.slice`, otherwise we'd run in -/// `user.slice` (which is isolated). We use `--uid` and `--gid` to keep running as the current -/// user. -/// - `--scope` only inherits the system environment, so the caller is expected to have already -/// forwarded the relevant variables (via `wrap_with_env`). -/// - The caller is expected to have already set the working directory on `bench_cmd`; it will be -/// propagated to `systemd-run` via [`CommandBuilder::wrap_with`], and `--same-dir` makes the -/// spawned scope inherit it. +/// Notes on the `systemd-run` flags: +/// - `--scope` (rather than a transient service) keeps the benchmark a child of +/// the runner, so the profiler can capture its events; see [`Isolation::Systemd`]. +/// - `--user` would land us in `user.slice`, not `codspeed.slice`; `--uid`/`--gid` +/// instead keep the scope running as the current user. +/// - `--scope` only inherits the system environment, so the caller must have +/// already forwarded the benchmark's variables (via `wrap_with_env`) and set +/// its working directory, which `--same-dir` propagates into the scope. #[cfg(target_os = "linux")] -pub fn wrap_isolation_scope(mut bench_cmd: CommandBuilder) -> Result { +fn wrap_isolation_scope(mut bench_cmd: CommandBuilder) -> Result { use crate::executor::helpers::env::is_codspeed_debug_enabled; let mut cmd_builder = CommandBuilder::new("systemd-run"); @@ -75,6 +186,45 @@ pub fn wrap_isolation_scope(mut bench_cmd: CommandBuilder) -> Result Result { +fn wrap_isolation_scope(bench_cmd: CommandBuilder) -> Result { Ok(bench_cmd) } + +#[cfg(test)] +mod tests { + use super::*; + use temp_env::with_vars; + + #[test] + fn isolation_false_disables() { + with_vars([(ISOLATION_ENV, Some("false"))], || { + assert!(matches!(Isolation::resolve(), Isolation::None)); + }); + } + + #[test] + fn systemd_requires_sudo_but_hooks_do_not() { + assert!(Isolation::Systemd.requires_sudo()); + assert!(!Isolation::Hooks.requires_sudo()); + assert!(!Isolation::None.requires_sudo()); + } + + #[test] + fn hook_wrap_prepends_wrap_bench() { + let mut cmd = CommandBuilder::new("bash"); + cmd.arg("/tmp/bench.sh"); + let wrapped = Isolation::Hooks.wrap_bench(cmd).unwrap(); + assert_eq!( + wrapped.as_command_line(), + format!("{WRAP_BENCH_HOOK} bash /tmp/bench.sh") + ); + } + + #[test] + fn none_wrap_is_passthrough() { + let mut cmd = CommandBuilder::new("bash"); + cmd.arg("/tmp/bench.sh"); + let wrapped = Isolation::None.wrap_bench(cmd).unwrap(); + assert_eq!(wrapped.as_command_line(), "bash /tmp/bench.sh"); + } +} diff --git a/src/executor/wall_time/profiler/mod.rs b/src/executor/wall_time/profiler/mod.rs index 0ee6becc8..ab7f62cb6 100644 --- a/src/executor/wall_time/profiler/mod.rs +++ b/src/executor/wall_time/profiler/mod.rs @@ -48,15 +48,16 @@ pub trait Profiler { /// Profilers stash any live state they need for the duration of the run /// (control fifos, output paths) on `self`. /// - /// `isolate` mirrors the decision the executor used to wrap the benchmark in - /// the systemd scope: when set, the profiler records system-wide under sudo; - /// otherwise it records its own descendant tree unprivileged. + /// When `requires_sudo` is set, the profiler must run elevated: the + /// isolation mechanism reparented the benchmark out of its subtree, so it can + /// only observe it with elevated privileges. Otherwise it records its own + /// descendant tree unprivileged. async fn wrap_command( &mut self, cmd: CommandBuilder, config: &ExecutorConfig, profile_folder: &Path, - isolate: bool, + requires_sudo: bool, ) -> anyhow::Result; /// The benchmarked process signaled the start of a measured region. diff --git a/src/executor/wall_time/profiler/perf/mod.rs b/src/executor/wall_time/profiler/perf/mod.rs index 5536af126..2c5514d24 100644 --- a/src/executor/wall_time/profiler/perf/mod.rs +++ b/src/executor/wall_time/profiler/perf/mod.rs @@ -91,7 +91,7 @@ impl Profiler for PerfProfiler { mut cmd_builder: CommandBuilder, config: &ExecutorConfig, profile_folder: &Path, - isolate: bool, + requires_sudo: bool, ) -> anyhow::Result { let perf_fifo = PerfFifo::new()?; let perf_file_path = profile_folder.join(PERF_PIPEDATA_FILE_NAME); @@ -183,10 +183,9 @@ impl Profiler for PerfProfiler { self.perf_fifo = Some(perf_fifo); self.perf_file_path = Some(perf_file_path); - // Isolated runs reparent the benchmark out of perf's subtree, so perf - // must record system-wide under sudo. Unisolated runs record perf's own - // descendant tree unprivileged. - let wrapped_builder = if isolate { + // When the benchmark was reparented out of perf's subtree, perf needs + // sudo to observe it; otherwise it records its own descendants unprivileged. + let wrapped_builder = if requires_sudo { wrap_with_sudo(wrapped_builder)? } else { wrapped_builder diff --git a/src/executor/wall_time/profiler/samply/mod.rs b/src/executor/wall_time/profiler/samply/mod.rs index 7a1a6f04e..97b5bffd9 100644 --- a/src/executor/wall_time/profiler/samply/mod.rs +++ b/src/executor/wall_time/profiler/samply/mod.rs @@ -86,7 +86,7 @@ impl Profiler for SamplyProfiler { mut cmd_builder: CommandBuilder, _config: &ExecutorConfig, profile_folder: &Path, - isolate: bool, + requires_sudo: bool, ) -> anyhow::Result { let output_path = profile_folder.join(SAMPLY_OUTPUT_FILE_NAME); @@ -157,10 +157,9 @@ impl Profiler for SamplyProfiler { self.output_path = Some(output_path); self.v8_log_dir = Some(v8_log_dir); - // Isolated runs reparent the benchmark out of samply's subtree, so samply - // must record system-wide under sudo. Unisolated runs record samply's own - // descendant tree unprivileged. - let cmd_builder = if isolate { + // When the benchmark was reparented out of samply's subtree, samply needs + // sudo to observe it; otherwise it records its own descendants unprivileged. + let cmd_builder = if requires_sudo { wrap_with_sudo(cmd_builder)? } else { cmd_builder