From 9a0e681842fa285fd181523ec6fef74f0ee45b26 Mon Sep 17 00:00:00 2001
From: Andrea Terzolo <andreaterzolo3@gmail.com>
Date: Thu, 5 Dec 2024 11:38:41 +0100
Subject: [PATCH] fix: use the fd in the exit event

Signed-off-by: Andrea Terzolo <andreaterzolo3@gmail.com>
---
 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<int64_t>();
+}
diff --git a/userspace/libsinsp/event.h b/userspace/libsinsp/event.h
index d16f3d1feb5..fa644f1b661 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; }
+
+	int64_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<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));
 		}
 
@@ -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<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) {
@@ -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