Add comments, use PATH_LEN_CLAMP and restore a missing stop condition

Added comments to help clarify some of the logic around d_path.

Used PATH_LEN_CLAMP instead of raw `& (PATH_MAX -1)` operations,
documenting the macro itself to make its purpose more obvious.

Restored the d_path stop condition of reaching the root of the current
task fs, which was accidentally removed when moving from the original
implementation.

Author: Molter73

fact-ebpf/src/bpf/bound_path.h

@@ -11,11 +11,8 @@
 #include <bpf/bpf_core_read.h>
 // clang-format on
 
-#define PATH_MAX_MASK (PATH_MAX - 1)
-#define path_len_clamp(len) ((len) & PATH_MAX_MASK)
-
 __always_inline static char* path_safe_access(char* p, unsigned int offset) {
-  return &p[path_len_clamp(offset)];
+  return &p[PATH_LEN_CLAMP(offset)];
 }
 
 __always_inline static void path_write_char(char* p, unsigned int offset, char c) {
@@ -34,7 +31,7 @@ __always_inline static struct bound_path_t* _path_read(struct path* path, bool u
   }
 
   // Ensure length is within PATH_MAX for the verifier
-  bound_path->len = path_len_clamp(bound_path->len);
+  bound_path->len = PATH_LEN_CLAMP(bound_path->len);
 
   return bound_path;
 }
@@ -63,7 +60,7 @@ __always_inline static enum path_append_status_t path_append_dentry(struct bound
   }
 
   char* path_offset = path_safe_access(path->path, path->len);
-  if (bpf_probe_read_kernel(path_offset, path_len_clamp(len), d_name.name)) {
+  if (bpf_probe_read_kernel(path_offset, PATH_LEN_CLAMP(len), d_name.name)) {
     return PATH_APPEND_READ_ERROR;
   }
 

fact-ebpf/src/bpf/d_path.h

@@ -9,9 +9,24 @@
 #include <bpf/bpf_core_read.h>
 // clang-format on
 
+/**
+ * PATH_MAX is defined in the kernel as 4096 which translates to 0x1000.
+ * This define gives as an easy way to clamp path lengths
+ */
+#define PATH_MAX_MASK (PATH_MAX - 1)
+
+/**
+ * Helper for keeping the verifier happy.
+ *
+ * Whenever a path is interacted with in a buffer, this macro can be
+ * used to convince the verifier no more than PATH_MAX bytes will be
+ * accessed.
+ */
+#define PATH_LEN_CLAMP(len) ((len) & PATH_MAX_MASK)
+
 struct d_path_ctx {
   struct helper_t* helper;
-  struct path root;
+  struct path* root;
   struct mount* mnt;
   struct dentry* dentry;
   int offset;
@@ -26,24 +41,42 @@ static long __d_path_inner(uint32_t index, void* _ctx) {
   struct mount* mnt = ctx->mnt;
   struct dentry* mnt_root = BPF_CORE_READ(mnt, mnt.mnt_root);
 
+  if (dentry == ctx->root->dentry && &mnt->mnt == ctx->root->mnt) {
+    // Found the root of the process, we are done
+    ctx->success = true;
+    return 1;
+  }
+
   if (dentry == mnt_root) {
     struct mount* m = BPF_CORE_READ(mnt, mnt_parent);
     if (m != mnt) {
+      // Current dentry is a mount root different to the previous one we
+      // had (to prevent looping), switch over to that mount position
+      // and keep walking up the path.
       ctx->dentry = BPF_CORE_READ(mnt, mnt_mountpoint);
       ctx->mnt = m;
       return 0;
     }
+
+    // Ended up in a global root, the path might need re-processing or
+    // the root is not attached yet, we are not getting a better path,
+    // so we assume we are correct and stop iterating.
     ctx->success = true;
     return 1;
   }
 
   if (dentry == parent) {
+    // We escaped the mounts and ended up at (most likely) the root of
+    // the device, the path we formed will be wrong.
+    //
+    // This may happen in race conditions where some dentries go away
+    // while we are iterating.
     return 1;
   }
 
   struct qstr d_name;
   BPF_CORE_READ_INTO(&d_name, dentry, d_name);
-  int len = d_name.len & (PATH_MAX - 1);
+  int len = PATH_LEN_CLAMP(d_name.len);
   if (len <= 0 || len >= ctx->buflen) {
     return 1;
   }
@@ -52,7 +85,7 @@ static long __d_path_inner(uint32_t index, void* _ctx) {
   if (offset <= 0) {
     return 1;
   }
-  offset &= PATH_MAX - 1;
+  offset = PATH_LEN_CLAMP(offset);
 
   if (bpf_probe_read_kernel(&ctx->helper->buf[offset], len, d_name.name) != 0) {
     return 1;
@@ -80,7 +113,7 @@ __always_inline static long __d_path(const struct path* path, char* buf, int buf
     return -1;
   }
 
-  int offset = (buflen - 1) & (PATH_MAX - 1);
+  int offset = PATH_LEN_CLAMP(buflen - 1);
   struct d_path_ctx ctx = {
       .buflen = buflen,
       .helper = get_helper(),
@@ -91,10 +124,10 @@ __always_inline static long __d_path(const struct path* path, char* buf, int buf
     return -1;
   }
 
-  struct task_struct* task = (struct task_struct*)bpf_get_current_task();
+  struct task_struct* task = (struct task_struct*)bpf_get_current_task_btf();
   ctx.helper->buf[offset] = '\0';  // Ensure null termination
 
-  BPF_CORE_READ_INTO(&ctx.root, task, fs, root);
+  ctx.root = &task->fs->root;
   ctx.mnt = container_of(path->mnt, struct mount, mnt);
   BPF_CORE_READ_INTO(&ctx.dentry, path, dentry);
 
@@ -103,7 +136,7 @@ __always_inline static long __d_path(const struct path* path, char* buf, int buf
     return -1;
   }
 
-  bpf_probe_read_str(buf, buflen, &ctx.helper->buf[ctx.offset & (PATH_MAX - 1)]);
+  bpf_probe_read_str(buf, buflen, &ctx.helper->buf[PATH_LEN_CLAMP(ctx.offset)]);
   return buflen - ctx.offset;
 }
 

fact-ebpf/src/bpf/types.h

@@ -6,8 +6,13 @@
  * other sources of bloat into this file.
  */
 
+/**
+ * Kernel constant, taken from:
+ * https://github.com/torvalds/linux/blob/f0b9d8eb98dfee8d00419aa07543bdc2c1a44fb1/include/uapi/linux/limits.h#L13
+ */
 #define PATH_MAX 4096
 #define TASK_COMM_LEN 16
+
 #define LINEAGE_MAX 2
 
 #define LPM_SIZE_MAX 256