new: scap ppme to sc mapping table #937

FedeDP · 2023-03-02T11:00:38Z

What type of PR is this?

/kind cleanup
/kind feature

Any specific area of the project related to this PR?

/area libscap
/area libsinsp

Does this PR require a change in the driver versions?

What this PR does / why we need it:

This PR is extracted from #889 .
It implements:

split scap ppm sc API (exposed by scap.h) in scap_platform library, under {linux,win32,osx}/scap_ppm_sc.c source file.
In the linux file, a new map[event_code][list of ppm_sc codes that generated that event] was created and it is needed to correctly retrieve all ppm_sc given an event_set.
Indeed, using the syscall_table to retrieve that information was very error prone because in userspace we only saw syscalls present on that architecture (included by our driver/syscall_compat_$arch.h); moreover there was no way to map OLD_VERSION events.
Ported all ppm_sc related scap APIs to use that table (that is linux specific, and scap_ppm_sc.c private).
This allows us to reliably retrieve all ppm_sc related to an event set (because we are no more iterating over the syscalls_table, but we directly map event -> ppm_sc).

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Most of the changes are in userspace/libscap/linux/scap_ppm_sc.c.
Macos and win32 scap_ppm_sc API impl does nothing.

Tests were cleaned up of now useless ifdef __NR_FOO checks, and <sys/syscall.h> includes.
Added a small test to guarantee that any new PPM_SC is also added to the new scap_ppm_sc.c table.

Other tests present in #889 will be added once all the PRs are in (ie: this one, #936 and the ppm_tp table refactor/merge into ppm_sc)

Does this PR introduce a user-facing change?:

NONE

FedeDP · 2023-03-02T11:01:11Z

userspace/libpman/include/libpman.h

+	 *
+	 * @return supported true or false.
+	 */
+	bool pman_check_support();


While i was at it, exported this function in the libpman header. Otherwise compilation complained about it.

FedeDP · 2023-03-02T12:57:03Z

userspace/libscap/linux/scap_ppm_sc.c

+	[PPME_SOCKET_LISTEN_X] = (ppm_sc_code[]){PPM_SC_LISTEN, -1},
+	[PPME_SOCKET_ACCEPT_E] = (ppm_sc_code[]){PPM_SC_ACCEPT, -1},
+	[PPME_SOCKET_ACCEPT_X] = (ppm_sc_code[]){PPM_SC_ACCEPT, -1},
+	[PPME_SOCKET_SEND_E] = NULL, // TODO (ppm_sc_code[]){PPM_SC_SEND, -1},


This will be addressed by #936 .

Idea here is:

is event actually sent but we still miss the PPM_SC mapping it (tracepoint ppm_sc that will be introduced once we merge the ppm_sc table with the tb table)? Then use PPM_SC_UNKNOWN.

else, use NULL

In this case, the event is not actually sent right now.

FedeDP · 2023-03-02T12:57:08Z

userspace/libscap/linux/scap_ppm_sc.c

+	[PPME_SOCKET_SEND_X] = NULL, // TODO (ppm_sc_code[]){PPM_SC_SEND, -1},
+	[PPME_SOCKET_SENDTO_E] = (ppm_sc_code[]){PPM_SC_SENDTO, -1},
+	[PPME_SOCKET_SENDTO_X] = (ppm_sc_code[]){PPM_SC_SENDTO, -1},
+	[PPME_SOCKET_RECV_E] = NULL, // TODO (ppm_sc_code[]){PPM_SC_RECV, -1},


This will be addressed by #936 .

FedeDP · 2023-03-06T08:36:56Z

/milestone 0.11.0

FedeDP · 2023-03-06T09:33:50Z

Rebased on top of new master.

Andreagit97

This LGTM thank you for splitting it, now it's very easy and clear! Left just one minor comment
/approve

Andreagit97 · 2023-03-06T12:33:51Z

userspace/libsinsp/test/public_sinsp_API/interesting_syscalls.cpp

