From d613b3e4eaeeb30118d395355f2d537fdc88e823 Mon Sep 17 00:00:00 2001 From: Federico Di Pierro 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 --- 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 -#include #include #include #include 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 #include #include +#include +#include +#include #include 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