Add fcntl enter arguments to exit event #1304
Conversation
poiana
added
release-note
dco-signoff: yes
kind/feature
|
Please double check driver/API_VERSION file. See versioning. /hold |
|
I'm not that familiar with the modern bpf driver, do I need to make changes to an additional location or does the modern bpf driver use the fillers in driver/bpf ? |
There was a problem hiding this comment.
@mstemm thank you for this! The modern probe uses its dedicated fillers so you need to change also them, moreover, we have added tests on all fillers so now the tests on fcntl exit events are falling because the expect 1 parameter instead of 3.
- you can find the modern bpf filler for
fcntlhere: driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/fcntl.bpf.c - you can find tests for
fcntlhere: test/drivers/test_suites/syscall_exit_suite/fcntl_x.cpp
Moreover, this PR could be a good example to see where to touch: https://github.com/falcosecurity/libs/pull/1270/files
mstemm
force-pushed
the
make-fcntl-args-available-in-exit-event
branch
3 times, most recently
from
6e587f3
to
75d15c2
Compare
For some filters on fcntl events, it's useful to have access to both the provided fd and cmd, which are parameters in the enter event, as well as the return value (e.g. the new fd, which is in the exit event). So add the enter event fields to the exit event. This matches the pattern we already use for lots of other events. Fcntl is a pretty old event, so we weren't following that pattern then. It's safe to add new fields to events to preserve backwards compatibility, but it's not safe to modify or reduce fields, so keep the parameters in the enter event. Also bump the schema minor version, as this adds fields to an existing event. Signed-off-by: Mark Stemm <mark.stemm@gmail.com>
mstemm
force-pushed
the
make-fcntl-args-available-in-exit-event
branch
from
75d15c2
to
063db0a
Compare
|
Ok, all the tests are passing now. PTAL again! |
|
/milestone 0.13.0 |
|
LGTM label has been added. Git tree hash: a9c4e74eb586732b819d75963b8a8049531cdfcb
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Andreagit97, incertum, mstemm The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
For some filters on fcntl events, it's useful to have access to both the provided fd and cmd, which are parameters in the enter event, as well as the return value (e.g. the new fd, which is in the exit event).
So add the enter event fields to the exit event. This matches the pattern we already use for lots of other events. Fcntl is a pretty old event, so we weren't following that pattern then.
It's safe to add new fields to events to preserve backwards compatibility, but it's not safe to modify or reduce fields, so keep the parameters in the enter event.
What type of PR is this?
/kind feature
Any specific area of the project related to this PR?
/area driver-kmod
/area driver-bpf
Does this PR require a change in the driver versions?
/version driver-API-version-minor
What this PR does / why we need it:
Add fcntl arguments to the exit event so the arguments and return values can be compared at the same time.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: