ROX-33198: Instrument inode tracking on file open lsm hook by JoukoVirtanen · Pull Request #391 · stackrox/fact

JoukoVirtanen · 2026-03-15T18:13:46Z

Description

Host file paths need to be reported correctly. When a file is created in a tracked directory its inode should be added to a hash set in kernel space. In user space an entry needs to be added into a map with the inode as the key and file path as the value.

Checklist

Investigated and inspected CI test results
Updated documentation accordingly

Automated testing

Added unit tests
Added integration tests
Added regression tests

If any of these don't apply, please comment below.

Integration tests were modified.

Testing Performed

cat .cargo/config.toml
[build]
rustflags = ["-C", "force-frame-pointers=yes"]

[target.'cfg(all())']
runner = "sudo -E"

The following command was run in the main branch and this branch.

FACT_LOGLEVEL=debug cargo run --bin fact -- -p '/etc/sensitive-files/**/*' -p /etc/sensitive-files --expose-metrics

The following commands were also run

etc$ sudo mkdir sensitive-files
/etc/sensitive-files$ sudo touch hello_world.txt
/etc/sensitive-files$ sudo vi hello_world.txt

One of the tests was run with bye_world.txt instead.

In the main branch

{
  "timestamp": 1773981129521946307,
  "hostname": "jvirtane-thinkpadp1gen3.rmtuswa.csb",
  "process": {
    "comm": "vim",
    "args": [
      "/usr/bin/vim",
      "bye_world.txt"
    ],
    "exe_path": "/usr/bin/vim",
    "container_id": null,
    "uid": 0,
    "username": "root",
    "gid": 0,
    "login_uid": 4203274,
    "pid": 1252416,
    "in_root_mount_ns": true,
    "lineage": [
      {
        "uid": 4203274,
        "exe_path": "/usr/bin/sudo"
      },
      {
        "uid": 4203274,
        "exe_path": "/usr/bin/sudo"
      }
    ]
  },
  "file": {
    "Chmod": {
      "inner": {
        "filename": "/etc/sensitive-files/bye_world.txt",
        "host_file": "",
        "inode": {
          "inode": 0,
          "dev": 0
        }
      },
      "new_mode": 33188,
      "old_mode": 33188
    }
  }
}

Note that the host_file is blank.

In this branch

{
  "timestamp": 1773980922622536024,
  "hostname": "jvirtane-thinkpadp1gen3.rmtuswa.csb",
  "process": {
    "comm": "vim",
    "args": [
      "/usr/bin/vim",
      "hello_world.txt"
    ],
    "exe_path": "/usr/bin/vim",
    "container_id": null,
    "uid": 0,
    "username": "root",
    "gid": 0,
    "login_uid": 4203274,
    "pid": 1251608,
    "in_root_mount_ns": true,
    "lineage": [
      {
        "uid": 4203274,
        "exe_path": "/usr/bin/sudo"
      },
      {
        "uid": 4203274,
        "exe_path": "/usr/bin/sudo"
      }
    ]
  },
  "file": {
    "Chmod": {
      "inner": {
        "filename": "/etc/sensitive-files/hello_world.txt",
        "host_file": "/etc/sensitive-files/hello_world.txt",
        "inode": {
          "inode": 74960529,
          "dev": 37
        },
        "parent_inode": {
          "inode": 0,
          "dev": 0
        }
      },
      "new_mode": 33188,
      "old_mode": 33188
    }
  }
}

Note that host_file is now populated.

fact-ebpf/src/bpf/main.c

fact/src/host_scanner.rs

tests/test_inode_tracking.py

JoukoVirtanen · 2026-03-15T18:39:37Z

tests/test_inode_tracking.py