-#endif
-
-#ifdef __NR_writev
+	//io_sc_set_truth.insert(PPM_SC_PWRITE);


why is this commented? I don't see any PPM_SC_PWRITE between our enums, is it correct?

Yep exactly; i don't know what that was indeed :)

I think we can clean it up later!

🤷‍♀️ I don't remember either and I was probably the one who added it? Plz cleanup and adjust as needed 🙏

Cleaned up!

FedeDP · 2023-03-06T12:45:17Z

Need to rebase :(

Andreagit97

/approve

jasondellaluce

LGTM!

Will wait for @incertum's feedback before approving.

jasondellaluce · 2023-03-06T15:30:03Z

cc @falcosecurity/libs-maintainers for visibility.

incertum

Thanks @FedeDP for this, on board!

Re timing let's wait for #936 to be merged first.

Since we are asking for more feedback perhaps we can wait until Wednesday this week before we merge it?

incertum · 2023-03-06T18:53:54Z

userspace/libsinsp/test/public_sinsp_API/events_set.cpp

+	auto generic_ev_set = libsinsp::events::set<ppm_event_code>({PPME_GENERIC_E, PPME_GENERIC_X});
+	auto generic_sc_set = libsinsp::events::event_set_to_sc_set(generic_ev_set);
+	auto final_ev_set = libsinsp::events::sc_set_to_event_set(generic_sc_set);
+	ASSERT_EQ(final_ev_set, generic_ev_set);


Consider using the new macro we added for consistency ASSERT_PPM_EVENT_CODES_EQ.
Also we can and should do another extension to test coverage and @Andreagit97 already has bunch of more tests lined up 🎉

Maybe another small test for this PR could be this test for non generic syscalls while also including non syscall events ...

incertum · 2023-03-06T18:56:54Z

userspace/libsinsp/test/public_sinsp_API/events_set.cpp

-#include "syscall_compat_aarch64.h"
-#elif __s390x__
-#include "syscall_compat_s390x.h"
-#endif /* __x86_64__ */


Amazing 😅

incertum · 2023-03-06T19:06:28Z

userspace/libsinsp/test/public_sinsp_API/interesting_syscalls.cpp

-#endif
-
-#ifdef __NR_writev
+	//io_sc_set_truth.insert(PPM_SC_PWRITE);


🤷‍♀️ I don't remember either and I was probably the one who added it? Plz cleanup and adjust as needed 🙏

…o map each PPME_EVENT to the PPM_SC that generate it. Moreover, small improvements to libsinsp::events API. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

Moreover, cleaned up interesting_syscalls test and added a new events_set test. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

incertum

Thanks @FedeDP!

/approve

Andreagit97

/approve

poiana · 2023-03-08T09:04:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, FedeDP, incertum

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,incertum]

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

poiana added kind/cleanup release-note-none dco-signoff: yes kind/feature New feature or request area/libscap area/libsinsp approved labels Mar 2, 2023

poiana requested review from hbrueckner and mstemm March 2, 2023 11:00

poiana added the size/XXL label Mar 2, 2023

FedeDP commented Mar 2, 2023

View reviewed changes

FedeDP mentioned this pull request Mar 2, 2023

wip: refactor(userspace,driver): merge ppm_sc_codes and ppm_tp_codes + add new scap private table that maps each event to all of the PPM_SC codes that generate it #889

Closed

FedeDP force-pushed the new/scap_ppme_to_sc_table branch from 5b86740 to 15e13d9 Compare March 2, 2023 11:17

FedeDP mentioned this pull request Mar 2, 2023

new: add support for the union map from all supported architectures. falcosecurity/syscalls-bumper#7

Merged

FedeDP commented Mar 2, 2023

View reviewed changes

poiana added this to the 0.11.0 milestone Mar 6, 2023

FedeDP force-pushed the new/scap_ppme_to_sc_table branch from 15e13d9 to c054a66 Compare March 6, 2023 09:33

