Skip to content

Conversation

@ekoops
Copy link
Contributor

@ekoops ekoops commented Sep 25, 2025

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind test

/kind feature

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area API-version

/area build

/area CI

/area driver-kmod

/area driver-bpf

/area driver-modern-bpf

/area libscap-engine-bpf

/area libscap-engine-gvisor

/area libscap-engine-kmod

/area libscap-engine-modern-bpf

/area libscap-engine-nodriver

/area libscap-engine-noop

/area libscap-engine-source-plugin

/area libscap-engine-savefile

/area libscap

/area libpman

/area libsinsp

/area tests

/area proposals

Does this PR require a change in the driver versions?

/version driver-API-version-major

/version driver-API-version-minor

/version driver-API-version-patch

/version driver-SCHEMA-version-major

/version driver-SCHEMA-version-minor

/version driver-SCHEMA-version-patch

What this PR does / why we need it:

This PR makes enter events related to TOCTOU mitigation managed by the scap converter. All old event versions are converter to their new corresponding versions, and the sinsp code is updated to support the new possibly-empty parameters.
Moreover, this PR adds the new scap converter action C_ACTION_STORE_AND_SKIP. This new action code allows to state the intention that a copy of the event should be stored in the scap converter's internal event storage and, at the same time, that another copy of the event should proceed to the upper layers.
As stated in the code, the current implementation of C_ACTION_STORE_AND_SKIP is identical to the C_ACTION_STORE one. However, in the future, C_ACTION_STORE will return something to indicate that the event should be dropped, while C_ACTION_STORE_AND_SKIP will return CONVERSION_SKIP (or whatever is the name we choose to indicate that the event should proceed... Maybe we can reuse CONVERSION_COMPLETE).
Finally, this PR adds some scap converter tests for the new scap converter table entries.
Notice that PPME_SOCKET_CONNECT_E was already flagged as EF_TMP_CONVERTER_MANAGED: for this event type, the PR just update the scap converter table's entry for the latest version (the one with two parameters) to use the new action code C_ACTION_STORE_AND_SKIP.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Not bumping the driver schema version is currently a practice we are following as we will bump it in a single shot once we are done with the #2068 proposal.

/milestone 0.22.0

Does this PR introduce a user-facing change?:

feat!: make `PPME_SYSCALL_OPENAT_2_{E,X}` "scap converter"-manage
feat!: make `PPME_SYSCALL_OPENAT2_{E,X}` "scap converter"-managed
feat!: make `PPME_SYSCALL_CREAT_{E,X}` "scap converter"-managed
feat!: make `PPME_SYSCALL_OPEN_{E,X}` "scap converter"-managed

Add `C_ACTION_STORE_AND_SKIP` converter action to indicate that a
copy of the event should be stored in the scap converter's internal
event storage and, at the same time, that another copy of the event
should proceed to the upper layers. As stated in the code, the current
implementation is identical to the `C_ACTION_STORE` one. However, in
the future, `C_ACTION_STORE` will return something to indicate that
the event should be dropped, while `C_ACTION_STORE_AND_SKIP` will
return `CONVERSION_SKIP` (or whatever is the name we choose to
indicate that the event should proceed... Maybe we can reuse
`CONVERSION_COMPLETE`). Update the implementation once we are ready to
change the `C_ACTION_STORE` logic.

Signed-off-by: Leonardo Di Giovanna <[email protected]>
Use the scap converter action `C_ACTION_STORE_AND_SKIP` to state the
intention to store events with type `PPME_SOCKET_CONNECT_E` and 2
parameters in the scap converter's internal event storage and, at the
same time, send an event copy to the upper layers. This is a refactor
and not a feature because, currently, `C_ACTION_STORE_AND_SKIP`
implementation is equivalent to `C_ACTION_STORE`.

Signed-off-by: Leonardo Di Giovanna <[email protected]>
BREAKING CHANGE: `PPME_SYSCALL_OPEN_{E,X}` evts with old layouts not
  delivered anymore to sinsp

Signed-off-by: Leonardo Di Giovanna <[email protected]>
BREAKING CHANGE: `PPME_SYSCALL_CREAT_{E,X}` evts with old layouts not
      delivered anymore to sinsp

Signed-off-by: Leonardo Di Giovanna <[email protected]>
BREAKING CHANGE: `PPME_SYSCALL_OPENAT2_{E,X}` evts with old layouts
  not delivered anymore to sinsp

Signed-off-by: Leonardo Di Giovanna <[email protected]>
BREAKING CHANGE: `PPME_SYSCALL_OPENAT_2_{E,X}` evts with old layouts
  not delivered anymore to sinsp

Signed-off-by: Leonardo Di Giovanna <[email protected]>
@poiana poiana added this to the 0.22.0 milestone Sep 25, 2025
@poiana poiana requested a review from hbrueckner September 25, 2025 15:49
@poiana poiana added the kind/test PRs increasing the test coverage without fixing any failing test label Sep 25, 2025
@poiana poiana requested a review from irozzo-1A September 25, 2025 15:49
@poiana
Copy link
Contributor

poiana commented Sep 25, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ekoops

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@github-actions
Copy link

Please double check driver/SCHEMA_VERSION file. See versioning.

/hold

@codecov
Copy link

codecov bot commented Sep 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.19%. Comparing base (027c258) to head (e6c2e6c).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2649   +/-   ##
=======================================
  Coverage   78.19%   78.19%           
=======================================
  Files         292      292           
  Lines       31793    31789    -4     
  Branches     4662     4655    -7     
=======================================
- Hits        24860    24857    -3     
+ Misses       6933     6932    -1     
Flag Coverage Δ
libsinsp 78.19% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 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.

Copy link
Contributor

@irozzo-1A irozzo-1A left a comment

Choose a reason for hiding this comment

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

LGTM, just two nits

@github-project-automation github-project-automation bot moved this from Todo to In progress in Falco Roadmap Sep 26, 2025
@ekoops
Copy link
Contributor Author

ekoops commented Sep 26, 2025

/hold cancel

@poiana poiana merged commit f45917b into falcosecurity:master Sep 26, 2025
55 of 57 checks passed
@github-project-automation github-project-automation bot moved this from In progress to Done in Falco Roadmap Sep 26, 2025
@ekoops ekoops deleted the ekoops/scap-conv-toctou branch September 26, 2025 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants