diff --git a/fact/src/host_scanner.rs b/fact/src/host_scanner.rs index b0415a0d..326701bf 100644 --- a/fact/src/host_scanner.rs +++ b/fact/src/host_scanner.rs @@ -20,6 +20,7 @@ use std::{ cell::RefCell, + collections::HashMap, fs::Metadata, io, os::linux::fs::MetadataExt, @@ -30,7 +31,7 @@ use std::{ use anyhow::{Context, bail}; use aya::{ - maps::{MapData, MapError}, + maps::{HashMap as AyaHashMap, MapData, MapError}, sys::SyscallError, }; use fact_ebpf::{inode_key_t, inode_value_t, monitored_t}; @@ -50,8 +51,10 @@ use crate::{ }; pub struct HostScanner { - kernel_inode_map: RefCell>, - inode_map: RefCell>, + kernel_inode_map: RefCell>, + inode_map: RefCell>, + + scan_count: RefCell, paths: watch::Receiver>, scan_interval: watch::Receiver, @@ -82,6 +85,7 @@ impl HostScanner { let host_scanner = HostScanner { kernel_inode_map, inode_map, + scan_count: RefCell::new(0), paths, scan_interval, running, @@ -113,32 +117,45 @@ impl HostScanner { Ok(builder.build()?) } + fn new_scan(&self) -> u8 { + let mut scan_count = self.scan_count.borrow_mut(); + *scan_count += 1; + *scan_count + } + fn scan(&self) -> anyhow::Result<()> { info!("Host scan started"); let start = Instant::now(); self.metrics.scan_inc(ScanLabels::Scans); - let config = self.paths.borrow(); + + let scan_count = self.new_scan(); + + for pattern in self.paths.borrow().iter() { + let path = host_info::prepend_host_mount(pattern); + self.scan_inner(&path, scan_count)?; + } // Cleanup any items that are either: // * Not configured to be monitored anymore. // * Are configured to be monitored but no longer are found in // the file system. - self.inode_map.borrow_mut().retain(|inode, path| { - if config.iter().any(|prefix| path.starts_with(prefix)) - && host_info::prepend_host_mount(path).exists() - { - true - } else { - let _ = self.kernel_inode_map.borrow_mut().remove(inode); - self.metrics.scan_inc(ScanLabels::InodeRemoved); - false - } - }); - - for pattern in self.paths.borrow().iter() { - let path = host_info::prepend_host_mount(pattern); - self.scan_inner(&path)?; + // + // The scan happens inline with processing of events, so at this + // point all elements in the map that we saw during the scan + // should have the corresponding scan count set to the same + // value as the one we have now. Everything that has a different + // value can be assumed to have been removed or no longer + // monitored. + for (inode, _) in self + .inode_map + .borrow_mut() + .extract_if(|_, (_, c)| *c != scan_count) + { + // TODO: Turn this into a bulk operation. https://github.com/stackrox/fact/issues/1133 + let _ = self.kernel_inode_map.borrow_mut().remove(&inode); + self.metrics.scan_inc(ScanLabels::InodeRemoved); } + let duration = start.elapsed(); self.metrics.scan_duration.observe(duration.as_secs_f64()); info!( @@ -149,7 +166,7 @@ impl HostScanner { Ok(()) } - fn scan_inner(&self, path: &Path) -> anyhow::Result<()> { + fn scan_inner(&self, path: &Path, scan_count: u8) -> anyhow::Result<()> { self.metrics.scan_inc(ScanLabels::ElementsScanned); let Some(glob_str) = path.to_str() else { @@ -176,11 +193,11 @@ impl HostScanner { if metadata.is_file() { self.metrics.scan_inc(ScanLabels::FileScanned); - self.update_entry(path.as_path(), &metadata) + self.update_entry(path.as_path(), &metadata, scan_count) .with_context(|| format!("Failed to update entry for {}", path.display()))?; } else if metadata.is_dir() { self.metrics.scan_inc(ScanLabels::DirectoryScanned); - self.update_entry(path.as_path(), &metadata) + self.update_entry(path.as_path(), &metadata, scan_count) .with_context(|| format!("Failed to update entry for {}", path.display()))?; } else { self.metrics.scan_inc(ScanLabels::FsItemIgnored); @@ -189,34 +206,40 @@ impl HostScanner { Ok(()) } - fn update_entry(&self, path: &Path, metadata: &Metadata) -> anyhow::Result<()> { + fn update_entry(&self, path: &Path, metadata: &Metadata, scan_count: u8) -> anyhow::Result<()> { let inode = inode_key_t { inode: metadata.st_ino(), dev: metadata.st_dev(), }; let host_path = host_info::remove_host_mount(path); - self.update_entry_with_inode(inode, host_path)?; + self.update_entry_with_inode(inode, host_path, scan_count)?; debug!("Added entry for {}: {inode:?}", path.display()); Ok(()) } /// Similar to update_entry except we are are directly using the inode instead of the path. - fn update_entry_with_inode(&self, inode: inode_key_t, path: PathBuf) -> anyhow::Result<()> { + fn update_entry_with_inode( + &self, + inode: inode_key_t, + path: PathBuf, + scan_count: u8, + ) -> anyhow::Result<()> { let mut inode_map = self.inode_map.borrow_mut(); match inode_map.get_mut(&inode) { - Some(p) => { + Some((p, c)) => { // inode is already tracked. if path != *p { *p = path; self.metrics.scan_inc(ScanLabels::FileUpdated); } + *c = scan_count; return Ok(()); } None => { self.metrics.scan_inc(ScanLabels::FileUpdated); - inode_map.insert(inode, path.clone()); + inode_map.insert(inode, (path.clone(), scan_count)); } }; @@ -240,7 +263,7 @@ You can increase this limit with: fn get_host_path(&self, inode: Option<&inode_key_t>) -> Option { // The path here needs to be cloned because we won't keep the // inode_map borrow long enough. - self.inode_map.borrow().get(inode?).cloned() + self.inode_map.borrow().get(inode?).cloned().map(|(p, _)| p) } /// Handle file creation events by adding new inodes to the map. @@ -259,7 +282,7 @@ You can increase this limit with: && let Some(parent_host_path) = self.get_host_path(Some(parent_inode)) { let host_path = parent_host_path.join(filename); - self.update_entry_with_inode(*inode, host_path) + self.update_entry_with_inode(*inode, host_path, *self.scan_count.borrow()) .with_context(|| { format!( "Failed to add creation event entry for {}", @@ -310,7 +333,7 @@ You can increase this limit with: warn!("Rename event did not have old host path for inode tracked item"); return; }; - self.inode_map.borrow_mut().retain(|inode, path| { + self.inode_map.borrow_mut().retain(|inode, (path, _)| { if !path.starts_with(old_host_path) { return true; } @@ -339,7 +362,7 @@ You can increase this limit with: // 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 { + let Some((new_host_parent, _)) = inode_map.get(event.get_parent_inode()) else { warn!("Failed to get parent host path"); return; }; @@ -355,7 +378,7 @@ You can increase this limit with: 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.values_mut() { + for (path, _) in inode_map.values_mut() { if let Ok(suffix) = path.strip_prefix(old_host_path) { if suffix == Path::new("") { *path = new_host_path.clone(); @@ -369,7 +392,7 @@ You can increase this limit with: event.set_host_path(new_host_path); } else { // New path is not tracked, remove old entries - inode_map.retain(|inode, path| { + inode_map.retain(|inode, (path, _)| { if !path.starts_with(old_host_path) { return true; } @@ -391,7 +414,7 @@ You can increase this limit with: // 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) + && let Some((path, _)) = self.inode_map.borrow().get(old_inode) { event.set_host_path(path.clone()); }