From 1e9b524cc7055ee5ae136d2cff25879722fc6c4b Mon Sep 17 00:00:00 2001 From: not-matthias Date: Tue, 28 Jul 2026 13:53:03 +0200 Subject: [PATCH 1/2] fix(memtrack): drop rss_stat samples from recycled mm_structs rss_stat ownership is keyed on the kernel's mm_id, a hash of the mm_struct pointer that the tracepoint never exposes, so an entry cannot be removed when the mm it describes goes away. Once a tracked process execs, its old mm_struct is freed while the pid lives on, and the next allocation from that slab slot hashes to the same id: the new address space's near-zero counters are emitted as the live owner's absolute RSS, from the context of whichever task populated or tore down the recycled mm. The owner then reads ~0 until its next in-context fault, while the folio-rmap reconstruction stays high because it keys ownership on the raw pointer and drops it on exec. Record the owner's mm alongside its pid when an entry is seeded in context, and require it to still be the owner's current mm before attributing an out-of-context update. pid_mm is now maintained by this hook too, so the check also holds when only rss_stat is attached and the rmap hooks never run. Genuine external reclaim is unaffected: the owner still holds the mm, so the comparison passes. Add a fixture that reproduces the collapse deterministically. It pins one CPU so the mm freed at exec stays at the head of the slab freelist, execs a child into a 256 MiB-resident image, then forks 256 short-lived children onto that slot. The test asserts both halves of the contradiction: no absolute anon sample for the live pid falls below a quarter of its peak, and the reconstructed rmap total shows the pages never moved. Refs COD-3285 --- crates/memtrack/src/ebpf/c/rss.bpf.h | 60 +++++++++---- crates/memtrack/testdata/rss/stale_mm_owner.c | 82 ++++++++++++++++++ crates/memtrack/tests/rss_tests.rs | 85 +++++++++++++++++++ 3 files changed, 209 insertions(+), 18 deletions(-) create mode 100644 crates/memtrack/testdata/rss/stale_mm_owner.c diff --git a/crates/memtrack/src/ebpf/c/rss.bpf.h b/crates/memtrack/src/ebpf/c/rss.bpf.h index 06e5b7e2d..b9edf9676 100644 --- a/crates/memtrack/src/ebpf/c/rss.bpf.h +++ b/crates/memtrack/src/ebpf/c/rss.bpf.h @@ -5,14 +5,18 @@ #include "utils/event_helpers.h" #include "utils/process_tracking.h" -/* (rss_stat mm_id << 32 | member) -> {owning tgid, last in-context size}. Keyed per - * counter so an external (curr==0) update is attributed only once that mm/member was - * established in-context. An external event may only lower a counter: any size above - * the last in-context value is dropped, so neither a stale/racing reclaim read nor an - * mm_id hash collision with another task can invent a peak. LRU eviction + re-seeding - * on the owner's next in-context event covers hash reuse, so no teardown hook needed. */ +/* (rss_stat mm_id << 32 | member) -> {owning tgid, that tgid's mm at seeding time, + * last in-context size}. Keyed per counter so an external (curr==0) update is + * attributed only once that mm/member was established in-context. An external event + * may only lower a counter: any size above the last in-context value is dropped, so + * neither a stale/racing reclaim read nor an mm_id hash collision with another task + * can invent a peak. mm_id is only a hash, so the pointer is stored alongside and + * revalidated against pid_mm on the external path: an entry whose mm the owner no + * longer holds describes a freed mm_struct whose slab slot (and therefore hash) has + * been recycled by an unrelated address space. */ struct rss_owner { __u32 pid; + __u64 mm; __u64 size; }; struct { @@ -20,7 +24,7 @@ struct { __uint(max_entries, 40960); __type(key, __u64); __type(value, struct rss_owner); -} mm_to_pid SEC(".maps"); +} rss_counter_owner SEC(".maps"); /* Foreign-actor rmap attribution: rmap events run by a task other than the mm's * owner (kswapd reclaim, another process's process_madvise, khugepaged, KSM, @@ -46,6 +50,16 @@ struct { } mm_owner SEC(".maps"); BPF_HASH_MAP(pid_mm, __u32, __u64, 10240); +/* Guard-then-write keeps the common case a read-only lookup (pid_mm is a plain + * hash, so a fresh entry never needs rewriting). The recorded mm is what the + * external rss_stat path validates against to detect a recycled mm_struct. */ +static __always_inline void refresh_pid_mm(__u32 pid, __u64 mm) { + __u64* cur_mm = bpf_map_lookup_elem(&pid_mm, &pid); + if (!cur_mm || *cur_mm != mm) { + bpf_map_update_elem(&pid_mm, &pid, &mm, BPF_ANY); + } +} + #define FOLIO_MAPPING_ANON 0x1UL const volatile __u32 page_shift = 12; @@ -73,10 +87,17 @@ int tracepoint_rss_stat(struct trace_event_raw_rss_stat* ctx) { return 0; } owner = cur; - struct rss_owner state = {.pid = cur, .size = size}; - bpf_map_update_elem(&mm_to_pid, &key, &state, BPF_ANY); + /* curr means current->mm is the mm the counter belongs to. Recording it + * (and keeping pid_mm current, which the rmap hooks may never do when + * only rss_stat is attached) is what lets the external path below tell a + * live mm from a recycled slab slot. */ + struct task_struct* task = bpf_get_current_task_btf(); + __u64 mm = (__u64)BPF_CORE_READ(task, mm); + struct rss_owner state = {.pid = cur, .mm = mm, .size = size}; + bpf_map_update_elem(&rss_counter_owner, &key, &state, BPF_ANY); + refresh_pid_mm(cur, mm); } else { - struct rss_owner* found = bpf_map_lookup_elem(&mm_to_pid, &key); + struct rss_owner* found = bpf_map_lookup_elem(&rss_counter_owner, &key); if (!found) { return 0; } @@ -87,6 +108,14 @@ int tracepoint_rss_stat(struct trace_event_raw_rss_stat* ctx) { if (cur == owner) { return 0; } + /* mm_id is a hash of a pointer the tracepoint never exposes, so the key + * survives the mm it was seeded from. Once the owner no longer holds that + * mm (it execed, or the entry predates a pid reuse), the hash now belongs + * to a recycled mm_struct and its counters describe another address space. */ + __u64* owner_mm = bpf_map_lookup_elem(&pid_mm, &owner); + if (!owner_mm || *owner_mm != found->mm) { + return 0; + } /* An external actor may only lower a counter. A larger value is a stale * reclaim read or an mm_id hash collision with another task; dropping it * keeps the reconstructed peak identical to the in-context timeline. */ @@ -177,18 +206,13 @@ static __always_inline int submit_rmap(struct vm_area_struct* vma, __s32 member, } /* Register ownership so foreign actors can later attribute to this pid. - * Both maps are validated (not just written) on every in-context event: - * the lookups keep the hot path cheap AND keep both entries LRU-fresh, - * since pid_mm is otherwise never read until exec/exit and could be - * evicted independently of its still-hot mm_owner twin. */ + * The guarded updates keep the hot path read-only in the common case and + * keep the LRU mm_owner entry fresh even when nothing else touches it. */ __u32* reg = bpf_map_lookup_elem(&mm_owner, &mm); if (!reg || *reg != pid) { bpf_map_update_elem(&mm_owner, &mm, &pid, BPF_ANY); } - __u64* cur_mm = bpf_map_lookup_elem(&pid_mm, &pid); - if (!cur_mm || *cur_mm != mm) { - bpf_map_update_elem(&pid_mm, &pid, &mm, BPF_ANY); - } + refresh_pid_mm(pid, mm); owner = pid; } else { /* Foreign actor (task->mm != mm, including kthreads whose task->mm is NULL): diff --git a/crates/memtrack/testdata/rss/stale_mm_owner.c b/crates/memtrack/testdata/rss/stale_mm_owner.c new file mode 100644 index 000000000..22a2912cb --- /dev/null +++ b/crates/memtrack/testdata/rss/stale_mm_owner.c @@ -0,0 +1,82 @@ +/* Recycled mm_struct, stale rss_stat owner. + * + * A child faults a page (registering mm0's hash as owned by that pid), execs + * into "big" (freeing mm0 while the pid lives on), and idles holding REGION_MIB + * of anon RSS on mm1. Each subsequent churn fork then allocates an mm_struct + * from the same slab and populates it from the PARENT's context before tearing + * it down from the child's, so both updates are out-of-context; mm0's slot sits + * at the head of the per-cpu freelist, so the whole burst hashes to mm0's id. + * + * Everything is pinned to one CPU to keep that freelist LIFO. + */ +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include + +#include "rss_report.h" + +#define REGION_MIB 256 +#define CHURN_FORKS 256 + +/* Keeps the mm_struct freed at exec at the head of this CPU's slab freelist. + * Fatal, because failing here would silently disable the reuse path. */ +static int pin_one_cpu(void) { + cpu_set_t one; + CPU_ZERO(&one); + CPU_SET(sched_getcpu(), &one); + return sched_setaffinity(0, sizeof(one), &one) != 0; +} + +static int run_big(const char* report_path) { + size_t len = (size_t)REGION_MIB * 1024 * 1024; + void* region = mmap(NULL, len, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + if (region == MAP_FAILED) return 1; + memset(region, 0x42, len); + if (write_rss_report(report_path) != 0) return 1; + /* Idle while the parent churns: no further fault means no in-context + * rss_stat event repairs a bogus sample. */ + pause(); + return 0; +} + +int main(int argc, char** argv) { + if (argc < 2) return 1; + if (argc >= 3 && strcmp(argv[2], "big") == 0) return run_big(argv[1]); + + /* Inherited across both the fork and the exec below. */ + if (pin_one_cpu() != 0) return 1; + + pid_t big = fork(); + if (big < 0) return 1; + if (big == 0) { + void* page = mmap(NULL, 4096, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + if (page == MAP_FAILED) _exit(1); + memset(page, 1, 4096); + char* args[] = {argv[0], argv[1], "big", NULL}; + execv("/proc/self/exe", args); + _exit(1); + } + + /* run_big writes the report right after its memset and the harness never + * pre-creates that path, so its existence means the region is resident. */ + for (int i = 0; access(argv[1], F_OK) != 0; i++) { + if (i > 5000) return 1; + usleep(1000); + } + + for (int i = 0; i < CHURN_FORKS; i++) { + pid_t churn = fork(); + if (churn < 0) return 1; + if (churn == 0) _exit(0); + int churn_status; + if (waitpid(churn, &churn_status, 0) < 0) return 1; + } + + if (kill(big, SIGKILL) != 0) return 1; + int status; + return waitpid(big, &status, 0) < 0; +} diff --git a/crates/memtrack/tests/rss_tests.rs b/crates/memtrack/tests/rss_tests.rs index a3ba085ae..3af002cfa 100644 --- a/crates/memtrack/tests/rss_tests.rs +++ b/crates/memtrack/tests/rss_tests.rs @@ -431,6 +431,91 @@ fn test_rss_external_reclaim(#[case] mode: Reclaim) -> Result<(), Box Result<(), Box> { + let (_report, events) = track_fixture( + include_str!("../testdata/rss/stale_mm_owner.c"), + "stale_mm_owner", + shared::track_command_with_rmap, + )?; + + // The fixture's only fork before the burst is the child that execs into the + // memory-holding image, so it keeps its pid across the exec. + let (_parent, big) = first_fork_pair(&events).ok_or("no fork event")?; + let exec_ts = events + .iter() + .find(|e| e.pid == big && matches!(e.kind, MemtrackEventKind::Exec)) + .map(|e| e.timestamp) + .ok_or("no exec event for the memory-holding child")?; + + // Only the post-exec address space is of interest: the pre-exec image + // faulted a single page, and that mm is the one whose slab slot gets reused. + let anon_after_exec: Vec<_> = events + .iter() + .filter_map(|e| match e.kind { + MemtrackEventKind::Rss { member: 1, size } if e.pid == big && e.timestamp > exec_ts => { + Some((e, size)) + } + _ => None, + }) + .collect(); + + let (peak_ts, peak) = anon_after_exec + .iter() + .max_by_key(|&&(_, size)| size) + .map(|&(e, size)| (e.timestamp, size)) + .ok_or("no anon rss_stat event after the exec")?; + assert!(peak >= 128 * MIB, "peak anon RSS too small: {peak}"); + + // Past the peak the region is held untouched until the fixture is killed, so + // no legitimate absolute sample may fall back near zero. Before it, samples + // are just the fault-in ramp. + let collapsed: Vec<_> = anon_after_exec + .iter() + .filter(|&&(e, size)| e.timestamp > peak_ts && size < peak / 4) + .map(|&(e, size)| (e.tid, size)) + .collect(); + assert!( + collapsed.is_empty(), + "{} absolute anon samples for pid {big} collapsed below {} bytes while it held {peak}: \ + (tid, size) = {:?}", + collapsed.len(), + peak / 4, + &collapsed[..collapsed.len().min(5)] + ); + + // The rmap-reconstructed peak confirms the pages became and stayed resident, + // so a collapsing absolute sample could only come from a recycled mm. The + // final net is unusable: only since v6.16 does trace_sched_process_exit fire + // before exit_mm, so older kernels attribute the SIGKILL teardown's foreign + // rmap removes while ownership is still bound, netting the sum to ~0. + let rmap_peak = events + .iter() + .sorted_by_key(|e| e.timestamp) + .filter_map(|e| match e.kind { + MemtrackEventKind::Rmap { member: 1, delta } if e.pid == big => Some(delta), + _ => None, + }) + .scan(0i64, |net, delta| { + *net += delta; + Some(*net) + }) + .max() + .unwrap_or(0) + * page_size() as i64; + assert!( + rmap_peak >= (peak / 2) as i64, + "fixture never held the region per rmap: peak net {rmap_peak} bytes" + ); + Ok(()) +} + /// A fork issued by a worker thread must still track the child: registration /// keys on the parent's tgid (task_newtask fires in the cloning task, whose /// pid_tgid upper half is the tgid), not on the raw creator tid. From b08209544538ff3a362fb0f8ea7980a9689eadd9 Mon Sep 17 00:00:00 2001 From: not-matthias Date: Wed, 29 Jul 2026 14:59:12 +0200 Subject: [PATCH 2/2] fix(memtrack): prevent stale rmap ownership attribution Drop stale mm ownership when a PID changes address spaces. Validate foreign rmap ownership and preserve live CLONE_VM parent bindings. Add deterministic exec-churn and clone-vm regressions plus xz/bzip2 coverage. Fixes COD-3285 --- crates/memtrack/src/ebpf/c/rss.bpf.h | 126 ++++--- crates/memtrack/src/ebpf/memtrack/maps.rs | 44 +++ crates/memtrack/src/ebpf/memtrack/mod.rs | 2 + crates/memtrack/src/ebpf/mod.rs | 4 +- crates/memtrack/src/ebpf/tracker.rs | 7 +- crates/memtrack/testdata/rss/file_region.h | 55 ++++ crates/memtrack/testdata/rss/madvise_extern.c | 42 +-- crates/memtrack/testdata/rss/vfork_spawn.c | 75 +++++ crates/memtrack/tests/rss_tests.rs | 309 ++++++++++++++---- crates/memtrack/tests/shared.rs | 76 ++++- 10 files changed, 573 insertions(+), 167 deletions(-) create mode 100644 crates/memtrack/testdata/rss/file_region.h create mode 100644 crates/memtrack/testdata/rss/vfork_spawn.c diff --git a/crates/memtrack/src/ebpf/c/rss.bpf.h b/crates/memtrack/src/ebpf/c/rss.bpf.h index b9edf9676..95f73d292 100644 --- a/crates/memtrack/src/ebpf/c/rss.bpf.h +++ b/crates/memtrack/src/ebpf/c/rss.bpf.h @@ -5,15 +5,11 @@ #include "utils/event_helpers.h" #include "utils/process_tracking.h" -/* (rss_stat mm_id << 32 | member) -> {owning tgid, that tgid's mm at seeding time, - * last in-context size}. Keyed per counter so an external (curr==0) update is - * attributed only once that mm/member was established in-context. An external event - * may only lower a counter: any size above the last in-context value is dropped, so - * neither a stale/racing reclaim read nor an mm_id hash collision with another task - * can invent a peak. mm_id is only a hash, so the pointer is stored alongside and - * revalidated against pid_mm on the external path: an entry whose mm the owner no - * longer holds describes a freed mm_struct whose slab slot (and therefore hash) has - * been recycled by an unrelated address space. */ +/* (rss_stat mm_id << 32 | member) -> {owning tgid, its mm when seeded, last + * in-context size}. An external (curr==0) update may only lower the counter, and + * only while pid_mm still binds the owner to the seeded mm: mm_id is a hash, so + * without both guards a stale reclaim read, a hash collision, or a recycled + * mm_struct slab slot could invent a peak. */ struct rss_owner { __u32 pid; __u64 mm; @@ -29,19 +25,11 @@ struct { /* Foreign-actor rmap attribution: rmap events run by a task other than the mm's * owner (kswapd reclaim, another process's process_madvise, khugepaged, KSM, * uffd) carry no owning-pid context, so mm_owner recovers it from the mm_struct - * pointer. pid_mm is the inverse, letting the exec and exit hooks remove an entry - * by value. + * pointer. pid_mm is the inverse, letting exec and exit remove an entry by + * value; attribution requires both to agree, so a stale mm fails closed. * - * Lifecycle invariant: every mm_owner entry is removed when its process execs - * (the old mm is freed mid-life) or when its thread group dies, whichever comes - * first; LRU eviction is only a backstop. A stale entry surviving mm-pointer - * reuse would misattribute another process's events, so ownership is only ever - * registered from an in-context (task->mm == mm) event. - * - * pid_mm is a plain hash on purpose: an LRU inverse could be evicted while its - * forward twin stays lookup-hot, leaving exec/exit unable to remove the live - * mm_owner entry. Like tracked_pids, its entries are bound to the process - * lifecycle and removed at group death. */ + * pid_mm must not use LRU eviction: losing the inverse binding would leave exec + * and exit unable to remove the forward entry. */ struct { __uint(type, BPF_MAP_TYPE_LRU_HASH); __uint(max_entries, 10240); @@ -50,13 +38,42 @@ struct { } mm_owner SEC(".maps"); BPF_HASH_MAP(pid_mm, __u32, __u64, 10240); -/* Guard-then-write keeps the common case a read-only lookup (pid_mm is a plain - * hash, so a fresh entry never needs rewriting). The recorded mm is what the - * external rss_stat path validates against to detect a recycled mm_struct. */ -static __always_inline void refresh_pid_mm(__u32 pid, __u64 mm) { - __u64* cur_mm = bpf_map_lookup_elem(&pid_mm, &pid); - if (!cur_mm || *cur_mm != mm) { +/* Rebind pid's address space; mm == 0 unbinds at exit. The mm_owner entry is only + * dropped while it still names pid: a live CLONE_VM sibling shares the mm and must + * keep its registration. */ +static __always_inline void set_pid_mm(__u32 pid, __u64 mm) { + __u64* cur = bpf_map_lookup_elem(&pid_mm, &pid); + if (cur && *cur == mm) { + return; + } + if (cur) { + __u32* owner = bpf_map_lookup_elem(&mm_owner, cur); + if (owner && *owner == pid) { + bpf_map_delete_elem(&mm_owner, cur); + } + } + if (mm) { bpf_map_update_elem(&pid_mm, &pid, &mm, BPF_ANY); + } else { + bpf_map_delete_elem(&pid_mm, &pid); + } +} + +/* Claim mm for pid without stealing from a live owner: CLONE_VM siblings share the + * mm, and overwriting would let the child's exec-time cleanup delete the entry out + * from under the still-live parent. */ +static __always_inline void mm_owner_take(__u64 mm, __u32 pid) { + __u32* reg = bpf_map_lookup_elem(&mm_owner, &mm); + if (!reg) { + bpf_map_update_elem(&mm_owner, &mm, &pid, BPF_ANY); + return; + } + if (*reg == pid) { + return; + } + __u64* reg_mm = bpf_map_lookup_elem(&pid_mm, reg); + if (!reg_mm || *reg_mm != mm) { + bpf_map_update_elem(&mm_owner, &mm, &pid, BPF_ANY); } } @@ -87,15 +104,13 @@ int tracepoint_rss_stat(struct trace_event_raw_rss_stat* ctx) { return 0; } owner = cur; - /* curr means current->mm is the mm the counter belongs to. Recording it - * (and keeping pid_mm current, which the rmap hooks may never do when - * only rss_stat is attached) is what lets the external path below tell a - * live mm from a recycled slab slot. */ + /* curr == 1 means current->mm is the counter's mm. pid_mm is maintained + * here too because the rmap hooks may not be attached. */ struct task_struct* task = bpf_get_current_task_btf(); __u64 mm = (__u64)BPF_CORE_READ(task, mm); struct rss_owner state = {.pid = cur, .mm = mm, .size = size}; bpf_map_update_elem(&rss_counter_owner, &key, &state, BPF_ANY); - refresh_pid_mm(cur, mm); + set_pid_mm(cur, mm); } else { struct rss_owner* found = bpf_map_lookup_elem(&rss_counter_owner, &key); if (!found) { @@ -108,10 +123,6 @@ int tracepoint_rss_stat(struct trace_event_raw_rss_stat* ctx) { if (cur == owner) { return 0; } - /* mm_id is a hash of a pointer the tracepoint never exposes, so the key - * survives the mm it was seeded from. Once the owner no longer holds that - * mm (it execed, or the entry predates a pid reuse), the hash now belongs - * to a recycled mm_struct and its counters describe another address space. */ __u64* owner_mm = bpf_map_lookup_elem(&pid_mm, &owner); if (!owner_mm || *owner_mm != found->mm) { return 0; @@ -205,14 +216,8 @@ static __always_inline int submit_rmap(struct vm_area_struct* vma, __s32 member, return 0; } - /* Register ownership so foreign actors can later attribute to this pid. - * The guarded updates keep the hot path read-only in the common case and - * keep the LRU mm_owner entry fresh even when nothing else touches it. */ - __u32* reg = bpf_map_lookup_elem(&mm_owner, &mm); - if (!reg || *reg != pid) { - bpf_map_update_elem(&mm_owner, &mm, &pid, BPF_ANY); - } - refresh_pid_mm(pid, mm); + mm_owner_take(mm, pid); + set_pid_mm(pid, mm); owner = pid; } else { /* Foreign actor (task->mm != mm, including kthreads whose task->mm is NULL): @@ -226,6 +231,12 @@ static __always_inline int submit_rmap(struct vm_area_struct* vma, __s32 member, if (!is_tracked(owner)) { return 0; } + /* An mm_struct address may be reused while a stale owner entry remains. + * Accept only the current inverse binding. */ + __u64* owner_mm = bpf_map_lookup_elem(&pid_mm, &owner); + if (!owner_mm || *owner_mm != mm) { + return 0; + } } /* header.tid is stamped from the current task; for a foreign actor it @@ -331,21 +342,6 @@ int tracepoint_task_newtask(struct trace_event_raw_task_newtask* ctx) { SUBMIT_EVENT_AS(child_pid, EVENT_TYPE_FORK, { e->data.fork.parent_pid = parent_pid; }); } -/* Remove pid's ownership registration. The mm_owner value is verified against - * pid before deleting: a stale pid_mm entry (LRU eviction skew) could otherwise - * point at an mm since re-registered by another process, and deleting that - * would silence a live owner's foreign attribution. */ -static __always_inline void drop_mm_ownership(__u32 pid) { - __u64* mm = bpf_map_lookup_elem(&pid_mm, &pid); - if (mm) { - __u32* owner = bpf_map_lookup_elem(&mm_owner, mm); - if (owner && *owner == pid) { - bpf_map_delete_elem(&mm_owner, mm); - } - } - bpf_map_delete_elem(&pid_mm, &pid); -} - SEC("tracepoint/sched/sched_process_exec") int tracepoint_sched_process_exec(void* ctx) { __u32 pid = bpf_get_current_pid_tgid() >> 32; @@ -353,17 +349,13 @@ int tracepoint_sched_process_exec(void* ctx) { return 0; } - /* Maintain ownership before submitting (SUBMIT_EVENT_AS returns from the - * function). Exec frees the old mm long before group death, so the stale - * pointer must be dropped here or a reused mm_struct would be misattributed. */ - drop_mm_ownership(pid); - + /* SUBMIT_EVENT_AS returns, so the rebind must precede it. */ struct task_struct* task = bpf_get_current_task_btf(); __u64 new_mm = (__u64)BPF_CORE_READ(task, mm); if (new_mm) { - bpf_map_update_elem(&mm_owner, &new_mm, &pid, BPF_ANY); - bpf_map_update_elem(&pid_mm, &pid, &new_mm, BPF_ANY); + mm_owner_take(new_mm, pid); } + set_pid_mm(pid, new_mm); SUBMIT_EVENT_AS(pid, EVENT_TYPE_EXEC, {}); } @@ -398,7 +390,7 @@ int tracepoint_sched_process_exit(void* ctx) { /* Drop the ownership mapping so foreign actors stop attributing to a pid * the kernel may reuse. */ - drop_mm_ownership(pid); + set_pid_mm(pid, 0); SUBMIT_EVENT_AS(pid, EVENT_TYPE_EXIT, {}); } diff --git a/crates/memtrack/src/ebpf/memtrack/maps.rs b/crates/memtrack/src/ebpf/memtrack/maps.rs index 42d2dd2e7..99e3d125a 100644 --- a/crates/memtrack/src/ebpf/memtrack/maps.rs +++ b/crates/memtrack/src/ebpf/memtrack/maps.rs @@ -67,6 +67,50 @@ impl MemtrackBpf { "dropped_events", ) } + + pub fn ownership_maps(&self) -> Result { + let mm_owner = entries(with_skel!(self, skel => &skel.maps.mm_owner))?; + let pid_mm = entries(with_skel!(self, skel => &skel.maps.pid_mm))?; + Ok(OwnershipMaps { + mm_owner: mm_owner + .into_iter() + .map(|(mm, pid)| (mm, pid as u32)) + .collect(), + pid_mm: pid_mm + .into_iter() + .map(|(pid, mm)| (pid as u32, mm)) + .collect(), + }) + } +} + +/// Live ownership bindings: `mm_owner` (`mm_struct` pointer -> owning pid) and its +/// inverse `pid_mm`, which foreign-actor rmap attribution validates against. +pub struct OwnershipMaps { + pub mm_owner: Vec<(u64, u32)>, + pub pid_mm: Vec<(u32, u64)>, +} + +/// Iteration is `BPF_MAP_GET_NEXT_KEY` followed by a separate lookup, so it is not +/// atomic: a key deleted in between is skipped rather than reported. +fn entries(map: &impl MapCore) -> Result> { + let mut entries = Vec::new(); + for key in map.keys() { + let value = map + .lookup(&key, libbpf_rs::MapFlags::ANY) + .context("Failed to read map entry")?; + if let Some(value) = value { + entries.push((le(&key), le(&value))); + } + } + Ok(entries) +} + +fn le(bytes: &[u8]) -> u64 { + bytes + .iter() + .rev() + .fold(0, |acc, &b| acc << 8 | u64::from(b)) } /// Read slot 0 of a single-entry `__u64` array map. diff --git a/crates/memtrack/src/ebpf/memtrack/mod.rs b/crates/memtrack/src/ebpf/memtrack/mod.rs index 1d031cdf5..c6678f2a8 100644 --- a/crates/memtrack/src/ebpf/memtrack/mod.rs +++ b/crates/memtrack/src/ebpf/memtrack/mod.rs @@ -22,6 +22,8 @@ mod allocator; mod maps; mod tracking; +pub use maps::OwnershipMaps; + use crate::bpf_token::has_delegated_bpf_token; /// Which attach mechanism a loaded skeleton uses for its uprobes. See diff --git a/crates/memtrack/src/ebpf/mod.rs b/crates/memtrack/src/ebpf/mod.rs index d44ca957f..454193b37 100644 --- a/crates/memtrack/src/ebpf/mod.rs +++ b/crates/memtrack/src/ebpf/mod.rs @@ -6,5 +6,7 @@ mod proc_fs; mod spawn; mod tracker; -pub use memtrack::{BpfVariant, MemtrackBpf, ResolvedSymbols, resolve_symbol_offsets}; +pub use memtrack::{ + BpfVariant, MemtrackBpf, OwnershipMaps, ResolvedSymbols, resolve_symbol_offsets, +}; pub use tracker::{Tracker, TrackerOptions}; diff --git a/crates/memtrack/src/ebpf/tracker.rs b/crates/memtrack/src/ebpf/tracker.rs index d40079f47..deecb2240 100644 --- a/crates/memtrack/src/ebpf/tracker.rs +++ b/crates/memtrack/src/ebpf/tracker.rs @@ -1,6 +1,6 @@ use crate::ebpf::attach_worker::AttachWorker; use crate::ebpf::spawn::{resume, spawn_stopped, wrap_stopped}; -use crate::ebpf::{BpfVariant, MemtrackBpf}; +use crate::ebpf::{BpfVariant, MemtrackBpf, OwnershipMaps}; use crate::prelude::*; use crate::session::Session; use parking_lot::Mutex; @@ -135,6 +135,11 @@ impl Tracker { self.bpf.lock().dropped_events_count() } + /// Only meaningful while the BPF object is alive; teardown frees the maps. + pub fn ownership_maps(&self) -> Result { + self.bpf.lock().ownership_maps() + } + /// Stop the attach worker, if any, and surface any fatal error it recorded, /// including missed exec mappings (incomplete allocator coverage). A tracker /// without an allocator watcher has no worker, so this is a no-op. diff --git a/crates/memtrack/testdata/rss/file_region.h b/crates/memtrack/testdata/rss/file_region.h new file mode 100644 index 000000000..a34d6de71 --- /dev/null +++ b/crates/memtrack/testdata/rss/file_region.h @@ -0,0 +1,55 @@ +#ifndef FILE_REGION_H +#define FILE_REGION_H + +#include +#include +#include +#include +#include +#include +#include +#include + +/* Maps len bytes of a private file mapping and faults every page in, so the + * pages land in the caller's RSS (MM_FILEPAGES) in-context and seed ownership. + * Returns the mapping, or NULL. + * + * The data file is derived from base rather than placed in /tmp, which is tmpfs + * on Ubuntu >= 25.04 and would account mapped file pages as shmem, not file. */ +static void* map_and_fault_file(const char* base, size_t len) { + char path[4096]; + snprintf(path, sizeof(path), "%s.data-XXXXXX", base); + int fd = mkstemp(path); + if (fd < 0) return NULL; + unlink(path); + if (ftruncate(fd, len) != 0) return NULL; + + void* mem = mmap(NULL, len, PROT_READ, MAP_PRIVATE, fd, 0); + if (mem == MAP_FAILED) return NULL; + + /* volatile so the reads are not elided. 4096 is the fault stride: a smaller + * one would fault the same page twice, a larger one would skip pages. */ + volatile char sink = 0; + for (size_t i = 0; i < len; i += 4096) sink ^= ((volatile char*)mem)[i]; + (void)sink; + return mem; +} + +/* Pages out [mem, mem+len) of the calling process from a forked child, and + * waits for it. The reclaim must run in another task's context to exercise + * foreign-actor attribution. Returns 0 on success. */ +static int reclaim_from_child(void* mem, size_t len) { + pid_t pid = fork(); + if (pid < 0) return 1; + if (pid == 0) { + int pidfd = syscall(SYS_pidfd_open, getppid(), 0); + if (pidfd < 0) _exit(1); + struct iovec iov = {.iov_base = mem, .iov_len = len}; + syscall(SYS_process_madvise, pidfd, &iov, 1UL, MADV_PAGEOUT, 0UL); + _exit(0); + } + int status; + return waitpid(pid, &status, 0) < 0; +} + +#endif diff --git a/crates/memtrack/testdata/rss/madvise_extern.c b/crates/memtrack/testdata/rss/madvise_extern.c index b981c38e3..d67f718b2 100644 --- a/crates/memtrack/testdata/rss/madvise_extern.c +++ b/crates/memtrack/testdata/rss/madvise_extern.c @@ -1,49 +1,19 @@ #define _GNU_SOURCE -#include -#include -#include -#include -#include -#include -#include #include +#include "file_region.h" + int main(int argc, char** argv) { if (argc != 2) return 1; sleep(1); /* let the tracker attach + enable + add root pid */ size_t len = 64UL * 1024 * 1024; + void* mem = map_and_fault_file(argv[1], len); + if (!mem) return 1; - /* Keep the data file next to the report: /tmp may be tmpfs (Ubuntu >= 25.04), - which accounts mapped file pages as shmem instead of file. */ - char path[4096]; - snprintf(path, sizeof(path), "%s.data-XXXXXX", argv[1]); - int fd = mkstemp(path); - if (fd < 0) return 1; - unlink(path); - if (ftruncate(fd, len) != 0) return 1; - - void* mem = mmap(NULL, len, PROT_READ, MAP_PRIVATE, fd, 0); - if (mem == MAP_FAILED) return 1; - - /* Fault the file pages into A's RSS (MM_FILEPAGES), in-context -> seeds ownership. */ - volatile char sink = 0; - for (size_t i = 0; i < len; i += 4096) sink ^= ((volatile char*)mem)[i]; - (void)sink; - - pid_t pid = fork(); - if (pid < 0) return 1; - if (pid == 0) { - /* B: page out A's (parent's) region from B's context. */ - int pidfd = syscall(SYS_pidfd_open, getppid(), 0); - if (pidfd < 0) _exit(1); - struct iovec iov = {.iov_base = mem, .iov_len = len}; - syscall(SYS_process_madvise, pidfd, &iov, 1UL, MADV_PAGEOUT, 0UL); - _exit(0); - } + /* B: page out A's (parent's) region from B's context. */ + if (reclaim_from_child(mem, len) != 0) return 1; - int status; - if (waitpid(pid, &status, 0) < 0) return 1; sleep(1); /* let the external decrement flush to the ring buffer */ /* No munmap: an in-context decrement would mask the external-path signal. */ return 0; diff --git a/crates/memtrack/testdata/rss/vfork_spawn.c b/crates/memtrack/testdata/rss/vfork_spawn.c new file mode 100644 index 000000000..ab919d812 --- /dev/null +++ b/crates/memtrack/testdata/rss/vfork_spawn.c @@ -0,0 +1,75 @@ +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include + +#include "file_region.h" + +/* Exercise ownership of an mm shared through CLONE_VM. + * + * The fixture pauses before later parent faults can refresh the binding, then + * reclaims the parent's file pages from another process. Ownership must remain + * with the parent throughout. + * + * argv: [1] = checkpoint-ready file to create, [2] = release file to wait for. */ + +#define REGION (64UL * 1024 * 1024) +#define SCRATCH (1UL * 1024 * 1024) +#define CHILD_STACK (256UL * 1024) + +static char* scratch; + +/* Fault fresh anonymous pages into the shared address space from the child. */ +static int clone_vm_child(void* arg) { + (void)arg; + memset(scratch, 0x42, SCRATCH); + /* POSIX guarantees /bin/sh; only the exec transition matters. */ + execl("/bin/sh", "sh", "-c", "exit 0", (char*)NULL); + _exit(127); +} + +int main(int argc, char** argv) { + if (argc < 3) return 2; + sleep(1); /* let the tracker attach + enable + add root pid */ + + void* mem = map_and_fault_file(argv[1], REGION); + if (!mem) return 1; + + /* Leave the mapping untouched so only the CLONE_VM child faults its pages. */ + scratch = mmap(NULL, SCRATCH, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + if (scratch == MAP_FAILED) return 1; + + char* stack = mmap(NULL, CHILD_STACK, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0); + if (stack == MAP_FAILED) return 1; + + /* glibc's posix_spawn uses CLONE_VM|CLONE_VFORK. CLONE_VFORK orders the + * child's exec before the later foreign reclaim. */ + pid_t c = clone(clone_vm_child, stack + CHILD_STACK, CLONE_VM | CLONE_VFORK | SIGCHLD, NULL); + if (c < 0) return 1; + int status; + if (waitpid(c, &status, 0) < 0) return 1; + /* A failed exec would skip the ownership transition under test. */ + if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) return 1; + + /* Pause before a parent-side fault can refresh the ownership binding. Use + * only already-faulted memory until the test releases the fixture. */ + int rfd = creat(argv[1], 0644); + if (rfd < 0) return 1; + close(rfd); + /* Bounded: a test that dies before writing the release file must not leave + * an orphan spinning here while holding the 64 MiB mapping. */ + for (int i = 0; access(argv[2], F_OK) != 0; i++) { + if (i > 1500) return 3; + usleep(20000); + } + + if (reclaim_from_child(mem, REGION) != 0) return 1; + sleep(1); /* let the external decrements flush to the ring buffer */ + /* No munmap: an in-context decrement would mask the external-path signal. */ + return 0; +} diff --git a/crates/memtrack/tests/rss_tests.rs b/crates/memtrack/tests/rss_tests.rs index 3af002cfa..f40658b42 100644 --- a/crates/memtrack/tests/rss_tests.rs +++ b/crates/memtrack/tests/rss_tests.rs @@ -5,7 +5,8 @@ use itertools::Itertools; use rstest::rstest; use runner_shared::artifacts::{MemtrackEvent, MemtrackEventKind}; use serde::Serialize; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; +use std::path::{Path, PathBuf}; use std::process::Command; use tempfile::TempDir; @@ -169,41 +170,108 @@ fn per_pid_peaks(events: &[MemtrackEvent]) -> (Vec, Vec) { (project(&rss), project(&rmap)) } -/// Compile a fixture that writes a `/proc` RSS report to its argv[1], run it under -/// `track`, and return the raw report text alongside the collected events. -/// -/// The report read is best-effort: some fixtures write no report. -fn track_fixture( +/// Headers the fixtures `#include`. Each fixture compiles in its own temp dir, +/// so they are copied in next to the source. +fn write_fixture_headers(dir: &Path) -> std::io::Result<()> { + for (name, source) in [ + ("rss_report.h", include_str!("../testdata/rss/rss_report.h")), + ( + "file_region.h", + include_str!("../testdata/rss/file_region.h"), + ), + ] { + std::fs::write(dir.join(name), source)?; + } + Ok(()) +} + +/// Compile a fixture and build the command that runs it against a report path +/// in the returned temp dir, which must outlive the run. +fn build_fixture( source: &str, name: &str, - track: impl FnOnce(Command) -> shared::TrackResult, -) -> Result<(Option, Vec), Box> { +) -> Result<(TempDir, PathBuf, Command), Box> { // Fixtures mmap data files created next to the report path, so the temp dir // must be disk-backed: /tmp may be tmpfs (Ubuntu >= 25.04), which accounts // mapped file pages as shmem instead of file. std::fs::create_dir_all(env!("CARGO_TARGET_TMPDIR"))?; let temp_dir = TempDir::new_in(env!("CARGO_TARGET_TMPDIR"))?; - std::fs::write( - temp_dir.path().join("rss_report.h"), - include_str!("../testdata/rss/rss_report.h"), - )?; + write_fixture_headers(temp_dir.path())?; let binary = shared::compile_c_source(source, name, temp_dir.path())?; let report_path = temp_dir.path().join(format!("{name}.report")); let mut command = Command::new(&binary); command.arg(&report_path); + Ok((temp_dir, report_path, command)) +} +/// Run a fixture under `track` and return the raw `/proc` RSS report it wrote to +/// its argv[1] alongside the collected events. +/// +/// The report read is best-effort: some fixtures write no report. +fn track_fixture( + source: &str, + name: &str, + track: impl FnOnce(Command) -> shared::TrackResult, +) -> Result<(Option, Vec), Box> { + let (_temp_dir, report_path, command) = build_fixture(source, name)?; let (events, thread_handle) = track(command)?; let raw_report = std::fs::read_to_string(&report_path).ok(); thread_handle.join().unwrap(); Ok((raw_report, events)) } +/// [`track_fixture`] returning the ownership maps snapshotted once the tracked +/// tree exited, instead of the report text. +fn track_fixture_with_maps( + source: &str, + name: &str, +) -> Result<(Vec, shared::OwnershipMaps), Box> { + let (_temp_dir, _report_path, command) = build_fixture(source, name)?; + let (events, maps, thread_handle) = shared::track_command_with_rmap_maps(command)?; + thread_handle.join().unwrap(); + Ok((events, maps)) +} + +/// Every fork as `(parent_pid, child_pid)`, in timestamp order. +fn fork_pairs(events: &[MemtrackEvent]) -> Vec<(i32, i32)> { + events + .iter() + .sorted_by_key(|e| e.timestamp) + .filter_map(|e| match e.kind { + MemtrackEventKind::Fork { parent_pid } => Some((parent_pid, e.pid)), + _ => None, + }) + .collect() +} + /// The first fork observed: `(parent_pid, child_pid)`. fn first_fork_pair(events: &[MemtrackEvent]) -> Option<(i32, i32)> { - events.iter().find_map(|e| match e.kind { - MemtrackEventKind::Fork { parent_pid } => Some((parent_pid, e.pid)), - _ => None, - }) + fork_pairs(events).first().copied() +} + +/// Summed rmap bytes for one pid's `member` counter, in one direction. `tid` +/// narrows to a single performing task, which is how a foreign actor's events +/// are told apart from the owner's own. +fn rmap_bytes( + events: &[MemtrackEvent], + pid: i32, + tid: Option, + member: i32, + positive: bool, +) -> u64 { + events + .iter() + .filter(|e| e.pid == pid && tid.is_none_or(|tid| e.tid == tid)) + .filter_map(|e| match e.kind { + MemtrackEventKind::Rmap { member: m, delta } + if m == member && (delta > 0) == positive => + { + Some(delta.unsigned_abs()) + } + _ => None, + }) + .sum::() + * page_size() } /// Pins reconstructed rmap addresses to the exact punched range: hole pages are @@ -392,16 +460,7 @@ fn test_rss_external_reclaim(#[case] mode: Reclaim) -> Result<(), Box { - let in_context_bytes = events - .iter() - .filter_map(|e| match e.kind { - MemtrackEventKind::Rmap { member: 0, delta } if e.pid == a && delta > 0 => { - Some(delta) - } - _ => None, - }) - .sum::() as u64 - * page_size(); + let in_context_bytes = rmap_bytes(&events, a, None, 0, true); assert!( in_context_bytes >= 32 * MIB, "in-context file rmap adds too small: {in_context_bytes}" @@ -410,18 +469,7 @@ fn test_rss_external_reclaim(#[case] mode: Reclaim) -> Result<(), Box - { - Some(-delta) - } - _ => None, - }) - .sum::() as u64 - * page_size(); + let external_bytes = rmap_bytes(&events, a, Some(b), 0, false); assert!( external_bytes >= 8 * MIB, "external MADV_PAGEOUT remove not attributed to the owner (pid=A, tid=B): only {external_bytes} bytes" @@ -439,10 +487,9 @@ fn test_rss_external_reclaim(#[case] mode: Reclaim) -> Result<(), Box Result<(), Box> { - let (_report, events) = track_fixture( + let (events, maps) = track_fixture_with_maps( include_str!("../testdata/rss/stale_mm_owner.c"), "stale_mm_owner", - shared::track_command_with_rmap, )?; // The fixture's only fork before the burst is the child that execs into the @@ -490,11 +537,9 @@ fn test_rss_stale_mm_owner_keeps_live_rss() -> Result<(), Box Result<(), Box= (peak / 2) as i64, "fixture never held the region per rmap: peak net {rmap_peak} bytes" ); + + let forks = events + .iter() + .filter(|e| matches!(e.kind, MemtrackEventKind::Fork { .. })) + .count(); + assert!( + forks >= 200, + "the mm_struct-recycling fork burst did not run tracked: {forks} forks" + ); + + // Every fixture process is dead by now, so no ownership entry may still name one. + let tracked: BTreeSet = events.iter().map(|e| e.pid).collect(); + let residue: Vec = maps + .pid_mm + .iter() + .filter(|(pid, _)| tracked.contains(&(*pid as i32))) + .map(|e| format!("pid_mm {e:?}")) + .chain( + maps.mm_owner + .iter() + .filter(|(_, owner)| tracked.contains(&(*owner as i32))) + .map(|e| format!("mm_owner {e:?}")), + ) + .collect(); + assert!( + residue.is_empty(), + "ownership entries outlived their pid: {residue:?}" + ); Ok(()) } @@ -536,20 +609,142 @@ fn test_rss_rmap_thread_fork_tracks_child() -> Result<(), Box= (REGION_MIB - 16) * MIB, + "child of a worker-thread fork missed rmap tracking: anon adds {child_anon} bytes, \ + expected ~{REGION_MIB} MiB" + ); + Ok(()) +} + +/// Both accountings restart from zero at EXEC, so past that point they must agree: +/// rss_stat reports the kernel's own counters while rmap sums folio add/remove +/// deltas, and a lost fork seed or a dropped in-context event shows up as a +/// reconstructed peak far below the counters. +#[test_with::env(GITHUB_ACTIONS)] +#[test] +fn test_rmap_matches_rss_stat_across_execs() -> Result<(), Box> { + // Two compressors in one shell so ancestor state spans both execs. xz -9's + // dictionary and bzip2 -9's 900k block buffer are what make the peaks large + // enough to compare; the input only has to be compressible, not random. + let temp_dir = TempDir::new()?; + let mut command = Command::new("bash"); + command + .arg("-c") + .arg( + "yes alpha bravo charlie delta | head -c 16777216 > corpus.tar \ + && xz -9 -k -f -T1 corpus.tar && bzip2 -9 -k -f corpus.tar", + ) + .current_dir(temp_dir.path()); + + let (events, thread_handle) = shared::track_command_with_rmap(command)?; + thread_handle.join().unwrap(); + + let (_order, rss, rmap) = per_pid_raw(&events); + + // Only pids that exec'd can be compared: a pid whose pages were already + // resident when the tracker attached has absolute rss_stat counters covering + // them but no rmap events, and nothing resets that deficit. EXEC does. + let exec_pids: BTreeSet = events .iter() - .filter_map(|e| match e.kind { - MemtrackEventKind::Rmap { member: 1, delta } if e.pid == child && delta > 0 => { - Some(delta) - } - _ => None, - }) - .sum(); + .filter(|e| matches!(e.kind, MemtrackEventKind::Exec)) + .map(|e| e.pid) + .collect(); + + let mut compared = 0; + for pid in &exec_pids { + // Large enough that page-level noise cannot explain a shortfall. + let rss_peak = rss.get(pid).map_or(0, |acc| acc.max_rss.max(0)) as u64; + if rss_peak <= 6 * MIB { + continue; + } + compared += 1; + let rmap_peak = rmap.get(pid).map_or(0, |acc| acc.max_rss.max(0)) as u64; + assert!( + rmap_peak * 4 >= rss_peak * 3, + "pid {pid}: rmap reconstructed {rmap_peak} bytes against rss_stat's {rss_peak}" + ); + } + assert!( + compared >= 2, + "expected xz and bzip2 above 6 MiB, compared {compared} of {} exec'd pids", + exec_pids.len() + ); + Ok(()) +} + +/// A CLONE_VM child shares its parent's mm until exec. Its faults must not +/// transfer ownership because exec cleanup would remove the parent's live +/// binding. +#[test_with::env(GITHUB_ACTIONS)] +#[test] +fn test_rmap_clone_vm_keeps_parent_ownership() -> Result<(), Box> { + // Disk-backed: the fixture mmaps a data file next to argv[1] (see build_fixture). + std::fs::create_dir_all(env!("CARGO_TARGET_TMPDIR"))?; + let tmp = TempDir::new_in(env!("CARGO_TARGET_TMPDIR"))?; + write_fixture_headers(tmp.path())?; + let binary = shared::compile_c_source( + include_str!("../testdata/rss/vfork_spawn.c"), + "vfork_spawn", + tmp.path(), + )?; + let ready = tmp.path().join("checkpoint-ready"); + let release = tmp.path().join("checkpoint-release"); + let mut command = Command::new(&binary); + command.arg(&ready).arg(&release); + + let (events, checkpoint, root_pid, thread_handle) = + shared::track_command_with_rmap_checkpoint(command, &ready, &release)?; + thread_handle.join().unwrap(); + + // A stale entry naming the parent is insufficient; check its current mm. + let parent_mm = checkpoint + .pid_mm + .iter() + .find_map(|&(pid, mm)| (pid == root_pid as u32).then_some(mm)) + .expect("parent has no pid_mm binding at the checkpoint"); + assert!( + checkpoint.mm_owner.contains(&(parent_mm, root_pid as u32)), + "mm_owner lost the parent's current mm after its CLONE_VM child exec'd \ + (parent mm {parent_mm:#x}, snapshot: {:?})", + checkpoint.mm_owner + ); + + let pairs = fork_pairs(&events); + let [(parent, shared_child), (reclaimer_parent, reclaimer), ..] = pairs[..] else { + return Err("expected forks for the CLONE_VM child and the reclaimer".into()); + }; + assert_eq!( + reclaimer_parent, parent, + "the reclaimer was not forked by the fixture" + ); + + // Confirm both processes produced the events needed to exercise ownership. + let owner_adds = rmap_bytes(&events, parent, None, 0, true); + assert!( + owner_adds >= 32 * MIB, + "in-context file rmap adds too small: {owner_adds}" + ); + + let shared_mm_adds = rmap_bytes(&events, shared_child, None, 1, true); + assert!( + shared_mm_adds >= 256 * 1024, + "CLONE_VM child faulted no anon pages into the shared mm: {shared_mm_adds} bytes" + ); + assert!( + events + .iter() + .any(|e| e.pid == shared_child && matches!(e.kind, MemtrackEventKind::Exec)), + "CLONE_VM child never exec'd, so its ownership cleanup never ran" + ); + + // process_madvise runs in the reclaimer's context, so removals require + // foreign ownership attribution. + let reclaimed = rmap_bytes(&events, parent, Some(reclaimer), 0, false); assert!( - child_anon * page_size() as i64 >= ((REGION_MIB - 16) * MIB) as i64, - "child of a worker-thread fork missed rmap tracking: anon adds {} bytes, expected ~{} MiB", - child_anon * page_size() as i64, - REGION_MIB + reclaimed >= 8 * MIB, + "foreign reclaim was not attributed to the owner after its CLONE_VM child exec'd: only {reclaimed} bytes" ); Ok(()) } diff --git a/crates/memtrack/tests/shared.rs b/crates/memtrack/tests/shared.rs index 0dda7e851..66c7d4bc1 100644 --- a/crates/memtrack/tests/shared.rs +++ b/crates/memtrack/tests/shared.rs @@ -1,5 +1,6 @@ #![allow(dead_code, unused)] +pub use memtrack::OwnershipMaps; use memtrack::prelude::*; use memtrack::{BpfVariant, Tracker, TrackerOptions}; use runner_shared::artifacts::{MemtrackEvent as Event, MemtrackEventKind}; @@ -208,11 +209,62 @@ pub fn track_command_with_rmap(command: Command) -> TrackResult { ) } +/// Probe selection used by the rmap-snapshot helpers: RSS reconstruction only, +/// without allocator uprobes. +fn rmap_only_options() -> TrackerOptions { + TrackerOptions::builder() + .allocators(false) + .rmap(true) + .build() +} + /// Track a command with an explicit probe selection rather than the environment's. pub fn track_command_with_opts(command: Command, options: TrackerOptions) -> TrackResult { track_command_with_tracker(command, Tracker::with_options(options)?) } +/// Track a command with rmap hooks and snapshot its ownership maps after the +/// tracked tree exits but before tracker teardown frees the BPF maps. +pub fn track_command_with_rmap_maps( + command: Command, +) -> anyhow::Result<(Vec, OwnershipMaps, std::thread::JoinHandle<()>)> { + let tracker = Tracker::with_options(rmap_only_options())?; + let (tracker, events, ()) = run_tracked(command, tracker, |_, _| Ok(()))?; + let maps = tracker.ownership_maps()?; + Ok((events, maps, std::thread::spawn(move || drop(tracker)))) +} + +/// Track a command with rmap hooks and snapshot the ownership maps at a +/// fixture-signalled checkpoint. +/// +/// The fixture creates `ready` and waits for `release`. The root pid is returned +/// for correlating snapshot entries with events. +pub fn track_command_with_rmap_checkpoint( + command: Command, + ready: &Path, + release: &Path, +) -> anyhow::Result<(Vec, OwnershipMaps, i32, std::thread::JoinHandle<()>)> { + let tracker = Tracker::with_options(rmap_only_options())?; + let (tracker, events, (maps, root_pid)) = run_tracked(command, tracker, |tracker, pid| { + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); + while !ready.exists() && std::time::Instant::now() < deadline { + std::thread::sleep(std::time::Duration::from_millis(20)); + } + let maps = tracker.ownership_maps()?; + // Release before bailing: nothing else reaps the fixture, which is + // parked holding its mapping. + std::fs::write(release, b"")?; + ensure!(ready.exists(), "fixture never reached the checkpoint"); + Ok((maps, pid)) + })?; + Ok(( + events, + maps, + root_pid, + std::thread::spawn(move || drop(tracker)), + )) +} + /// 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. @@ -281,10 +333,28 @@ pub fn for_each_variant( } fn track_command_with_tracker(command: Command, tracker: Tracker) -> TrackResult { + let (tracker, events, ()) = run_tracked(command, tracker, |_, _| Ok(()))?; + + // 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)); + + Ok((events, thread_handle)) +} + +/// Run `command` to completion under `tracker` and drain its events, handing +/// back the still-live tracker so BPF state can be read before teardown. +/// `checkpoint` runs while the tracked tree is still live. +fn run_tracked( + command: Command, + tracker: Tracker, + checkpoint: impl FnOnce(&Tracker, i32) -> anyhow::Result, +) -> anyhow::Result<(Tracker, Vec, T)> { tracker.enable_tracking()?; let mut session = tracker.spawn(&command, None)?; let rx = session.take_events()?; + let checkpoint = checkpoint(&tracker, session.pid())?; session.wait()?; // Dropping the session does a final ring buffer drain and closes the @@ -294,12 +364,8 @@ fn track_command_with_tracker(command: Command, tracker: Tracker) -> TrackResult tracker.finish()?; - // 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()); trace!("Events: {events:#?}"); - Ok((events, thread_handle)) + Ok((tracker, events, checkpoint)) }