diff --git a/test/libscap/test_suites/userspace/event_table.cpp b/test/libscap/test_suites/userspace/event_table.cpp index f1cc0bef35..252a368348 100644 --- a/test/libscap/test_suites/userspace/event_table.cpp +++ b/test/libscap/test_suites/userspace/event_table.cpp @@ -167,3 +167,27 @@ TEST(event_table, check_usage_of_EC_UNKNOWN_flag) { } } } + +// 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_NE(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 95aadd7ca1..3b253385a0 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..222b680141 100644 --- a/userspace/libscap/scap_event.c +++ b/userspace/libscap/scap_event.c @@ -496,3 +496,48 @@ 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); + ASSERT(scap_get_event_info_table()[etype].flags & EF_TMP_CONVERTER_MANAGED); + + // 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_WRITE_X: + case PPME_SYSCALL_PREAD_X: + case PPME_SYSCALL_PWRITE_X: + location = 2; + break; + default: + break; + } + return location; +} diff --git a/userspace/libsinsp/event.h b/userspace/libsinsp/event.h index d16f3d1feb..6caac297bc 100644 --- a/userspace/libsinsp/event.h +++ b/userspace/libsinsp/event.h @@ -761,6 +761,8 @@ class SINSP_PUBLIC sinsp_evt { } } + inline bool uses_fd() const { return get_info_flags() & EF_USES_FD; } + private: sinsp* m_inspector; scap_evt* m_pevt; diff --git a/userspace/libsinsp/parsers.cpp b/userspace/libsinsp/parsers.cpp index b8abbaefe9..2649bd219c 100644 --- a/userspace/libsinsp/parsers.cpp +++ b/userspace/libsinsp/parsers.cpp @@ -581,18 +581,17 @@ 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); - ASSERT(evt->get_param_info(fd_location)->type == PT_FD); + int fd_location = get_enter_event_fd_location((ppm_event_code)etype); 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)); } @@ -645,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 @@ -654,6 +653,8 @@ bool sinsp_parser::reset(sinsp_evt *evt) { tinfo->m_lastevent_fd = evt->get_param(1)->as(); } + // todo!: This is ok until we have enter events because `m_lastevent_fd` should be + // populated by them. We must modify the logic when we will have only exit events. evt->set_fd_info(tinfo->get_fd(tinfo->m_lastevent_fd)); if(evt->get_fd_info() == NULL) { @@ -5257,17 +5258,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