From bd4821d85d5920035d0e7d95854c088ddce8ef1a Mon Sep 17 00:00:00 2001 From: Mauro Ezequiel Moltrasio Date: Thu, 18 Dec 2025 18:24:52 +0100 Subject: [PATCH 1/2] feat(ebpf): use bpf_loop for local d_path implementation Part of ROX-32059. The local implementation of d_path now uses bpf_loop, which enables it to iterate further than the previously implemented loop that did at most 16 levels of path. --- fact-ebpf/src/bpf/d_path.h | 132 ++++++++++++++++++++++--------------- fact/src/bpf/mod.rs | 81 ++++++++++++----------- 2 files changed, 122 insertions(+), 91 deletions(-) diff --git a/fact-ebpf/src/bpf/d_path.h b/fact-ebpf/src/bpf/d_path.h index 506b1c06..f74f37f5 100644 --- a/fact-ebpf/src/bpf/d_path.h +++ b/fact-ebpf/src/bpf/d_path.h @@ -1,13 +1,74 @@ #pragma once // clang-format off -#include "maps.h" #include "vmlinux.h" +#include "maps.h" + #include #include // clang-format on +struct d_path_ctx { + struct helper_t* helper; + struct path root; + struct mount* mnt; + struct dentry* dentry; + int offset; + int buflen; + bool success; +}; + +static long __d_path_inner(uint32_t index, void* _ctx) { + struct d_path_ctx* ctx = (struct d_path_ctx*)_ctx; + struct dentry* dentry = ctx->dentry; + struct dentry* parent = BPF_CORE_READ(dentry, d_parent); + struct mount* mnt = ctx->mnt; + struct dentry* mnt_root = BPF_CORE_READ(mnt, mnt.mnt_root); + + if (dentry == mnt_root) { + struct mount* m = BPF_CORE_READ(mnt, mnt_parent); + if (m != mnt) { + ctx->dentry = BPF_CORE_READ(mnt, mnt_mountpoint); + ctx->mnt = m; + return 0; + } + ctx->success = true; + return 1; + } + + if (dentry == parent) { + return 1; + } + + struct qstr d_name; + BPF_CORE_READ_INTO(&d_name, dentry, d_name); + int len = d_name.len & (PATH_MAX - 1); + if (len <= 0 || len >= ctx->buflen) { + return 1; + } + + int offset = ctx->offset - len; + if (offset <= 0) { + return 1; + } + offset &= PATH_MAX - 1; + + if (bpf_probe_read_kernel(&ctx->helper->buf[offset], len, d_name.name) != 0) { + return 1; + } + + offset--; + if (offset <= 0) { + return 1; + } + ctx->helper->buf[offset] = '/'; + + ctx->offset = offset; + ctx->dentry = parent; + return 0; +} + /** * Reimplementation of the kernel d_path function. * @@ -19,66 +80,31 @@ __always_inline static long __d_path(const struct path* path, char* buf, int buf return -1; } - struct helper_t* helper = get_helper(); - if (helper == NULL) { + int offset = (buflen - 1) & (PATH_MAX - 1); + struct d_path_ctx ctx = { + .buflen = buflen, + .helper = get_helper(), + .offset = offset, + }; + + if (ctx.helper == NULL) { return -1; } struct task_struct* task = (struct task_struct*)bpf_get_current_task(); - int offset = (buflen - 1) & (PATH_MAX - 1); - helper->buf[offset] = '\0'; // Ensure null termination - - struct path root; - BPF_CORE_READ_INTO(&root, task, fs, root); - struct mount* mnt = container_of(path->mnt, struct mount, mnt); - struct dentry* dentry; - BPF_CORE_READ_INTO(&dentry, path, dentry); - - for (int i = 0; i < 16 && (dentry != root.dentry || &mnt->mnt != root.mnt); i++) { - struct dentry* parent = BPF_CORE_READ(dentry, d_parent); - struct dentry* mnt_root = BPF_CORE_READ(mnt, mnt.mnt_root); - - if (dentry == mnt_root) { - struct mount* m = BPF_CORE_READ(mnt, mnt_parent); - if (m != mnt) { - dentry = BPF_CORE_READ(mnt, mnt_mountpoint); - mnt = m; - continue; - } - break; - } - - if (dentry == parent) { - return -1; - } + ctx.helper->buf[offset] = '\0'; // Ensure null termination - struct qstr d_name; - BPF_CORE_READ_INTO(&d_name, dentry, d_name); - int len = d_name.len; - if (len <= 0 || len >= buflen) { - return -1; - } + BPF_CORE_READ_INTO(&ctx.root, task, fs, root); + ctx.mnt = container_of(path->mnt, struct mount, mnt); + BPF_CORE_READ_INTO(&ctx.dentry, path, dentry); - offset -= len; - if (offset <= 0) { - return -1; - } - - if (bpf_probe_read_kernel(&helper->buf[offset], len, d_name.name) != 0) { - return -1; - } - - offset--; - if (offset <= 0) { - return -1; - } - helper->buf[offset] = '/'; - - dentry = parent; + long res = bpf_loop(PATH_MAX, __d_path_inner, &ctx, 0); + if (res <= 0 || !ctx.success) { + return -1; } - bpf_probe_read_str(buf, buflen, &helper->buf[offset]); - return buflen - offset; + bpf_probe_read_str(buf, buflen, &ctx.helper->buf[ctx.offset & (PATH_MAX - 1)]); + return buflen - ctx.offset; } __always_inline static long d_path(struct path* path, char* buf, int buflen, bool use_bpf_helper) { diff --git a/fact/src/bpf/mod.rs b/fact/src/bpf/mod.rs index 2a6b85e2..d8d6babf 100644 --- a/fact/src/bpf/mod.rs +++ b/fact/src/bpf/mod.rs @@ -227,7 +227,6 @@ impl Bpf { mod bpf_tests { use std::{env, path::PathBuf, time::Duration}; - use anyhow::Context; use fact_ebpf::file_activity_type_t; use tempfile::NamedTempFile; use tokio::{sync::watch, time::timeout}; @@ -241,16 +240,8 @@ mod bpf_tests { use super::*; - fn get_executor() -> anyhow::Result { - let executor = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .context("Failed building tokio runtime")?; - Ok(executor) - } - - #[test] - fn test_basic() { + #[tokio::test] + async fn test_basic() { if let Ok(value) = std::env::var("FACT_LOGLEVEL") { let value = value.to_lowercase(); if value == "debug" || value == "trace" { @@ -258,55 +249,69 @@ mod bpf_tests { } } - let executor = get_executor().unwrap(); let monitored_path = env!("CARGO_MANIFEST_DIR"); let monitored_path = PathBuf::from(monitored_path); let paths = vec![monitored_path.clone()]; let mut config = FactConfig::default(); config.set_paths(paths); let reloader = Reloader::from(config); - executor.block_on(async { - let (tx, mut rx) = mpsc::channel(100); - let mut bpf = Bpf::new(reloader.paths(), reloader.config().ringbuf_size(), tx) - .expect("Failed to load BPF code"); - let (run_tx, run_rx) = watch::channel(true); - // Create a metrics exporter, but don't start it - let exporter = Exporter::new(bpf.take_metrics().unwrap()); + let (tx, mut rx) = mpsc::channel(100); + let mut bpf = Bpf::new(reloader.paths(), reloader.config().ringbuf_size(), tx) + .expect("Failed to load BPF code"); + let (run_tx, run_rx) = watch::channel(true); + // Create a metrics exporter, but don't start it + let exporter = Exporter::new(bpf.take_metrics().unwrap()); + + let handle = bpf.start(run_rx, exporter.metrics.bpf_worker.clone()); - let handle = bpf.start(run_rx, exporter.metrics.bpf_worker.clone()); + tokio::time::sleep(Duration::from_millis(500)).await; - tokio::time::sleep(Duration::from_millis(500)).await; + // Create a file + let file = NamedTempFile::new_in(monitored_path).expect("Failed to create temporary file"); + println!("Created {file:?}"); - // Create a file - let file = - NamedTempFile::new_in(monitored_path).expect("Failed to create temporary file"); - println!("Created {file:?}"); + let current = Process::current(); + let file_path = file.path().to_path_buf(); - let expected = Event::new( + let expected_events = [ + Event::new( file_activity_type_t::FILE_ACTIVITY_CREATION, host_info::get_hostname(), - file.path().to_path_buf(), + file_path.clone(), + PathBuf::new(), // host path is resolved by HostScanner + current.clone(), + ) + .unwrap(), + Event::new( + file_activity_type_t::FILE_ACTIVITY_UNLINK, + host_info::get_hostname(), + file_path, PathBuf::new(), // host path is resolved by HostScanner - Process::current(), + current, ) - .unwrap(); + .unwrap(), + ]; - println!("Expected: {expected:?}"); - let wait = timeout(Duration::from_secs(1), async move { + // Close the file, removing it + file.close().expect("Failed to close temp file"); + + println!("Expected: {expected_events:?}"); + let wait = timeout(Duration::from_secs(1), async move { + for expected in expected_events { while let Some(event) = rx.recv().await { println!("{event:?}"); if event == expected { break; } } - }); - - tokio::select! { - res = wait => res.unwrap(), - res = handle => res.unwrap().unwrap(), } - - run_tx.send(false).unwrap(); }); + + tokio::select! { + res = wait => res.unwrap(), + res = handle => res.unwrap().unwrap(), + } + + run_tx.send(false).unwrap(); } } From 49ca5193d7819a7af6f86011e4c0e6d6881b47d4 Mon Sep 17 00:00:00 2001 From: Mauro Ezequiel Moltrasio Date: Wed, 7 Jan 2026 17:48:29 +0100 Subject: [PATCH 2/2] Add comments, use PATH_LEN_CLAMP and restore a missing stop condition Added comments to help clarify some of the logic around d_path. Used PATH_LEN_CLAMP instead of raw `& (PATH_MAX -1)` operations, documenting the macro itself to make its purpose more obvious. Restored the d_path stop condition of reaching the root of the current task fs, which was accidentally removed when moving from the original implementation. --- fact-ebpf/src/bpf/bound_path.h | 9 +++---- fact-ebpf/src/bpf/d_path.h | 47 +++++++++++++++++++++++++++++----- fact-ebpf/src/bpf/types.h | 5 ++++ 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/fact-ebpf/src/bpf/bound_path.h b/fact-ebpf/src/bpf/bound_path.h index 1c9430cf..0cbe7ebb 100644 --- a/fact-ebpf/src/bpf/bound_path.h +++ b/fact-ebpf/src/bpf/bound_path.h @@ -11,11 +11,8 @@ #include // clang-format on -#define PATH_MAX_MASK (PATH_MAX - 1) -#define path_len_clamp(len) ((len) & PATH_MAX_MASK) - __always_inline static char* path_safe_access(char* p, unsigned int offset) { - return &p[path_len_clamp(offset)]; + return &p[PATH_LEN_CLAMP(offset)]; } __always_inline static void path_write_char(char* p, unsigned int offset, char c) { @@ -34,7 +31,7 @@ __always_inline static struct bound_path_t* _path_read(struct path* path, bool u } // Ensure length is within PATH_MAX for the verifier - bound_path->len = path_len_clamp(bound_path->len); + bound_path->len = PATH_LEN_CLAMP(bound_path->len); return bound_path; } @@ -63,7 +60,7 @@ __always_inline static enum path_append_status_t path_append_dentry(struct bound } char* path_offset = path_safe_access(path->path, path->len); - if (bpf_probe_read_kernel(path_offset, path_len_clamp(len), d_name.name)) { + if (bpf_probe_read_kernel(path_offset, PATH_LEN_CLAMP(len), d_name.name)) { return PATH_APPEND_READ_ERROR; } diff --git a/fact-ebpf/src/bpf/d_path.h b/fact-ebpf/src/bpf/d_path.h index f74f37f5..a922600e 100644 --- a/fact-ebpf/src/bpf/d_path.h +++ b/fact-ebpf/src/bpf/d_path.h @@ -9,9 +9,24 @@ #include // clang-format on +/** + * PATH_MAX is defined in the kernel as 4096 which translates to 0x1000. + * This define gives as an easy way to clamp path lengths + */ +#define PATH_MAX_MASK (PATH_MAX - 1) + +/** + * Helper for keeping the verifier happy. + * + * Whenever a path is interacted with in a buffer, this macro can be + * used to convince the verifier no more than PATH_MAX bytes will be + * accessed. + */ +#define PATH_LEN_CLAMP(len) ((len) & PATH_MAX_MASK) + struct d_path_ctx { struct helper_t* helper; - struct path root; + struct path* root; struct mount* mnt; struct dentry* dentry; int offset; @@ -26,24 +41,42 @@ static long __d_path_inner(uint32_t index, void* _ctx) { struct mount* mnt = ctx->mnt; struct dentry* mnt_root = BPF_CORE_READ(mnt, mnt.mnt_root); + if (dentry == ctx->root->dentry && &mnt->mnt == ctx->root->mnt) { + // Found the root of the process, we are done + ctx->success = true; + return 1; + } + if (dentry == mnt_root) { struct mount* m = BPF_CORE_READ(mnt, mnt_parent); if (m != mnt) { + // Current dentry is a mount root different to the previous one we + // had (to prevent looping), switch over to that mount position + // and keep walking up the path. ctx->dentry = BPF_CORE_READ(mnt, mnt_mountpoint); ctx->mnt = m; return 0; } + + // Ended up in a global root, the path might need re-processing or + // the root is not attached yet, we are not getting a better path, + // so we assume we are correct and stop iterating. ctx->success = true; return 1; } if (dentry == parent) { + // We escaped the mounts and ended up at (most likely) the root of + // the device, the path we formed will be wrong. + // + // This may happen in race conditions where some dentries go away + // while we are iterating. return 1; } struct qstr d_name; BPF_CORE_READ_INTO(&d_name, dentry, d_name); - int len = d_name.len & (PATH_MAX - 1); + int len = PATH_LEN_CLAMP(d_name.len); if (len <= 0 || len >= ctx->buflen) { return 1; } @@ -52,7 +85,7 @@ static long __d_path_inner(uint32_t index, void* _ctx) { if (offset <= 0) { return 1; } - offset &= PATH_MAX - 1; + offset = PATH_LEN_CLAMP(offset); if (bpf_probe_read_kernel(&ctx->helper->buf[offset], len, d_name.name) != 0) { return 1; @@ -80,7 +113,7 @@ __always_inline static long __d_path(const struct path* path, char* buf, int buf return -1; } - int offset = (buflen - 1) & (PATH_MAX - 1); + int offset = PATH_LEN_CLAMP(buflen - 1); struct d_path_ctx ctx = { .buflen = buflen, .helper = get_helper(), @@ -91,10 +124,10 @@ __always_inline static long __d_path(const struct path* path, char* buf, int buf return -1; } - struct task_struct* task = (struct task_struct*)bpf_get_current_task(); + struct task_struct* task = (struct task_struct*)bpf_get_current_task_btf(); ctx.helper->buf[offset] = '\0'; // Ensure null termination - BPF_CORE_READ_INTO(&ctx.root, task, fs, root); + ctx.root = &task->fs->root; ctx.mnt = container_of(path->mnt, struct mount, mnt); BPF_CORE_READ_INTO(&ctx.dentry, path, dentry); @@ -103,7 +136,7 @@ __always_inline static long __d_path(const struct path* path, char* buf, int buf return -1; } - bpf_probe_read_str(buf, buflen, &ctx.helper->buf[ctx.offset & (PATH_MAX - 1)]); + bpf_probe_read_str(buf, buflen, &ctx.helper->buf[PATH_LEN_CLAMP(ctx.offset)]); return buflen - ctx.offset; } diff --git a/fact-ebpf/src/bpf/types.h b/fact-ebpf/src/bpf/types.h index 17c1f43b..a3a6a92b 100644 --- a/fact-ebpf/src/bpf/types.h +++ b/fact-ebpf/src/bpf/types.h @@ -6,8 +6,13 @@ * other sources of bloat into this file. */ +/** + * Kernel constant, taken from: + * https://github.com/torvalds/linux/blob/f0b9d8eb98dfee8d00419aa07543bdc2c1a44fb1/include/uapi/linux/limits.h#L13 + */ #define PATH_MAX 4096 #define TASK_COMM_LEN 16 + #define LINEAGE_MAX 2 #define LPM_SIZE_MAX 256