From 468f39645776026f68f76559d2d45b53e25b5416 Mon Sep 17 00:00:00 2001 From: Andrea Terzolo Date: Sat, 14 Dec 2024 15:20:19 +0100 Subject: [PATCH 1/5] fix(sinsp): don't skip event reset for exit events Signed-off-by: Andrea Terzolo --- userspace/libsinsp/parsers.cpp | 8 ++++---- userspace/libsinsp/test/events_net.ut.cpp | 17 +++++++++-------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/userspace/libsinsp/parsers.cpp b/userspace/libsinsp/parsers.cpp index 8605fce300..f5acf8c0dc 100644 --- a/userspace/libsinsp/parsers.cpp +++ b/userspace/libsinsp/parsers.cpp @@ -622,10 +622,10 @@ bool sinsp_parser::reset(sinsp_evt *evt) { tinfo->set_lastevent_data_validity(true); } else { tinfo->set_lastevent_data_validity(false); - - if(tinfo->get_lastevent_type() != PPME_TRACER_E) { - return false; - } + // We cannot be sure that the lastevent_fd is something valid, it could be the socket of + // the previous `socket` syscall or it could be something completely unrelated, for now + // we don't trust it in any case. + tinfo->m_lastevent_fd = -1; } // diff --git a/userspace/libsinsp/test/events_net.ut.cpp b/userspace/libsinsp/test/events_net.ut.cpp index 5440109005..faedbcf878 100644 --- a/userspace/libsinsp/test/events_net.ut.cpp +++ b/userspace/libsinsp/test/events_net.ut.cpp @@ -806,7 +806,6 @@ TEST_F(sinsp_with_test_input, net_connect_enter_event_is_missing_wo_fd_param_exi add_default_init_thread(); open_inspector(); sinsp_evt* evt = NULL; - sinsp_fdinfo* fdinfo = NULL; int64_t client_fd = 7; add_event_advance_ts(increasing_ts(), @@ -830,7 +829,9 @@ TEST_F(sinsp_with_test_input, net_connect_enter_event_is_missing_wo_fd_param_exi /* We dropped connect enter! */ - /* We read an old scap file with a connect exit event with just 2 params (no fd!) */ + /* todo!: revisit this when we will manage CONNECT_X in scap files. + * We simulate an event from an old scap file, a connect exit event with just 2 params (no fd!) + */ std::vector socktuple = test_utils::pack_socktuple(reinterpret_cast(&client), reinterpret_cast(&server)); @@ -841,8 +842,13 @@ TEST_F(sinsp_with_test_input, net_connect_enter_event_is_missing_wo_fd_param_exi return_value, scap_const_sized_buffer{socktuple.data(), socktuple.size()}); + /* We cannot recover the file descriptor from the enter event neither from the exit event */ + ASSERT_EQ(evt->get_fd_info(), nullptr); + + /* We recover this from the tuple */ + ASSERT_EQ(get_field_as_string(evt, "fd.name"), "80.9.11.45:12->152.40.111.222:25632"); + /* Check that we are not able to load any info */ - ASSERT_EQ(get_field_as_string(evt, "fd.name"), ""); ASSERT_FALSE(field_has_value(evt, "fd.sip")); ASSERT_FALSE(field_has_value(evt, "fd.cip")); ASSERT_FALSE(field_has_value(evt, "fd.rip")); @@ -851,9 +857,4 @@ TEST_F(sinsp_with_test_input, net_connect_enter_event_is_missing_wo_fd_param_exi ASSERT_FALSE(field_has_value(evt, "fd.sport")); ASSERT_FALSE(field_has_value(evt, "fd.lport")); ASSERT_FALSE(field_has_value(evt, "fd.rport")); - - /* The parser is not able to obtain an updated fdname because the syscall fails and the parser - * flow is truncated */ - fdinfo = evt->get_fd_info(); - ASSERT_EQ(fdinfo, nullptr); } From 72a6d4f39aef6a8c75c6dc0053af452e08d22fa4 Mon Sep 17 00:00:00 2001 From: Andrea Terzolo Date: Sat, 14 Dec 2024 15:22:40 +0100 Subject: [PATCH 2/5] fix(converter): always process enter events like in live captures Signed-off-by: Andrea Terzolo --- .../engine/savefile/converter/converter.cpp | 14 ++++++-------- userspace/libscap/engine/savefile/scap_savefile.c | 5 +---- .../libsinsp/test/scap_files/converter_tests.cpp | 13 ++++++------- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/userspace/libscap/engine/savefile/converter/converter.cpp b/userspace/libscap/engine/savefile/converter/converter.cpp index a0c8195df4..4ca85a27a5 100644 --- a/userspace/libscap/engine/savefile/converter/converter.cpp +++ b/userspace/libscap/engine/savefile/converter/converter.cpp @@ -317,6 +317,10 @@ static conversion_result convert_event(scap_evt *new_evt, uint16_t params_offset = 0; int param_to_populate = 0; + // We copy the entire event in any case so that we are ready to handle `CONVERSION_SKIP` cases + // without further actions. + memcpy(new_evt, evt_to_convert, evt_to_convert->len); + switch(ci.m_action) { case C_ACTION_SKIP: return CONVERSION_SKIP; @@ -326,7 +330,6 @@ static conversion_result convert_event(scap_evt *new_evt, return CONVERSION_SKIP; case C_ACTION_ADD_PARAMS: - memcpy(new_evt, evt_to_convert, sizeof(scap_evt)); // The new number of params is the previous one plus the number of conversion instructions. new_evt->nparams = evt_to_convert->nparams + ci.m_instrs.size(); params_offset = copy_old_params(new_evt, evt_to_convert); @@ -334,7 +337,6 @@ static conversion_result convert_event(scap_evt *new_evt, break; case C_ACTION_CHANGE_TYPE: - memcpy(new_evt, evt_to_convert, sizeof(scap_evt)); // The new number of params is the number of conversion instructions. new_evt->nparams = ci.m_instrs.size(); new_evt->type = ci.m_desired_type; @@ -355,8 +357,6 @@ static conversion_result convert_event(scap_evt *new_evt, PRINT_EVENT(new_evt, PRINT_HEADER); scap_evt *tmp_evt = NULL; - // If this is true at the end of the for loop we will free its memory. - bool used_enter_event = false; // We iterate over the instructions for(int i = 0; i < ci.m_instrs.size(); i++, param_to_populate++) { @@ -390,7 +390,6 @@ static conversion_result convert_event(scap_evt *new_evt, get_direction_char((ppm_event_code)tmp_evt->type)); return CONVERSION_ERROR; } - used_enter_event = true; break; case C_INSTR_FROM_OLD: @@ -429,11 +428,10 @@ static conversion_result convert_event(scap_evt *new_evt, } } - if(used_enter_event) { - // We can free the enter event because we don't need it anymore. + if(PPME_IS_EXIT(evt_to_convert->type)) { + // We can free the enter event for this thread because we don't need it anymore. clear_evt(evt_to_convert->tid); } - new_evt->len = params_offset; PRINT_MESSAGE("Final event:\n"); diff --git a/userspace/libscap/engine/savefile/scap_savefile.c b/userspace/libscap/engine/savefile/scap_savefile.c index 214611f06c..e4b7da1d41 100644 --- a/userspace/libscap/engine/savefile/scap_savefile.c +++ b/userspace/libscap/engine/savefile/scap_savefile.c @@ -2027,7 +2027,6 @@ static int32_t next(struct scap_engine_handle engine, uint16_t *pdevid, uint32_t *pflags) { struct savefile_engine *handle = engine.m_handle; -read_event:; int32_t res = next_event_from_file(handle, pevent, pdevid, pflags); // If we fail we don't convert the event. if(res != SCAP_SUCCESS) { @@ -2065,10 +2064,8 @@ read_event:; case CONVERSION_ERROR: return SCAP_FAILURE; + // today with CONVERSION_SKIP we send the event to userspace, tomorrow we could drop it. case CONVERSION_SKIP: - // Probably an enter event that we don't want to consider. So we read another event. - goto read_event; - case CONVERSION_COMPLETED: case CONVERSION_CONTINUE: default: diff --git a/userspace/libsinsp/test/scap_files/converter_tests.cpp b/userspace/libsinsp/test/scap_files/converter_tests.cpp index 3bb42f5876..1e7817af0b 100644 --- a/userspace/libsinsp/test/scap_files/converter_tests.cpp +++ b/userspace/libsinsp/test/scap_files/converter_tests.cpp @@ -27,13 +27,12 @@ limitations under the License. // READ //////////////////////////// -TEST_F(scap_file_test, no_read_e) { +TEST_F(scap_file_test, read_e_same_number_of_events) { open_filename("scap_2013.scap"); - // we shouldn't see the enter events - assert_no_event_type(PPME_SYSCALL_READ_E); + assert_num_event_type(PPME_SYSCALL_READ_E, 24956); } -TEST_F(scap_file_test, read_x_same_number_of_exit_events) { +TEST_F(scap_file_test, read_x_same_number_of_events) { open_filename("scap_2013.scap"); assert_num_event_type(PPME_SYSCALL_READ_X, 24957); } @@ -80,12 +79,12 @@ TEST_F(scap_file_test, read_x_check_final_converted_event) { // PREAD //////////////////////////// -TEST_F(scap_file_test, no_pread_e) { +TEST_F(scap_file_test, pread_e_same_number_of_events) { open_filename("kexec_arm64.scap"); - assert_no_event_type(PPME_SYSCALL_PREAD_E); + assert_num_event_type(PPME_SYSCALL_PREAD_E, 3216); } -TEST_F(scap_file_test, pread_x_same_number_of_exit_events) { +TEST_F(scap_file_test, pread_x_same_number_of_events) { open_filename("kexec_arm64.scap"); assert_num_event_type(PPME_SYSCALL_PREAD_X, 3216); } From 2684d5c42a474a8b10dac354aee891a9a11b2b82 Mon Sep 17 00:00:00 2001 From: Andrea Terzolo Date: Sat, 14 Dec 2024 15:24:24 +0100 Subject: [PATCH 3/5] update: use fd in the exit event if available Signed-off-by: Andrea Terzolo --- .../test_suites/userspace/event_table.cpp | 24 +++++++++++ userspace/libscap/scap.h | 3 +- userspace/libscap/scap_event.c | 42 +++++++++++++++++++ userspace/libsinsp/event.h | 4 ++ userspace/libsinsp/parsers.cpp | 29 ++++++------- userspace/libsinsp/parsers.h | 1 - 6 files changed, 84 insertions(+), 19 deletions(-) diff --git a/test/libscap/test_suites/userspace/event_table.cpp b/test/libscap/test_suites/userspace/event_table.cpp index 56fcf0ae8d..55dc2a81eb 100644 --- a/test/libscap/test_suites/userspace/event_table.cpp +++ b/test/libscap/test_suites/userspace/event_table.cpp @@ -190,3 +190,27 @@ TEST(event_table, check_exit_param_names) { } } } + +// todo!: revisit this test after we remove the enter event support in sinsp +TEST(event_table, check_EF_USED_FD) { + for(int evt = 0; evt < PPM_EVENT_MAX; evt++) { + auto evt_info = scap_get_event_info_table()[evt]; + if((evt_info.flags & EF_USES_FD) == 0) { + continue; + } + + if(PPME_IS_ENTER(evt)) { + int location = get_enter_event_fd_location((ppm_event_code)evt); + ASSERT_EQ(evt_info.params[location].type, PT_FD) + << "event_type " << evt << " uses a wrong location " << location; + } + + if(PPME_IS_EXIT(evt) && evt_info.flags & EF_TMP_CONVERTER_MANAGED) { + int location = get_exit_event_fd_location((ppm_event_code)evt); + ASSERT_NE(location, -1) + << "event_type " << evt << " uses a wrong location " << location; + ASSERT_EQ(evt_info.params[location].type, PT_FD) + << "event_type " << evt << " uses a wrong location " << location; + } + } +} diff --git a/userspace/libscap/scap.h b/userspace/libscap/scap.h index 7ffae1a56b..6685f9f32b 100644 --- a/userspace/libscap/scap.h +++ b/userspace/libscap/scap.h @@ -874,7 +874,8 @@ typedef enum scap_print_info { PRINT_FULL, } scap_print_info; void scap_print_event(scap_evt* ev, scap_print_info i); - +int get_enter_event_fd_location(ppm_event_code etype); +int get_exit_event_fd_location(ppm_event_code etype); /*@}*/ /////////////////////////////////////////////////////////////////////////////// diff --git a/userspace/libscap/scap_event.c b/userspace/libscap/scap_event.c index d375fb7666..c13a55efb5 100644 --- a/userspace/libscap/scap_event.c +++ b/userspace/libscap/scap_event.c @@ -496,3 +496,45 @@ scap_evt *scap_create_event(char *error, va_end(args); return ret; } + +// Only enter events have a convention on the fd parameter position. +// Should be always the first parameter apart from some exceptions. +int get_enter_event_fd_location(ppm_event_code etype) { + ASSERT(etype < PPM_EVENT_MAX); + ASSERT(PPME_IS_ENTER(etype)); + ASSERT(scap_get_event_info_table()[etype].flags & EF_USES_FD); + + // For almost all parameters the default position is `0` + int location = 0; + switch(etype) { + case PPME_SYSCALL_MMAP_E: + case PPME_SYSCALL_MMAP2_E: + location = 4; + break; + case PPME_SYSCALL_SPLICE_E: + location = 1; + break; + default: + break; + } + return location; +} + +// In the exit events we don't have a precise convension on the fd parameter position. +int get_exit_event_fd_location(ppm_event_code etype) { + ASSERT(etype < PPM_EVENT_MAX); + ASSERT(PPME_IS_EXIT(etype)); + ASSERT(scap_get_event_info_table()[etype].flags & EF_USES_FD); + + // we want to return -1 as location if we forgot to handle something + int location = -1; + switch(etype) { + case PPME_SYSCALL_READ_X: + case PPME_SYSCALL_PREAD_X: + location = 2; + break; + default: + break; + } + return location; +} diff --git a/userspace/libsinsp/event.h b/userspace/libsinsp/event.h index ac1e4a3824..4a34d5eac5 100644 --- a/userspace/libsinsp/event.h +++ b/userspace/libsinsp/event.h @@ -769,6 +769,10 @@ class SINSP_PUBLIC sinsp_evt { } } + inline bool uses_fd() const { return get_info_flags() & EF_USES_FD; } + + inline bool creates_fd() const { return get_info_flags() & EF_CREATES_FD; } + private: sinsp* m_inspector; scap_evt* m_pevt; diff --git a/userspace/libsinsp/parsers.cpp b/userspace/libsinsp/parsers.cpp index f5acf8c0dc..c79cbcd31c 100644 --- a/userspace/libsinsp/parsers.cpp +++ b/userspace/libsinsp/parsers.cpp @@ -586,17 +586,19 @@ bool sinsp_parser::reset(sinsp_evt *evt) { evt->get_tinfo()->m_flags |= PPM_CL_ACTIVE; } + // todo!: at the end of we work we should remove this if/else and ideally we should set the + // fdinfos directly here and return if they are not present if(PPME_IS_ENTER(etype)) { evt->get_tinfo()->m_lastevent_fd = -1; evt->get_tinfo()->set_lastevent_type(etype); - if(eflags & EF_USES_FD) { + if(evt->uses_fd()) { // // Get the fd. // An fd will usually be the first parameter of the enter event, // but there are exceptions, as is the case with mmap, mmap2 // - int fd_location = get_fd_location(etype); + int fd_location = get_enter_event_fd_location((ppm_event_code)etype); ASSERT(evt->get_param_info(fd_location)->type == PT_FD); evt->get_tinfo()->m_lastevent_fd = evt->get_param(fd_location)->as(); evt->set_fd_info(evt->get_tinfo()->get_fd(evt->get_tinfo()->m_lastevent_fd)); @@ -642,7 +644,7 @@ bool sinsp_parser::reset(sinsp_evt *evt) { // // Retrieve the fd // - if(eflags & EF_USES_FD) { + if(evt->uses_fd()) { // // The copy_file_range syscall has the peculiarity of using two fds // Set as m_lastevent_fd the output fd @@ -651,6 +653,13 @@ bool sinsp_parser::reset(sinsp_evt *evt) { tinfo->m_lastevent_fd = evt->get_param(1)->as(); } + // todo!: this should become the unique logic when we'll disable the enter events. + if(tinfo->m_lastevent_fd == -1) { + int fd_location = get_exit_event_fd_location((ppm_event_code)etype); + if(fd_location != -1) { + tinfo->m_lastevent_fd = evt->get_param(fd_location)->as(); + } + } evt->set_fd_info(tinfo->get_fd(tinfo->m_lastevent_fd)); if(evt->get_fd_info() == NULL) { @@ -5259,17 +5268,3 @@ void sinsp_parser::parse_pidfd_getfd_exit(sinsp_evt *evt) { } evt->get_tinfo()->add_fd(fd, targetfd_fdinfo->clone()); } - -int sinsp_parser::get_fd_location(uint16_t etype) { - int location; - switch(etype) { - case PPME_SYSCALL_MMAP_E: - case PPME_SYSCALL_MMAP2_E: - location = 4; - break; - default: - location = 0; - break; - } - return location; -} diff --git a/userspace/libsinsp/parsers.h b/userspace/libsinsp/parsers.h index ceaa8ad3af..364aad5e9f 100644 --- a/userspace/libsinsp/parsers.h +++ b/userspace/libsinsp/parsers.h @@ -151,7 +151,6 @@ class sinsp_parser { void swap_addresses(sinsp_fdinfo* fdinfo); uint8_t* reserve_event_buffer(); void free_event_buffer(uint8_t*); - inline int get_fd_location(uint16_t etype); // // Pointers to inspector context From 68f6a46d35b9f28b744e68cd7e582fbbc7fbc358 Mon Sep 17 00:00:00 2001 From: Andrea Terzolo Date: Sat, 14 Dec 2024 15:24:53 +0100 Subject: [PATCH 4/5] tests: add test for exit events Signed-off-by: Andrea Terzolo --- userspace/libsinsp/test/parsers/parse_brk.cpp | 84 +++++++++++++ .../libsinsp/test/parsers/parse_pread.cpp | 88 ++++++++++++++ .../libsinsp/test/parsers/parse_read.cpp | 84 +++++++++++++ .../libsinsp/test/sinsp_with_test_input.cpp | 112 ++++++++++++++++++ .../libsinsp/test/sinsp_with_test_input.h | 16 ++- 5 files changed, 382 insertions(+), 2 deletions(-) create mode 100644 userspace/libsinsp/test/parsers/parse_brk.cpp create mode 100644 userspace/libsinsp/test/parsers/parse_pread.cpp create mode 100644 userspace/libsinsp/test/parsers/parse_read.cpp diff --git a/userspace/libsinsp/test/parsers/parse_brk.cpp b/userspace/libsinsp/test/parsers/parse_brk.cpp new file mode 100644 index 0000000000..fcd3dc6182 --- /dev/null +++ b/userspace/libsinsp/test/parsers/parse_brk.cpp @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: Apache-2.0 +/* +Copyright (C) 2024 The Falco Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +#include + +TEST_F(sinsp_with_test_input, parse_brk_updated_prog_break) { + add_default_init_thread(); + open_inspector(); + + // if the program break is updated the res should be equal to `addr` + uint64_t res = 83983092; + uint32_t vm_size = 294; + uint32_t vm_rss = 295; + uint32_t vm_swap = 296; + + auto evt = add_event_advance_ts(increasing_ts(), + INIT_TID, + PPME_SYSCALL_BRK_4_X, + 4, + res, + vm_size, + vm_rss, + vm_swap); + + auto init_tinfo = m_inspector.get_thread_ref(INIT_TID, false, true).get(); + ASSERT_TRUE(init_tinfo); + ASSERT_EQ(init_tinfo->m_vmsize_kb, vm_size); + ASSERT_EQ(init_tinfo->m_vmrss_kb, vm_rss); + ASSERT_EQ(init_tinfo->m_vmswap_kb, vm_swap); + + assert_return_value(evt, res); + + ASSERT_EQ(get_field_as_string(evt, "evt.arg[1]"), std::to_string(vm_size)); + ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.vm_size"), std::to_string(vm_size)); + + ASSERT_EQ(get_field_as_string(evt, "evt.arg[2]"), std::to_string(vm_rss)); + ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.vm_rss"), std::to_string(vm_rss)); + + ASSERT_EQ(get_field_as_string(evt, "evt.arg[3]"), std::to_string(vm_swap)); + ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.vm_swap"), std::to_string(vm_swap)); +} + +TEST_F(sinsp_with_test_input, parse_brk_no_update) { + add_default_init_thread(); + open_inspector(); + + // if the program break is different from `addr`. + uint64_t res = 83983090; + uint32_t vm_size = 294; + uint32_t vm_rss = 295; + uint32_t vm_swap = 296; + + auto evt = add_event_advance_ts(increasing_ts(), + INIT_TID, + PPME_SYSCALL_BRK_4_X, + 4, + res, + vm_size, + vm_rss, + vm_swap); + + auto init_tinfo = m_inspector.get_thread_ref(INIT_TID, false, true).get(); + ASSERT_TRUE(init_tinfo); + // We should always update the info + ASSERT_EQ(init_tinfo->m_vmsize_kb, vm_size); + ASSERT_EQ(init_tinfo->m_vmrss_kb, vm_rss); + ASSERT_EQ(init_tinfo->m_vmswap_kb, vm_swap); + + // BRK can update or not update the program break according to the value we provide. Today we + // don't consider a failure if the program break in not updated, we consider a failure only if + // the syscall sets an errno. + assert_return_value(evt, res); +} diff --git a/userspace/libsinsp/test/parsers/parse_pread.cpp b/userspace/libsinsp/test/parsers/parse_pread.cpp new file mode 100644 index 0000000000..211c75cc4f --- /dev/null +++ b/userspace/libsinsp/test/parsers/parse_pread.cpp @@ -0,0 +1,88 @@ + +// SPDX-License-Identifier: Apache-2.0 +/* +Copyright (C) 2024 The Falco Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +#include + +TEST_F(sinsp_with_test_input, parse_pread_success) { + add_default_init_thread(); + open_inspector(); + + auto evt = generate_open_x_event(); + ASSERT_TRUE(evt->get_fd_info()); + + std::string data = "hello"; + uint32_t size = data.size(); + uint64_t pos = 0; + evt = add_event_advance_ts(increasing_ts(), + INIT_TID, + PPME_SYSCALL_PREAD_X, + 5, + (int64_t)size, + scap_const_sized_buffer{data.c_str(), size}, + sinsp_test_input::open_params::default_fd, + size, + pos); + + ASSERT_TRUE(evt->get_fd_info()); + assert_fd_fields(evt, + sinsp_test_input::fd_info_fields{ + .fd_num = sinsp_test_input::open_params::default_fd, + .fd_name = sinsp_test_input::open_params::default_path, + .fd_name_raw = sinsp_test_input::open_params::default_path, + .fd_directory = sinsp_test_input::open_params::default_directory, + .fd_filename = sinsp_test_input::open_params::default_filename}); + + assert_return_value(evt, size); + + ASSERT_EQ(get_field_as_string(evt, "evt.arg[1]"), data); + ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.data"), data); + + ASSERT_EQ(get_field_as_string(evt, "evt.arg[2]"), + std::string("") + sinsp_test_input::open_params::default_path); + ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.fd"), + std::to_string(sinsp_test_input::open_params::default_fd)); + + ASSERT_EQ(get_field_as_string(evt, "evt.arg[3]"), std::to_string(size)); + ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.size"), std::to_string(size)); +} + +TEST_F(sinsp_with_test_input, parse_pread_failure) { + add_default_init_thread(); + open_inspector(); + + auto evt = generate_open_x_event(); + + std::string data = "hello"; + uint32_t size = data.size(); + int64_t errno_code = -3; + uint64_t pos = 0; + evt = add_event_advance_ts(increasing_ts(), + INIT_TID, + PPME_SYSCALL_PREAD_X, + 5, + errno_code, + scap_const_sized_buffer{data.c_str(), size}, + sinsp_test_input::open_params::default_fd, + size, + pos); + + // Check we have the correct fd info associated with the event + auto fdinfo = evt->get_fd_info(); + ASSERT_TRUE(fdinfo); + ASSERT_EQ(fdinfo->m_fd, sinsp_test_input::open_params::default_fd); + + // Assert return value filterchecks + assert_return_value(evt, errno_code); +} diff --git a/userspace/libsinsp/test/parsers/parse_read.cpp b/userspace/libsinsp/test/parsers/parse_read.cpp new file mode 100644 index 0000000000..3d5ed2da31 --- /dev/null +++ b/userspace/libsinsp/test/parsers/parse_read.cpp @@ -0,0 +1,84 @@ + +// SPDX-License-Identifier: Apache-2.0 +/* +Copyright (C) 2024 The Falco Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +#include + +TEST_F(sinsp_with_test_input, parse_read_success) { + add_default_init_thread(); + open_inspector(); + + auto evt = generate_open_x_event(); + ASSERT_TRUE(evt->get_fd_info()); + + std::string data = "hello"; + uint32_t size = data.size(); + evt = add_event_advance_ts(increasing_ts(), + INIT_TID, + PPME_SYSCALL_READ_X, + 4, + (int64_t)size, + scap_const_sized_buffer{data.c_str(), size}, + sinsp_test_input::open_params::default_fd, + size); + + ASSERT_TRUE(evt->get_fd_info()); + assert_fd_fields(evt, + sinsp_test_input::fd_info_fields{ + .fd_num = sinsp_test_input::open_params::default_fd, + .fd_name = sinsp_test_input::open_params::default_path, + .fd_name_raw = sinsp_test_input::open_params::default_path, + .fd_directory = sinsp_test_input::open_params::default_directory, + .fd_filename = sinsp_test_input::open_params::default_filename}); + + assert_return_value(evt, size); + + ASSERT_EQ(get_field_as_string(evt, "evt.arg[1]"), data); + ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.data"), data); + + ASSERT_EQ(get_field_as_string(evt, "evt.arg[2]"), + std::string("") + sinsp_test_input::open_params::default_path); + ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.fd"), + std::to_string(sinsp_test_input::open_params::default_fd)); + + ASSERT_EQ(get_field_as_string(evt, "evt.arg[3]"), std::to_string(size)); + ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.size"), std::to_string(size)); +} + +TEST_F(sinsp_with_test_input, parse_read_failure) { + add_default_init_thread(); + open_inspector(); + + auto evt = generate_open_x_event(); + + std::string data = "hello"; + uint32_t size = data.size(); + int64_t errno_code = -3; + evt = add_event_advance_ts(increasing_ts(), + INIT_TID, + PPME_SYSCALL_READ_X, + 4, + errno_code, + scap_const_sized_buffer{data.c_str(), size}, + sinsp_test_input::open_params::default_fd, + size); + + // Check we have the correct fd info associated with the event + auto fdinfo = evt->get_fd_info(); + ASSERT_TRUE(fdinfo); + ASSERT_EQ(fdinfo->m_fd, sinsp_test_input::open_params::default_fd); + + // Assert return value filterchecks + assert_return_value(evt, errno_code); +} diff --git a/userspace/libsinsp/test/sinsp_with_test_input.cpp b/userspace/libsinsp/test/sinsp_with_test_input.cpp index e6694a132e..e997b5265e 100644 --- a/userspace/libsinsp/test/sinsp_with_test_input.cpp +++ b/userspace/libsinsp/test/sinsp_with_test_input.cpp @@ -693,3 +693,115 @@ sinsp_evt* sinsp_with_test_input::next_event() { auto result = m_inspector.next(&evt); return result == SCAP_SUCCESS ? evt : nullptr; } + +void sinsp_with_test_input::assert_return_value(sinsp_evt* evt, int64_t expected_retval) { + // First the event we provide should have the return value + ASSERT_TRUE(evt->has_return_value()); + + // The raw value should be equal to what we expect + ASSERT_EQ(evt->get_syscall_return_value(), expected_retval); + + // We need to create the filtercheck name for the first parameter, usually `res`/`fd` but not + // always, could be also `addr` for example... + std::string evt_rawarg_name = std::string("evt.rawarg.") + std::string(evt->get_param_name(0)); + + // SUCCESS CASE: we consider success when retval >= 0 (so != errno) + if(expected_retval >= 0) { + ///////////// + // Res + ///////////// + ASSERT_EQ(get_field_as_string(evt, "evt.res"), "SUCCESS"); + ASSERT_EQ(get_field_as_string(evt, "evt.rawres"), std::to_string(expected_retval)); + ASSERT_EQ(get_field_as_string(evt, "evt.failed"), "false"); + + ///////////// + // First parameter (arg0) + ///////////// + auto arg0_info = evt->get_param_info(0); + ASSERT_TRUE(arg0_info); + // Default values + std::string arg0 = std::to_string(expected_retval); + std::string raw_arg0 = std::to_string(expected_retval); + + // Some cases in which we need to modify `arg0` or `raw_arg0` + if(arg0_info->type == PT_FD) { + // If the return value is a file descriptor the value of `evt.arg[0]` is the path + // prefixed by `` + auto fdinfo = evt->get_fd_info(); + ASSERT_TRUE(fdinfo); + arg0 = std::string("") + fdinfo->m_name; + + // raw_arg0 is ok! + } + + if(arg0_info->fmt == PF_HEX && arg0_info->type == PT_UINT64) { + // both `evt.arg[0]` and `evt.rawarg` become hexadecimal + char buffer[100]; + std::snprintf(buffer, sizeof(buffer), "%" PRIX64, expected_retval); + arg0 = buffer; + raw_arg0 = buffer; + } + + ASSERT_EQ(get_field_as_string(evt, "evt.arg[0]"), arg0); + ASSERT_EQ(get_field_as_string(evt, evt_rawarg_name), raw_arg0); + } else { + ///////////// + // Res + ///////////// + ASSERT_EQ(get_field_as_string(evt, "evt.res"), sinsp_utils::errno_to_str(expected_retval)); + ASSERT_EQ(get_field_as_string(evt, "evt.rawres"), std::to_string(expected_retval)); + ASSERT_EQ(get_field_as_string(evt, "evt.failed"), "true"); + + ///////////// + // First parameter (arg0) + ///////////// + ASSERT_EQ(get_field_as_string(evt, "evt.arg[0]"), get_field_as_string(evt, "evt.res")); + ASSERT_EQ(get_field_as_string(evt, evt_rawarg_name), + get_field_as_string(evt, "evt.rawres")); + } +} + +void sinsp_with_test_input::assert_fd_fields(sinsp_evt* evt, + sinsp_test_input::fd_info_fields fields) { + auto fdinfo = evt->get_fd_info(); + + // Right now we only assert if the fdinfo is present, but we want to be sure that for some + // events the fdinfo should be here + if((evt->creates_fd() || evt->uses_fd()) && evt->get_syscall_return_value() >= 0) { + ASSERT_TRUE(fdinfo); + } + + if(fdinfo) { + if(fields.fd_num.has_value()) { + ASSERT_EQ(fdinfo->m_fd, fields.fd_num.value()); + } + + if(fields.fd_name.has_value()) { + ASSERT_EQ(fdinfo->m_name, fields.fd_name.value()); + } + + if(fields.fd_name_raw.has_value()) { + ASSERT_EQ(fdinfo->m_name_raw, fields.fd_name_raw.value()); + } + } + + if(fields.fd_num.has_value()) { + ASSERT_EQ(get_field_as_string(evt, "fd.num"), std::to_string(fields.fd_num.value())); + } + + if(fields.fd_name.has_value()) { + ASSERT_EQ(get_field_as_string(evt, "fd.name"), fields.fd_name.value()); + } + + if(fields.fd_name_raw.has_value()) { + ASSERT_EQ(get_field_as_string(evt, "fd.nameraw"), fields.fd_name_raw.value()); + } + + if(fields.fd_directory.has_value()) { + ASSERT_EQ(get_field_as_string(evt, "fd.directory"), fields.fd_directory.value()); + } + + if(fields.fd_filename.has_value()) { + ASSERT_EQ(get_field_as_string(evt, "fd.filename"), fields.fd_filename.value()); + } +} diff --git a/userspace/libsinsp/test/sinsp_with_test_input.h b/userspace/libsinsp/test/sinsp_with_test_input.h index 5ec82aeffb..77e243edcf 100644 --- a/userspace/libsinsp/test/sinsp_with_test_input.h +++ b/userspace/libsinsp/test/sinsp_with_test_input.h @@ -36,19 +36,28 @@ limitations under the License. namespace sinsp_test_input { struct open_params { - static constexpr int32_t default_fd = 4; + static constexpr int64_t default_fd = 4; static constexpr const char* default_path = "/home/file.txt"; // Used for some filter checks static constexpr const char* default_directory = "/home"; static constexpr const char* default_filename = "file.txt"; - int32_t fd = default_fd; + int64_t fd = default_fd; const char* path = default_path; uint32_t flags = 0; uint32_t mode = 0; uint32_t dev = 0; uint64_t ino = 0; }; + +struct fd_info_fields { + std::optional fd_num = std::nullopt; + std::optional fd_name = std::nullopt; + std::optional fd_name_raw = std::nullopt; + std::optional fd_directory = std::nullopt; + std::optional fd_filename = std::nullopt; +}; + } // namespace sinsp_test_input class sinsp_with_test_input : public ::testing::Test { @@ -281,6 +290,9 @@ class sinsp_with_test_input : public ::testing::Test { bool filter_compiles(std::string_view filter_str); bool filter_compiles(std::string_view filter_str, filter_check_list&); + void assert_return_value(sinsp_evt* evt, int64_t expected_retval); + void assert_fd_fields(sinsp_evt* evt, sinsp_test_input::fd_info_fields fields = {}); + sinsp_evt* next_event(); scap_test_input_data m_test_data; From d6658b7e69b8f697e07dc5bb8d7a15be4a4c4b3a Mon Sep 17 00:00:00 2001 From: Andrea Terzolo Date: Mon, 16 Dec 2024 11:52:52 +0100 Subject: [PATCH 5/5] cleanup: remove some duplicate code since write has the EF_USES_FLAG, we call `set_fd_info` in `sinsp_parser::reset` Signed-off-by: Andrea Terzolo --- userspace/libsinsp/parsers.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/userspace/libsinsp/parsers.cpp b/userspace/libsinsp/parsers.cpp index c79cbcd31c..f4f05ab30c 100644 --- a/userspace/libsinsp/parsers.cpp +++ b/userspace/libsinsp/parsers.cpp @@ -116,12 +116,6 @@ void sinsp_parser::process_event(sinsp_evt *evt) { case PPME_SYSCALL_EXECVEAT_E: store_event(evt); break; - case PPME_SYSCALL_WRITE_E: - if(!m_inspector->is_dumping() && evt->get_tinfo() != nullptr) { - // note(jasondellaluce): this may be useless now that we removed tracers support - evt->set_fd_info(evt->get_tinfo()->get_fd(evt->get_tinfo()->m_lastevent_fd)); - } - break; case PPME_SYSCALL_MKDIR_X: case PPME_SYSCALL_RMDIR_X: case PPME_SYSCALL_LINK_X: