From d613b3e4eaeeb30118d395355f2d537fdc88e823 Mon Sep 17 00:00:00 2001
From: Federico Di Pierro <nierro92@gmail.com>
Date: Mon, 9 Dec 2024 14:25:16 +0100
Subject: [PATCH] chore(userspace/libsinsp): move user group manager on
 container_id changed refresh to a RAII object.

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
---
 userspace/libsinsp/parsers.cpp | 29 ------------------
 userspace/libsinsp/sinsp.cpp   |  5 +++
 userspace/libsinsp/user.cpp    |  1 -
 userspace/libsinsp/user.h      | 56 ++++++++++++++++++++++++++++++++--
 4 files changed, 59 insertions(+), 32 deletions(-)

diff --git a/userspace/libsinsp/parsers.cpp b/userspace/libsinsp/parsers.cpp
index d14c8ebdf9..61324c9fb6 100644
--- a/userspace/libsinsp/parsers.cpp
+++ b/userspace/libsinsp/parsers.cpp
@@ -1274,12 +1274,6 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid) {
 		return;
 	}
 
-	/* Refresh user / group */
-	if(new_child->m_container_id.empty() == false) {
-		new_child->set_group(new_child->m_gid);
-		new_child->set_user(new_child->m_uid);
-	}
-
 	/* If there's a listener, invoke it */
 	if(m_inspector->get_observer()) {
 		m_inspector->get_observer()->on_clone(evt, new_child.get(), tid_collision);
@@ -1764,12 +1758,6 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt) {
 	 */
 	evt->set_tinfo(new_child.get());
 
-	/* Refresh user / group */
-	if(new_child->m_container_id.empty() == false) {
-		new_child->set_group(new_child->m_gid);
-		new_child->set_user(new_child->m_uid);
-	}
-
 	//
 	// If there's a listener, invoke it
 	//
@@ -2239,15 +2227,6 @@ void sinsp_parser::parse_execve_exit(sinsp_evt *evt) {
 	//
 	evt->get_tinfo()->compute_program_hash();
 
-	//
-	// Refresh user / group
-	// if we happen to change container id
-	//
-	if(container_id != evt->get_tinfo()->m_container_id) {
-		evt->get_tinfo()->set_group(evt->get_tinfo()->m_gid);
-		evt->get_tinfo()->set_user(evt->get_tinfo()->m_uid);
-	}
-
 	//
 	// If there's a listener, invoke it
 	//
@@ -4992,14 +4971,6 @@ void sinsp_parser::parse_chroot_exit(sinsp_evt *evt) {
 		m_inspector->m_container_manager.resolve_container(
 		        evt->get_tinfo(),
 		        m_inspector->is_live() || m_inspector->is_syscall_plugin());
-		//
-		// Refresh user / group
-		// if we happen to change container id
-		//
-		if(container_id != evt->get_tinfo()->m_container_id) {
-			evt->get_tinfo()->set_group(evt->get_tinfo()->m_gid);
-			evt->get_tinfo()->set_user(evt->get_tinfo()->m_uid);
-		}
 	}
 }
 
diff --git a/userspace/libsinsp/sinsp.cpp b/userspace/libsinsp/sinsp.cpp
index 7b116f73b3..65b6772c12 100644
--- a/userspace/libsinsp/sinsp.cpp
+++ b/userspace/libsinsp/sinsp.cpp
@@ -1297,6 +1297,11 @@ int32_t sinsp::next(sinsp_evt** puevt) {
 	{
 		// Object that uses RAII to enable event filtered out flag
 		sinsp_evt_filter evt_filter(evt);
+		// Object that uses RAII to automatically update user/group associated with a threadinfo
+		// upon threadinfo's container_id changes.
+		// Since the threadinfo state might get changed from a plugin parser,
+		// evaluate this one after all parsers get run.
+		user_group_updater usr_grp_updater(evt);
 
 		if(!evt->is_filtered_out()) {
 			//
diff --git a/userspace/libsinsp/user.cpp b/userspace/libsinsp/user.cpp
index 69c7b5daa1..2397b858df 100644
--- a/userspace/libsinsp/user.cpp
+++ b/userspace/libsinsp/user.cpp
@@ -17,7 +17,6 @@ limitations under the License.
 */
 
 #include <libsinsp/user.h>
-#include <libsinsp/event.h>
 #include <libsinsp/procfs_utils.h>
 #include <libsinsp/utils.h>
 #include <libsinsp/logger.h>
diff --git a/userspace/libsinsp/user.h b/userspace/libsinsp/user.h
index 4151c74858..fbacc1d22c 100644
--- a/userspace/libsinsp/user.h
+++ b/userspace/libsinsp/user.h
@@ -24,17 +24,69 @@ limitations under the License.
 #include <memory>
 #include <libsinsp/container_info.h>
 #include <libsinsp/procfs_utils.h>
+#include <libsinsp/event.h>
+#include <libsinsp/dumper.h>
+#include <libsinsp/threadinfo.h>
 #include <libscap/scap.h>
 
 class sinsp;
-class sinsp_dumper;
-class sinsp_evt;
 namespace libsinsp {
 namespace procfs_utils {
 class ns_helper;
 }
 }  // namespace libsinsp
 
+// RAII struct to manage threadinfos automatic user/group refresh
+// upon container_id updates.
+struct user_group_updater {
+	explicit user_group_updater(sinsp_evt *evt) {
+		switch(evt->get_type()) {
+		case PPME_SYSCALL_CLONE_11_X:
+		case PPME_SYSCALL_CLONE_16_X:
+		case PPME_SYSCALL_CLONE_17_X:
+		case PPME_SYSCALL_CLONE_20_X:
+		case PPME_SYSCALL_FORK_X:
+		case PPME_SYSCALL_FORK_17_X:
+		case PPME_SYSCALL_FORK_20_X:
+		case PPME_SYSCALL_VFORK_X:
+		case PPME_SYSCALL_VFORK_17_X:
+		case PPME_SYSCALL_VFORK_20_X:
+		case PPME_SYSCALL_CLONE3_X:
+		case PPME_SYSCALL_EXECVE_8_X:
+		case PPME_SYSCALL_EXECVE_13_X:
+		case PPME_SYSCALL_EXECVE_14_X:
+		case PPME_SYSCALL_EXECVE_15_X:
+		case PPME_SYSCALL_EXECVE_16_X:
+		case PPME_SYSCALL_EXECVE_17_X:
+		case PPME_SYSCALL_EXECVE_18_X:
+		case PPME_SYSCALL_EXECVE_19_X:
+		case PPME_SYSCALL_EXECVEAT_X:
+		case PPME_SYSCALL_CHROOT_X:
+			m_evt = evt;
+			if(m_evt->get_tinfo() != nullptr) {
+				m_container_id = m_evt->get_tinfo()->m_container_id;
+			}
+			break;
+		default:
+			m_evt = nullptr;
+			break;
+		}
+	}
+
+	~user_group_updater() {
+		if(m_evt != nullptr && m_evt->get_tinfo() != nullptr) {
+			if(m_evt->get_tinfo()->m_container_id != m_container_id) {
+				// Refresh user/group
+				m_evt->get_tinfo()->set_group(m_evt->get_tinfo()->m_gid);
+				m_evt->get_tinfo()->set_user(m_evt->get_tinfo()->m_uid);
+			}
+		}
+	}
+
+	sinsp_evt *m_evt;
+	std::string m_container_id;
+};
+
 /*
  * Basic idea:
  * * when container_manager tries to resolve a threadinfo container, it will update