ROX-33216: implement inode tracking for path_rename

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);
 }

fact-ebpf/src/bpf/file.h

@@ -27,34 +27,33 @@ __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;
 }
 
 // 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;

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;

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);
 

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

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<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    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 };

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;
                                     }