From fd0fd1ce95c9e2a4301e0e96d6eb0c56013e1e52 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Thu, 9 Apr 2026 10:13:00 -0700 Subject: [PATCH 01/34] X-Smart-Branch-Parent: main From 5b2167e0e48b8a813925db260f9540fce8c9f1e1 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Tue, 31 Mar 2026 16:41:02 -0700 Subject: [PATCH 02/34] Added integration tests --- tests/test_path_mkdir.py | 69 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 tests/test_path_mkdir.py diff --git a/tests/test_path_mkdir.py b/tests/test_path_mkdir.py new file mode 100644 index 00000000..5242894d --- /dev/null +++ b/tests/test_path_mkdir.py @@ -0,0 +1,69 @@ +import os + +import pytest + +from event import Event, EventType, Process + + +def test_mkdir_nested(monitored_dir, server): + """ + Tests that creating nested directories tracks all inodes correctly. + + Args: + monitored_dir: Temporary directory path for creating the test directory. + server: The server instance to communicate with. + """ + process = Process.from_proc() + + # Create nested directories + level1 = os.path.join(monitored_dir, 'level1') + level2 = os.path.join(level1, 'level2') + level3 = os.path.join(level2, 'level3') + + os.mkdir(level1) + os.mkdir(level2) + os.mkdir(level3) + + # Create a file in the deepest directory + test_file = os.path.join(level3, 'deep_file.txt') + with open(test_file, 'w') as f: + f.write('nested content') + + events = [ + Event(process=process, event_type=EventType.CREATION, + file=level1, host_path=level1), + Event(process=process, event_type=EventType.CREATION, + file=level2, host_path=level2), + Event(process=process, event_type=EventType.CREATION, + file=level3, host_path=level3), + Event(process=process, event_type=EventType.CREATION, + file=test_file, host_path=test_file), + ] + + server.wait_events(events) + + +def test_mkdir_ignored(monitored_dir, ignored_dir, server): + """ + Tests that directories created outside monitored paths are ignored. + + Args: + monitored_dir: Temporary directory path that is monitored. + ignored_dir: Temporary directory path that is not monitored. + server: The server instance to communicate with. + """ + process = Process.from_proc() + + # Create directory in ignored path - should not be tracked + ignored_subdir = os.path.join(ignored_dir, 'ignored_subdir') + os.mkdir(ignored_subdir) + + # Create directory in monitored path - should be tracked + monitored_subdir = os.path.join(monitored_dir, 'monitored_subdir') + os.mkdir(monitored_subdir) + + # Only the monitored directory should generate an event + e = Event(process=process, event_type=EventType.CREATION, + file=monitored_subdir, host_path=monitored_subdir) + + server.wait_events([e]) From 12f602eb08fd8f6f0b70a40f6ad4b34507eab4cc Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Wed, 1 Apr 2026 10:50:56 -0700 Subject: [PATCH 03/34] Instrument inode tracking on directory being created --- fact-ebpf/src/bpf/events.h | 14 ++++ fact-ebpf/src/bpf/file.h | 16 +++++ fact-ebpf/src/bpf/main.c | 107 +++++++++++++++++++++++++++++ fact-ebpf/src/bpf/maps.h | 20 ++++++ fact-ebpf/src/bpf/types.h | 8 +++ fact/src/metrics/kernel_metrics.rs | 18 +++++ 6 files changed, 183 insertions(+) diff --git a/fact-ebpf/src/bpf/events.h b/fact-ebpf/src/bpf/events.h index 26254778..5fc3d3a6 100644 --- a/fact-ebpf/src/bpf/events.h +++ b/fact-ebpf/src/bpf/events.h @@ -129,3 +129,17 @@ __always_inline static void submit_rename_event(struct metrics_by_hook_t* m, __submit_event(event, m, FILE_ACTIVITY_RENAME, new_filename, new_inode, new_parent_inode, path_hooks_support_bpf_d_path); } + +__always_inline static void submit_mkdir_event(struct metrics_by_hook_t* m, + const char filename[PATH_MAX], + inode_key_t* inode, + inode_key_t* parent_inode) { + struct event_t* event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); + if (event == NULL) { + m->ringbuffer_full++; + return; + } + + // d_instantiate doesn't support bpf_d_path, so we use false and rely on the stashed path from path_mkdir + __submit_event(event, m, FILE_ACTIVITY_CREATION, filename, inode, parent_inode, false); +} diff --git a/fact-ebpf/src/bpf/file.h b/fact-ebpf/src/bpf/file.h index d0fdc8b1..eac1b429 100644 --- a/fact-ebpf/src/bpf/file.h +++ b/fact-ebpf/src/bpf/file.h @@ -43,3 +43,19 @@ __always_inline static inode_monitored_t is_monitored(inode_key_t inode, struct return NOT_MONITORED; } + +// Check if a new directory should be tracked based on its parent and path. +// This is used during mkdir operations where the child inode doesn't exist yet. +__always_inline static inode_monitored_t should_track_mkdir(inode_key_t parent_inode, struct bound_path_t* child_path) { + const inode_value_t* volatile parent_value = inode_get(&parent_inode); + + if (parent_value != NULL) { + return PARENT_MONITORED; + } + + if (path_is_monitored(child_path)) { + return MONITORED; + } + + return NOT_MONITORED; +} diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index 5ac35159..bef5df6c 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -19,6 +19,11 @@ char _license[] SEC("license") = "Dual MIT/GPL"; #define FMODE_PWRITE ((fmode_t)(1 << 4)) #define FMODE_CREATED ((fmode_t)(1 << 20)) +// File type constants from linux/stat.h +#define S_IFMT 00170000 +#define S_IFDIR 0040000 +#define S_ISDIR(m) (((m) & S_IFMT) == S_IFDIR) + SEC("lsm/file_open") int BPF_PROG(trace_file_open, struct file* file) { struct metrics_t* m = get_metrics(); @@ -231,3 +236,105 @@ int BPF_PROG(trace_path_rename, struct path* old_dir, m->path_rename.error++; return 0; } + +SEC("lsm/path_mkdir") +int BPF_PROG(trace_path_mkdir, struct path* dir, struct dentry* dentry, umode_t mode) { + struct metrics_t* m = get_metrics(); + if (m == NULL) { + return 0; + } + + m->path_mkdir.total++; + + struct bound_path_t* path = path_read_append_d_entry(dir, dentry); + if (path == NULL) { + bpf_printk("Failed to read path"); + m->path_mkdir.error++; + return 0; + } + + struct dentry* parent_dentry = BPF_CORE_READ(dir, dentry); + struct inode* parent_inode_ptr = BPF_CORE_READ(parent_dentry, d_inode); + inode_key_t parent_inode = inode_to_key(parent_inode_ptr); + + if (should_track_mkdir(parent_inode, path) == NOT_MONITORED) { + m->path_mkdir.ignored++; + return 0; + } + + // Stash mkdir context for security_d_instantiate + __u64 pid_tgid = bpf_get_current_pid_tgid(); + struct mkdir_context_t* mkdir_ctx = get_mkdir_context(); + if (mkdir_ctx == NULL) { + bpf_printk("Failed to get mkdir context buffer"); + m->path_mkdir.error++; + return 0; + } + + long path_copy_len = bpf_probe_read_str(mkdir_ctx->path, PATH_MAX, path->path); + if (path_copy_len < 0) { + bpf_printk("Failed to copy path string"); + m->path_mkdir.error++; + return 0; + } + mkdir_ctx->parent_inode = parent_inode; + + if (bpf_map_update_elem(&mkdir_context, &pid_tgid, mkdir_ctx, BPF_ANY) != 0) { + bpf_printk("Failed to stash mkdir context"); + m->path_mkdir.error++; + return 0; + } + + return 0; +} + +SEC("lsm/d_instantiate") +int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) { + struct metrics_t* m = get_metrics(); + if (m == NULL) { + return 0; + } + + m->d_instantiate.total++; + + if (inode == NULL) { + m->d_instantiate.ignored++; + return 0; + } + + __u64 pid_tgid = bpf_get_current_pid_tgid(); + struct mkdir_context_t* mkdir_ctx = bpf_map_lookup_elem(&mkdir_context, &pid_tgid); + if (mkdir_ctx == NULL) { + m->d_instantiate.ignored++; + return 0; + } + + // Check if this is a directory + umode_t mode = BPF_CORE_READ(inode, i_mode); + if (!S_ISDIR(mode)) { + bpf_map_delete_elem(&mkdir_context, &pid_tgid); + m->d_instantiate.ignored++; + return 0; + } + + // Get the inode key for the new directory + inode_key_t inode_key = inode_to_key(inode); + + // Add the new directory inode to tracking + if (inode_add(&inode_key) == 0) { + m->d_instantiate.added++; + } else { + m->d_instantiate.error++; + } + + // Submit creation event using the stashed path + submit_mkdir_event(&m->d_instantiate, + mkdir_ctx->path, + &inode_key, + &mkdir_ctx->parent_inode); + + // Clean up context + bpf_map_delete_elem(&mkdir_context, &pid_tgid); + + return 0; +} diff --git a/fact-ebpf/src/bpf/maps.h b/fact-ebpf/src/bpf/maps.h index eca822f0..3758ea80 100644 --- a/fact-ebpf/src/bpf/maps.h +++ b/fact-ebpf/src/bpf/maps.h @@ -83,6 +83,26 @@ struct { __uint(map_flags, BPF_F_NO_PREALLOC); } inode_map SEC(".maps"); +struct { + __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY); + __type(key, __u32); + __type(value, struct mkdir_context_t); + __uint(max_entries, 1); +} mkdir_context_heap SEC(".maps"); + +__always_inline static struct mkdir_context_t* get_mkdir_context() { + unsigned int zero = 0; + return bpf_map_lookup_elem(&mkdir_context_heap, &zero); +} + +struct { + __uint(type, BPF_MAP_TYPE_HASH); + __type(key, __u64); + __type(value, struct mkdir_context_t); + __uint(max_entries, 16384); + __uint(map_flags, BPF_F_NO_PREALLOC); +} mkdir_context SEC(".maps"); + struct { __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY); __type(key, __u32); diff --git a/fact-ebpf/src/bpf/types.h b/fact-ebpf/src/bpf/types.h index 55005c00..9059c7aa 100644 --- a/fact-ebpf/src/bpf/types.h +++ b/fact-ebpf/src/bpf/types.h @@ -96,6 +96,12 @@ struct path_prefix_t { const char path[LPM_SIZE_MAX]; }; +// Context for correlating mkdir operations +struct mkdir_context_t { + char path[PATH_MAX]; + inode_key_t parent_inode; +}; + // Metrics types struct metrics_by_hook_t { unsigned long long total; @@ -111,4 +117,6 @@ struct metrics_t { struct metrics_by_hook_t path_chmod; struct metrics_by_hook_t path_chown; struct metrics_by_hook_t path_rename; + struct metrics_by_hook_t path_mkdir; + struct metrics_by_hook_t d_instantiate; }; diff --git a/fact/src/metrics/kernel_metrics.rs b/fact/src/metrics/kernel_metrics.rs index d1a3a242..9caa1ff3 100644 --- a/fact/src/metrics/kernel_metrics.rs +++ b/fact/src/metrics/kernel_metrics.rs @@ -13,6 +13,8 @@ pub struct KernelMetrics { path_chmod: EventCounter, path_chown: EventCounter, path_rename: EventCounter, + path_mkdir: EventCounter, + d_instantiate: EventCounter, map: PerCpuArray, } @@ -43,12 +45,24 @@ impl KernelMetrics { "Events processed by the path_rename LSM hook", &[], // Labels are not needed since `collect` will add them all ); + let path_mkdir = EventCounter::new( + "kernel_path_mkdir_events", + "Events processed by the path_mkdir LSM hook", + &[], // Labels are not needed since `collect` will add them all + ); + let d_instantiate = EventCounter::new( + "kernel_d_instantiate_events", + "Events processed by the d_instantiate LSM hook", + &[], // Labels are not needed since `collect` will add them all + ); file_open.register(reg); path_unlink.register(reg); path_chmod.register(reg); path_chown.register(reg); path_rename.register(reg); + path_mkdir.register(reg); + d_instantiate.register(reg); KernelMetrics { file_open, @@ -56,6 +70,8 @@ impl KernelMetrics { path_chmod, path_chown, path_rename, + path_mkdir, + d_instantiate, map: kernel_metrics, } } @@ -105,6 +121,8 @@ impl KernelMetrics { KernelMetrics::refresh_labels(&self.path_chmod, &metrics.path_chmod); KernelMetrics::refresh_labels(&self.path_chown, &metrics.path_chown); KernelMetrics::refresh_labels(&self.path_rename, &metrics.path_rename); + KernelMetrics::refresh_labels(&self.path_mkdir, &metrics.path_mkdir); + KernelMetrics::refresh_labels(&self.d_instantiate, &metrics.d_instantiate); Ok(()) } From 8406a5afd157195551b005a293c180ae9b00a87f Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Wed, 1 Apr 2026 15:29:42 -0700 Subject: [PATCH 04/34] Only tracking directories if the parent is monitored --- fact-ebpf/src/bpf/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index bef5df6c..1ec2837d 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -257,7 +257,7 @@ int BPF_PROG(trace_path_mkdir, struct path* dir, struct dentry* dentry, umode_t struct inode* parent_inode_ptr = BPF_CORE_READ(parent_dentry, d_inode); inode_key_t parent_inode = inode_to_key(parent_inode_ptr); - if (should_track_mkdir(parent_inode, path) == NOT_MONITORED) { + if (should_track_mkdir(parent_inode, path) != PARENT_MONITORED) { m->path_mkdir.ignored++; return 0; } From 03bd26ebcff3edb940d4740a2ecd0974adf1616f Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Wed, 1 Apr 2026 16:41:52 -0700 Subject: [PATCH 05/34] Removed some comments --- fact-ebpf/src/bpf/main.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index 1ec2837d..ea0781ea 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -309,7 +309,6 @@ int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) { return 0; } - // Check if this is a directory umode_t mode = BPF_CORE_READ(inode, i_mode); if (!S_ISDIR(mode)) { bpf_map_delete_elem(&mkdir_context, &pid_tgid); @@ -317,23 +316,19 @@ int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) { return 0; } - // Get the inode key for the new directory inode_key_t inode_key = inode_to_key(inode); - // Add the new directory inode to tracking if (inode_add(&inode_key) == 0) { m->d_instantiate.added++; } else { m->d_instantiate.error++; } - // Submit creation event using the stashed path submit_mkdir_event(&m->d_instantiate, mkdir_ctx->path, &inode_key, &mkdir_ctx->parent_inode); - // Clean up context bpf_map_delete_elem(&mkdir_context, &pid_tgid); return 0; From b1fc6f57fe562a742d72a39fcde73a6e0a506ff4 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Thu, 2 Apr 2026 10:43:48 -0700 Subject: [PATCH 06/34] Combined two uses of BPF_CORE_READ --- fact-ebpf/src/bpf/main.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index ea0781ea..743ad73c 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -253,8 +253,7 @@ int BPF_PROG(trace_path_mkdir, struct path* dir, struct dentry* dentry, umode_t return 0; } - struct dentry* parent_dentry = BPF_CORE_READ(dir, dentry); - struct inode* parent_inode_ptr = BPF_CORE_READ(parent_dentry, d_inode); + struct inode* parent_inode_ptr = BPF_CORE_READ(dir, dentry, d_inode); inode_key_t parent_inode = inode_to_key(parent_inode_ptr); if (should_track_mkdir(parent_inode, path) != PARENT_MONITORED) { From dbea1a72e9f07a964c2c41970d0721af7f71d2ab Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Thu, 2 Apr 2026 11:25:58 -0700 Subject: [PATCH 07/34] Added DIR_ACTIVITY_CREATION --- fact-ebpf/src/bpf/events.h | 2 +- fact-ebpf/src/bpf/types.h | 1 + fact/src/event/mod.rs | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/fact-ebpf/src/bpf/events.h b/fact-ebpf/src/bpf/events.h index 5fc3d3a6..fe521450 100644 --- a/fact-ebpf/src/bpf/events.h +++ b/fact-ebpf/src/bpf/events.h @@ -141,5 +141,5 @@ __always_inline static void submit_mkdir_event(struct metrics_by_hook_t* m, } // d_instantiate doesn't support bpf_d_path, so we use false and rely on the stashed path from path_mkdir - __submit_event(event, m, FILE_ACTIVITY_CREATION, filename, inode, parent_inode, false); + __submit_event(event, m, DIR_ACTIVITY_CREATION, filename, inode, parent_inode, false); } diff --git a/fact-ebpf/src/bpf/types.h b/fact-ebpf/src/bpf/types.h index 9059c7aa..95c67e86 100644 --- a/fact-ebpf/src/bpf/types.h +++ b/fact-ebpf/src/bpf/types.h @@ -55,6 +55,7 @@ typedef enum file_activity_type_t { FILE_ACTIVITY_CHMOD, FILE_ACTIVITY_CHOWN, FILE_ACTIVITY_RENAME, + DIR_ACTIVITY_CREATION, } file_activity_type_t; struct event_t { diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index bc9d7897..ed83b899 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -311,6 +311,7 @@ impl FileData { let file = match event_type { file_activity_type_t::FILE_ACTIVITY_OPEN => FileData::Open(inner), file_activity_type_t::FILE_ACTIVITY_CREATION => FileData::Creation(inner), + file_activity_type_t::DIR_ACTIVITY_CREATION => FileData::Creation(inner), file_activity_type_t::FILE_ACTIVITY_UNLINK => FileData::Unlink(inner), file_activity_type_t::FILE_ACTIVITY_CHMOD => { let data = ChmodFileData { From d1b4136ebfbb5570d9b8a3aa058d5025c04def4a Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Thu, 2 Apr 2026 11:29:22 -0700 Subject: [PATCH 08/34] Added permalink to linux/stat.h --- fact-ebpf/src/bpf/main.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index 743ad73c..e40cf1c1 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -20,6 +20,7 @@ char _license[] SEC("license") = "Dual MIT/GPL"; #define FMODE_CREATED ((fmode_t)(1 << 20)) // File type constants from linux/stat.h +// https://github.com/torvalds/linux/blob/5619b098e2fbf3a23bf13d91897056a1fe238c6d/include/uapi/linux/stat.h #define S_IFMT 00170000 #define S_IFDIR 0040000 #define S_ISDIR(m) (((m) & S_IFMT) == S_IFDIR) From a1200841270e02127165b39039bf96a106ca0d23 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Thu, 2 Apr 2026 11:43:15 -0700 Subject: [PATCH 09/34] Removing map entry in case of early return in lsm/d_instantiate --- fact-ebpf/src/bpf/main.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index e40cf1c1..a7841284 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -309,11 +309,11 @@ int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) { return 0; } + // From this point on, we must clean up mkdir_context before returning umode_t mode = BPF_CORE_READ(inode, i_mode); if (!S_ISDIR(mode)) { - bpf_map_delete_elem(&mkdir_context, &pid_tgid); m->d_instantiate.ignored++; - return 0; + goto cleanup; } inode_key_t inode_key = inode_to_key(inode); @@ -329,7 +329,7 @@ int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) { &inode_key, &mkdir_ctx->parent_inode); +cleanup: bpf_map_delete_elem(&mkdir_context, &pid_tgid); - return 0; } From fb7ce804b200a288040f34a153464d076b3ac5a3 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Thu, 2 Apr 2026 15:25:20 -0700 Subject: [PATCH 10/34] Using os.makedirs instead of os.mkdir three times --- tests/test_path_mkdir.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_path_mkdir.py b/tests/test_path_mkdir.py index 5242894d..759e0022 100644 --- a/tests/test_path_mkdir.py +++ b/tests/test_path_mkdir.py @@ -20,9 +20,7 @@ def test_mkdir_nested(monitored_dir, server): level2 = os.path.join(level1, 'level2') level3 = os.path.join(level2, 'level3') - os.mkdir(level1) - os.mkdir(level2) - os.mkdir(level3) + os.makedirs(level3, exist_ok=True) # Create a file in the deepest directory test_file = os.path.join(level3, 'deep_file.txt') From 60f96eb6f7a9f7979490dddc59b0005d76962688 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Fri, 3 Apr 2026 10:55:54 -0700 Subject: [PATCH 11/34] Accumulate m.path_mkdir and m.d_instantiate --- fact-ebpf/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fact-ebpf/src/lib.rs b/fact-ebpf/src/lib.rs index bd84ee08..c4c95ec6 100644 --- a/fact-ebpf/src/lib.rs +++ b/fact-ebpf/src/lib.rs @@ -125,6 +125,8 @@ impl metrics_t { m.path_chmod = m.path_chmod.accumulate(&other.path_chmod); m.path_chown = m.path_chown.accumulate(&other.path_chown); m.path_rename = m.path_rename.accumulate(&other.path_rename); + m.path_mkdir = m.path_mkdir.accumulate(&other.path_mkdir); + m.d_instantiate = m.d_instantiate.accumulate(&other.d_instantiate); m } } From 651edd47ceb2a4cd41972fb1483522a3dd562519 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Fri, 3 Apr 2026 11:58:44 -0700 Subject: [PATCH 12/34] Checking pid_tgid earlier so if inode is null we still cleanup --- fact-ebpf/src/bpf/main.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index a7841284..2fead541 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -297,19 +297,19 @@ int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) { m->d_instantiate.total++; - if (inode == NULL) { - m->d_instantiate.ignored++; - return 0; - } - __u64 pid_tgid = bpf_get_current_pid_tgid(); struct mkdir_context_t* mkdir_ctx = bpf_map_lookup_elem(&mkdir_context, &pid_tgid); + if (mkdir_ctx == NULL) { m->d_instantiate.ignored++; return 0; } - // From this point on, we must clean up mkdir_context before returning + if (inode == NULL) { + m->d_instantiate.ignored++; + goto cleanup; + } + umode_t mode = BPF_CORE_READ(inode, i_mode); if (!S_ISDIR(mode)) { m->d_instantiate.ignored++; From a57baf7e2aa6face7da768b9e24cd97e88fb3e78 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Mon, 6 Apr 2026 17:09:10 -0700 Subject: [PATCH 13/34] Not using BPF_F_NO_PREALLOC --- fact-ebpf/src/bpf/main.c | 18 ++++++++++-------- fact-ebpf/src/bpf/maps.h | 13 ------------- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index 2fead541..a6521cfd 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -264,9 +264,16 @@ int BPF_PROG(trace_path_mkdir, struct path* dir, struct dentry* dentry, umode_t // Stash mkdir context for security_d_instantiate __u64 pid_tgid = bpf_get_current_pid_tgid(); - struct mkdir_context_t* mkdir_ctx = get_mkdir_context(); + + if (bpf_map_update_elem(&mkdir_context, &pid_tgid, NULL, BPF_ANY) != 0) { + bpf_printk("Failed to create mkdir context entry"); + m->path_mkdir.error++; + return 0; + } + + struct mkdir_context_t* mkdir_ctx = bpf_map_lookup_elem(&mkdir_context, &pid_tgid); if (mkdir_ctx == NULL) { - bpf_printk("Failed to get mkdir context buffer"); + bpf_printk("Failed to lookup mkdir context after creation"); m->path_mkdir.error++; return 0; } @@ -275,16 +282,11 @@ int BPF_PROG(trace_path_mkdir, struct path* dir, struct dentry* dentry, umode_t if (path_copy_len < 0) { bpf_printk("Failed to copy path string"); m->path_mkdir.error++; + bpf_map_delete_elem(&mkdir_context, &pid_tgid); return 0; } mkdir_ctx->parent_inode = parent_inode; - if (bpf_map_update_elem(&mkdir_context, &pid_tgid, mkdir_ctx, BPF_ANY) != 0) { - bpf_printk("Failed to stash mkdir context"); - m->path_mkdir.error++; - return 0; - } - return 0; } diff --git a/fact-ebpf/src/bpf/maps.h b/fact-ebpf/src/bpf/maps.h index 3758ea80..acd498f7 100644 --- a/fact-ebpf/src/bpf/maps.h +++ b/fact-ebpf/src/bpf/maps.h @@ -83,24 +83,11 @@ struct { __uint(map_flags, BPF_F_NO_PREALLOC); } inode_map SEC(".maps"); -struct { - __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY); - __type(key, __u32); - __type(value, struct mkdir_context_t); - __uint(max_entries, 1); -} mkdir_context_heap SEC(".maps"); - -__always_inline static struct mkdir_context_t* get_mkdir_context() { - unsigned int zero = 0; - return bpf_map_lookup_elem(&mkdir_context_heap, &zero); -} - struct { __uint(type, BPF_MAP_TYPE_HASH); __type(key, __u64); __type(value, struct mkdir_context_t); __uint(max_entries, 16384); - __uint(map_flags, BPF_F_NO_PREALLOC); } mkdir_context SEC(".maps"); struct { From 1fa3c77d91b39693a1df835342e5b438d09b2498 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Mon, 6 Apr 2026 18:46:35 -0700 Subject: [PATCH 14/34] Switched from BPF_MAP_TYPE_HASH to BPF_MAP_TYPE_LRU_HASH --- fact-ebpf/src/bpf/maps.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fact-ebpf/src/bpf/maps.h b/fact-ebpf/src/bpf/maps.h index acd498f7..c8595829 100644 --- a/fact-ebpf/src/bpf/maps.h +++ b/fact-ebpf/src/bpf/maps.h @@ -84,7 +84,7 @@ struct { } inode_map SEC(".maps"); struct { - __uint(type, BPF_MAP_TYPE_HASH); + __uint(type, BPF_MAP_TYPE_LRU_HASH); __type(key, __u64); __type(value, struct mkdir_context_t); __uint(max_entries, 16384); From 0d1929d667e9b79d6821c4b208c8ae0750f260f9 Mon Sep 17 00:00:00 2001 From: Jouko Virtanen Date: Mon, 6 Apr 2026 18:53:01 -0700 Subject: [PATCH 15/34] Apply suggestion from @Molter73 Co-authored-by: Mauro Ezequiel Moltrasio --- fact/src/event/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index ed83b899..70e96916 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -310,7 +310,7 @@ impl FileData { let inner = BaseFileData::new(filename, inode, parent_inode)?; let file = match event_type { file_activity_type_t::FILE_ACTIVITY_OPEN => FileData::Open(inner), - file_activity_type_t::FILE_ACTIVITY_CREATION => FileData::Creation(inner), + file_activity_type_t::FILE_ACTIVITY_CREATION | file_activity_type_t::DIR_ACTIVITY_CREATION => FileData::Creation(inner), file_activity_type_t::FILE_ACTIVITY_UNLINK => FileData::Unlink(inner), file_activity_type_t::FILE_ACTIVITY_CHMOD => { From e9634e57bb48f5f4a31c9240b20e8d3c2c1050d5 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Mon, 6 Apr 2026 19:01:45 -0700 Subject: [PATCH 16/34] Parameterized test --- tests/test_path_mkdir.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/test_path_mkdir.py b/tests/test_path_mkdir.py index 759e0022..a3953e50 100644 --- a/tests/test_path_mkdir.py +++ b/tests/test_path_mkdir.py @@ -5,20 +5,27 @@ from event import Event, EventType, Process -def test_mkdir_nested(monitored_dir, server): +@pytest.mark.parametrize("dirname", [ + pytest.param('level3', id='ASCII'), + pytest.param('café', id='French'), + pytest.param('файл', id='Cyrillic'), + pytest.param('日本語', id='Japanese'), +]) +def test_mkdir_nested(monitored_dir, server, dirname): """ Tests that creating nested directories tracks all inodes correctly. Args: monitored_dir: Temporary directory path for creating the test directory. server: The server instance to communicate with. + dirname: Final directory name to test (including UTF-8 variants). """ process = Process.from_proc() # Create nested directories level1 = os.path.join(monitored_dir, 'level1') level2 = os.path.join(level1, 'level2') - level3 = os.path.join(level2, 'level3') + level3 = os.path.join(level2, dirname) os.makedirs(level3, exist_ok=True) From 11eba2e14c57b97e922e0dbb47f7216e9b8e292f Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Mon, 6 Apr 2026 22:13:37 -0700 Subject: [PATCH 17/34] Fixed verifier issue. Not sending directory creation events --- fact-ebpf/src/bpf/main.c | 22 ++++++++++++---------- fact/src/event/mod.rs | 8 ++++++++ fact/src/host_scanner.rs | 9 +++++++++ tests/test_path_mkdir.py | 18 ++++++++++-------- 4 files changed, 39 insertions(+), 18 deletions(-) diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index a6521cfd..f480418c 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -264,18 +264,20 @@ int BPF_PROG(trace_path_mkdir, struct path* dir, struct dentry* dentry, umode_t // Stash mkdir context for security_d_instantiate __u64 pid_tgid = bpf_get_current_pid_tgid(); - - if (bpf_map_update_elem(&mkdir_context, &pid_tgid, NULL, BPF_ANY) != 0) { - bpf_printk("Failed to create mkdir context entry"); - m->path_mkdir.error++; - return 0; - } - struct mkdir_context_t* mkdir_ctx = bpf_map_lookup_elem(&mkdir_context, &pid_tgid); if (mkdir_ctx == NULL) { - bpf_printk("Failed to lookup mkdir context after creation"); - m->path_mkdir.error++; - return 0; + static const struct mkdir_context_t empty_ctx = {0}; + if (bpf_map_update_elem(&mkdir_context, &pid_tgid, &empty_ctx, BPF_NOEXIST) != 0) { + bpf_printk("Failed to create mkdir context entry"); + m->path_mkdir.error++; + return 0; + } + mkdir_ctx = bpf_map_lookup_elem(&mkdir_context, &pid_tgid); + if (mkdir_ctx == NULL) { + bpf_printk("Failed to lookup mkdir context after creation"); + m->path_mkdir.error++; + return 0; + } } long path_copy_len = bpf_probe_read_str(mkdir_ctx->path, PATH_MAX, path->path); diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index 70e96916..2d801c54 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -74,6 +74,8 @@ pub struct Event { hostname: &'static str, process: Process, file: FileData, + #[serde(skip)] + event_type: file_activity_type_t, } impl Event { @@ -123,6 +125,7 @@ impl Event { hostname, process, file, + event_type: file_activity_type_t::FILE_ACTIVITY_CREATION, }) } @@ -133,6 +136,10 @@ impl Event { pub fn is_unlink(&self) -> bool { matches!(self.file, FileData::Unlink(_)) } + + pub fn is_dir_creation(&self) -> bool { + self.event_type == file_activity_type_t::DIR_ACTIVITY_CREATION + } /// Unwrap the inner FileData and return the inode that triggered /// the event. @@ -263,6 +270,7 @@ impl TryFrom<&event_t> for Event { hostname: host_info::get_hostname(), process, file, + event_type: value.type_, }) } } diff --git a/fact/src/host_scanner.rs b/fact/src/host_scanner.rs index 19d5b719..b755332e 100644 --- a/fact/src/host_scanner.rs +++ b/fact/src/host_scanner.rs @@ -306,6 +306,15 @@ impl HostScanner { self.metrics.events.dropped(); warn!("Failed to send event: {e}"); } + + // Skip directory creation events - we track them internally but don't send to sensor + if !event.is_dir_creation() { + let event = Arc::new(event); + if let Err(e) = self.tx.send(event) { + self.metrics.events.dropped(); + warn!("Failed to send event: {e}"); + } + } }, _ = scan_trigger.notified() => self.scan()?, _ = self.paths.changed() => self.scan()?, diff --git a/tests/test_path_mkdir.py b/tests/test_path_mkdir.py index a3953e50..89fd3b72 100644 --- a/tests/test_path_mkdir.py +++ b/tests/test_path_mkdir.py @@ -34,13 +34,9 @@ def test_mkdir_nested(monitored_dir, server, dirname): with open(test_file, 'w') as f: f.write('nested content') + # Directory creation events are tracked internally but not sent to sensor + # Only the file creation event should be sent events = [ - Event(process=process, event_type=EventType.CREATION, - file=level1, host_path=level1), - Event(process=process, event_type=EventType.CREATION, - file=level2, host_path=level2), - Event(process=process, event_type=EventType.CREATION, - file=level3, host_path=level3), Event(process=process, event_type=EventType.CREATION, file=test_file, host_path=test_file), ] @@ -62,13 +58,19 @@ def test_mkdir_ignored(monitored_dir, ignored_dir, server): # Create directory in ignored path - should not be tracked ignored_subdir = os.path.join(ignored_dir, 'ignored_subdir') os.mkdir(ignored_subdir) + ignored_file = os.path.join(ignored_subdir, 'ignored.txt') + with open(ignored_file, 'w') as f: + f.write('ignored') # Create directory in monitored path - should be tracked monitored_subdir = os.path.join(monitored_dir, 'monitored_subdir') os.mkdir(monitored_subdir) + monitored_file = os.path.join(monitored_subdir, 'monitored.txt') + with open(monitored_file, 'w') as f: + f.write('monitored') - # Only the monitored directory should generate an event + # Only the monitored file should generate an event (directories are tracked internally) e = Event(process=process, event_type=EventType.CREATION, - file=monitored_subdir, host_path=monitored_subdir) + file=monitored_file, host_path=monitored_file) server.wait_events([e]) From 008b580b889be7758307906682ee4ae763ce3bf6 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Tue, 7 Apr 2026 07:40:47 -0700 Subject: [PATCH 18/34] make format --- fact/src/event/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index 2d801c54..a69d4dce 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -318,8 +318,8 @@ impl FileData { let inner = BaseFileData::new(filename, inode, parent_inode)?; let file = match event_type { file_activity_type_t::FILE_ACTIVITY_OPEN => FileData::Open(inner), - file_activity_type_t::FILE_ACTIVITY_CREATION | - file_activity_type_t::DIR_ACTIVITY_CREATION => FileData::Creation(inner), + file_activity_type_t::FILE_ACTIVITY_CREATION + | file_activity_type_t::DIR_ACTIVITY_CREATION => FileData::Creation(inner), file_activity_type_t::FILE_ACTIVITY_UNLINK => FileData::Unlink(inner), file_activity_type_t::FILE_ACTIVITY_CHMOD => { let data = ChmodFileData { From e86fa4eef6a74a8ae271c2df800e6caf6cace7cc Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Wed, 8 Apr 2026 08:57:44 -0700 Subject: [PATCH 19/34] Improved test Event constructor --- fact/src/event/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index a69d4dce..fad42d42 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -97,16 +97,16 @@ impl Event { inode: Default::default(), parent_inode: Default::default(), }; - let file = match data { - EventTestData::Creation => FileData::Creation(inner), - EventTestData::Unlink => FileData::Unlink(inner), + let (file, event_type) = match data { + EventTestData::Creation => (FileData::Creation(inner), file_activity_type_t::FILE_ACTIVITY_CREATION), + EventTestData::Unlink => (FileData::Unlink(inner), file_activity_type_t::FILE_ACTIVITY_UNLINK), EventTestData::Chmod(new_mode, old_mode) => { let data = ChmodFileData { inner, new_mode, old_mode, }; - FileData::Chmod(data) + (FileData::Chmod(data), file_activity_type_t::FILE_ACTIVITY_CHMOD) } EventTestData::Rename(old_path) => { let data = RenameFileData { @@ -116,7 +116,7 @@ impl Event { ..Default::default() }, }; - FileData::Rename(data) + (FileData::Rename(data), file_activity_type_t::FILE_ACTIVITY_RENAME) } }; @@ -125,7 +125,7 @@ impl Event { hostname, process, file, - event_type: file_activity_type_t::FILE_ACTIVITY_CREATION, + event_type, }) } From fd6bcbb4a3ccd61a28faab7371f4ad98ff92ed38 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Wed, 8 Apr 2026 09:17:06 -0700 Subject: [PATCH 20/34] Remove is_dir_creation --- fact/src/host_scanner.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fact/src/host_scanner.rs b/fact/src/host_scanner.rs index b755332e..b544cecb 100644 --- a/fact/src/host_scanner.rs +++ b/fact/src/host_scanner.rs @@ -308,7 +308,7 @@ impl HostScanner { } // Skip directory creation events - we track them internally but don't send to sensor - if !event.is_dir_creation() { + if event.event_type() != fact_ebpf::file_activity_type_t::DIR_ACTIVITY_CREATION { let event = Arc::new(event); if let Err(e) = self.tx.send(event) { self.metrics.events.dropped(); From 5f83efa1d701e5bd7d0528d85976e9a97dc28616 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Wed, 8 Apr 2026 09:45:02 -0700 Subject: [PATCH 21/34] Removed unneeded S_ISDIR --- fact-ebpf/src/bpf/main.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index f480418c..8a601e15 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -19,12 +19,6 @@ char _license[] SEC("license") = "Dual MIT/GPL"; #define FMODE_PWRITE ((fmode_t)(1 << 4)) #define FMODE_CREATED ((fmode_t)(1 << 20)) -// File type constants from linux/stat.h -// https://github.com/torvalds/linux/blob/5619b098e2fbf3a23bf13d91897056a1fe238c6d/include/uapi/linux/stat.h -#define S_IFMT 00170000 -#define S_IFDIR 0040000 -#define S_ISDIR(m) (((m) & S_IFMT) == S_IFDIR) - SEC("lsm/file_open") int BPF_PROG(trace_file_open, struct file* file) { struct metrics_t* m = get_metrics(); @@ -314,12 +308,6 @@ int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) { goto cleanup; } - umode_t mode = BPF_CORE_READ(inode, i_mode); - if (!S_ISDIR(mode)) { - m->d_instantiate.ignored++; - goto cleanup; - } - inode_key_t inode_key = inode_to_key(inode); if (inode_add(&inode_key) == 0) { From b97f03ec0147eda1762b2c69bbaeff2c5e4b192f Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Wed, 8 Apr 2026 09:51:21 -0700 Subject: [PATCH 22/34] make format --- fact/src/event/mod.rs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index fad42d42..41f2cb67 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -98,15 +98,24 @@ impl Event { parent_inode: Default::default(), }; let (file, event_type) = match data { - EventTestData::Creation => (FileData::Creation(inner), file_activity_type_t::FILE_ACTIVITY_CREATION), - EventTestData::Unlink => (FileData::Unlink(inner), file_activity_type_t::FILE_ACTIVITY_UNLINK), + EventTestData::Creation => ( + FileData::Creation(inner), + file_activity_type_t::FILE_ACTIVITY_CREATION, + ), + EventTestData::Unlink => ( + FileData::Unlink(inner), + file_activity_type_t::FILE_ACTIVITY_UNLINK, + ), EventTestData::Chmod(new_mode, old_mode) => { let data = ChmodFileData { inner, new_mode, old_mode, }; - (FileData::Chmod(data), file_activity_type_t::FILE_ACTIVITY_CHMOD) + ( + FileData::Chmod(data), + file_activity_type_t::FILE_ACTIVITY_CHMOD, + ) } EventTestData::Rename(old_path) => { let data = RenameFileData { @@ -116,7 +125,10 @@ impl Event { ..Default::default() }, }; - (FileData::Rename(data), file_activity_type_t::FILE_ACTIVITY_RENAME) + ( + FileData::Rename(data), + file_activity_type_t::FILE_ACTIVITY_RENAME, + ) } }; From 0874869bad414a06cb3cc4ed46e7e0c26079e899 Mon Sep 17 00:00:00 2001 From: Jouko Virtanen Date: Thu, 9 Apr 2026 10:11:48 -0700 Subject: [PATCH 23/34] Update tests/test_path_mkdir.py Co-authored-by: Mauro Ezequiel Moltrasio --- tests/test_path_mkdir.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/test_path_mkdir.py b/tests/test_path_mkdir.py index 89fd3b72..88685f8f 100644 --- a/tests/test_path_mkdir.py +++ b/tests/test_path_mkdir.py @@ -23,14 +23,11 @@ def test_mkdir_nested(monitored_dir, server, dirname): process = Process.from_proc() # Create nested directories - level1 = os.path.join(monitored_dir, 'level1') - level2 = os.path.join(level1, 'level2') - level3 = os.path.join(level2, dirname) - - os.makedirs(level3, exist_ok=True) + test_dir = os.path.join(monitored_dir, 'level1', 'level2', dirname) + os.makedirs(test_dir, exist_ok=True) # Create a file in the deepest directory - test_file = os.path.join(level3, 'deep_file.txt') + test_file = os.path.join(test_dir, 'deep_file.txt') with open(test_file, 'w') as f: f.write('nested content') From ba14b12255bf7a361bb3cf8acb3c74b7f18d4831 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Thu, 9 Apr 2026 10:51:35 -0700 Subject: [PATCH 24/34] Fixes after rebase --- fact/src/event/mod.rs | 4 ++++ fact/src/host_scanner.rs | 6 ------ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index 41f2cb67..3bb10fb0 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -145,6 +145,10 @@ impl Event { matches!(self.file, FileData::Creation(_)) } + pub(crate) fn event_type(&self) -> file_activity_type_t { + self.event_type + } + pub fn is_unlink(&self) -> bool { matches!(self.file, FileData::Unlink(_)) } diff --git a/fact/src/host_scanner.rs b/fact/src/host_scanner.rs index b544cecb..010d2331 100644 --- a/fact/src/host_scanner.rs +++ b/fact/src/host_scanner.rs @@ -301,12 +301,6 @@ impl HostScanner { self.handle_unlink_event(&event); } - let event = Arc::new(event); - if let Err(e) = self.tx.send(event) { - self.metrics.events.dropped(); - warn!("Failed to send event: {e}"); - } - // Skip directory creation events - we track them internally but don't send to sensor if event.event_type() != fact_ebpf::file_activity_type_t::DIR_ACTIVITY_CREATION { let event = Arc::new(event); From e4cd88235b9c8f61dbf4980df0af4e83cb16ef6b Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Thu, 9 Apr 2026 10:56:48 -0700 Subject: [PATCH 25/34] Moved the check for null inode up --- fact-ebpf/src/bpf/main.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index 8a601e15..d262f2d8 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -296,16 +296,17 @@ int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) { m->d_instantiate.total++; __u64 pid_tgid = bpf_get_current_pid_tgid(); - struct mkdir_context_t* mkdir_ctx = bpf_map_lookup_elem(&mkdir_context, &pid_tgid); - if (mkdir_ctx == NULL) { + if (inode == NULL) { m->d_instantiate.ignored++; - return 0; + goto cleanup; } - if (inode == NULL) { + struct mkdir_context_t* mkdir_ctx = bpf_map_lookup_elem(&mkdir_context, &pid_tgid); + + if (mkdir_ctx == NULL) { m->d_instantiate.ignored++; - goto cleanup; + return 0; } inode_key_t inode_key = inode_to_key(inode); From 34f8ff41f26876702730687013545d030bd99498 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Thu, 9 Apr 2026 13:42:41 -0700 Subject: [PATCH 26/34] Not adding a new event type field to Event struct --- fact/src/event/mod.rs | 56 +++++++++++++++++++--------------------- fact/src/host_scanner.rs | 4 +-- 2 files changed, 29 insertions(+), 31 deletions(-) diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index 3bb10fb0..9f87430b 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -7,6 +7,7 @@ use std::{ }; use globset::GlobSet; +use log::warn; use serde::Serialize; use fact_ebpf::{PATH_MAX, event_t, file_activity_type_t, inode_key_t}; @@ -74,8 +75,6 @@ pub struct Event { hostname: &'static str, process: Process, file: FileData, - #[serde(skip)] - event_type: file_activity_type_t, } impl Event { @@ -97,25 +96,16 @@ impl Event { inode: Default::default(), parent_inode: Default::default(), }; - let (file, event_type) = match data { - EventTestData::Creation => ( - FileData::Creation(inner), - file_activity_type_t::FILE_ACTIVITY_CREATION, - ), - EventTestData::Unlink => ( - FileData::Unlink(inner), - file_activity_type_t::FILE_ACTIVITY_UNLINK, - ), + let file = match data { + EventTestData::Creation => FileData::Creation(inner), + EventTestData::Unlink => FileData::Unlink(inner), EventTestData::Chmod(new_mode, old_mode) => { let data = ChmodFileData { inner, new_mode, old_mode, }; - ( - FileData::Chmod(data), - file_activity_type_t::FILE_ACTIVITY_CHMOD, - ) + FileData::Chmod(data) } EventTestData::Rename(old_path) => { let data = RenameFileData { @@ -125,10 +115,7 @@ impl Event { ..Default::default() }, }; - ( - FileData::Rename(data), - file_activity_type_t::FILE_ACTIVITY_RENAME, - ) + FileData::Rename(data) } }; @@ -137,25 +124,24 @@ impl Event { hostname, process, file, - event_type, }) } pub fn is_creation(&self) -> bool { + matches!(self.file, FileData::Creation(_) | FileData::MkDir(_)) + } + + pub fn is_file_creation(&self) -> bool { matches!(self.file, FileData::Creation(_)) } - pub(crate) fn event_type(&self) -> file_activity_type_t { - self.event_type + pub fn is_mkdir(&self) -> bool { + matches!(self.file, FileData::MkDir(_)) } pub fn is_unlink(&self) -> bool { matches!(self.file, FileData::Unlink(_)) } - - pub fn is_dir_creation(&self) -> bool { - self.event_type == file_activity_type_t::DIR_ACTIVITY_CREATION - } /// Unwrap the inner FileData and return the inode that triggered /// the event. @@ -166,6 +152,7 @@ impl Event { match &self.file { FileData::Open(data) => &data.inode, FileData::Creation(data) => &data.inode, + FileData::MkDir(data) => &data.inode, FileData::Unlink(data) => &data.inode, FileData::Chmod(data) => &data.inner.inode, FileData::Chown(data) => &data.inner.inode, @@ -178,6 +165,7 @@ impl Event { match &self.file { FileData::Open(data) => &data.parent_inode, FileData::Creation(data) => &data.parent_inode, + FileData::MkDir(data) => &data.parent_inode, FileData::Unlink(data) => &data.parent_inode, FileData::Chmod(data) => &data.inner.parent_inode, FileData::Chown(data) => &data.inner.parent_inode, @@ -199,6 +187,7 @@ impl Event { match &self.file { FileData::Open(data) => &data.filename, FileData::Creation(data) => &data.filename, + FileData::MkDir(data) => &data.filename, FileData::Unlink(data) => &data.filename, FileData::Chmod(data) => &data.inner.filename, FileData::Chown(data) => &data.inner.filename, @@ -217,6 +206,7 @@ impl Event { match &self.file { FileData::Open(data) => &data.host_file, FileData::Creation(data) => &data.host_file, + FileData::MkDir(data) => &data.host_file, FileData::Unlink(data) => &data.host_file, FileData::Chmod(data) => &data.inner.host_file, FileData::Chown(data) => &data.inner.host_file, @@ -232,6 +222,7 @@ impl Event { match &mut self.file { FileData::Open(data) => data.host_file = host_path, FileData::Creation(data) => data.host_file = host_path, + FileData::MkDir(data) => data.host_file = host_path, FileData::Unlink(data) => data.host_file = host_path, FileData::Chmod(data) => data.inner.host_file = host_path, FileData::Chown(data) => data.inner.host_file = host_path, @@ -286,7 +277,6 @@ impl TryFrom<&event_t> for Event { hostname: host_info::get_hostname(), process, file, - event_type: value.type_, }) } } @@ -317,6 +307,7 @@ impl PartialEq for Event { pub enum FileData { Open(BaseFileData), Creation(BaseFileData), + MkDir(BaseFileData), Unlink(BaseFileData), Chmod(ChmodFileData), Chown(ChownFileData), @@ -334,8 +325,8 @@ impl FileData { let inner = BaseFileData::new(filename, inode, parent_inode)?; let file = match event_type { file_activity_type_t::FILE_ACTIVITY_OPEN => FileData::Open(inner), - file_activity_type_t::FILE_ACTIVITY_CREATION - | file_activity_type_t::DIR_ACTIVITY_CREATION => FileData::Creation(inner), + file_activity_type_t::FILE_ACTIVITY_CREATION => FileData::Creation(inner), + file_activity_type_t::DIR_ACTIVITY_CREATION => FileData::MkDir(inner), file_activity_type_t::FILE_ACTIVITY_UNLINK => FileData::Unlink(inner), file_activity_type_t::FILE_ACTIVITY_CHMOD => { let data = ChmodFileData { @@ -384,6 +375,12 @@ impl From for fact_api::file_activity::File { let f_act = fact_api::FileCreation { activity }; fact_api::file_activity::File::Creation(f_act) } + FileData::MkDir(event) => { + warn!("MkDir event reached protobuf conversion - converting to Creation (filtering may have failed)"); + let activity = Some(fact_api::FileActivityBase::from(event)); + let f_act = fact_api::FileCreation { activity }; + fact_api::file_activity::File::Creation(f_act) + } FileData::Unlink(event) => { let activity = Some(fact_api::FileActivityBase::from(event)); let f_act = fact_api::FileUnlink { activity }; @@ -411,6 +408,7 @@ impl PartialEq for FileData { match (self, other) { (FileData::Open(this), FileData::Open(other)) => this == other, (FileData::Creation(this), FileData::Creation(other)) => this == other, + (FileData::MkDir(this), FileData::MkDir(other)) => this == other, (FileData::Unlink(this), FileData::Unlink(other)) => this == other, (FileData::Chmod(this), FileData::Chmod(other)) => this == other, (FileData::Rename(this), FileData::Rename(other)) => this == other, diff --git a/fact/src/host_scanner.rs b/fact/src/host_scanner.rs index 010d2331..f49c621f 100644 --- a/fact/src/host_scanner.rs +++ b/fact/src/host_scanner.rs @@ -280,7 +280,7 @@ impl HostScanner { }; self.metrics.events.added(); - // Handle file creation events by adding new inodes to the map + // Handle file and directory creation events by adding new inodes to the map if event.is_creation() && let Err(e) = self.handle_creation_event(&event) { warn!("Failed to handle creation event: {e}"); @@ -302,7 +302,7 @@ impl HostScanner { } // Skip directory creation events - we track them internally but don't send to sensor - if event.event_type() != fact_ebpf::file_activity_type_t::DIR_ACTIVITY_CREATION { + if !event.is_mkdir() { let event = Arc::new(event); if let Err(e) = self.tx.send(event) { self.metrics.events.dropped(); From db4645b1fea63d199342c14f59e2ff2fe94b80a2 Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Thu, 9 Apr 2026 13:43:08 -0700 Subject: [PATCH 27/34] make format --- fact/src/event/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index 9f87430b..1854d7af 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -376,7 +376,9 @@ impl From for fact_api::file_activity::File { fact_api::file_activity::File::Creation(f_act) } FileData::MkDir(event) => { - warn!("MkDir event reached protobuf conversion - converting to Creation (filtering may have failed)"); + warn!( + "MkDir event reached protobuf conversion - converting to Creation (filtering may have failed)" + ); let activity = Some(fact_api::FileActivityBase::from(event)); let f_act = fact_api::FileCreation { activity }; fact_api::file_activity::File::Creation(f_act) From c4ef0acb506e2f32bef8bcfee66dbd7581f5a75a Mon Sep 17 00:00:00 2001 From: JoukoVirtanen Date: Thu, 9 Apr 2026 20:54:02 -0700 Subject: [PATCH 28/34] Empty commit From 74e5543fdd7cc8c01f9a4f350695ef160edd0a78 Mon Sep 17 00:00:00 2001 From: Mauro Ezequiel Moltrasio Date: Wed, 1 Apr 2026 10:59:21 +0200 Subject: [PATCH 29/34] cleanup: refactor submit event functions A new submit_event_args_t type is added to group together common arguments used by all submit event functions. This reduces the number of arguments that need to be passed into these functions and make it harder to make mistakes in the ordering of the actual arguments. --- fact-ebpf/src/bpf/events.h | 135 +++++++++++++++++-------------------- fact-ebpf/src/bpf/main.c | 123 +++++++++++++++++---------------- tests/test_path_rename.py | 4 +- 3 files changed, 131 insertions(+), 131 deletions(-) diff --git a/fact-ebpf/src/bpf/events.h b/fact-ebpf/src/bpf/events.h index fe521450..e5aea55f 100644 --- a/fact-ebpf/src/bpf/events.h +++ b/fact-ebpf/src/bpf/events.h @@ -12,134 +12,125 @@ #include // clang-format on -__always_inline static void __submit_event(struct event_t* event, - struct metrics_by_hook_t* m, - file_activity_type_t event_type, - const char filename[PATH_MAX], - inode_key_t* inode, - inode_key_t* parent_inode, - bool use_bpf_d_path) { - event->type = event_type; +struct submit_event_args_t { + struct event_t* event; + struct metrics_by_hook_t* metrics; + const char* filename; + inode_key_t* inode; + inode_key_t parent_inode; + bool use_bpf_d_path; +}; + +__always_inline static void __submit_event(struct submit_event_args_t* args) { + struct event_t* event = args->event; event->timestamp = bpf_ktime_get_boot_ns(); - inode_copy_or_reset(&event->inode, inode); - inode_copy_or_reset(&event->parent_inode, parent_inode); - bpf_probe_read_str(event->filename, PATH_MAX, filename); + inode_copy_or_reset(&event->inode, args->inode); + inode_copy_or_reset(&event->parent_inode, &args->parent_inode); + bpf_probe_read_str(event->filename, PATH_MAX, args->filename); struct helper_t* helper = get_helper(); if (helper == NULL) { goto error; } - int64_t err = process_fill(&event->process, use_bpf_d_path); + int64_t err = process_fill(&event->process, args->use_bpf_d_path); if (err) { bpf_printk("Failed to fill process information: %d", err); goto error; } - m->added++; + args->metrics->added++; bpf_ringbuf_submit(event, 0); return; error: - m->error++; + args->metrics->error++; bpf_ringbuf_discard(event, 0); } -__always_inline static void submit_open_event(struct metrics_by_hook_t* m, - file_activity_type_t event_type, - const char filename[PATH_MAX], - inode_key_t* inode, - inode_key_t* parent_inode) { - struct event_t* event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); - if (event == NULL) { - m->ringbuffer_full++; +__always_inline static void submit_open_event(struct submit_event_args_t* args, + file_activity_type_t event_type) { + args->event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); + if (args->event == NULL) { + args->metrics->ringbuffer_full++; return; } + args->event->type = event_type; - __submit_event(event, m, event_type, filename, inode, parent_inode, true); + __submit_event(args); } -__always_inline static void submit_unlink_event(struct metrics_by_hook_t* m, - const char filename[PATH_MAX], - inode_key_t* inode, - inode_key_t* parent_inode) { - struct event_t* event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); - if (event == NULL) { - m->ringbuffer_full++; +__always_inline static void submit_unlink_event(struct submit_event_args_t* args) { + args->event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); + if (args->event == NULL) { + args->metrics->ringbuffer_full++; return; } + args->event->type = FILE_ACTIVITY_UNLINK; - __submit_event(event, m, FILE_ACTIVITY_UNLINK, filename, inode, parent_inode, path_hooks_support_bpf_d_path); + __submit_event(args); } -__always_inline static void submit_mode_event(struct metrics_by_hook_t* m, - const char filename[PATH_MAX], - inode_key_t* inode, - inode_key_t* parent_inode, +__always_inline static void submit_mode_event(struct submit_event_args_t* args, umode_t mode, umode_t old_mode) { - struct event_t* event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); - if (event == NULL) { - m->ringbuffer_full++; + args->event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); + if (args->event == NULL) { + args->metrics->ringbuffer_full++; return; } - event->chmod.new = mode; - event->chmod.old = old_mode; + args->event->type = FILE_ACTIVITY_CHMOD; + args->event->chmod.new = mode; + args->event->chmod.old = old_mode; - __submit_event(event, m, FILE_ACTIVITY_CHMOD, filename, inode, parent_inode, path_hooks_support_bpf_d_path); + __submit_event(args); } -__always_inline static void submit_ownership_event(struct metrics_by_hook_t* m, - const char filename[PATH_MAX], - inode_key_t* inode, - inode_key_t* parent_inode, +__always_inline static void submit_ownership_event(struct submit_event_args_t* args, unsigned long long uid, unsigned long long gid, unsigned long long old_uid, unsigned long long old_gid) { - struct event_t* event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); - if (event == NULL) { - m->ringbuffer_full++; + args->event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); + if (args->event == NULL) { + args->metrics->ringbuffer_full++; return; } - event->chown.new.uid = uid; - event->chown.new.gid = gid; - event->chown.old.uid = old_uid; - event->chown.old.gid = old_gid; + args->event->type = FILE_ACTIVITY_CHOWN; + args->event->chown.new.uid = uid; + args->event->chown.new.gid = gid; + args->event->chown.old.uid = old_uid; + args->event->chown.old.gid = old_gid; - __submit_event(event, m, FILE_ACTIVITY_CHOWN, filename, inode, parent_inode, path_hooks_support_bpf_d_path); + __submit_event(args); } -__always_inline static void submit_rename_event(struct metrics_by_hook_t* m, - const char new_filename[PATH_MAX], +__always_inline static void submit_rename_event(struct submit_event_args_t* args, const char old_filename[PATH_MAX], - inode_key_t* new_inode, - inode_key_t* old_inode, - inode_key_t* new_parent_inode) { - struct event_t* event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); - if (event == NULL) { - m->ringbuffer_full++; + inode_key_t* old_inode) { + args->event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); + if (args->event == NULL) { + args->metrics->ringbuffer_full++; return; } - bpf_probe_read_str(event->rename.old_filename, PATH_MAX, old_filename); - inode_copy_or_reset(&event->rename.old_inode, old_inode); + args->event->type = FILE_ACTIVITY_RENAME; + bpf_probe_read_str(args->event->rename.old_filename, PATH_MAX, old_filename); + inode_copy_or_reset(&args->event->rename.old_inode, old_inode); - __submit_event(event, m, FILE_ACTIVITY_RENAME, new_filename, new_inode, new_parent_inode, path_hooks_support_bpf_d_path); + __submit_event(args); } -__always_inline static void submit_mkdir_event(struct metrics_by_hook_t* m, - const char filename[PATH_MAX], - inode_key_t* inode, - inode_key_t* parent_inode) { - struct event_t* event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); - if (event == NULL) { - m->ringbuffer_full++; +__always_inline static void submit_mkdir_event(struct submit_event_args_t* args) { + args->event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); + if (args->event == NULL) { + args->metrics->ringbuffer_full++; return; } + args->event->type = DIR_ACTIVITY_CREATION; // d_instantiate doesn't support bpf_d_path, so we use false and rely on the stashed path from path_mkdir - __submit_event(event, m, DIR_ACTIVITY_CREATION, filename, inode, parent_inode, false); + __submit_event(args); } diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index d262f2d8..1cf315d8 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -25,8 +25,12 @@ int BPF_PROG(trace_file_open, struct file* file) { if (m == NULL) { return 0; } + struct submit_event_args_t args = { + .metrics = &m->file_open, + .use_bpf_d_path = true, + }; - m->file_open.total++; + args.metrics->total++; file_activity_type_t event_type = FILE_ACTIVITY_INIT; if ((file->f_mode & FMODE_CREATED) != 0) { @@ -43,15 +47,16 @@ int BPF_PROG(trace_file_open, struct file* file) { m->file_open.error++; return 0; } + args.filename = path->path; inode_key_t inode_key = inode_to_key(file->f_inode); - inode_key_t* inode_to_submit = &inode_key; + args.inode = &inode_key; struct dentry* parent_dentry = BPF_CORE_READ(file, f_path.dentry, d_parent); struct inode* parent_inode_ptr = parent_dentry ? BPF_CORE_READ(parent_dentry, d_inode) : NULL; - inode_key_t parent_key = inode_to_key(parent_inode_ptr); + args.parent_inode = inode_to_key(parent_inode_ptr); - inode_monitored_t status = is_monitored(inode_key, path, &parent_key, &inode_to_submit); + inode_monitored_t status = is_monitored(inode_key, path, &args.parent_inode, &args.inode); if (status == PARENT_MONITORED && event_type == FILE_ACTIVITY_CREATION) { inode_add(&inode_key); @@ -61,7 +66,7 @@ int BPF_PROG(trace_file_open, struct file* file) { goto ignored; } - submit_open_event(&m->file_open, event_type, path->path, inode_to_submit, &parent_key); + submit_open_event(&args, event_type); return 0; @@ -76,8 +81,12 @@ int BPF_PROG(trace_path_unlink, struct path* dir, struct dentry* dentry) { if (m == NULL) { return 0; } + struct submit_event_args_t args = { + .metrics = &m->path_unlink, + .use_bpf_d_path = path_hooks_support_bpf_d_path, + }; - m->path_unlink.total++; + args.metrics->total++; struct bound_path_t* path = path_read_append_d_entry(dir, dentry); if (path == NULL) { @@ -85,11 +94,12 @@ int BPF_PROG(trace_path_unlink, struct path* dir, struct dentry* dentry) { m->path_unlink.error++; return 0; } + args.filename = path->path; inode_key_t inode_key = inode_to_key(dentry->d_inode); - inode_key_t* inode_to_submit = &inode_key; + args.inode = &inode_key; - if (is_monitored(inode_key, path, NULL, &inode_to_submit) == NOT_MONITORED) { + if (is_monitored(inode_key, path, NULL, &args.inode) == NOT_MONITORED) { m->path_unlink.ignored++; return 0; } @@ -97,10 +107,7 @@ int BPF_PROG(trace_path_unlink, struct path* dir, struct dentry* dentry) { // We only support files with one link for now inode_remove(&inode_key); - submit_unlink_event(&m->path_unlink, - path->path, - inode_to_submit, - NULL); + submit_unlink_event(&args); return 0; } @@ -110,31 +117,31 @@ int BPF_PROG(trace_path_chmod, struct path* path, umode_t mode) { if (m == NULL) { return 0; } + struct submit_event_args_t args = { + .metrics = &m->path_chmod, + .use_bpf_d_path = path_hooks_support_bpf_d_path, + }; - m->path_chmod.total++; + args.metrics->total++; struct bound_path_t* bound_path = path_read(path); if (bound_path == NULL) { bpf_printk("Failed to read path"); - m->path_chmod.error++; + args.metrics->error++; return 0; } + args.filename = bound_path->path; inode_key_t inode_key = inode_to_key(path->dentry->d_inode); - inode_key_t* inode_to_submit = &inode_key; + args.inode = &inode_key; - if (is_monitored(inode_key, bound_path, NULL, &inode_to_submit) == NOT_MONITORED) { - m->path_chmod.ignored++; + if (is_monitored(inode_key, bound_path, NULL, &args.inode) == NOT_MONITORED) { + args.metrics->ignored++; return 0; } umode_t old_mode = BPF_CORE_READ(path, dentry, d_inode, i_mode); - submit_mode_event(&m->path_chmod, - bound_path->path, - inode_to_submit, - NULL, - mode, - old_mode); + submit_mode_event(&args, mode, old_mode); return 0; } @@ -148,21 +155,26 @@ int BPF_PROG(trace_path_chown, struct path* path, unsigned long long uid, unsign if (m == NULL) { return 0; } + struct submit_event_args_t args = { + .metrics = &m->path_chown, + .use_bpf_d_path = path_hooks_support_bpf_d_path, + }; - m->path_chown.total++; + args.metrics->total++; struct bound_path_t* bound_path = path_read(path); if (bound_path == NULL) { bpf_printk("Failed to read path"); - m->path_chown.error++; + args.metrics->error++; return 0; } + args.filename = bound_path->path; inode_key_t inode_key = inode_to_key(path->dentry->d_inode); - inode_key_t* inode_to_submit = &inode_key; + args.inode = &inode_key; - if (is_monitored(inode_key, bound_path, NULL, &inode_to_submit) == NOT_MONITORED) { - m->path_chown.ignored++; + if (is_monitored(inode_key, bound_path, NULL, &args.inode) == NOT_MONITORED) { + args.metrics->ignored++; return 0; } @@ -170,14 +182,7 @@ int BPF_PROG(trace_path_chown, struct path* path, unsigned long long uid, unsign unsigned long long old_uid = BPF_CORE_READ(d, d_inode, i_uid.val); unsigned long long old_gid = BPF_CORE_READ(d, d_inode, i_gid.val); - submit_ownership_event(&m->path_chown, - bound_path->path, - inode_to_submit, - NULL, - uid, - gid, - old_uid, - old_gid); + submit_ownership_event(&args, uid, gid, old_uid, old_gid); return 0; } @@ -190,14 +195,19 @@ int BPF_PROG(trace_path_rename, struct path* old_dir, if (m == NULL) { return 0; } + struct submit_event_args_t args = { + .metrics = &m->path_rename, + .use_bpf_d_path = path_hooks_support_bpf_d_path, + }; - m->path_rename.total++; + args.metrics->total++; struct bound_path_t* new_path = path_read_append_d_entry(new_dir, new_dentry); if (new_path == NULL) { bpf_printk("Failed to read path"); goto error; } + args.filename = new_path->path; struct bound_path_t* old_path = path_read_alt_append_d_entry(old_dir, old_dentry); if (old_path == NULL) { @@ -209,26 +219,21 @@ int BPF_PROG(trace_path_rename, struct path* old_dir, inode_key_t new_inode = inode_to_key(new_dentry->d_inode); inode_key_t* old_inode_submit = &old_inode; - inode_key_t* new_inode_submit = &new_inode; + args.inode = &new_inode; inode_monitored_t old_monitored = is_monitored(old_inode, old_path, NULL, &old_inode_submit); - inode_monitored_t new_monitored = is_monitored(new_inode, new_path, NULL, &new_inode_submit); + inode_monitored_t new_monitored = is_monitored(new_inode, new_path, NULL, &args.inode); if (old_monitored == NOT_MONITORED && new_monitored == NOT_MONITORED) { - m->path_rename.ignored++; + args.metrics->ignored++; return 0; } - submit_rename_event(&m->path_rename, - new_path->path, - old_path->path, - old_inode_submit, - new_inode_submit, - NULL); + submit_rename_event(&args, old_path->path, old_inode_submit); return 0; error: - m->path_rename.error++; + args.metrics->error++; return 0; } @@ -292,35 +297,39 @@ int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) { if (m == NULL) { return 0; } + struct submit_event_args_t args = { + .metrics = &m->d_instantiate, + .use_bpf_d_path = false, + }; - m->d_instantiate.total++; + args.metrics->total++; __u64 pid_tgid = bpf_get_current_pid_tgid(); if (inode == NULL) { - m->d_instantiate.ignored++; + args.metrics->ignored++; goto cleanup; } struct mkdir_context_t* mkdir_ctx = bpf_map_lookup_elem(&mkdir_context, &pid_tgid); if (mkdir_ctx == NULL) { - m->d_instantiate.ignored++; + args.metrics->ignored++; return 0; } + args.filename = mkdir_ctx->path; + args.parent_inode = mkdir_ctx->parent_inode; inode_key_t inode_key = inode_to_key(inode); + args.inode = &inode_key; - if (inode_add(&inode_key) == 0) { - m->d_instantiate.added++; + if (inode_add(args.inode) == 0) { + args.metrics->added++; } else { - m->d_instantiate.error++; + args.metrics->error++; } - submit_mkdir_event(&m->d_instantiate, - mkdir_ctx->path, - &inode_key, - &mkdir_ctx->parent_inode); + submit_mkdir_event(&args); cleanup: bpf_map_delete_elem(&mkdir_context, &pid_tgid); diff --git a/tests/test_path_rename.py b/tests/test_path_rename.py index a3600eff..98c3447a 100644 --- a/tests/test_path_rename.py +++ b/tests/test_path_rename.py @@ -48,9 +48,9 @@ def test_rename(monitored_dir, server, filename): Event(process=Process.from_proc(), event_type=EventType.CREATION, file=old_fut, host_path=old_fut), Event(process=Process.from_proc(), event_type=EventType.RENAME, - file=fut, host_path=old_fut, old_file=old_fut, old_host_path=''), + file=fut, host_path='', old_file=old_fut, old_host_path=old_fut), Event(process=Process.from_proc(), event_type=EventType.RENAME, - file=old_fut, host_path=old_fut, old_file=fut, old_host_path=''), + file=old_fut, host_path='', old_file=fut, old_host_path=old_fut), ] server.wait_events(events) From 13e58f38b9affc061048b6611d0e24fbd2905014 Mon Sep 17 00:00:00 2001 From: Mauro Ezequiel Moltrasio Date: Mon, 6 Apr 2026 16:22:22 +0200 Subject: [PATCH 30/34] chore: simplify inode handling kernel side --- fact-ebpf/src/bpf/events.h | 8 ++++---- fact-ebpf/src/bpf/file.h | 6 +++--- fact-ebpf/src/bpf/inode.h | 16 +++++++-------- fact-ebpf/src/bpf/main.c | 41 ++++++++++++++++---------------------- 4 files changed, 32 insertions(+), 39 deletions(-) diff --git a/fact-ebpf/src/bpf/events.h b/fact-ebpf/src/bpf/events.h index e5aea55f..c4cb4e8c 100644 --- a/fact-ebpf/src/bpf/events.h +++ b/fact-ebpf/src/bpf/events.h @@ -16,7 +16,7 @@ struct submit_event_args_t { struct event_t* event; struct metrics_by_hook_t* metrics; const char* filename; - inode_key_t* inode; + inode_key_t inode; inode_key_t parent_inode; bool use_bpf_d_path; }; @@ -24,8 +24,8 @@ struct submit_event_args_t { __always_inline static void __submit_event(struct submit_event_args_t* args) { struct event_t* event = args->event; event->timestamp = bpf_ktime_get_boot_ns(); - inode_copy_or_reset(&event->inode, args->inode); - inode_copy_or_reset(&event->parent_inode, &args->parent_inode); + inode_copy(&event->inode, &args->inode); + inode_copy(&event->parent_inode, &args->parent_inode); bpf_probe_read_str(event->filename, PATH_MAX, args->filename); struct helper_t* helper = get_helper(); @@ -118,7 +118,7 @@ __always_inline static void submit_rename_event(struct submit_event_args_t* args args->event->type = FILE_ACTIVITY_RENAME; bpf_probe_read_str(args->event->rename.old_filename, PATH_MAX, old_filename); - inode_copy_or_reset(&args->event->rename.old_inode, old_inode); + inode_copy(&args->event->rename.old_inode, old_inode); __submit_event(args); } diff --git a/fact-ebpf/src/bpf/file.h b/fact-ebpf/src/bpf/file.h index eac1b429..efdccfc5 100644 --- a/fact-ebpf/src/bpf/file.h +++ b/fact-ebpf/src/bpf/file.h @@ -27,8 +27,8 @@ __always_inline static bool path_is_monitored(struct bound_path_t* path) { return res; } -__always_inline static inode_monitored_t is_monitored(inode_key_t inode, struct bound_path_t* path, const inode_key_t* parent, inode_key_t** submit) { - const inode_value_t* volatile inode_value = inode_get(&inode); +__always_inline static inode_monitored_t is_monitored(inode_key_t* inode, struct bound_path_t* path, const inode_key_t* parent) { + const inode_value_t* volatile inode_value = inode_get(inode); const inode_value_t* volatile parent_value = inode_get(parent); inode_monitored_t status = inode_is_monitored(inode_value, parent_value); @@ -36,7 +36,7 @@ __always_inline static inode_monitored_t is_monitored(inode_key_t inode, struct return status; } - *submit = NULL; + inode_reset(inode); if (path_is_monitored(path)) { return MONITORED; } diff --git a/fact-ebpf/src/bpf/inode.h b/fact-ebpf/src/bpf/inode.h index 481313e3..6b213c3b 100644 --- a/fact-ebpf/src/bpf/inode.h +++ b/fact-ebpf/src/bpf/inode.h @@ -80,6 +80,11 @@ __always_inline static long inode_remove(struct inode_key_t* inode) { return bpf_map_delete_elem(&inode_map, inode); } +__always_inline static void inode_reset(struct inode_key_t* inode) { + inode->inode = 0; + inode->dev = 0; +} + typedef enum inode_monitored_t { NOT_MONITORED = 0, MONITORED, @@ -99,16 +104,11 @@ __always_inline static inode_monitored_t inode_is_monitored(const inode_value_t* return NOT_MONITORED; } -__always_inline static void inode_copy_or_reset(inode_key_t* dst, const inode_key_t* src) { +__always_inline static void inode_copy(inode_key_t* dst, const inode_key_t* src) { if (dst == NULL) { return; } - if (src != NULL) { - dst->inode = src->inode; - dst->dev = src->dev; - } else { - dst->inode = 0; - dst->dev = 0; - } + dst->inode = src->inode; + dst->dev = src->dev; } diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index 1cf315d8..5ccf4e47 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -49,17 +49,16 @@ int BPF_PROG(trace_file_open, struct file* file) { } args.filename = path->path; - inode_key_t inode_key = inode_to_key(file->f_inode); - args.inode = &inode_key; + args.inode = inode_to_key(file->f_inode); struct dentry* parent_dentry = BPF_CORE_READ(file, f_path.dentry, d_parent); struct inode* parent_inode_ptr = parent_dentry ? BPF_CORE_READ(parent_dentry, d_inode) : NULL; args.parent_inode = inode_to_key(parent_inode_ptr); - inode_monitored_t status = is_monitored(inode_key, path, &args.parent_inode, &args.inode); + inode_monitored_t status = is_monitored(&args.inode, path, &args.parent_inode); if (status == PARENT_MONITORED && event_type == FILE_ACTIVITY_CREATION) { - inode_add(&inode_key); + inode_add(&args.inode); } if (status == NOT_MONITORED) { @@ -96,16 +95,15 @@ int BPF_PROG(trace_path_unlink, struct path* dir, struct dentry* dentry) { } args.filename = path->path; - inode_key_t inode_key = inode_to_key(dentry->d_inode); - args.inode = &inode_key; + args.inode = inode_to_key(dentry->d_inode); - if (is_monitored(inode_key, path, NULL, &args.inode) == NOT_MONITORED) { + if (is_monitored(&args.inode, path, NULL) == NOT_MONITORED) { m->path_unlink.ignored++; return 0; } // We only support files with one link for now - inode_remove(&inode_key); + inode_remove(&args.inode); submit_unlink_event(&args); return 0; @@ -132,10 +130,9 @@ int BPF_PROG(trace_path_chmod, struct path* path, umode_t mode) { } args.filename = bound_path->path; - inode_key_t inode_key = inode_to_key(path->dentry->d_inode); - args.inode = &inode_key; + args.inode = inode_to_key(path->dentry->d_inode); - if (is_monitored(inode_key, bound_path, NULL, &args.inode) == NOT_MONITORED) { + if (is_monitored(&args.inode, bound_path, NULL) == NOT_MONITORED) { args.metrics->ignored++; return 0; } @@ -170,10 +167,9 @@ int BPF_PROG(trace_path_chown, struct path* path, unsigned long long uid, unsign } args.filename = bound_path->path; - inode_key_t inode_key = inode_to_key(path->dentry->d_inode); - args.inode = &inode_key; + args.inode = inode_to_key(path->dentry->d_inode); - if (is_monitored(inode_key, bound_path, NULL, &args.inode) == NOT_MONITORED) { + if (is_monitored(&args.inode, bound_path, NULL) == NOT_MONITORED) { args.metrics->ignored++; return 0; } @@ -215,21 +211,19 @@ int BPF_PROG(trace_path_rename, struct path* old_dir, goto error; } - inode_key_t old_inode = inode_to_key(old_dentry->d_inode); - inode_key_t new_inode = inode_to_key(new_dentry->d_inode); + args.inode = inode_to_key(new_dentry->d_inode); - inode_key_t* old_inode_submit = &old_inode; - args.inode = &new_inode; + inode_key_t old_inode = inode_to_key(old_dentry->d_inode); - inode_monitored_t old_monitored = is_monitored(old_inode, old_path, NULL, &old_inode_submit); - inode_monitored_t new_monitored = is_monitored(new_inode, new_path, NULL, &args.inode); + inode_monitored_t old_monitored = is_monitored(&old_inode, old_path, NULL); + inode_monitored_t new_monitored = is_monitored(&args.inode, new_path, NULL); if (old_monitored == NOT_MONITORED && new_monitored == NOT_MONITORED) { args.metrics->ignored++; return 0; } - submit_rename_event(&args, old_path->path, old_inode_submit); + submit_rename_event(&args, old_path->path, &old_inode); return 0; error: @@ -320,10 +314,9 @@ int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) { args.filename = mkdir_ctx->path; args.parent_inode = mkdir_ctx->parent_inode; - inode_key_t inode_key = inode_to_key(inode); - args.inode = &inode_key; + args.inode = inode_to_key(inode); - if (inode_add(args.inode) == 0) { + if (inode_add(&args.inode) == 0) { args.metrics->added++; } else { args.metrics->error++; From 96c8f45d838f455ee2f52b78d507f5998bebf124 Mon Sep 17 00:00:00 2001 From: Mauro Ezequiel Moltrasio Date: Fri, 10 Apr 2026 12:40:45 +0200 Subject: [PATCH 31/34] cleanup: move ringbuf event reservation to its own function --- fact-ebpf/src/bpf/events.h | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/fact-ebpf/src/bpf/events.h b/fact-ebpf/src/bpf/events.h index c4cb4e8c..18ad38ae 100644 --- a/fact-ebpf/src/bpf/events.h +++ b/fact-ebpf/src/bpf/events.h @@ -21,6 +21,15 @@ struct submit_event_args_t { bool use_bpf_d_path; }; +__always_inline static bool reserve_event(struct submit_event_args_t* args) { + args->event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); + if (args->event == NULL) { + args->metrics->ringbuffer_full++; + return false; + } + return true; +} + __always_inline static void __submit_event(struct submit_event_args_t* args) { struct event_t* event = args->event; event->timestamp = bpf_ktime_get_boot_ns(); @@ -50,9 +59,7 @@ __always_inline static void __submit_event(struct submit_event_args_t* args) { __always_inline static void submit_open_event(struct submit_event_args_t* args, file_activity_type_t event_type) { - args->event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); - if (args->event == NULL) { - args->metrics->ringbuffer_full++; + if (!reserve_event(args)) { return; } args->event->type = event_type; @@ -61,9 +68,7 @@ __always_inline static void submit_open_event(struct submit_event_args_t* args, } __always_inline static void submit_unlink_event(struct submit_event_args_t* args) { - args->event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); - if (args->event == NULL) { - args->metrics->ringbuffer_full++; + if (!reserve_event(args)) { return; } args->event->type = FILE_ACTIVITY_UNLINK; @@ -74,9 +79,7 @@ __always_inline static void submit_unlink_event(struct submit_event_args_t* args __always_inline static void submit_mode_event(struct submit_event_args_t* args, umode_t mode, umode_t old_mode) { - args->event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); - if (args->event == NULL) { - args->metrics->ringbuffer_full++; + if (!reserve_event(args)) { return; } @@ -92,9 +95,7 @@ __always_inline static void submit_ownership_event(struct submit_event_args_t* a unsigned long long gid, unsigned long long old_uid, unsigned long long old_gid) { - args->event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); - if (args->event == NULL) { - args->metrics->ringbuffer_full++; + if (!reserve_event(args)) { return; } @@ -110,9 +111,7 @@ __always_inline static void submit_ownership_event(struct submit_event_args_t* a __always_inline static void submit_rename_event(struct submit_event_args_t* args, const char old_filename[PATH_MAX], inode_key_t* old_inode) { - args->event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); - if (args->event == NULL) { - args->metrics->ringbuffer_full++; + if (!reserve_event(args)) { return; } @@ -124,9 +123,7 @@ __always_inline static void submit_rename_event(struct submit_event_args_t* args } __always_inline static void submit_mkdir_event(struct submit_event_args_t* args) { - args->event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0); - if (args->event == NULL) { - args->metrics->ringbuffer_full++; + if (!reserve_event(args)) { return; } args->event->type = DIR_ACTIVITY_CREATION; From d6fe74c77ccd0c8480fd5176c169ef392e76e108 Mon Sep 17 00:00:00 2001 From: Mauro Ezequiel Moltrasio Date: Fri, 10 Apr 2026 16:13:52 +0200 Subject: [PATCH 32/34] fix: prevent verifier error on bpf_d_path not being removed --- fact-ebpf/src/bpf/events.h | 18 +++++++++--------- fact-ebpf/src/bpf/main.c | 30 ++++++------------------------ 2 files changed, 15 insertions(+), 33 deletions(-) diff --git a/fact-ebpf/src/bpf/events.h b/fact-ebpf/src/bpf/events.h index 18ad38ae..276a1d97 100644 --- a/fact-ebpf/src/bpf/events.h +++ b/fact-ebpf/src/bpf/events.h @@ -18,7 +18,6 @@ struct submit_event_args_t { const char* filename; inode_key_t inode; inode_key_t parent_inode; - bool use_bpf_d_path; }; __always_inline static bool reserve_event(struct submit_event_args_t* args) { @@ -30,7 +29,8 @@ __always_inline static bool reserve_event(struct submit_event_args_t* args) { return true; } -__always_inline static void __submit_event(struct submit_event_args_t* args) { +__always_inline static void __submit_event(struct submit_event_args_t* args, + bool use_bpf_d_path) { struct event_t* event = args->event; event->timestamp = bpf_ktime_get_boot_ns(); inode_copy(&event->inode, &args->inode); @@ -42,7 +42,7 @@ __always_inline static void __submit_event(struct submit_event_args_t* args) { goto error; } - int64_t err = process_fill(&event->process, args->use_bpf_d_path); + int64_t err = process_fill(&event->process, use_bpf_d_path); if (err) { bpf_printk("Failed to fill process information: %d", err); goto error; @@ -64,7 +64,7 @@ __always_inline static void submit_open_event(struct submit_event_args_t* args, } args->event->type = event_type; - __submit_event(args); + __submit_event(args, true); } __always_inline static void submit_unlink_event(struct submit_event_args_t* args) { @@ -73,7 +73,7 @@ __always_inline static void submit_unlink_event(struct submit_event_args_t* args } args->event->type = FILE_ACTIVITY_UNLINK; - __submit_event(args); + __submit_event(args, path_hooks_support_bpf_d_path); } __always_inline static void submit_mode_event(struct submit_event_args_t* args, @@ -87,7 +87,7 @@ __always_inline static void submit_mode_event(struct submit_event_args_t* args, args->event->chmod.new = mode; args->event->chmod.old = old_mode; - __submit_event(args); + __submit_event(args, path_hooks_support_bpf_d_path); } __always_inline static void submit_ownership_event(struct submit_event_args_t* args, @@ -105,7 +105,7 @@ __always_inline static void submit_ownership_event(struct submit_event_args_t* a args->event->chown.old.uid = old_uid; args->event->chown.old.gid = old_gid; - __submit_event(args); + __submit_event(args, path_hooks_support_bpf_d_path); } __always_inline static void submit_rename_event(struct submit_event_args_t* args, @@ -119,7 +119,7 @@ __always_inline static void submit_rename_event(struct submit_event_args_t* args bpf_probe_read_str(args->event->rename.old_filename, PATH_MAX, old_filename); inode_copy(&args->event->rename.old_inode, old_inode); - __submit_event(args); + __submit_event(args, path_hooks_support_bpf_d_path); } __always_inline static void submit_mkdir_event(struct submit_event_args_t* args) { @@ -129,5 +129,5 @@ __always_inline static void submit_mkdir_event(struct submit_event_args_t* args) args->event->type = DIR_ACTIVITY_CREATION; // d_instantiate doesn't support bpf_d_path, so we use false and rely on the stashed path from path_mkdir - __submit_event(args); + __submit_event(args, false); } diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index 5ccf4e47..c6effe8d 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -25,10 +25,7 @@ int BPF_PROG(trace_file_open, struct file* file) { if (m == NULL) { return 0; } - struct submit_event_args_t args = { - .metrics = &m->file_open, - .use_bpf_d_path = true, - }; + struct submit_event_args_t args = {.metrics = &m->file_open}; args.metrics->total++; @@ -80,10 +77,7 @@ int BPF_PROG(trace_path_unlink, struct path* dir, struct dentry* dentry) { if (m == NULL) { return 0; } - struct submit_event_args_t args = { - .metrics = &m->path_unlink, - .use_bpf_d_path = path_hooks_support_bpf_d_path, - }; + struct submit_event_args_t args = {.metrics = &m->path_unlink}; args.metrics->total++; @@ -115,10 +109,7 @@ int BPF_PROG(trace_path_chmod, struct path* path, umode_t mode) { if (m == NULL) { return 0; } - struct submit_event_args_t args = { - .metrics = &m->path_chmod, - .use_bpf_d_path = path_hooks_support_bpf_d_path, - }; + struct submit_event_args_t args = {.metrics = &m->path_chmod}; args.metrics->total++; @@ -152,10 +143,7 @@ int BPF_PROG(trace_path_chown, struct path* path, unsigned long long uid, unsign if (m == NULL) { return 0; } - struct submit_event_args_t args = { - .metrics = &m->path_chown, - .use_bpf_d_path = path_hooks_support_bpf_d_path, - }; + struct submit_event_args_t args = {.metrics = &m->path_chown}; args.metrics->total++; @@ -191,10 +179,7 @@ int BPF_PROG(trace_path_rename, struct path* old_dir, if (m == NULL) { return 0; } - struct submit_event_args_t args = { - .metrics = &m->path_rename, - .use_bpf_d_path = path_hooks_support_bpf_d_path, - }; + struct submit_event_args_t args = {.metrics = &m->path_rename}; args.metrics->total++; @@ -291,10 +276,7 @@ int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) { if (m == NULL) { return 0; } - struct submit_event_args_t args = { - .metrics = &m->d_instantiate, - .use_bpf_d_path = false, - }; + struct submit_event_args_t args = {.metrics = &m->d_instantiate}; args.metrics->total++; From be9b5397f6b4a4d405095903ebab0c7adb18a5e6 Mon Sep 17 00:00:00 2001 From: Mauro Ezequiel Moltrasio Date: Wed, 8 Apr 2026 17:34:19 +0200 Subject: [PATCH 33/34] ROX-33216: implement inode tracking for path_rename This is a first attempt at adding inode tracking to the path_rename LSM hook. A number of changes were needed in order to cover all the possible edge cases in this operation. The most important change is that inodes are now always sent to userspace alongside with a value that indicates how the event is triggered (by matching an inode or a path), this was needed because the path_rename operation will use an inode number of 0 the same we were doing when the new path is not populated (i.e there is no file on the path we are landing). As a side effect of this, inodes on the kernel side cannot be NULL, some additional cleanup might be needed. Properly tracking rename operations requires collaboration between kernelspace and userspace, particularly when renaming a directory. In this case, the LSM hook will trigger once and we have no way of iterating through subdirectories and files contained in the directories involved, so the kernel does its best effort with the available information and leaves the heavy lifting to userspace. Where possible, userspace will use the available information in memory to adjust the paths being tracked, any discrepancies will be sorted by (hopefully) subsequent events or a periodic scan of the host. The logic on both sides is documented as thoroughly as possible. --- fact-ebpf/src/bpf/events.h | 10 +- fact-ebpf/src/bpf/file.h | 13 ++- fact-ebpf/src/bpf/inode.h | 17 +-- fact-ebpf/src/bpf/main.c | 98 +++++++++++++---- fact-ebpf/src/bpf/types.h | 14 ++- fact-ebpf/src/lib.rs | 21 ++++ fact/src/bpf/mod.rs | 13 ++- fact/src/event/mod.rs | 64 +++++++++-- fact/src/host_scanner.rs | 179 +++++++++++++++++++++++++++++-- fact/src/metrics/host_scanner.rs | 7 +- fact/src/metrics/mod.rs | 13 +++ tests/test_path_rename.py | 72 ++++++------- 12 files changed, 421 insertions(+), 100 deletions(-) diff --git a/fact-ebpf/src/bpf/events.h b/fact-ebpf/src/bpf/events.h index 276a1d97..a3d30ea9 100644 --- a/fact-ebpf/src/bpf/events.h +++ b/fact-ebpf/src/bpf/events.h @@ -18,6 +18,7 @@ struct submit_event_args_t { const char* filename; inode_key_t inode; inode_key_t parent_inode; + monitored_t monitored; }; __always_inline static bool reserve_event(struct submit_event_args_t* args) { @@ -33,6 +34,7 @@ __always_inline static void __submit_event(struct submit_event_args_t* args, bool use_bpf_d_path) { struct event_t* event = args->event; event->timestamp = bpf_ktime_get_boot_ns(); + event->monitored = args->monitored; inode_copy(&event->inode, &args->inode); inode_copy(&event->parent_inode, &args->parent_inode); bpf_probe_read_str(event->filename, PATH_MAX, args->filename); @@ -110,14 +112,16 @@ __always_inline static void submit_ownership_event(struct submit_event_args_t* a __always_inline static void submit_rename_event(struct submit_event_args_t* args, const char old_filename[PATH_MAX], - inode_key_t* old_inode) { + inode_key_t* old_inode, + monitored_t old_monitored) { if (!reserve_event(args)) { return; } args->event->type = FILE_ACTIVITY_RENAME; - bpf_probe_read_str(args->event->rename.old_filename, PATH_MAX, old_filename); - inode_copy(&args->event->rename.old_inode, old_inode); + bpf_probe_read_str(args->event->rename.filename, PATH_MAX, old_filename); + inode_copy(&args->event->rename.inode, old_inode); + args->event->rename.monitored = old_monitored; __submit_event(args, path_hooks_support_bpf_d_path); } diff --git a/fact-ebpf/src/bpf/file.h b/fact-ebpf/src/bpf/file.h index efdccfc5..942ecbdf 100644 --- a/fact-ebpf/src/bpf/file.h +++ b/fact-ebpf/src/bpf/file.h @@ -27,18 +27,17 @@ __always_inline static bool path_is_monitored(struct bound_path_t* path) { return res; } -__always_inline static inode_monitored_t is_monitored(inode_key_t* inode, struct bound_path_t* path, const inode_key_t* parent) { +__always_inline static monitored_t is_monitored(const inode_key_t* inode, struct bound_path_t* path, const inode_key_t* parent) { const inode_value_t* volatile inode_value = inode_get(inode); const inode_value_t* volatile parent_value = inode_get(parent); - inode_monitored_t status = inode_is_monitored(inode_value, parent_value); + monitored_t status = inode_is_monitored(inode_value, parent_value); if (status != NOT_MONITORED) { return status; } - inode_reset(inode); if (path_is_monitored(path)) { - return MONITORED; + return MONITORED_BY_PATH; } return NOT_MONITORED; @@ -46,15 +45,15 @@ __always_inline static inode_monitored_t is_monitored(inode_key_t* inode, struct // Check if a new directory should be tracked based on its parent and path. // This is used during mkdir operations where the child inode doesn't exist yet. -__always_inline static inode_monitored_t should_track_mkdir(inode_key_t parent_inode, struct bound_path_t* child_path) { +__always_inline static monitored_t should_track_mkdir(inode_key_t parent_inode, struct bound_path_t* child_path) { const inode_value_t* volatile parent_value = inode_get(&parent_inode); if (parent_value != NULL) { - return PARENT_MONITORED; + return MONITORED_BY_PARENT; } if (path_is_monitored(child_path)) { - return MONITORED; + return MONITORED_BY_PATH; } return NOT_MONITORED; diff --git a/fact-ebpf/src/bpf/inode.h b/fact-ebpf/src/bpf/inode.h index 6b213c3b..01f01814 100644 --- a/fact-ebpf/src/bpf/inode.h +++ b/fact-ebpf/src/bpf/inode.h @@ -85,20 +85,21 @@ __always_inline static void inode_reset(struct inode_key_t* inode) { inode->dev = 0; } -typedef enum inode_monitored_t { - NOT_MONITORED = 0, - MONITORED, - PARENT_MONITORED, -} inode_monitored_t; +/* + * Check if the supplied inode is empty + */ +__always_inline static bool inode_is_empty(struct inode_key_t* inode) { + return inode->inode == 0 && inode->dev == 0; +} // Check if the provided inode or its parent is being monitored. -__always_inline static inode_monitored_t inode_is_monitored(const inode_value_t* volatile inode, const inode_value_t* volatile parent_inode) { +__always_inline static monitored_t inode_is_monitored(const inode_value_t* volatile inode, const inode_value_t* volatile parent_inode) { if (inode != NULL) { - return MONITORED; + return MONITORED_BY_INODE; } if (parent_inode != NULL) { - return PARENT_MONITORED; + return MONITORED_BY_PARENT; } return NOT_MONITORED; diff --git a/fact-ebpf/src/bpf/main.c b/fact-ebpf/src/bpf/main.c index c6effe8d..258aba0c 100644 --- a/fact-ebpf/src/bpf/main.c +++ b/fact-ebpf/src/bpf/main.c @@ -52,14 +52,13 @@ int BPF_PROG(trace_file_open, struct file* file) { struct inode* parent_inode_ptr = parent_dentry ? BPF_CORE_READ(parent_dentry, d_inode) : NULL; args.parent_inode = inode_to_key(parent_inode_ptr); - inode_monitored_t status = is_monitored(&args.inode, path, &args.parent_inode); - - if (status == PARENT_MONITORED && event_type == FILE_ACTIVITY_CREATION) { - inode_add(&args.inode); + args.monitored = is_monitored(&args.inode, path, &args.parent_inode); + if (args.monitored == NOT_MONITORED) { + goto ignored; } - if (status == NOT_MONITORED) { - goto ignored; + if (args.monitored == MONITORED_BY_PARENT && event_type == FILE_ACTIVITY_CREATION) { + inode_add(&args.inode); } submit_open_event(&args, event_type); @@ -90,8 +89,9 @@ int BPF_PROG(trace_path_unlink, struct path* dir, struct dentry* dentry) { args.filename = path->path; args.inode = inode_to_key(dentry->d_inode); + args.monitored = is_monitored(&args.inode, path, NULL); - if (is_monitored(&args.inode, path, NULL) == NOT_MONITORED) { + if (args.monitored == NOT_MONITORED) { m->path_unlink.ignored++; return 0; } @@ -122,8 +122,9 @@ int BPF_PROG(trace_path_chmod, struct path* path, umode_t mode) { args.filename = bound_path->path; args.inode = inode_to_key(path->dentry->d_inode); + args.monitored = is_monitored(&args.inode, bound_path, NULL); - if (is_monitored(&args.inode, bound_path, NULL) == NOT_MONITORED) { + if (args.monitored == NOT_MONITORED) { args.metrics->ignored++; return 0; } @@ -156,8 +157,9 @@ int BPF_PROG(trace_path_chown, struct path* path, unsigned long long uid, unsign args.filename = bound_path->path; args.inode = inode_to_key(path->dentry->d_inode); + args.monitored = is_monitored(&args.inode, bound_path, NULL); - if (is_monitored(&args.inode, bound_path, NULL) == NOT_MONITORED) { + if (args.monitored == NOT_MONITORED) { args.metrics->ignored++; return 0; } @@ -197,18 +199,75 @@ int BPF_PROG(trace_path_rename, struct path* old_dir, } args.inode = inode_to_key(new_dentry->d_inode); + args.parent_inode = inode_to_key(new_dir->dentry->d_inode); + args.monitored = is_monitored(&args.inode, new_path, &args.parent_inode); inode_key_t old_inode = inode_to_key(old_dentry->d_inode); - - inode_monitored_t old_monitored = is_monitored(&old_inode, old_path, NULL); - inode_monitored_t new_monitored = is_monitored(&args.inode, new_path, NULL); - - if (old_monitored == NOT_MONITORED && new_monitored == NOT_MONITORED) { - args.metrics->ignored++; - return 0; + monitored_t old_monitored = is_monitored(&old_inode, old_path, NULL); + + // From this point on we need to handle inode tracking. + // + // The result will be a combination of whether we are already tracking + // the old inode or not and whether the target path has an existing + // object that is about to be overwritten and if said object is + // tracked by inode or not. + switch (args.monitored) { + case NOT_MONITORED: + if (old_monitored == NOT_MONITORED) { + m->path_rename.ignored++; + return 0; + } + + if (old_monitored == MONITORED_BY_INODE) { + // Old inode is monitored, new path is not. + // If the old path is a directory userspace will remove any + // subdirectories and files too. + inode_remove(&old_inode); + } + break; + + case MONITORED_BY_PATH: + if (old_monitored == MONITORED_BY_INODE) { + // New path is not inode tracked, old path is. + // + // This implies the inode will be crossing a FS mountpoint, + // which should never happen. When the inode crosses into a new + // mount, a new inode is created altogether. Still, we can cover + // our bases. + inode_remove(&old_inode); + } + break; + + case MONITORED_BY_PARENT: + if (old_monitored != MONITORED_BY_INODE) { + // Old inode is not monitored, new parent is. + if (inode_is_empty(&args.inode)) { + // Landing on an empty path, we track the inode in case we + // need to, userspace will double check in detail. + inode_add(&old_inode); + } + } else if (!inode_is_empty(&args.inode)) { + // Old inode is monitored and will land on a path that has a + // monitored parent but the path itself is not monitored, we + // stop tracking the inode + inode_remove(&old_inode); + } + break; + + case MONITORED_BY_INODE: + // If we landed here, the new path already has an inode that is + // being tracked and is about to be overwritten, we need to remove + // it from the map + inode_remove(&args.inode); + if (old_monitored != MONITORED_BY_INODE) { + // Old inode is not monitored, but is landing in a monitored + // path that uses inode tracking. + inode_add(&old_inode); + } + break; } - submit_rename_event(&args, old_path->path, &old_inode); + submit_rename_event(&args, old_path->path, &old_inode, old_monitored); return 0; error: @@ -235,7 +294,8 @@ int BPF_PROG(trace_path_mkdir, struct path* dir, struct dentry* dentry, umode_t struct inode* parent_inode_ptr = BPF_CORE_READ(dir, dentry, d_inode); inode_key_t parent_inode = inode_to_key(parent_inode_ptr); - if (should_track_mkdir(parent_inode, path) != PARENT_MONITORED) { + monitored_t monitored = should_track_mkdir(parent_inode, path); + if (monitored != MONITORED_BY_PARENT) { m->path_mkdir.ignored++; return 0; } @@ -266,6 +326,7 @@ int BPF_PROG(trace_path_mkdir, struct path* dir, struct dentry* dentry, umode_t return 0; } mkdir_ctx->parent_inode = parent_inode; + mkdir_ctx->monitored = monitored; return 0; } @@ -295,6 +356,7 @@ int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) { } args.filename = mkdir_ctx->path; args.parent_inode = mkdir_ctx->parent_inode; + args.monitored = mkdir_ctx->monitored; args.inode = inode_to_key(inode); diff --git a/fact-ebpf/src/bpf/types.h b/fact-ebpf/src/bpf/types.h index 95c67e86..67d8df49 100644 --- a/fact-ebpf/src/bpf/types.h +++ b/fact-ebpf/src/bpf/types.h @@ -42,6 +42,13 @@ typedef struct inode_key_t { unsigned long dev; } inode_key_t; +typedef enum monitored_t { + NOT_MONITORED = 0, + MONITORED_BY_INODE, + MONITORED_BY_PATH, + MONITORED_BY_PARENT, +} monitored_t; + // We can't use bool here because it is not a standard C type, we would // need to include vmlinux.h but that would explode our Rust bindings. // For the time being we just keep a char. @@ -64,6 +71,7 @@ struct event_t { char filename[PATH_MAX]; inode_key_t inode; inode_key_t parent_inode; + monitored_t monitored; file_activity_type_t type; union { struct { @@ -77,8 +85,9 @@ struct event_t { } old, new; } chown; struct { - char old_filename[PATH_MAX]; - inode_key_t old_inode; + char filename[PATH_MAX]; + inode_key_t inode; + monitored_t monitored; } rename; }; }; @@ -101,6 +110,7 @@ struct path_prefix_t { struct mkdir_context_t { char path[PATH_MAX]; inode_key_t parent_inode; + monitored_t monitored; }; // Metrics types diff --git a/fact-ebpf/src/lib.rs b/fact-ebpf/src/lib.rs index c4c95ec6..5d5984ab 100644 --- a/fact-ebpf/src/lib.rs +++ b/fact-ebpf/src/lib.rs @@ -103,6 +103,27 @@ impl Serialize for inode_key_t { unsafe impl Pod for inode_key_t {} +impl Default for monitored_t { + fn default() -> Self { + monitored_t::NOT_MONITORED + } +} + +impl Serialize for monitored_t { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + match *self { + monitored_t::NOT_MONITORED => "not monitored".serialize(serializer), + monitored_t::MONITORED_BY_INODE => "by inode".serialize(serializer), + monitored_t::MONITORED_BY_PATH => "by path".serialize(serializer), + monitored_t::MONITORED_BY_PARENT => "by parent".serialize(serializer), + _ => unreachable!("Invalid monitored_t value: {self:?}"), + } + } +} + impl metrics_by_hook_t { fn accumulate(&self, other: &metrics_by_hook_t) -> metrics_by_hook_t { let mut m = metrics_by_hook_t { ..*self }; diff --git a/fact/src/bpf/mod.rs b/fact/src/bpf/mod.rs index 38fe269d..94fd6a08 100644 --- a/fact/src/bpf/mod.rs +++ b/fact/src/bpf/mod.rs @@ -9,7 +9,7 @@ use aya::{ use checks::Checks; use globset::{Glob, GlobSet, GlobSetBuilder}; use libc::c_char; -use log::{error, info}; +use log::{error, info, warn}; use tokio::{ io::unix::AsyncFd, sync::{mpsc, watch}, @@ -160,7 +160,9 @@ impl Bpf { // Remove old prefixes for p in self.paths.iter().filter(|p| !new_paths.contains(p)) { - path_prefix.remove(&(*p).into())?; + if let Err(e) = path_prefix.remove(&(*p).into()) { + warn!("Failed to remove path prefix: {e:#?}"); + } } self.paths = new_paths; @@ -228,7 +230,12 @@ impl Bpf { let event: &event_t = unsafe { &*(event.as_ptr() as *const _) }; let event = match Event::try_from(event) { Ok(event) => { - if event.is_ignored(&self.paths_globset) { + // If the event is monitored by parent, we need to check + // its host path, but we don't have that context here, + // so we let the event go into HostScanner and make the + // decision there. + if !event.is_monitored_by_parent() && + event.is_ignored(&self.paths_globset) { event_counter.dropped(); continue; } diff --git a/fact/src/event/mod.rs b/fact/src/event/mod.rs index 1854d7af..a5e96c47 100644 --- a/fact/src/event/mod.rs +++ b/fact/src/event/mod.rs @@ -10,7 +10,7 @@ use globset::GlobSet; use log::warn; use serde::Serialize; -use fact_ebpf::{PATH_MAX, event_t, file_activity_type_t, inode_key_t}; +use fact_ebpf::{PATH_MAX, event_t, file_activity_type_t, inode_key_t, monitored_t}; use crate::host_info; use process::Process; @@ -95,6 +95,7 @@ impl Event { host_file, inode: Default::default(), parent_inode: Default::default(), + monitored: Default::default(), }; let file = match data { EventTestData::Creation => FileData::Creation(inner), @@ -143,6 +144,10 @@ impl Event { matches!(self.file, FileData::Unlink(_)) } + pub fn is_rename(&self) -> bool { + matches!(self.file, FileData::Rename(_)) + } + /// Unwrap the inner FileData and return the inode that triggered /// the event. /// @@ -195,7 +200,7 @@ impl Event { } } - fn get_old_filename(&self) -> Option<&PathBuf> { + pub fn get_old_filename(&self) -> Option<&PathBuf> { match &self.file { FileData::Rename(data) => Some(&data.old.filename), _ => None, @@ -214,6 +219,13 @@ impl Event { } } + pub fn get_old_host_path(&self) -> Option<&PathBuf> { + match &self.file { + FileData::Rename(data) => Some(&data.old.host_file), + _ => None, + } + } + /// Set the `host_file` field of the event to the one provided. /// /// In the case of operations that involve two paths, like rename, @@ -238,6 +250,25 @@ impl Event { } } + pub fn get_monitored(&self) -> monitored_t { + match &self.file { + FileData::Open(data) => data.monitored, + FileData::Creation(data) => data.monitored, + FileData::MkDir(data) => data.monitored, + FileData::Unlink(data) => data.monitored, + FileData::Chmod(data) => data.inner.monitored, + FileData::Chown(data) => data.inner.monitored, + FileData::Rename(data) => data.new.monitored, + } + } + + pub fn get_old_monitored(&self) -> Option { + match &self.file { + FileData::Rename(data) => Some(data.old.monitored), + _ => None, + } + } + /// Determine if the event should be ignored. /// /// With wildcards, the kernel can only match on the inode and @@ -249,13 +280,19 @@ impl Event { /// /// We also need to check the old values for rename events. pub fn is_ignored(&self, globset: &GlobSet) -> bool { - self.get_inode().empty() - && self.get_old_inode().is_none_or(|inode| inode.empty()) + self.get_monitored() != monitored_t::MONITORED_BY_INODE + && self + .get_old_monitored() + .is_none_or(|m| m != monitored_t::MONITORED_BY_INODE) && !globset.is_match(self.get_filename()) && self .get_old_filename() .is_none_or(|path| !globset.is_match(path)) } + + pub fn is_monitored_by_parent(&self) -> bool { + self.get_monitored() == monitored_t::MONITORED_BY_PARENT + } } impl TryFrom<&event_t> for Event { @@ -269,6 +306,7 @@ impl TryFrom<&event_t> for Event { value.filename, value.inode, value.parent_inode, + value.monitored, value.__bindgen_anon_1, )?; @@ -320,9 +358,10 @@ impl FileData { filename: [c_char; PATH_MAX as usize], inode: inode_key_t, parent_inode: inode_key_t, + monitored: monitored_t, extra_data: fact_ebpf::event_t__bindgen_ty_1, ) -> anyhow::Result { - let inner = BaseFileData::new(filename, inode, parent_inode)?; + let inner = BaseFileData::new(filename, inode, parent_inode, monitored)?; let file = match event_type { file_activity_type_t::FILE_ACTIVITY_OPEN => FileData::Open(inner), file_activity_type_t::FILE_ACTIVITY_CREATION => FileData::Creation(inner), @@ -347,11 +386,17 @@ impl FileData { FileData::Chown(data) } file_activity_type_t::FILE_ACTIVITY_RENAME => { - let old_filename = unsafe { extra_data.rename.old_filename }; - let old_inode = unsafe { extra_data.rename.old_inode }; + let old_filename = unsafe { extra_data.rename.filename }; + let old_inode = unsafe { extra_data.rename.inode }; + let old_monitored = unsafe { extra_data.rename.monitored }; let data = RenameFileData { new: inner, - old: BaseFileData::new(old_filename, old_inode, Default::default())?, + old: BaseFileData::new( + old_filename, + old_inode, + Default::default(), + old_monitored, + )?, }; FileData::Rename(data) } @@ -425,6 +470,7 @@ pub struct BaseFileData { host_file: PathBuf, inode: inode_key_t, parent_inode: inode_key_t, + monitored: monitored_t, } impl BaseFileData { @@ -432,12 +478,14 @@ impl BaseFileData { filename: [c_char; PATH_MAX as usize], inode: inode_key_t, parent_inode: inode_key_t, + monitored: monitored_t, ) -> anyhow::Result { Ok(BaseFileData { filename: sanitize_d_path(&filename), host_file: PathBuf::new(), // this field is set by HostScanner inode, parent_inode, + monitored, }) } } diff --git a/fact/src/host_scanner.rs b/fact/src/host_scanner.rs index f49c621f..af143e44 100644 --- a/fact/src/host_scanner.rs +++ b/fact/src/host_scanner.rs @@ -28,7 +28,8 @@ use std::{ use anyhow::{Context, bail}; use aya::maps::MapData; -use fact_ebpf::{inode_key_t, inode_value_t}; +use fact_ebpf::{inode_key_t, inode_value_t, monitored_t}; +use globset::{Glob, GlobSet, GlobSetBuilder}; use log::{debug, info, warn}; use tokio::{ sync::{Notify, broadcast, mpsc, watch}, @@ -54,6 +55,8 @@ pub struct HostScanner { tx: broadcast::Sender>, metrics: HostScannerMetrics, + + paths_globset: GlobSet, } impl HostScanner { @@ -68,6 +71,7 @@ impl HostScanner { let kernel_inode_map = RefCell::new(bpf.take_inode_map()?); let inode_map = RefCell::new(std::collections::HashMap::new()); let (tx, _) = broadcast::channel(100); + let paths_globset = HostScanner::build_globset(paths.borrow().as_slice())?; let host_scanner = HostScanner { kernel_inode_map, @@ -78,6 +82,7 @@ impl HostScanner { rx, tx, metrics, + paths_globset, }; // Run an initial scan to fill in the inode map @@ -86,6 +91,22 @@ impl HostScanner { Ok(host_scanner) } + fn build_globset(paths: &[PathBuf]) -> anyhow::Result { + let mut builder = GlobSetBuilder::new(); + for p in paths.iter() { + let Some(glob_str) = p.to_str() else { + bail!("failed to convert path {} to string", p.display()); + }; + + builder.add( + Glob::new(glob_str) + .with_context(|| format!("invalid glob {}", glob_str)) + .unwrap(), + ); + } + Ok(builder.build()?) + } + fn scan(&self) -> anyhow::Result<()> { debug!("Host scan started"); self.metrics.scan_inc(ScanLabels::Scans); @@ -233,6 +254,127 @@ impl HostScanner { self.metrics.scan_inc(ScanLabels::FileRemoved); } + fn handle_rename_event(&self, event: &mut Event) { + match event.get_monitored() { + monitored_t::MONITORED_BY_INODE => { + // This condition means a file is being renamed and taking the + // place of an existing, tracked file. We need to remove the + // inode we are landing on and put the associated host path in + // the old inode. + let mut inode_map = self.inode_map.borrow_mut(); + let Some(path) = inode_map.remove(event.get_inode()) else { + warn!("Old path was not found for inode tracked event"); + return; + }; + let Some(old_inode) = event.get_old_inode() else { + unreachable!("old inode not found for rename event"); + }; + inode_map.insert(*old_inode, path); + } + monitored_t::NOT_MONITORED + if event.get_old_monitored() == Some(monitored_t::MONITORED_BY_INODE) => + { + // We are landing on a path that is not tracked at all, remove + // the entries for the old path from the map + let Some(old_host_path) = event.get_old_host_path() else { + warn!("Rename event did not have old host path for inode tracked item"); + return; + }; + self.inode_map.borrow_mut().retain(|inode, path| { + if !path.starts_with(old_host_path) { + return true; + } + + let _ = self.kernel_inode_map.borrow_mut().remove(inode); + false + }); + } + monitored_t::NOT_MONITORED => { + // The new path is not monitored and the old path is most likely + // matching by path, we don't need to do anything in this case. + } + monitored_t::MONITORED_BY_PARENT if !event.get_inode().empty() => { + // The parent for the target is monitored, but the file itself + // is not. Remove the entry for the old file from the map. + self.inode_map.borrow_mut().remove( + event + .get_old_inode() + .expect("rename event did not have old inode"), + ); + } + monitored_t::MONITORED_BY_PARENT + if event.get_old_monitored() == Some(monitored_t::MONITORED_BY_INODE) => + { + // The target is monitored by parent and we are landing on a + // path that didn't hold anything, we need to figure out the + // host path and check if we should track it. + let mut inode_map = self.inode_map.borrow_mut(); + let Some(new_host_parent) = inode_map.get(event.get_parent_inode()) else { + warn!("Failed to get parent host path"); + return; + }; + let Some(filename) = event.get_filename().file_name() else { + warn!("Failed to get last component from event: {event:#?}"); + return; + }; + let new_host_path = new_host_parent.join(filename); + let Some(old_host_path) = event.get_old_host_path() else { + unreachable!("Rename event did not have an old host path"); + }; + + if self.paths_globset.is_match(&new_host_path) { + // New path needs to be tracked. + // Move all entries for the old host path to the new one + for (_, path) in inode_map.iter_mut() { + if let Ok(suffix) = path.strip_prefix(old_host_path) { + if suffix == Path::new("") { + *path = new_host_path.clone(); + } else { + *path = new_host_path.join(suffix); + } + } + } + + // Add the new host path to the event + event.set_host_path(new_host_path); + } else { + // New path is not tracked, remove old entries + inode_map.retain(|inode, path| { + if !path.starts_with(old_host_path) { + return true; + } + if let Err(e) = self.kernel_inode_map.borrow_mut().remove(inode) { + warn!("Failed to remove inode kernel entry: {e:?}"); + } + false + }); + } + } + monitored_t::MONITORED_BY_PARENT => { + // In this case, the target location might be monitored, but we + // don't have any information of the host path for the old path, + // best we can do is attempt to scan the file system and fix the + // inode maps that way. + if let Err(e) = self.scan() { + warn!("Scan failed: {e:?}"); + } + + // Attempt to update the host path with the old inode + if let Some(old_inode) = event.get_old_inode() + && let Some(path) = self.inode_map.borrow().get(old_inode) + { + event.set_host_path(path.clone()); + } + } + monitored_t::MONITORED_BY_PATH => { + // Nothing to do here, having one side of the rename monitored + // by path means at best the other side is also monitored by + // path, no inode tracking is involved. + } + _ => unreachable!("Invalid monitored value"), + } + } + /// Periodically notify the host scanner main task that a scan needs /// to happen. /// @@ -286,6 +428,11 @@ impl HostScanner { warn!("Failed to handle creation event: {e}"); } + // Skip directory creation events - we track them internally but don't send to sensor + if event.is_mkdir() { + continue; + } + if let Some(host_path) = self.get_host_path(Some(event.get_inode())) { self.metrics.scan_inc(ScanLabels::InodeHit); event.set_host_path(host_path); @@ -301,17 +448,31 @@ impl HostScanner { self.handle_unlink_event(&event); } - // Skip directory creation events - we track them internally but don't send to sensor - if !event.is_mkdir() { - let event = Arc::new(event); - if let Err(e) = self.tx.send(event) { - self.metrics.events.dropped(); - warn!("Failed to send event: {e}"); - } + if event.is_rename() { self.handle_rename_event(&mut event); } + + if event.is_monitored_by_parent() && + !self.paths_globset.is_match(event.get_host_path()) { + // The event was monitored by parent, but the host + // path is not to be monitored, so we ignore the + // event and attempt to remove the inode from the + // maps to prevent it from sending more events. + self.inode_map.borrow_mut().remove(event.get_inode()); + let _ = self.kernel_inode_map.borrow_mut().remove(event.get_inode()); + self.metrics.events.ignored(); + continue; + } + + let event = Arc::new(event); + if let Err(e) = self.tx.send(event) { + self.metrics.events.dropped(); + warn!("Failed to send event: {e}"); } }, _ = scan_trigger.notified() => self.scan()?, - _ = self.paths.changed() => self.scan()?, + _ = self.paths.changed() => { + self.paths_globset = HostScanner::build_globset(self.paths.borrow().as_slice())?; + self.scan()?; + } _ = self.running.changed() => { if !*self.running.borrow() { break; diff --git a/fact/src/metrics/host_scanner.rs b/fact/src/metrics/host_scanner.rs index f5197cf5..81bef60c 100644 --- a/fact/src/metrics/host_scanner.rs +++ b/fact/src/metrics/host_scanner.rs @@ -33,7 +33,12 @@ pub struct HostScannerMetrics { impl HostScannerMetrics { pub(super) fn new() -> Self { - let labels = [EventLabels::Total, EventLabels::Added, EventLabels::Dropped]; + let labels = [ + EventLabels::Total, + EventLabels::Added, + EventLabels::Dropped, + EventLabels::Ignored, + ]; let events = EventCounter::new( "host_scanner_events", "Events processed by the host scanner component", diff --git a/fact/src/metrics/mod.rs b/fact/src/metrics/mod.rs index 97213c83..c7a7cd73 100644 --- a/fact/src/metrics/mod.rs +++ b/fact/src/metrics/mod.rs @@ -100,6 +100,19 @@ impl EventCounter { .unwrap() .inc_by(n); } + + /// Increment the counter for the Ignored label. + /// + /// Panics if the counter did not add the Ignored label as part of + /// its creation step. + pub fn ignored(&self) { + self.counter + .get(&MetricEvents { + label: LabelValues::Ignored, + }) + .expect("Ignored label not found") + .inc(); + } } #[derive(Debug, Clone)] diff --git a/tests/test_path_rename.py b/tests/test_path_rename.py index 98c3447a..6942c3ea 100644 --- a/tests/test_path_rename.py +++ b/tests/test_path_rename.py @@ -28,7 +28,6 @@ def test_rename(monitored_dir, server, filename): fut = join_path_with_filename(monitored_dir, filename) old_fut = os.path.join(monitored_dir, 'file.txt') - # Create a new file, it will have an empty host_path. with open(old_fut, 'w') as f: f.write('This is a test') os.rename(old_fut, fut) @@ -37,23 +36,15 @@ def test_rename(monitored_dir, server, filename): # Convert fut to string for the Event, replacing invalid UTF-8 with U+FFFD fut = path_to_string(fut) - # TODO: Current behavior is incorrect. The inode map should be updated - # during rename events so that host_path reflects the new path. - # Expected correct behavior: - # - First rename: host_path should be `fut` (new path), old_host_path should be `old_fut` - # - Second rename: host_path should be `old_fut`, old_host_path should be `fut` - # Current behavior: host_path remains the original path (old_fut) because - # the inode map is not updated on rename events. old_host_path is empty. - events = [ - Event(process=Process.from_proc(), event_type=EventType.CREATION, + p = Process.from_proc() + server.wait_events([ + Event(process=p, event_type=EventType.CREATION, file=old_fut, host_path=old_fut), - Event(process=Process.from_proc(), event_type=EventType.RENAME, - file=fut, host_path='', old_file=old_fut, old_host_path=old_fut), - Event(process=Process.from_proc(), event_type=EventType.RENAME, - file=old_fut, host_path='', old_file=fut, old_host_path=old_fut), - ] - - server.wait_events(events) + Event(process=p, event_type=EventType.RENAME, file=fut, + host_path=fut, old_file=old_fut, old_host_path=old_fut), + Event(process=p, event_type=EventType.RENAME, file=old_fut, + host_path=old_fut, old_file=fut, old_host_path=fut), + ]) def test_ignored(monitored_dir, ignored_dir, server): @@ -67,6 +58,7 @@ def test_ignored(monitored_dir, ignored_dir, server): server: The server instance to communicate with. """ ignored_path = os.path.join(ignored_dir, 'test.txt') + p = Process.from_proc() with open(ignored_path, 'w') as f: f.write('This is to be ignored') @@ -75,26 +67,22 @@ def test_ignored(monitored_dir, ignored_dir, server): # Renaming in between ignored paths should not generate events os.rename(ignored_path, new_ignored_path) - # Renaming to a monitored path generates an event + # Renaming to a monitored path requires a scan, we need to wait for + # it before we can continue modifying the FS new_path = os.path.join(monitored_dir, 'rename.txt') os.rename(new_ignored_path, new_path) + server.wait_events([ + Event(process=p, event_type=EventType.RENAME, + file=new_path, host_path=new_path, old_file=new_ignored_path, old_host_path=''), + ]) # Renaming from a monitored path generates an event too os.rename(new_path, ignored_path) - p = Process.from_proc() - # TODO: Current behavior is incorrect for rename events. - # Expected: When renaming from ignored to monitored, host_path should be new_path. - # When renaming from monitored to ignored, old_host_path should be new_path. - # Current: The inode map is not updated on renames, and old_host_path is not populated. - events = [ - Event(process=p, event_type=EventType.RENAME, - file=new_path, host_path='', old_file=new_ignored_path, old_host_path=''), + server.wait_events([ Event(process=p, event_type=EventType.RENAME, - file=ignored_path, host_path='', old_file=new_path, old_host_path=''), - ] - - server.wait_events(events) + file=ignored_path, host_path='', old_file=new_path, old_host_path=new_path), + ]) def test_rename_dir(monitored_dir, ignored_dir, server): @@ -127,24 +115,26 @@ def test_rename_dir(monitored_dir, ignored_dir, server): # This rename should generate no events os.rename(ignored_dut, new_ignored_dut) - # The next three renames need to generate one event each + # Going from a non-monitored directory to a monitored one requires a scan of + # the filesystem to add any subdirectories and files, so we need to wait for + # it to end before we can continue modifying the FS. os.rename(new_ignored_dut, dut) + + p = Process.from_proc() + server.wait_events([ + Event(process=p, event_type=EventType.RENAME, file=dut, + host_path=dut, old_file=new_ignored_dut, old_host_path=''), + ]) + + # The following two event should produce full events without scanning the FS. os.rename(dut, new_dut) os.rename(new_dut, ignored_dut) - p = Process.from_proc() - # TODO: Current behavior is incorrect for rename events. - # Expected: host_path should reflect the new path after rename, - # old_host_path should reflect the old path if it was monitored. - # Current: The inode map is not updated on renames, so host_path remains empty - # or shows the wrong path. old_host_path is not populated. events = [ - Event(process=p, event_type=EventType.RENAME, file=dut, - host_path='', old_file=new_ignored_dut, old_host_path=''), Event(process=p, event_type=EventType.RENAME, - file=new_dut, host_path='', old_file=dut, old_host_path=''), + file=new_dut, host_path=new_dut, old_file=dut, old_host_path=dut), Event(process=p, event_type=EventType.RENAME, - file=ignored_dut, host_path='', old_file=new_dut, old_host_path=''), + file=ignored_dut, host_path='', old_file=new_dut, old_host_path=new_dut), ] server.wait_events(events) From c115b4ddb21f1dfd08b9a606910348dc27731e71 Mon Sep 17 00:00:00 2001 From: Mauro Ezequiel Moltrasio Date: Fri, 10 Apr 2026 12:04:32 +0200 Subject: [PATCH 34/34] ROX-33216: add more test cases for path_rename In order to validate more of the inode tracking functionality in path_rename more test cases and additional validations are added. --- tests/test_path_rename.py | 161 +++++++++++++++++++++++++++++++++++--- 1 file changed, 152 insertions(+), 9 deletions(-) diff --git a/tests/test_path_rename.py b/tests/test_path_rename.py index 6942c3ea..371f870b 100644 --- a/tests/test_path_rename.py +++ b/tests/test_path_rename.py @@ -101,6 +101,20 @@ def test_rename_dir(monitored_dir, ignored_dir, server): ignored_dir: Temporary directory path that is not monitored by fact. server: The server instance to communicate with. """ + def touch_test_files(directory, process=None) -> list[Event]: + events = [] + for i in range(3): + with open(os.path.join(directory, f'{i}.txt'), 'w') as f: + f.write('This is a test') + if process is not None: + events.append( + Event(process=process, + event_type=EventType.OPEN, + file=os.path.join(directory, f'{i}.txt'), + host_path=os.path.join(directory, f'{i}.txt')) + ) + return events + # Directory Under Test dut = os.path.join(monitored_dir, 'some-dir') new_dut = os.path.join(monitored_dir, 'other-dir') @@ -108,9 +122,7 @@ def test_rename_dir(monitored_dir, ignored_dir, server): new_ignored_dut = os.path.join(ignored_dir, 'other-dir') os.mkdir(ignored_dut) - for i in range(3): - with open(os.path.join(ignored_dut, f'{i}.txt'), 'w') as f: - f.write('This is a test') + touch_test_files(ignored_dut) # This rename should generate no events os.rename(ignored_dut, new_ignored_dut) @@ -126,16 +138,81 @@ def test_rename_dir(monitored_dir, ignored_dir, server): host_path=dut, old_file=new_ignored_dut, old_host_path=''), ]) - # The following two event should produce full events without scanning the FS. + events = touch_test_files(dut, p) + + # The following renames should produce full events without scanning the FS. os.rename(dut, new_dut) - os.rename(new_dut, ignored_dut) + events.extend([ + Event(process=p, event_type=EventType.RENAME, file=new_dut, + host_path=new_dut, old_file=dut, old_host_path=dut), + # Check the renamed subfiles are properly tracked + *touch_test_files(new_dut, p), + ]) - events = [ - Event(process=p, event_type=EventType.RENAME, - file=new_dut, host_path=new_dut, old_file=dut, old_host_path=dut), + os.rename(new_dut, ignored_dut) + events.append( Event(process=p, event_type=EventType.RENAME, file=ignored_dut, host_path='', old_file=new_dut, old_host_path=new_dut), - ] + ) + + server.wait_events(events) + + +@pytest.mark.parametrize('from_monitored,to_monitored', [ + pytest.param(True, True, id='both_monitored'), + pytest.param(False, True, id='target_monitored'), + pytest.param(True, False, id='bullet_monitored'), +]) +def test_rename_overwrite(from_monitored, to_monitored, monitored_dir, ignored_dir, server): + events = [] + p = Process.from_proc() + if from_monitored: + bullet = os.path.join(monitored_dir, 'bullet.txt') + events.append( + Event(process=p, + event_type=EventType.CREATION, + file=bullet, + host_path=bullet, + )) + else: + bullet = os.path.join(ignored_dir, 'bullet.txt') + + if to_monitored: + target = os.path.join(monitored_dir, 'target.txt') + events.append( + Event(process=p, + event_type=EventType.CREATION, + file=target, + host_path=target, + )) + else: + target = os.path.join(ignored_dir, 'target.txt') + + # Create both files in the order they are expected in `events` + for path in [bullet, target]: + with open(path, 'w') as f: + f.write('This is a test') + + os.rename(bullet, target) + events.append( + Event(process=p, + event_type=EventType.RENAME, + file=target, + host_path=target if to_monitored else '', + old_file=bullet, + old_host_path=bullet if from_monitored else '', + )) + + if to_monitored: + # One final event to check the mapping is persisted correctly + with open(target, 'w') as f: + f.write('Check mapping') + events.append( + Event(process=p, + event_type=EventType.OPEN, + file=target, + host_path=target, + )) server.wait_events(events) @@ -203,3 +280,69 @@ def test_mounted_dir(test_container, ignored_dir, server): ] server.wait_events(events) + + +def test_cross_mountpoints(test_container, monitored_dir, server): + """ + Attempt to rename files/directories across mountpoints + + This test does not necessarily fit here, since it won't trigger the + path_rename LSM hook, but the test ensures that this hook is + defintely not triggered when an inode crosses a mount point, so it + doesn't fit but it kind of does? ¯\\_(ツ)_/¯ + """ + mounted_file = '/unmonitored/test.txt' + host_path = os.path.join(monitored_dir, 'test.txt') + ovfs_file = '/container-dir/test.txt' + + test_container.exec_run(f'touch {mounted_file}') + # Get owner uid and gid before moving the file + stat = os.stat(host_path) + owner_uid = stat.st_uid + owner_gid = stat.st_gid + mode = stat.st_mode + + test_container.exec_run(f'mv {mounted_file} {ovfs_file}') + test_container.exec_run(f'mv {ovfs_file} {mounted_file}') + + touch = Process.in_container( + exe_path='/usr/bin/touch', + args=f'touch {mounted_file}', + name='touch', + container_id=test_container.id[:12], + ) + first_rename = Process.in_container( + exe_path='/usr/bin/mv', + args=f'mv {mounted_file} {ovfs_file}', + name='mv', + container_id=test_container.id[:12], + ) + second_rename = Process.in_container( + exe_path='/usr/bin/mv', + args=f'mv {ovfs_file} {mounted_file}', + name='mv', + container_id=test_container.id[:12], + ) + + server.wait_events([ + Event(process=touch, event_type=EventType.OPEN, + file=mounted_file, host_path=host_path), + Event(process=first_rename, event_type=EventType.CREATION, + file=ovfs_file, host_path=''), + Event(process=first_rename, event_type=EventType.OPEN, + file=ovfs_file, host_path=''), + Event(process=first_rename, event_type=EventType.OWNERSHIP, + file=ovfs_file, host_path='', owner_uid=owner_uid, owner_gid=owner_gid), + Event(process=first_rename, event_type=EventType.PERMISSION, + file=ovfs_file, host_path='', mode=mode), + Event(process=first_rename, event_type=EventType.UNLINK, + file=mounted_file, host_path=host_path), + Event(process=second_rename, event_type=EventType.CREATION, + file=mounted_file, host_path=host_path), + Event(process=second_rename, event_type=EventType.OWNERSHIP, + file=mounted_file, host_path=host_path, owner_uid=owner_uid, owner_gid=owner_gid), + Event(process=second_rename, event_type=EventType.PERMISSION, + file=mounted_file, host_path=host_path, mode=mode), + Event(process=second_rename, event_type=EventType.UNLINK, + file=ovfs_file, host_path=''), + ])