Andreagit97 previously approved these changes Mar 6, 2023

View reviewed changes

poiana assigned Andreagit97 Mar 6, 2023

poiana added the lgtm label Mar 6, 2023

FedeDP dismissed Andreagit97’s stale review via 8890f72 March 6, 2023 12:59

FedeDP force-pushed the new/scap_ppme_to_sc_table branch 2 times, most recently from 8890f72 to d5381c2 Compare March 6, 2023 13:01

Andreagit97 previously approved these changes Mar 6, 2023

View reviewed changes

jasondellaluce reviewed Mar 6, 2023

View reviewed changes

incertum mentioned this pull request Mar 6, 2023

Support Linux Security Modules hook for more accurate security tracing #252

Open

23 tasks

incertum reviewed Mar 6, 2023

View reviewed changes

FedeDP added 4 commits March 7, 2023 14:28

new(libscap,libsinsp): added a new g_events_to_sc_map libscap table t…

2f5ced6

…o map each PPME_EVENT to the PPM_SC that generate it. Moreover, small improvements to libsinsp::events API. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

cleanup(libsinsp): cleaned up libsinsp tests by removing ifdefs.

83eed03

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

fix(libsinsp/test): fixed tests.

b460ae0

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

fix(libscap,libsinsp): fixed g_events_to_sc_map after rebase.

da53d7d

Moreover, cleaned up interesting_syscalls test and added a new events_set test. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

FedeDP dismissed Andreagit97’s stale review via da53d7d March 7, 2023 13:50

FedeDP force-pushed the new/scap_ppme_to_sc_table branch from d5381c2 to da53d7d Compare March 7, 2023 13:50

incertum approved these changes Mar 7, 2023

View reviewed changes

poiana assigned incertum Mar 7, 2023

Andreagit97 approved these changes Mar 8, 2023

View reviewed changes

poiana merged commit 7aa695d into master Mar 8, 2023

poiana deleted the new/scap_ppme_to_sc_table branch March 8, 2023 09:05

Andreagit97 mentioned this pull request Mar 8, 2023

fix(driver)!: create dedicated events for umount2 syscall and rename them to "umount2" #944

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new: scap ppme to sc mapping table #937

new: scap ppme to sc mapping table #937

FedeDP commented Mar 2, 2023 •

edited

Loading

FedeDP Mar 2, 2023

FedeDP Mar 2, 2023

FedeDP Mar 2, 2023

FedeDP Mar 2, 2023

FedeDP commented Mar 6, 2023

FedeDP commented Mar 6, 2023

Andreagit97 left a comment

Andreagit97 Mar 6, 2023

FedeDP Mar 6, 2023

FedeDP Mar 6, 2023

incertum Mar 6, 2023

FedeDP Mar 7, 2023

FedeDP commented Mar 6, 2023

Andreagit97 left a comment

jasondellaluce left a comment

jasondellaluce commented Mar 6, 2023

incertum left a comment

incertum Mar 6, 2023

incertum Mar 6, 2023

FedeDP Mar 7, 2023

incertum Mar 6, 2023

incertum Mar 6, 2023

incertum left a comment

Andreagit97 left a comment

poiana commented Mar 8, 2023

new: scap ppme to sc mapping table #937

new: scap ppme to sc mapping table #937

Conversation

FedeDP commented Mar 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FedeDP commented Mar 6, 2023

FedeDP commented Mar 6, 2023

Andreagit97 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FedeDP commented Mar 6, 2023

Andreagit97 left a comment

Choose a reason for hiding this comment

jasondellaluce left a comment

Choose a reason for hiding this comment

jasondellaluce commented Mar 6, 2023

incertum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

incertum left a comment

Choose a reason for hiding this comment

Andreagit97 left a comment

Choose a reason for hiding this comment

poiana commented Mar 8, 2023

FedeDP commented Mar 2, 2023 •

edited

Loading