From 623f01e402265d6bbc56aeeead51b3c092d86f44 Mon Sep 17 00:00:00 2001 From: Andrea Terzolo Date: Thu, 5 Dec 2024 11:38:41 +0100 Subject: [PATCH] fix: use the fd in the exit event Signed-off-by: Andrea Terzolo --- userspace/libsinsp/event.cpp | 15 +++++++++++++++ userspace/libsinsp/event.h | 8 ++++++++ userspace/libsinsp/parsers.cpp | 31 ++++++++++++------------------- userspace/libsinsp/parsers.h | 1 - 4 files changed, 35 insertions(+), 20 deletions(-) diff --git a/userspace/libsinsp/event.cpp b/userspace/libsinsp/event.cpp index d65565ff741..2d5dbc5dd61 100644 --- a/userspace/libsinsp/event.cpp +++ b/userspace/libsinsp/event.cpp @@ -1839,3 +1839,18 @@ void sinsp_evt_param::throw_invalid_len_error(size_t requested_length) const { const ppm_param_info *sinsp_evt_param::get_info() const { return &(m_evt->get_info()->params[m_idx]); } + +int64_t sinsp_evt::get_used_fd() { + if(!uses_fd()) { + throw sinsp_exception( + "Called get_used_fd() on an event that does not uses an FD. " + "Event type: " + + std::to_string(get_type())); + } + // we could add some extra checks in release mode but we should also trust something + ASSERT(get_param_by_name("fd")) + // todo!: ideally the event table should provide the position of the fd to avoid the loop. + // we could do a manual switch but this seems less error prone. Maybe we could add some static + // checks on the event table to ensure the fd is there. + return get_param_by_name("fd")->as(); +} diff --git a/userspace/libsinsp/event.h b/userspace/libsinsp/event.h index d16f3d1feb5..d77a0548a72 100644 --- a/userspace/libsinsp/event.h +++ b/userspace/libsinsp/event.h @@ -761,6 +761,14 @@ class SINSP_PUBLIC sinsp_evt { } } + inline bool uses_fd() const { return get_info_flags() & EF_USES_FD; } + + // todo!: remove it at the end of the work. Please note with "full" we mean that it has the + // enter event parameters insisde. + inline bool is_full_exit_event() const { return get_info_flags() & EF_TMP_CONVERTER_MANAGED; } + + int32_t get_used_fd(); + private: sinsp* m_inspector; scap_evt* m_pevt; diff --git a/userspace/libsinsp/parsers.cpp b/userspace/libsinsp/parsers.cpp index b8abbaefe99..c65d56a3651 100644 --- a/userspace/libsinsp/parsers.cpp +++ b/userspace/libsinsp/parsers.cpp @@ -581,19 +581,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); - ASSERT(evt->get_param_info(fd_location)->type == PT_FD); - evt->get_tinfo()->m_lastevent_fd = evt->get_param(fd_location)->as(); + evt->get_tinfo()->m_lastevent_fd = evt->get_used_fd(); evt->set_fd_info(evt->get_tinfo()->get_fd(evt->get_tinfo()->m_lastevent_fd)); } @@ -645,7 +645,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 +654,13 @@ bool sinsp_parser::reset(sinsp_evt *evt) { tinfo->m_lastevent_fd = evt->get_param(1)->as(); } + // New exit events don't rely on the enter event to obtain the fd. + // todo!: we can simplify this logic at the + // end of the work since we won't have enter events at all + if(evt->is_full_exit_event()) { + tinfo->m_lastevent_fd = evt->get_used_fd(); + } + evt->set_fd_info(tinfo->get_fd(tinfo->m_lastevent_fd)); if(evt->get_fd_info() == NULL) { @@ -5257,17 +5264,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 ceaa8ad3af9..364aad5e9fd 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