-
Notifications
You must be signed in to change notification settings - Fork 165
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.
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
fix(driver): fix dynamic snaplen logic (SCAP_FULLCAPTURE_PORT_RANGE and SCAP_STATSD_PORT) #2006
fix(driver): fix dynamic snaplen logic (SCAP_FULLCAPTURE_PORT_RANGE and SCAP_STATSD_PORT) #2006
Conversation
Perf diff from master - unit tests
Heap diff from master - unit tests
Heap diff from master - scap file
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2006 +/- ##
==========================================
+ Coverage 74.14% 74.21% +0.07%
==========================================
Files 253 253
Lines 30834 30845 +11
Branches 5404 5419 +15
==========================================
+ Hits 22863 22893 +30
- Misses 7946 7952 +6
+ Partials 25 0 -25
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com>
fallback logic to extract remote port with UDP Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com>
Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com>
Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com>
Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com>
Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com>
Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com>
da541a6
to
8e5d1a8
Compare
Please double check driver/SCHEMA_VERSION file. See versioning. /hold |
Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com>
Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com>
kernel testing results: https://github.com/falcosecurity/libs/actions/runs/10562104486/job/29259687424 x86
arm64
|
/milestone next-driver |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
LGTM label has been added. Git tree hash: 2c14369473911ada817e3cc21efee1ebaf2598ad
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Andreagit97, FedeDP 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 |
/unhold |
What type of PR is this?
/kind bug
/kind cleanup
Any specific area of the project related to this PR?
/area driver-kmod
/area driver-bpf
/area driver-modern-bpf
/area tests
Does this PR require a change in the driver versions?
What this PR does / why we need it:
This PR tries to fix some dynamic snaplen logics (SCAP_FULLCAPTURE_PORT_RANGE and SCAP_STATSD_PORT). More in detail:
compute_snaplen
logic for the legacy ebpf and kernel module. We now use the local kernel socket when available, if not, we fallback to the userspace structs.read,write,sendto,sendmsg,recvfrom,recvmsg
Which issue(s) this PR fixes:
Special notes for your reviewer:
The PR seems huge but apart from the snaplen rework there are only tests.
Working on this I noticed some other warring failures on the old drivers (legacy ebpf and kmod). They are now treated as skipped tests, but we need to fix them at a certain point. More in detail:
We are in the exit event and we rely on the sockaddr, more in detail on
addrlen != 0
, but in the exit tracepoint we will always obtainaddrlen=0
. I verified it with bpftrace, the kernel seems to clear themsg_namelen
before returning fromrecvmsg
Our code checks for
if (usrsockaddr != NULL && addrlen != 0)
so if the sockaddr is NULL we will send an empty tuple. In the modern ebpf we try to recover the info from the kernel if available, we should do the same thing...In case of negative fd (failure) in the enter event, we don't send sockaddr.
We don't send the tuple if the sockaddr is NULL
The pain point is that to fix these we probably need to completely rewrite the tuple collection logic in the old drivers :/
Our userspace seems able to handle these missing data relying on previous data like the
connect/accept
once. But for sure here there is room for improvement. Again these issues are only related to the legacy ebpf and the kmodDoes this PR introduce a user-facing change?: