new(driver): update exit events PPME_SYSCALL_SETUID_X with enter params #2414
Conversation
|
Welcome @terror96! It looks like this is your first PR to falcosecurity/libs 🎉 |
Perf diff from master - unit testsHeap diff from master - unit testsHeap diff from master - scap fileBenchmarks diff from master |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2414 +/- ##
==========================================
+ Coverage 77.27% 77.28% +0.01%
==========================================
Files 227 227
Lines 30343 30357 +14
Branches 4642 4642
==========================================
+ Hits 23448 23462 +14
Misses 6895 6895
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi @terror96 , thank you for the contribution! It seems that there is one test to fix for all drivers: |
userspace/libsinsp/parsers.cpp
Outdated
|
|
||
| if(retval == 0 && retrieve_enter_event(*enter_evt, evt)) { | ||
| uint32_t new_euid = enter_evt->get_param(0)->as<uint32_t>(); | ||
| if(retval == 0) { |
There was a problem hiding this comment.
I guess here we should check that the event has the "new format", this means checking that the number of parameter is greater than 1, before accessing the uid:
| if(retval == 0) { | |
| if(retval == 0 && evt.get_num_params() > 1) { |
Yes. Thanks for the pointer, we will look into it. This is the part of testing which is not completely clear for me. According to other tests, e.g. the drivers are emitting the events correctly and I would assume if libscap and libsinsp unit tests all pass that the new events are also handled correctly in the user space. However, the python test program does not seem to receive the new entry events. Do you have any further suggestions where to start looking for the reason why sinsp.py does not see the new exit events? Is there something that needs to also be modified in the e2e test code to handle the new exit event format correctly? EDIT: or is it just that the filter's "evt.args" field for '<' events is expecting still only a single parameter? |
|
...and we will also fix those "format code" issues. |
Most probably that's it! You can see a similar fix in b15675b |
|
Btw thanks for opening this PR, great job! |
driver/SCHEMA_VERSION
Outdated
| @@ -1 +1 @@ | |||
| 3.6.1 | |||
| 3.7.1 | |||
There was a problem hiding this comment.
Actually, the schema version should become 3.7.0 😄
To make your life easier for this and any feature contribution, you can configure git hooks in your local clone of the repo, using the pre-commit framework, as detailed here: https://github.com/falcosecurity/libs/blob/master/Contributing.md#enforce-the-style-locally |
This update is part of the proposal for disabling support for syscall enter events. It implements the following steps: 1. Add enter parameters to the exit event. 2. Adapt sinsp state to work just with exit events. 3. Create a scap-file conversion (in a dedicated scap-file converter) to convert ENTER events into merged EXIT ones. 4. Add some tests replaying scap-files. for the setuid syscall. Signed-off-by: Tero Kauppinen <[email protected]>
terror96
force-pushed
the
setuid-exit-event-improvement
branch
from
b214a07 to
84c49ee
Compare
|
Great job! I will trigger our kernel test matrix workflow against this branch to check if we broke any verifier :) https://github.com/falcosecurity/libs/actions/runs/15158621269 |
Thank you and thanks for all the help and pointers. |
|
ARM64:
AMD64:
No new issues! |
|
LGTM label has been added. DetailsGit tree hash: f08e4f1aaf204b509392dc7c705a96d4cbbbcaaf |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ekoops, FedeDP, terror96 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
5 participants
This update is part of the proposal for disabling support for syscall enter events. It implements the following steps:
for the setuid syscall.
What type of PR is this?
/kind feature
Any specific area of the project related to this PR?
/area driver-kmod
/area driver-bpf
/area driver-modern-bpf
/area libscap-engine-bpf
/area libscap-engine-gvisor
/area libscap-engine-savefile
/area libscap
/area libsinsp
/area tests
Does this PR require a change in the driver versions?
/version driver-API-version-minor
/version driver-SCHEMA-version-minor
What this PR does / why we need it:
This PR is part of #2068.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: