From b8d3c9ad1b18b39e23b4dd7829b91377ab6eeb4a Mon Sep 17 00:00:00 2001 From: Tero Kauppinen Date: Wed, 9 Jul 2025 12:33:07 +0300 Subject: [PATCH] feat(libscap/libsinsp): fcntl scap conversion This update adds scap event conversion for the fcntl syscall exit events and it eliminates the need to store the entry event in libsinsp. Signed-off-by: Tero Kauppinen --- driver/event_table.c | 4 +- .../engines/savefile/converter.cpp | 51 +++++++++++++++++++ .../engine/savefile/converter/table.cpp | 6 +++ userspace/libscap/scap_event.c | 1 + userspace/libsinsp/parsers.cpp | 24 +++------ userspace/libsinsp/parsers.h | 1 - .../test/scap_files/converter_tests.cpp | 33 ++++++++++++ 7 files changed, 101 insertions(+), 19 deletions(-) diff --git a/driver/event_table.c b/driver/event_table.c index d3f8925269..a955abec79 100644 --- a/driver/event_table.c +++ b/driver/event_table.c @@ -906,13 +906,13 @@ const struct ppm_event_info g_event_info[] = { {{"ratio", PT_UINT32, PF_DEC}}}, [PPME_SYSCALL_FCNTL_E] = {"fcntl", EC_IO_OTHER | EC_SYSCALL, - EF_USES_FD | EF_MODIFIES_STATE, + EF_USES_FD | EF_MODIFIES_STATE | EF_TMP_CONVERTER_MANAGED, 2, {{"fd", PT_FD, PF_DEC}, {"cmd", PT_ENUMFLAGS8, PF_DEC, fcntl_commands}}}, [PPME_SYSCALL_FCNTL_X] = {"fcntl", EC_IO_OTHER | EC_SYSCALL, - EF_USES_FD | EF_MODIFIES_STATE, + EF_USES_FD | EF_MODIFIES_STATE | EF_TMP_CONVERTER_MANAGED, 3, {{"res", PT_FD, PF_DEC}, {"fd", PT_FD, PF_DEC}, diff --git a/test/libscap/test_suites/engines/savefile/converter.cpp b/test/libscap/test_suites/engines/savefile/converter.cpp index 9731c20981..e04362e244 100644 --- a/test/libscap/test_suites/engines/savefile/converter.cpp +++ b/test/libscap/test_suites/engines/savefile/converter.cpp @@ -710,6 +710,57 @@ TEST_F(convert_event_test, PPME_SYSCALL_SETRLIMIT_X_to_4_params_with_enter) { create_safe_scap_event(ts, tid, PPME_SYSCALL_SETRLIMIT_X, 4, res, cur, max, resource)); } +//////////////////////////// +// FCNTL +//////////////////////////// + +TEST_F(convert_event_test, PPME_SYSCALL_FCNTL_E_store) { + constexpr uint64_t ts = 12; + constexpr int64_t tid = 25; + + constexpr int64_t fd = 19; + constexpr uint8_t cmd = 5; + + const auto evt = create_safe_scap_event(ts, tid, PPME_SYSCALL_FCNTL_E, 2, fd, cmd); + assert_single_conversion_skip(evt); + assert_event_storage_presence(evt); +} + +TEST_F(convert_event_test, PPME_SYSCALL_FCNTL_X_to_3_params_no_enter) { + constexpr uint64_t ts = 12; + constexpr int64_t tid = 25; + + constexpr int64_t res = 89; + + // Defaulted to 0 + constexpr int64_t fd = 0; + constexpr uint8_t cmd = 0; + + assert_single_conversion_success( + conversion_result::CONVERSION_COMPLETED, + create_safe_scap_event(ts, tid, PPME_SYSCALL_FCNTL_X, 1, res), + create_safe_scap_event(ts, tid, PPME_SYSCALL_FCNTL_X, 3, res, fd, cmd)); +} + +TEST_F(convert_event_test, PPME_SYSCALL_FCNTL_X_to_3_params_with_enter) { + constexpr uint64_t ts = 12; + constexpr int64_t tid = 25; + + constexpr int64_t fd = 19; + constexpr uint8_t cmd = 5; + constexpr int64_t res = 89; + + // After the first conversion we should have the storage + const auto evt = create_safe_scap_event(ts, tid, PPME_SYSCALL_FCNTL_E, 2, fd, cmd); + assert_single_conversion_skip(evt); + assert_event_storage_presence(evt); + + assert_single_conversion_success( + conversion_result::CONVERSION_COMPLETED, + create_safe_scap_event(ts, tid, PPME_SYSCALL_FCNTL_X, 1, res), + create_safe_scap_event(ts, tid, PPME_SYSCALL_FCNTL_X, 3, res, fd, cmd)); +} + //////////////////////////// // BRK //////////////////////////// diff --git a/userspace/libscap/engine/savefile/converter/table.cpp b/userspace/libscap/engine/savefile/converter/table.cpp index 8652bbb506..ed26106395 100644 --- a/userspace/libscap/engine/savefile/converter/table.cpp +++ b/userspace/libscap/engine/savefile/converter/table.cpp @@ -91,6 +91,12 @@ const std::unordered_map g_conversion_table = { {conversion_key{PPME_SYSCALL_SETRLIMIT_E, 1}, conversion_info().action(C_ACTION_STORE)}, {conversion_key{PPME_SYSCALL_SETRLIMIT_X, 3}, conversion_info().action(C_ACTION_ADD_PARAMS).instrs({{C_INSTR_FROM_ENTER, 0}})}, + /*====================== FCNTL ======================*/ + {conversion_key{PPME_SYSCALL_FCNTL_E, 2}, conversion_info().action(C_ACTION_STORE)}, + {conversion_key{PPME_SYSCALL_FCNTL_X, 1}, + conversion_info() + .action(C_ACTION_ADD_PARAMS) + .instrs({{C_INSTR_FROM_ENTER, 0}, {C_INSTR_FROM_ENTER, 1}})}, /*====================== BRK ======================*/ {conversion_key{PPME_SYSCALL_BRK_4_E, 1}, conversion_info().action(C_ACTION_STORE)}, {conversion_key{PPME_SYSCALL_BRK_4_X, 4}, diff --git a/userspace/libscap/scap_event.c b/userspace/libscap/scap_event.c index 78ae51975e..6e753daabc 100644 --- a/userspace/libscap/scap_event.c +++ b/userspace/libscap/scap_event.c @@ -552,6 +552,7 @@ int get_exit_event_fd_location(ppm_event_code etype) { case PPME_SYSCALL_GETDENTS64_X: case PPME_SYSCALL_FLOCK_X: case PPME_SYSCALL_COPY_FILE_RANGE_X: + case PPME_SYSCALL_FCNTL_X: location = 1; break; case PPME_SYSCALL_READ_X: diff --git a/userspace/libsinsp/parsers.cpp b/userspace/libsinsp/parsers.cpp index bff8610302..4a2b75a819 100644 --- a/userspace/libsinsp/parsers.cpp +++ b/userspace/libsinsp/parsers.cpp @@ -229,9 +229,6 @@ void sinsp_parser::process_event(sinsp_evt &evt, sinsp_parser_verdict &verdict) case PPME_SYSCALL_CLOSE_X: parse_close_exit(evt, verdict); break; - case PPME_SYSCALL_FCNTL_E: - parse_fcntl_enter(evt); - break; case PPME_SYSCALL_FCNTL_X: parse_fcntl_exit(evt); break; @@ -4140,30 +4137,25 @@ void sinsp_parser::parse_select_poll_ppoll_epollwait(sinsp_evt &evt) { *(uint64_t *)evt.get_tinfo()->get_last_event_data() = evt.get_ts(); } -void sinsp_parser::parse_fcntl_enter(sinsp_evt &evt) { +void sinsp_parser::parse_fcntl_exit(sinsp_evt &evt) const { if(evt.get_tinfo() == nullptr) { return; } - const auto cmd = evt.get_param(1)->as(); - - if(cmd == PPM_FCNTL_F_DUPFD || cmd == PPM_FCNTL_F_DUPFD_CLOEXEC) { - store_event(evt); - } -} - -void sinsp_parser::parse_fcntl_exit(sinsp_evt &evt) const { - sinsp_evt *enter_evt = &m_tmp_evt; - // // Extract the return value // const int64_t retval = evt.get_syscall_return_value(); // - // If this is not a F_DUPFD or F_DUPFD_CLOEXEC command, ignore it + // Extract the command + // + const auto cmd = evt.get_param(2)->as(); + + // + // If not a F_DUPFD or F_DUPFD_CLOEXEC command, ignore the event // - if(!retrieve_enter_event(*enter_evt, evt)) { + if(!(cmd == PPM_FCNTL_F_DUPFD || cmd == PPM_FCNTL_F_DUPFD_CLOEXEC)) { return; } diff --git a/userspace/libsinsp/parsers.h b/userspace/libsinsp/parsers.h index 176234ec55..a46f6427f9 100644 --- a/userspace/libsinsp/parsers.h +++ b/userspace/libsinsp/parsers.h @@ -116,7 +116,6 @@ class sinsp_parser { void parse_getrlimit_setrlimit_exit(sinsp_evt& evt) const; void parse_prlimit_exit(sinsp_evt& evt) const; void parse_select_poll_ppoll_epollwait(sinsp_evt& evt); - void parse_fcntl_enter(sinsp_evt& evt); void parse_fcntl_exit(sinsp_evt& evt) const; static void parse_prctl_exit_event(sinsp_evt& evt); static void parse_context_switch(sinsp_evt& evt); diff --git a/userspace/libsinsp/test/scap_files/converter_tests.cpp b/userspace/libsinsp/test/scap_files/converter_tests.cpp index 797e68b26e..2f0c2942be 100644 --- a/userspace/libsinsp/test/scap_files/converter_tests.cpp +++ b/userspace/libsinsp/test/scap_files/converter_tests.cpp @@ -63,6 +63,7 @@ TEST_F(scap_file_test, same_number_of_events) { {PPME_SYSCALL_SPLICE_E, 253}, {PPME_SYSCALL_SPLICE_X, 253}, {PPME_SYSCALL_LSEEK_E, 329}, {PPME_SYSCALL_LSEEK_X, 329}, {PPME_SYSCALL_WRITEV_E, 5}, {PPME_SYSCALL_WRITEV_X, 5}, + {PPME_SYSCALL_FCNTL_E, 9817}, {PPME_SYSCALL_FCNTL_X, 9817}, // Add further checks regarding the expected number of events in this scap file here. }); @@ -428,6 +429,38 @@ TEST_F(scap_file_test, setrlimit_x_check_final_converted_event) { create_safe_scap_event(ts, tid, PPME_SYSCALL_SETRLIMIT_X, 4, res, cur, max, resource)); } +//////////////////////////// +// FCNTL +//////////////////////////// + +TEST_F(scap_file_test, fcntl_x_check_final_converted_event) { + open_filename("kexec_arm64.scap"); + + // Inside the scap-file the event `906671` is the following: + // - type=PPME_SYSCALL_FCNTL_X, + // - ts=1687966734198994052 + // - tid=114093 + // - args=res=0(/dev/null) + // + // And its corresponding enter event `906670` is the following: + // - type=PPME_SYSCALL_FCNTL_E + // - ts=1687966734198993412 + // - tid=114093 + // - args=fd=19(/sys/fs/cgroup/kubelet.slice/kubelet-kubepods.slice/kubelet-kubepods- + // besteffort.slice/kubelet-kubepods-besteffort-pod03e86e4b_ac6e_4488_883e_e4b50b1be176. + // slice/cgroup.procs) + // cmd=5(F_SETFL) + // + // Let's see the new PPME_SYSCALL_FCNTL_X event! + constexpr uint64_t ts = 1687966734198994052; + constexpr int64_t tid = 114093; + constexpr int64_t res = 0; + constexpr int64_t fd = 19; + constexpr uint8_t cmd = 5; + + assert_event_presence(create_safe_scap_event(ts, tid, PPME_SYSCALL_FCNTL_X, 3, res, fd, cmd)); +} + //////////////////////////// ///// BRK ////////////////////////////