+    """
+    cwd = os.getcwd()
+    config = {
+        'paths': [f'{monitored_dir}/**', '/mounted/**', '/container-dir/**'],


I am not sure what globbing to use here.

fact-ebpf/src/bpf/main.c

fact/src/host_scanner.rs

tests/test_inode_tracking.py

JoukoVirtanen · 2026-03-16T18:27:30Z

fact-ebpf/src/bpf/inode.h

  }

-  unsigned long magic = inode->i_sb->s_magic;
+  unsigned long magic = BPF_CORE_READ(inode, i_sb, s_magic);


inode_to_key needs to be able to use untrusted pointers, so that it can get the key for a parent inode. To get it to be able to use untrusted pointers, BPF_CORE_READ is used.

fact-ebpf/src/bpf/main.c

fact-ebpf/src/bpf/events.h

JoukoVirtanen · 2026-03-20T05:26:48Z

tests/test_editors/test_nvim.py

    fut = '/mounted/test.txt'
+    fut_host = f'{ignored_dir}/test.txt'
    fut_backup = f'{fut}~'
+    fut_backup_host = f'{ignored_dir}/test.txt~'


fut_host and fut_backup_host should probably be blank.

JoukoVirtanen · 2026-03-20T05:27:33Z

tests/test_editors/test_sed.py

+def test_sed(vi_container, server, ignored_dir):
    # File Under Test
    fut = '/mounted/test.txt'
+    fut_host = f'{ignored_dir}/test.txt'


Same as above.

Molter73

There seems to be a lot of style changes, is this due to you using a newer Rust version? Can you check what version you are using? Is it 1.92+? If this is due to a version difference and yours is newer, can you open a new PR with those style changes? that way we can focus this PR on the actual inode tracking.

Molter73 · 2026-03-20T11:38:43Z

There seems to be a lot of style changes, is this due to you using a newer Rust version? Can you check what version you are using? Is it 1.92+? If this is due to a version difference and yours is newer, can you open a new PR with those style changes? that way we can focus this PR on the actual inode tracking.

Actually, on a second read-through, it might also be related to the bump to version 2024, if so, please move the bump to a separate PR and we can stack this one on top of that one, sorry for the inconvenience!

Molter73

I still need to go through the integration test changes, but I see them failing in the PR, do you need help debugging the failures?

Molter73 · 2026-03-20T11:46:18Z

fact/src/host_scanner.rs

                        self.update_entry(path.as_path()).with_context(|| {
                            format!("Failed to update entry for {}", path.display())
                        })?;
+                    } else if path.is_dir() {


This is slightly related to a conversation I've been having with @Stringy. IMO, in the case of finding a directory that matches a glob pattern, we should recursively scan into it, the reason being that tracking the directory itself isn't of much value if the pattern doesn't hit any files inside the directory.

For now we can leave it like this, but I think we need to take a close look at what users might expect when configuring different types of glob patterns (i.e do they want that strict set of patterns to be monitored, or are they expressing a set of paths that work as the base to be monitored?)

Molter73 · 2026-03-20T11:57:07Z

fact/src/host_scanner.rs

+    /// We use the parent inode provided by the eBPF code
+    /// to look up the parent directory's host path, then construct the full
+    /// path by appending the new file's name.
+    fn handle_creation_event(&self, event: &Event) -> anyhow::Result<()> {


I'm hoping this is still WIP and the debug statements are for testing, we really don't need a debug statement on each return condition, particularly not in the hot path where monitoring sets of files mildly noisy will just flood the logs and make it hard to see what is actually going on.

Please cut down on the debug statements, if something is expected to happen, don't put a statement there.

I tried removing all the debug statements in this method and managed to boil it down from 50+ lines to just this:

let inode = event.get_inode(); let parent_inode = event.get_parent_inode(); if self.get_host_path(Some(inode)).is_some() || parent_inode.empty() { return Ok(()); } if let Some(filename) = event.get_filename().file_name() && 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) .with_context(|| { format!( "Failed to add creation event entry for {}", filename.display() ) })?; } Ok(())

Given this is a lot more compact and easy to ready and the additional verbosity in the logs the implementation in the PR doesn't bring much of a benefit IMO, I would propose we try to move the implementation closer to this.

I have made this change, except I was not able to combine the if statements because the edition is still 2021.

The PR bumping the edition is already in the main branch, you can rebase and get this working.

JoukoVirtanen · 2026-03-23T03:54:04Z

I still need to go through the integration test changes, but I see them failing in the PR, do you need help debugging the failures?

The CI tests are now passing. I think there might be a flake involving unlinking, but that might need to be fixed in the future PR for the file_unlink LSM hook.

…er space

… can now use untrusted pointers

…et the inode and then add to the map

…y times

Molter73 · 2026-03-23T14:39:21Z

fact-ebpf/src/bpf/file.h

Could we unify the logic for checking if an inode is monitored in inode_is_monitored? Just have that method take both the inode and parent_inode, then return one of MONITORED, NOT_MONITORED or PARENT_MONITRED values? I think that might be a bit cleaner and we don't need a dedicated _with_parent function.

Molter73 · 2026-03-23T14:54:37Z

fact/src/host_scanner.rs

+    }
+
+    /// 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<()> {


I don't think we need to take inode by reference here, if anything, it is causing the call from update_entry to create the inode and then copy it inside here, meaning we copy the data twice. On the other hand, the other usage in handle_creation_event gets a reference to an inode_key_t because it gets if from event.get_inode(), if anything, handle_creation_event should create the copy explicitly instead.

Molter73 · 2026-03-23T15:15:51Z

tests/conftest.py

+            f'{monitored_dir}',
+            f'{monitored_dir}/**/*',
+            '/mounted',
+            '/mounted/**/*',
+            '/container-dir',
+            '/container-dir/**/*',


Can you explain why we need these many monitored paths? Is the one with wildcards not hitting the parent directory?

JoukoVirtanen requested a review from Molter73 March 15, 2026 18:14

JoukoVirtanen commented Mar 15, 2026

View reviewed changes

fact-ebpf/src/bpf/main.c Outdated Show resolved Hide resolved

JoukoVirtanen commented Mar 15, 2026

View reviewed changes

fact/src/host_scanner.rs Outdated Show resolved Hide resolved

JoukoVirtanen commented Mar 15, 2026

View reviewed changes

tests/test_inode_tracking.py Outdated Show resolved Hide resolved

JoukoVirtanen commented Mar 15, 2026

View reviewed changes

Molter73 reviewed Mar 16, 2026

View reviewed changes

JoukoVirtanen commented Mar 16, 2026

View reviewed changes

Molter73 reviewed Mar 17, 2026

View reviewed changes

fact-ebpf/src/bpf/main.c Outdated Show resolved Hide resolved

JoukoVirtanen commented Mar 18, 2026

View reviewed changes

fact-ebpf/src/bpf/events.h Show resolved Hide resolved

JoukoVirtanen requested a review from Molter73 March 20, 2026 05:22

JoukoVirtanen marked this pull request as ready for review March 20, 2026 05:23

JoukoVirtanen requested review from a team and rhacs-bot as code owners March 20, 2026 05:23

JoukoVirtanen commented Mar 20, 2026

View reviewed changes

Molter73 reviewed Mar 20, 2026

View reviewed changes

JoukoVirtanen force-pushed the jv-ROX-33198-instrument-inode-tracking-on-file_open-lsm-hook branch from 8fc3c95 to 4f1686a Compare March 20, 2026 17:31

JoukoVirtanen requested a review from Molter73 March 23, 2026 03:45

JoukoVirtanen added 7 commits March 23, 2026 07:39

X-Smart-Branch-Parent: main

7b92eab

When a monitored file is created it is added to maps in kernel and us…

0d9cf75

…er space

Using inode_to_key directly instead of duplicating code. inode_to_key…

b0113c2

… can now use untrusted pointers

Parent inode is added to events. That is used to get the correct path

2bf0f3d

Using the inode directly to add to map instead of using the path to g…

1b64a72

…et the inode and then add to the map

Added host path to tests

35462c4

Only the open file event returns the parent inode, the rest return null

411c115

JoukoVirtanen added 11 commits March 23, 2026 07:39

Removed blank space

3b09253

Fixed formatting errors

f37b40b

Refactored update_entry_with_inode and update_entry

283f50a

Removed tests/test_inode_tracking.py

600d08c

Removed debugging and refactored handle_creation_event

1d1e383

Not expecting host_path for ignored directories

65f53a0

Fixed style. Removed unused ignored_dir

950c402

Logic for checking if a file should be monitored moved to is_monitored

a8471b0

Refactored so that NULL doesn't have to be passed to is_monitored man…

5e145ca

…y times

Removed unneeded TODOs

0184180

Fixed format after rebasing and updating edition to 2024

2b139ff

JoukoVirtanen force-pushed the jv-ROX-33198-instrument-inode-tracking-on-file_open-lsm-hook branch from cd4df3f to 2b139ff Compare March 23, 2026 14:46

Molter73 reviewed Mar 23, 2026

View reviewed changes

Collapsed if statements

93dd1d8

Conversation

JoukoVirtanen commented Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Testing Performed

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Molter73 left a comment

Choose a reason for hiding this comment

Uh oh!

Molter73 commented Mar 20, 2026

Uh oh!

Molter73 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoukoVirtanen commented Mar 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JoukoVirtanen commented Mar 15, 2026 •

edited

Loading