Skip to content

Commit

Permalink
fix: use the fd in the exit event
Browse files Browse the repository at this point in the history
Signed-off-by: Andrea Terzolo <[email protected]>
  • Loading branch information
Andreagit97 committed Dec 5, 2024
1 parent 1d0673e commit 9a0e681
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 20 deletions.
15 changes: 15 additions & 0 deletions userspace/libsinsp/event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<int64_t>();
}
8 changes: 8 additions & 0 deletions userspace/libsinsp/event.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }

int64_t get_used_fd();

private:
sinsp* m_inspector;
scap_evt* m_pevt;
Expand Down
31 changes: 12 additions & 19 deletions userspace/libsinsp/parsers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<int64_t>();
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));
}

Expand Down Expand Up @@ -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
Expand All @@ -654,6 +654,13 @@ bool sinsp_parser::reset(sinsp_evt *evt) {
tinfo->m_lastevent_fd = evt->get_param(1)->as<int64_t>();
}

// 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) {
Expand Down Expand Up @@ -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;
}
1 change: 0 additions & 1 deletion userspace/libsinsp/parsers.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 9a0e681

Please sign in to comment.