Skip to content

ROX-33218: Instrument inode tracking on directory being deleted #530

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JoukoVirtanen wants to merge 12 commits into
base: main
Choose a base branch
from
Open

ROX-33218: Instrument inode tracking on directory being deleted #530

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
861b4eb
X-Smart-Branch-Parent: main
JoukoVirtanen Apr 20, 2026
0279e75
Added tests. Test check for both events and metrics. In the future th…
JoukoVirtanen Apr 16, 2026
19bd586
Tests pass now
JoukoVirtanen Apr 16, 2026
b5366ed
Minor change to logging
JoukoVirtanen Apr 17, 2026
d9e5b2d
Added get_metric_value helper function
JoukoVirtanen Apr 17, 2026
5318c90
Checking kernel_path_rmdir_events metrics
JoukoVirtanen Apr 18, 2026
13ec3fa
Using rm instead shutils.rmtree
JoukoVirtanen Apr 18, 2026
29dd966
Not sending directory deletion events
JoukoVirtanen Apr 18, 2026
87859e9
Tests do not expect directory deletion events
JoukoVirtanen Apr 18, 2026
85818e3
Cleaned up some comments
JoukoVirtanen Apr 18, 2026
cd1a06a
Improved one logging statement
JoukoVirtanen Apr 18, 2026
d35aa7c
Removed unneeded comment
JoukoVirtanen Apr 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions fact-ebpf/src/bpf/events.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,16 @@ __always_inline static void submit_mkdir_event(struct metrics_by_hook_t* m,
// d_instantiate doesn't support bpf_d_path, so we use false and rely on the stashed path from path_mkdir
__submit_event(event, m, DIR_ACTIVITY_CREATION, filename, inode, parent_inode, false);
}

__always_inline static void submit_rmdir_event(struct metrics_by_hook_t* m,
const char filename[PATH_MAX],
inode_key_t* inode,
inode_key_t* parent_inode) {
struct event_t* event = bpf_ringbuf_reserve(&rb, sizeof(struct event_t), 0);
if (event == NULL) {
m->ringbuffer_full++;
return;
}

__submit_event(event, m, DIR_ACTIVITY_UNLINK, filename, inode, parent_inode, path_hooks_support_bpf_d_path);
}
33 changes: 33 additions & 0 deletions fact-ebpf/src/bpf/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,3 +326,36 @@ int BPF_PROG(trace_d_instantiate, struct dentry* dentry, struct inode* inode) {
bpf_map_delete_elem(&mkdir_context, &pid_tgid);
return 0;
}

SEC("lsm/path_rmdir")
int BPF_PROG(trace_path_rmdir, struct path* dir, struct dentry* dentry) {
struct metrics_t* m = get_metrics();
if (m == NULL) {
return 0;
}

m->path_rmdir.total++;

struct bound_path_t* path = path_read_append_d_entry(dir, dentry);
if (path == NULL) {
bpf_printk("Failed to read directory path");
m->path_rmdir.error++;
return 0;
}

inode_key_t inode_key = inode_to_key(dentry->d_inode);
inode_key_t* inode_to_submit = &inode_key;

if (is_monitored(inode_key, path, NULL, &inode_to_submit) == NOT_MONITORED) {
Copy link
Copy Markdown
Contributor Author

@JoukoVirtanen JoukoVirtanen Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This follows the same pattern as file unlinking, but I wonder if both could be simplified. I don't think we need to check is_monitored. I think it is probably better to just directly call inode_remove. If the inode is in the map then it needs to be removed. There could potentially be a bug where is_monitored return NOT_MONITORED, but the inode is in the map and needs to be removed. If the inode is in the map then removing it does no harm. inode_remove reports if the inode was in the map and that information can be used to increment the metric if needed.

m->path_rmdir.ignored++;
return 0;
}

inode_remove(&inode_key);

submit_rmdir_event(&m->path_rmdir,
path->path,
inode_to_submit,
NULL);
return 0;
}
2 changes: 2 additions & 0 deletions fact-ebpf/src/bpf/types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ typedef enum file_activity_type_t {
FILE_ACTIVITY_CHOWN,
FILE_ACTIVITY_RENAME,
DIR_ACTIVITY_CREATION,
DIR_ACTIVITY_UNLINK,
} file_activity_type_t;

struct event_t {
Expand Down Expand Up @@ -120,4 +121,5 @@ struct metrics_t {
struct metrics_by_hook_t path_rename;
struct metrics_by_hook_t path_mkdir;
struct metrics_by_hook_t d_instantiate;
struct metrics_by_hook_t path_rmdir;
};
1 change: 1 addition & 0 deletions fact-ebpf/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ impl metrics_t {
m.path_chown = m.path_chown.accumulate(&other.path_chown);
m.path_rename = m.path_rename.accumulate(&other.path_rename);
m.path_mkdir = m.path_mkdir.accumulate(&other.path_mkdir);
m.path_rmdir = m.path_rmdir.accumulate(&other.path_rmdir);
m.d_instantiate = m.d_instantiate.accumulate(&other.d_instantiate);
m
}
Expand Down
17 changes: 16 additions & 1 deletion fact/src/event/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,12 @@ impl Event {
matches!(self.file, FileData::MkDir(_))
}

pub fn is_rmdir(&self) -> bool {
matches!(self.file, FileData::RmDir(_))
}

pub fn is_unlink(&self) -> bool {
matches!(self.file, FileData::Unlink(_))
matches!(self.file, FileData::Unlink(_) | FileData::RmDir(_))
}

/// Unwrap the inner FileData and return the inode that triggered
Expand All @@ -148,6 +152,7 @@ impl Event {
FileData::Open(data) => &data.inode,
FileData::Creation(data) => &data.inode,
FileData::MkDir(data) => &data.inode,
FileData::RmDir(data) => &data.inode,
FileData::Unlink(data) => &data.inode,
FileData::Chmod(data) => &data.inner.inode,
FileData::Chown(data) => &data.inner.inode,
Expand All @@ -161,6 +166,7 @@ impl Event {
FileData::Open(data) => &data.parent_inode,
FileData::Creation(data) => &data.parent_inode,
FileData::MkDir(data) => &data.parent_inode,
FileData::RmDir(data) => &data.parent_inode,
FileData::Unlink(data) => &data.parent_inode,
FileData::Chmod(data) => &data.inner.parent_inode,
FileData::Chown(data) => &data.inner.parent_inode,
Expand All @@ -183,6 +189,7 @@ impl Event {
FileData::Open(data) => &data.filename,
FileData::Creation(data) => &data.filename,
FileData::MkDir(data) => &data.filename,
FileData::RmDir(data) => &data.filename,
FileData::Unlink(data) => &data.filename,
FileData::Chmod(data) => &data.inner.filename,
FileData::Chown(data) => &data.inner.filename,
Expand All @@ -202,6 +209,7 @@ impl Event {
FileData::Open(data) => &data.host_file,
FileData::Creation(data) => &data.host_file,
FileData::MkDir(data) => &data.host_file,
FileData::RmDir(data) => &data.host_file,
FileData::Unlink(data) => &data.host_file,
FileData::Chmod(data) => &data.inner.host_file,
FileData::Chown(data) => &data.inner.host_file,
Expand All @@ -218,6 +226,7 @@ impl Event {
FileData::Open(data) => data.host_file = host_path,
FileData::Creation(data) => data.host_file = host_path,
FileData::MkDir(data) => data.host_file = host_path,
FileData::RmDir(data) => data.host_file = host_path,
FileData::Unlink(data) => data.host_file = host_path,
FileData::Chmod(data) => data.inner.host_file = host_path,
FileData::Chown(data) => data.inner.host_file = host_path,
Expand Down Expand Up @@ -303,6 +312,7 @@ pub enum FileData {
Open(BaseFileData),
Creation(BaseFileData),
MkDir(BaseFileData),
RmDir(BaseFileData),
Unlink(BaseFileData),
Chmod(ChmodFileData),
Chown(ChownFileData),
Expand All @@ -322,6 +332,7 @@ impl FileData {
file_activity_type_t::FILE_ACTIVITY_OPEN => FileData::Open(inner),
file_activity_type_t::FILE_ACTIVITY_CREATION => FileData::Creation(inner),
file_activity_type_t::DIR_ACTIVITY_CREATION => FileData::MkDir(inner),
file_activity_type_t::DIR_ACTIVITY_UNLINK => FileData::RmDir(inner),
file_activity_type_t::FILE_ACTIVITY_UNLINK => FileData::Unlink(inner),
file_activity_type_t::FILE_ACTIVITY_CHMOD => {
let data = ChmodFileData {
Expand Down Expand Up @@ -373,6 +384,9 @@ impl From<FileData> for fact_api::file_activity::File {
FileData::MkDir(_) => {
unreachable!("MkDir event reached protobuf conversion");
}
FileData::RmDir(_) => {
unreachable!("RmDir event reached protobuf conversion");
}
FileData::Unlink(event) => {
let activity = Some(fact_api::FileActivityBase::from(event));
let f_act = fact_api::FileUnlink { activity };
Expand Down Expand Up @@ -401,6 +415,7 @@ impl PartialEq for FileData {
(FileData::Open(this), FileData::Open(other)) => this == other,
(FileData::Creation(this), FileData::Creation(other)) => this == other,
(FileData::MkDir(this), FileData::MkDir(other)) => this == other,
(FileData::RmDir(this), FileData::RmDir(other)) => this == other,
(FileData::Unlink(this), FileData::Unlink(other)) => this == other,
(FileData::Chmod(this), FileData::Chmod(other)) => this == other,
(FileData::Rename(this), FileData::Rename(other)) => this == other,
Expand Down
4 changes: 2 additions & 2 deletions fact/src/host_scanner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,8 @@ impl HostScanner {
self.handle_unlink_event(&event);
}

// Skip directory creation events - we track them internally but don't send to sensor
if event.is_mkdir() {
// Skip directory creation and deletion events - we track them internally but don't send to sensor
if event.is_mkdir() || event.is_rmdir() {
continue;
}

Expand Down
9 changes: 9 additions & 0 deletions fact/src/metrics/kernel_metrics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pub struct KernelMetrics {
path_chown: EventCounter,
path_rename: EventCounter,
path_mkdir: EventCounter,
path_rmdir: EventCounter,
d_instantiate: EventCounter,
map: PerCpuArray<MapData, metrics_t>,
}
Expand Down Expand Up @@ -50,6 +51,11 @@ impl KernelMetrics {
"Events processed by the path_mkdir LSM hook",
&[], // Labels are not needed since `collect` will add them all
);
let path_rmdir = EventCounter::new(
"kernel_path_rmdir_events",
"Events processed by the path_rmdir LSM hook",
&[], // Labels are not needed since `collect` will add them all
);
let d_instantiate = EventCounter::new(
"kernel_d_instantiate_events",
"Events processed by the d_instantiate LSM hook",
Expand All @@ -62,6 +68,7 @@ impl KernelMetrics {
path_chown.register(reg);
path_rename.register(reg);
path_mkdir.register(reg);
path_rmdir.register(reg);
d_instantiate.register(reg);

KernelMetrics {
Expand All @@ -71,6 +78,7 @@ impl KernelMetrics {
path_chown,
path_rename,
path_mkdir,
path_rmdir,
d_instantiate,
map: kernel_metrics,
}
Expand Down Expand Up @@ -122,6 +130,7 @@ impl KernelMetrics {
KernelMetrics::refresh_labels(&self.path_chown, &metrics.path_chown);
KernelMetrics::refresh_labels(&self.path_rename, &metrics.path_rename);
KernelMetrics::refresh_labels(&self.path_mkdir, &metrics.path_mkdir);
KernelMetrics::refresh_labels(&self.path_rmdir, &metrics.path_rmdir);
KernelMetrics::refresh_labels(&self.d_instantiate, &metrics.d_instantiate);

Ok(())
Expand Down
Loading
Loading