Skip to content

fix(bpf): disable preemption during hook processing - #1569

Open
Molter73 wants to merge 3 commits into
mainfrom
mauro/feat/disable-preemption
Open

fix(bpf): disable preemption during hook processing #1569
Molter73 wants to merge 3 commits into
mainfrom
mauro/feat/disable-preemption

Conversation

@Molter73

@Molter73 Molter73 commented Aug 25, 2026

Copy link
Copy Markdown
Member

Description

This prevents the hooks from being preempted and replaced by another hook that might overwrite the contents of the shared per-CPU maps before sending the event to the ringbuffer. A similar issue was found to be corrupting events in Falco and is addressed in this PR of our fork, see the discussion there for more details: stackrox/falcosecurity-libs#98

In order to call bpf_preempt_enable/disable aya needs to support ksyms, which is currently only supported on main, therefore we pin the dependency to a commit known to work. We will unpin this once a new aya release is created.

As a final simplification, to prevent new hooks and branches in existing one from having to remember to re-enable preemption before exiting some macro magic is used. We can potentially leverage these macros to define some other common patterns in the future.

Checklist

  • Patch has a change log entry OR does not need one.
  • 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.

Testing Performed

CI should be enough.

Summary by CodeRabbit

  • Bug Fixes

    • Improved compatibility with updated kernel interfaces and map operations.
    • Enhanced reliability across file, path, inode, mount, rename, directory, symlink, instantiation, extended-attribute, and access-control event handling.
    • Preserved existing event tracking and error handling across supported monitoring hooks.
    • Improved operation across environments with differing kernel capabilities.
  • Refactor

    • Standardized monitoring hook setup while preserving existing behavior.

@Molter73
Molter73 requested review from a team and rhacs-bot as code owners August 25, 2026 10:10
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Enterprise

Run ID: f19444c1-014e-4abb-a6c8-8646562a4ba4

📥 Commits

Reviewing files that changed from the base of the PR and between 26c7079 and 2371024.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • Cargo.toml
  • fact-ebpf/src/bpf/main.c
  • fact/src/bpf/mod.rs
  • fact/src/host_scanner.rs

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The PR pins aya to a Git revision, updates map insertion calls for reference-based APIs, and replaces manual LSM BPF entry points with generated wrappers that preserve existing handler logic and preemption guards.

Changes

AYA and LSM compatibility

Layer / File(s) Summary
AYA revision and map API alignment
Cargo.toml, fact/src/bpf/mod.rs, fact/src/host_scanner.rs
The workspace pins aya to a Git revision. LPM trie and inode map insertions now pass references to their arguments.
Generated LSM wrapper macros
fact-ebpf/src/bpf/main.c
Variadic macros generate typed LSM entry points, inline handlers, preemption guards, and shared return paths.
LSM hook migration
fact-ebpf/src/bpf/main.c
File, path, inode, mount, rename, directory, symlink, instantiation, xattr, and ACL hooks use the generated wrappers. Existing event processing, metrics handling, null handling, and error behavior remain in place.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: joukovirtanen

Merge Risk: ⚪ Minimal · up to 23710

The reviewed compatibility updates and generated hook migration do not leave an identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary change: disabling BPF preemption during hook processing.
Description check ✅ Passed The description explains the purpose, implementation approach, dependency pin, checklist status, and testing approach. Unchecked documentation and automated-test items are acceptable because the autho…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch mauro/feat/disable-preemption

Comment @coderabbitai help to get the list of available commands.

@Molter73 Molter73 changed the title Mauro/feat/disable preemption fix(bpf): disable preemption during hook processing Aug 25, 2026
@codecov-commenter

codecov-commenter commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 33.40%. Comparing base (26c7079) to head (2371024).

Files with missing lines Patch % Lines
fact/src/bpf/mod.rs 0.00% 1 Missing ⚠️
fact/src/host_scanner.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1569   +/-   ##
=======================================
  Coverage   33.40%   33.40%           
=======================================
  Files          22       22           
  Lines        3628     3628           
  Branches     3628     3628           
=======================================
  Hits         1212     1212           
  Misses       2407     2407           
  Partials        9        9           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@erthalion

Copy link
Copy Markdown
Contributor

Can you be more specific about what shared per-CPU maps could be affected?

@Molter73

Molter73 commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

Can you be more specific about what shared per-CPU maps could be affected?

  • The helper map, which is used capturing the process cgroup and reading paths with our custom d_path implementation:
    struct {
    __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
    __type(key, __u32);
    __type(value, struct helper_t);
    __uint(max_entries, 1);
    } helper_map SEC(".maps");
  • The bound_path heap, also used for reading paths via d_path and bpf_d_path:
    struct {
    __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
    __type(key, __u32);
    __type(value, struct bound_path_t);
    __uint(max_entries, 2);
    } bound_path_heap SEC(".maps");
  • Not critical, but metrics might show some bogus information if some event counter is loaded in cache and it not correctly updated between preemptions:
    struct {
    __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
    __type(key, __u32);
    __type(value, struct metrics_t);
    __uint(max_entries, 1);
    } metrics SEC(".maps");

This prevents the hooks from being preempted and replaced by another
hook that might overwrite the contents of the shared per-CPU maps before
sending the event to the ringbuffer. A similar issue was found to be
corrupting events in Falco and is addressed in this PR of our fork, see
the discussion there for more details: stackrox/falcosecurity-libs#98

In order to call `bpf_preempt_enable/disable` aya needs to support
ksyms, which is currently only supported on main, therefore we pin the
dependency to a commit known to work. We will unpin this once a new aya
release is created.
These kfuncs where added in kernel version 6.12 and causes verifier
issues on RHCOS 4.16 and 4.18 on our CI.
@Molter73
Molter73 force-pushed the mauro/feat/disable-preemption branch from 61cebd7 to 2371024 Compare September 11, 2026 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants