From 72bc770f51677e7a106e40dc0a61687fd40d0a91 Mon Sep 17 00:00:00 2001 From: Giles Hutton Date: Mon, 9 Mar 2026 14:47:53 +0000 Subject: [PATCH 1/3] ROX-33471: do not attach progs when no paths configured --- fact-ebpf/src/bpf/file.h | 5 --- fact-ebpf/src/bpf/maps.h | 21 ----------- fact/src/bpf/mod.rs | 57 +++++++++++++++++++++++------- tests/test_config_hotreload.py | 64 ++++++++++++++++++++++++++++++++++ 4 files changed, 108 insertions(+), 39 deletions(-) diff --git a/fact-ebpf/src/bpf/file.h b/fact-ebpf/src/bpf/file.h index 4e3d6369..bb131f80 100644 --- a/fact-ebpf/src/bpf/file.h +++ b/fact-ebpf/src/bpf/file.h @@ -13,11 +13,6 @@ // clang-format on __always_inline static bool path_is_monitored(struct bound_path_t* path) { - if (!filter_by_prefix()) { - // no path configured, allow all - return true; - } - // Backup bytes length and restore it before exiting unsigned int len = path->len; diff --git a/fact-ebpf/src/bpf/maps.h b/fact-ebpf/src/bpf/maps.h index 048e3934..2a2a97b6 100644 --- a/fact-ebpf/src/bpf/maps.h +++ b/fact-ebpf/src/bpf/maps.h @@ -27,27 +27,6 @@ __always_inline static struct helper_t* get_helper() { return bpf_map_lookup_elem(&helper_map, &zero); } -/** - * A map with a single entry, determining whether prefix filtering - * should be done based on the `path_prefix` map. - */ -struct { - __uint(type, BPF_MAP_TYPE_ARRAY); - __type(key, __u32); - __type(value, char); - __uint(max_entries, 1); -} filter_by_prefix_map SEC(".maps"); - -/// Whether we should filter by path prefix or not. -__always_inline static bool filter_by_prefix() { - unsigned int zero = 0; - char* res = bpf_map_lookup_elem(&filter_by_prefix_map, &zero); - - // The NULL check is simply here to satisfy some verifiers, the result - // will never actually be NULL. - return res == NULL || *res != 0; -} - struct { __uint(type, BPF_MAP_TYPE_LPM_TRIE); __type(key, struct path_prefix_t); diff --git a/fact/src/bpf/mod.rs b/fact/src/bpf/mod.rs index 5604b821..c8f47cb3 100644 --- a/fact/src/bpf/mod.rs +++ b/fact/src/bpf/mod.rs @@ -1,9 +1,9 @@ -use std::{io, path::PathBuf}; +use std::{collections, io, path::PathBuf}; use anyhow::{bail, Context}; use aya::{ - maps::{Array, HashMap, LpmTrie, MapData, PerCpuArray, RingBuf}, - programs::Program, + maps::{HashMap, LpmTrie, MapData, PerCpuArray, RingBuf}, + programs::{lsm::LsmLinkId, Program}, Btf, Ebpf, }; use checks::Checks; @@ -33,6 +33,10 @@ pub struct Bpf { paths_config: watch::Receiver>, paths_globset: GlobSet, + + link_ids: collections::HashMap, + + progs_loaded: bool, } impl Bpf { @@ -65,10 +69,13 @@ impl Bpf { paths, paths_config, paths_globset: GlobSet::empty(), + link_ids: collections::HashMap::new(), + progs_loaded: false, }; bpf.load_paths()?; bpf.load_progs(&btf)?; + bpf.progs_loaded = true; Ok(bpf) } @@ -116,12 +123,18 @@ impl Bpf { } fn load_paths(&mut self) -> anyhow::Result<()> { - let paths_config = self.paths_config.borrow(); - let Some(filter_by_prefix) = self.obj.map_mut("filter_by_prefix_map") else { - bail!("filter_by_prefix_map map not found"); - }; - let mut filter_by_prefix: Array<&mut MapData, c_char> = Array::try_from(filter_by_prefix)?; - filter_by_prefix.set(0, !paths_config.is_empty() as c_char, 0)?; + if self.paths_config.borrow().is_empty() { + if self.progs_loaded { + self.detach_progs()?; + } + self.paths.clear(); + self.paths_globset = GlobSet::empty(); + return Ok(()); + } + + if self.progs_loaded && self.link_ids.is_empty() { + self.attach_progs()?; + } let Some(path_prefix) = self.obj.map_mut("path_prefix") else { bail!("path_prefix map not found"); @@ -130,6 +143,7 @@ impl Bpf { LpmTrie::try_from(path_prefix)?; // Add the new prefixes + let paths_config = self.paths_config.borrow(); let mut new_paths = Vec::with_capacity(paths_config.len()); let mut builder = GlobSetBuilder::new(); for p in paths_config.iter() { @@ -176,11 +190,25 @@ impl Bpf { } fn attach_progs(&mut self) -> anyhow::Result<()> { - for (_, prog) in self.obj.programs_mut() { - match prog { + for (name, prog) in self.obj.programs_mut() { + let name = name.to_string(); + let link_id = match prog { Program::Lsm(prog) => prog.attach()?, u => unimplemented!("{u:?}"), }; + self.link_ids.insert(name, link_id); + } + Ok(()) + } + + fn detach_progs(&mut self) -> anyhow::Result<()> { + for (name, prog) in self.obj.programs_mut() { + if let Some(link_id) = self.link_ids.remove(name) { + match prog { + Program::Lsm(prog) => prog.detach(link_id)?, + u => unimplemented!("{u:?}"), + }; + } } Ok(()) } @@ -194,8 +222,10 @@ impl Bpf { info!("Starting BPF worker..."); tokio::spawn(async move { - self.attach_progs() - .context("Failed to attach ebpf programs")?; + if !self.paths.is_empty() { + self.attach_progs() + .context("Failed to attach ebpf programs")?; + } let rb = self.take_ringbuffer()?; let mut fd = AsyncFd::new(rb)?; @@ -379,4 +409,5 @@ mod bpf_tests { run_tx.send(false).unwrap(); } + } diff --git a/tests/test_config_hotreload.py b/tests/test_config_hotreload.py index e23fdde1..c8bbc3da 100644 --- a/tests/test_config_hotreload.py +++ b/tests/test_config_hotreload.py @@ -154,6 +154,70 @@ def test_paths(fact, fact_config, monitored_dir, ignored_dir, server): server.wait_events([e]) +def test_no_paths_then_add(fact, fact_config, monitored_dir, server): + """ + Start with no paths configured, verify no events are produced, + then add paths via hot-reload and verify events appear. + """ + p = Process.from_proc() + + # Remove all paths + config, config_file = fact_config + config['paths'] = [] + reload_config(fact, config, config_file, delay=0.5) + + # Write to a file — should NOT produce events + fut = os.path.join(monitored_dir, 'test2.txt') + with open(fut, 'w') as f: + f.write('This should be ignored') + sleep(1) + + assert server.is_empty(), 'Should not receive events with no paths configured' + + # Add paths back + config['paths'] = [f'{monitored_dir}/**/*'] + reload_config(fact, config, config_file, delay=0.5) + + # Write to a file — should produce events + with open(fut, 'w') as f: + f.write('This should be detected') + + e = Event(process=p, event_type=EventType.OPEN, + file=fut, host_path=fut) + + server.wait_events([e]) + + +def test_paths_then_remove(fact, fact_config, monitored_dir, server): + """ + Start with paths configured, verify events are produced, + then remove all paths via hot-reload and verify events stop. + """ + p = Process.from_proc() + + # Write to a file — should produce events + fut = os.path.join(monitored_dir, 'test2.txt') + with open(fut, 'w') as f: + f.write('This is a test') + + e = Event(process=p, event_type=EventType.CREATION, + file=fut, host_path='') + + server.wait_events([e]) + + # Remove all paths + config, config_file = fact_config + config['paths'] = [] + reload_config(fact, config, config_file, delay=0.5) + + # Write to a file — should NOT produce events + with open(fut, 'w') as f: + f.write('This should be ignored') + sleep(1) + + assert server.is_empty(), 'Should not receive events after removing paths' + + def test_paths_addition(fact, fact_config, monitored_dir, ignored_dir, server): p = Process.from_proc() From fb954e6da294dede87c9f47025742be1d6dc9c16 Mon Sep 17 00:00:00 2001 From: Giles Hutton Date: Mon, 9 Mar 2026 15:00:07 +0000 Subject: [PATCH 2/3] Fmt; really must fix my nvim config --- fact/src/bpf/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/fact/src/bpf/mod.rs b/fact/src/bpf/mod.rs index c8f47cb3..daa551b7 100644 --- a/fact/src/bpf/mod.rs +++ b/fact/src/bpf/mod.rs @@ -409,5 +409,4 @@ mod bpf_tests { run_tx.send(false).unwrap(); } - } From 2f99ec2b60a26bc561a3d3095b80f6e7ef556d24 Mon Sep 17 00:00:00 2001 From: Giles Hutton Date: Tue, 10 Mar 2026 13:26:25 +0000 Subject: [PATCH 3/3] simplifies the link storage and dropping --- fact/src/bpf/mod.rs | 53 +++++++++++++++++---------------------------- 1 file changed, 20 insertions(+), 33 deletions(-) diff --git a/fact/src/bpf/mod.rs b/fact/src/bpf/mod.rs index daa551b7..7f654cd9 100644 --- a/fact/src/bpf/mod.rs +++ b/fact/src/bpf/mod.rs @@ -1,9 +1,9 @@ -use std::{collections, io, path::PathBuf}; +use std::{io, path::PathBuf}; use anyhow::{bail, Context}; use aya::{ maps::{HashMap, LpmTrie, MapData, PerCpuArray, RingBuf}, - programs::{lsm::LsmLinkId, Program}, + programs::{lsm::LsmLink, Program}, Btf, Ebpf, }; use checks::Checks; @@ -34,9 +34,7 @@ pub struct Bpf { paths_globset: GlobSet, - link_ids: collections::HashMap, - - progs_loaded: bool, + links: Vec, } impl Bpf { @@ -69,13 +67,11 @@ impl Bpf { paths, paths_config, paths_globset: GlobSet::empty(), - link_ids: collections::HashMap::new(), - progs_loaded: false, + links: Vec::new(), }; - bpf.load_paths()?; bpf.load_progs(&btf)?; - bpf.progs_loaded = true; + bpf.load_paths()?; Ok(bpf) } @@ -124,15 +120,13 @@ impl Bpf { fn load_paths(&mut self) -> anyhow::Result<()> { if self.paths_config.borrow().is_empty() { - if self.progs_loaded { - self.detach_progs()?; - } + self.detach_progs(); self.paths.clear(); self.paths_globset = GlobSet::empty(); return Ok(()); } - if self.progs_loaded && self.link_ids.is_empty() { + if self.links.is_empty() { self.attach_progs()?; } @@ -189,28 +183,26 @@ impl Bpf { Ok(()) } + /// Attaches all BPF programs. If any attach fails, all previously + /// attached programs are automatically detached via drop. fn attach_progs(&mut self) -> anyhow::Result<()> { - for (name, prog) in self.obj.programs_mut() { - let name = name.to_string(); - let link_id = match prog { - Program::Lsm(prog) => prog.attach()?, + let mut links = Vec::new(); + for (_name, prog) in self.obj.programs_mut() { + match prog { + Program::Lsm(prog) => { + let link_id = prog.attach()?; + links.push(prog.take_link(link_id)?); + } u => unimplemented!("{u:?}"), }; - self.link_ids.insert(name, link_id); } + self.links = links; Ok(()) } - fn detach_progs(&mut self) -> anyhow::Result<()> { - for (name, prog) in self.obj.programs_mut() { - if let Some(link_id) = self.link_ids.remove(name) { - match prog { - Program::Lsm(prog) => prog.detach(link_id)?, - u => unimplemented!("{u:?}"), - }; - } - } - Ok(()) + /// Detaches all BPF programs by dropping owned links. + fn detach_progs(&mut self) { + self.links.clear(); } // Gather events from the ring buffer and print them out. @@ -222,11 +214,6 @@ impl Bpf { info!("Starting BPF worker..."); tokio::spawn(async move { - if !self.paths.is_empty() { - self.attach_progs() - .context("Failed to attach ebpf programs")?; - } - let rb = self.take_ringbuffer()?; let mut fd = AsyncFd::new(rb)?;