fix(driver): take the unix path directly from the kernel #2215

Andreagit97 · 2024-12-20T17:44:46Z

What type of PR is this?

/kind bug

Any specific area of the project related to this PR?

/area driver-kmod

/area driver-bpf

/area driver-modern-bpf

Does this PR require a change in the driver versions?

No

What this PR does / why we need it:

Today in the modern ebpf we extract the UNIX path directly from the kernel socket. This means that symlink are automatically solved. In the legacy ebpf and kmod we take the path from the syscall sockaddr param. This PR adapts the old driver behaviors to the modern one

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

Andreagit97 · 2024-12-20T17:46:19Z

driver/bpf/filler_helpers.h

@@ -606,8 +606,7 @@ static __always_inline int unix_socket_path(char *dest, const char *user_ptr, si
 	 * specified length of the address structure.
 	 */
 	if(res == 1) {
-		dest[0] = '@';
-		res = bpf_probe_read_kernel_str(dest + 1,
+		res = bpf_probe_read_kernel_str(dest,


in the modern we don't prepend the @ so it's probably better to be uniform between drivers, we can add it in the modern if we want but not sure what is the value of having the initial @

github-actions · 2024-12-20T17:51:01Z

Perf diff from master - unit tests

     9.95%     +0.78%  [.] sinsp_parser::reset
    11.98%     -0.74%  [.] sinsp::next
     1.55%     +0.39%  [.] next
     3.78%     -0.37%  [.] sinsp_thread_manager::find_thread
     1.50%     -0.34%  [.] std::_Hashtable<unsigned long, std::pair<unsigned long const, std::shared_ptr<ppm_evt_hdr> >, std::allocator<std::pair<unsigned long const, std::shared_ptr<ppm_evt_hdr> > >, std::__detail::_Select1st, std::equal_to<unsigned long>, std::hash<unsigned long>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::_M_find_before_node
     1.03%     +0.24%  [.] sinsp_evt::get_direction
     0.97%     +0.22%  [.] std::_Hashtable<long, std::pair<long const, std::shared_ptr<sinsp_threadinfo> >, std::allocator<std::pair<long const, std::shared_ptr<sinsp_threadinfo> > >, std::__detail::_Select1st, std::equal_to<long>, std::hash<long>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::find
     0.57%     +0.22%  [.] sinsp_fdtable::find
     0.62%     +0.22%  [.] std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>
     1.26%     -0.22%  [.] sinsp_evt::get_ts

Heap diff from master - unit tests

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Heap diff from master - scap file

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Benchmarks diff from master

Comparing gbench_data.json to /root/actions-runner/_work/libs/libs/build/gbench_data.json
Benchmark                                                         Time             CPU      Time Old      Time New       CPU Old       CPU New
----------------------------------------------------------------------------------------------------------------------------------------------
BM_sinsp_split_mean                                            -0.0223         -0.0223           148           145           148           145
BM_sinsp_split_median                                          -0.0254         -0.0253           149           146           149           146
BM_sinsp_split_stddev                                          -0.0771         -0.0765             1             1             1             1
BM_sinsp_split_cv                                              -0.0560         -0.0555             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_mean                  +0.0146         +0.0146            56            57            56            57
BM_sinsp_concatenate_paths_relative_path_median                +0.0147         +0.0147            56            57            56            57
BM_sinsp_concatenate_paths_relative_path_stddev                +0.0432         +0.0465             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_cv                    +0.0281         +0.0314             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_mean                     -0.0366         -0.0366            25            24            25            24
BM_sinsp_concatenate_paths_empty_path_median                   -0.0313         -0.0313            25            24            25            24
BM_sinsp_concatenate_paths_empty_path_stddev                   -0.9219         -0.9222             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_cv                       -0.9189         -0.9192             0             0             0             0
BM_sinsp_concatenate_paths_absolute_path_mean                  +0.0095         +0.0095            56            56            56            56
BM_sinsp_concatenate_paths_absolute_path_median                +0.0031         +0.0031            56            56            56            56
BM_sinsp_concatenate_paths_absolute_path_stddev                +2.6893         +2.6851             0             1             0             1
BM_sinsp_concatenate_paths_absolute_path_cv                    +2.6545         +2.6503             0             0             0             0
BM_sinsp_split_container_image_mean                            +0.0121         +0.0121           383           388           383           388
BM_sinsp_split_container_image_median                          +0.0095         +0.0095           384           387           384           387
BM_sinsp_split_container_image_stddev                          -0.2767         -0.2766             3             2             3             2
BM_sinsp_split_container_image_cv                              -0.2854         -0.2852             0             0             0             0

codecov · 2024-12-20T18:46:40Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.44%. Comparing base (9ee57c8) to head (bf16be2).
Report is 18 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2215      +/-   ##
==========================================
+ Coverage   75.38%   75.44%   +0.06%     
==========================================
  Files         264      265       +1     
  Lines       34014    34057      +43     
  Branches     5805     5800       -5     
==========================================
+ Hits        25642    25696      +54     
+ Misses       8372     8361      -11

Flag	Coverage Δ
libsinsp	`75.44% <ø> (+0.06%)`	⬆️

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.

Andreagit97 · 2024-12-30T10:24:38Z

driver/bpf/filler_helpers.h

 		size = 1 + 8 + 8;
-


to simplify the verification we remove the fallback to the userspace struct like we do in the modern, we keep it only in the kernel module

poiana · 2024-12-30T11:05:12Z

LGTM label has been added.

Git tree hash: 49e6580a392193473af5bc03412d6d621c0b45ea

FedeDP

/approve

poiana · 2025-01-02T09:03:23Z

LGTM label has been added.

Git tree hash: 1c40a3184dc2040f536c16855695b14b89349013

Andreagit97 · 2025-01-02T10:27:58Z

For some reason, e2e tests are failing :/

Signed-off-by: Andrea Terzolo <[email protected]>

Andreagit97 · 2025-01-02T11:56:42Z

driver/ppm_events.c

+		// `us_name` should contain the socket path extracted from the kernel if we cannot retrieve
+		// it we can fallback to the user-provided address
+		// Note that we check the second byte of `us_name`, see `sock_getname` for more details.
+		// Some times `usrsockaddr` is provided as a NULL pointer, checking `use_userdata` should be


This is why e2e tests were failing we were dereferencing usrsockaddr without checking it was != NULL

FedeDP

/approve
Thanks andre!

poiana · 2025-01-02T13:16:14Z

LGTM label has been added.

Git tree hash: d3a29d5a1a33113e71cc949e693132e9403b4ad8

poiana · 2025-01-02T13:16:20Z

[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:

~~OWNERS~~ [Andreagit97,FedeDP]

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

FedeDP · 2025-01-02T15:02:32Z

I ran kernel-testing matrix against this PR and it works fine 🚀

poiana added kind/bug Something isn't working release-note-none dco-signoff: yes area/driver-kmod area/driver-bpf area/driver-modern-bpf approved labels Dec 20, 2024

poiana requested review from hbrueckner and LucaGuerra December 20, 2024 17:44

Andreagit97 changed the title ~~update(driver): take the unix path directly from the kernel~~ fix(driver): take the unix path directly from the kernel Dec 20, 2024

poiana added the size/M label Dec 20, 2024

Andreagit97 commented Dec 20, 2024

View reviewed changes

Andreagit97 added this to the next-driver milestone Dec 20, 2024

Andreagit97 force-pushed the take_unix_path_from_kernel_kmod branch from 2bc1408 to 847cdaa Compare December 30, 2024 10:23

poiana added size/L and removed size/M labels Dec 30, 2024

Andreagit97 commented Dec 30, 2024

View reviewed changes

jasondellaluce previously approved these changes Dec 30, 2024

View reviewed changes

poiana assigned jasondellaluce Dec 30, 2024

poiana added the lgtm label Dec 30, 2024

Andreagit97 dismissed jasondellaluce’s stale review via 5803382 December 30, 2024 11:36

Andreagit97 force-pushed the take_unix_path_from_kernel_kmod branch from 847cdaa to 5803382 Compare December 30, 2024 11:36

poiana removed the lgtm label Dec 30, 2024

poiana requested a review from jasondellaluce December 30, 2024 11:37

FedeDP previously approved these changes Jan 2, 2025

View reviewed changes

poiana assigned FedeDP Jan 2, 2025

poiana added the lgtm label Jan 2, 2025

update(driver): take the unix path directly from the kernel

bf16be2

Signed-off-by: Andrea Terzolo <[email protected]>

Andreagit97 dismissed FedeDP’s stale review via bf16be2 January 2, 2025 11:55

Andreagit97 force-pushed the take_unix_path_from_kernel_kmod branch from 5803382 to bf16be2 Compare January 2, 2025 11:55

poiana removed the lgtm label Jan 2, 2025

poiana requested a review from FedeDP January 2, 2025 11:55

Andreagit97 commented Jan 2, 2025

View reviewed changes

FedeDP approved these changes Jan 2, 2025

View reviewed changes

poiana added the lgtm label Jan 2, 2025

leogr approved these changes Jan 7, 2025

View reviewed changes

poiana assigned leogr Jan 7, 2025

poiana merged commit 258b4b4 into falcosecurity:master Jan 7, 2025
55 of 57 checks passed

FedeDP mentioned this pull request Jan 14, 2025

feat(libsinsp_e2e): add unix_udp_client_server_read test #2231

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(driver): take the unix path directly from the kernel #2215

fix(driver): take the unix path directly from the kernel #2215

Andreagit97 commented Dec 20, 2024

Andreagit97 Dec 20, 2024

github-actions bot commented Dec 20, 2024 •

edited

Loading

codecov bot commented Dec 20, 2024 •

edited

Loading

Andreagit97 Dec 30, 2024

poiana commented Dec 30, 2024

FedeDP left a comment

poiana commented Jan 2, 2025

Andreagit97 commented Jan 2, 2025

Andreagit97 Jan 2, 2025

FedeDP left a comment

poiana commented Jan 2, 2025

poiana commented Jan 2, 2025

FedeDP commented Jan 2, 2025

fix(driver): take the unix path directly from the kernel #2215

fix(driver): take the unix path directly from the kernel #2215

Conversation

Andreagit97 commented Dec 20, 2024

Andreagit97 Dec 20, 2024

Choose a reason for hiding this comment

github-actions bot commented Dec 20, 2024 • edited Loading

Perf diff from master - unit tests

Heap diff from master - unit tests

Heap diff from master - scap file

Benchmarks diff from master

codecov bot commented Dec 20, 2024 • edited Loading

Codecov Report

Andreagit97 Dec 30, 2024

Choose a reason for hiding this comment

poiana commented Dec 30, 2024

FedeDP left a comment

Choose a reason for hiding this comment

poiana commented Jan 2, 2025

Andreagit97 commented Jan 2, 2025

Andreagit97 Jan 2, 2025

Choose a reason for hiding this comment

FedeDP left a comment

Choose a reason for hiding this comment

poiana commented Jan 2, 2025

poiana commented Jan 2, 2025

FedeDP commented Jan 2, 2025

github-actions bot commented Dec 20, 2024 •

edited

Loading

codecov bot commented Dec 20, 2024 •

edited

Loading