diff --git a/driver/ppm_fillers.c b/driver/ppm_fillers.c index f5950d5fb1..4bbc7016c2 100644 --- a/driver/ppm_fillers.c +++ b/driver/ppm_fillers.c @@ -1468,19 +1468,6 @@ int f_proc_startupdate(struct event_filler_arguments *args) { fput(exe_file); } - /* The trusted_exepath could end with the suffix " (deleted)". - * https://github.com/torvalds/linux/blob/2dde18cd1d8fac735875f2e4987f11817cc0bc2c/fs/d_path.c#L255 - * This is unhandy to manage in userspace, for this reason, we can remove it here - */ - if(trusted_exepath != NULL) { - char deleted_suffix[] = " (deleted)"; - int diff_len = strlen(trusted_exepath) - strlen(deleted_suffix); - if(diff_len > 0 && - (strncmp(&trusted_exepath[diff_len], deleted_suffix, sizeof(deleted_suffix)) == 0)) { - trusted_exepath[diff_len] = '\0'; - } - } - if(exe_writable) { flags |= PPM_EXE_WRITABLE; } @@ -7358,19 +7345,6 @@ int f_sched_prog_exec(struct event_filler_arguments *args) { fput(exe_file); } - /* The trusted_exepath could end with the suffix " (deleted)". - * https://github.com/torvalds/linux/blob/2dde18cd1d8fac735875f2e4987f11817cc0bc2c/fs/d_path.c#L255 - * This is unhandy to manage in userspace, for this reason, we can remove it here - */ - if(trusted_exepath != NULL) { - char deleted_suffix[] = " (deleted)"; - int diff_len = strlen(trusted_exepath) - strlen(deleted_suffix); - if(diff_len > 0 && - (strncmp(&trusted_exepath[diff_len], deleted_suffix, sizeof(deleted_suffix)) == 0)) { - trusted_exepath[diff_len] = '\0'; - } - } - if(exe_writable) { flags |= PPM_EXE_WRITABLE; } diff --git a/test/drivers/test_suites/generic_tracepoints_suite/sched_process_exec.cpp b/test/drivers/test_suites/generic_tracepoints_suite/sched_process_exec.cpp index f85ebcc95d..b9bab7c61e 100644 --- a/test/drivers/test_suites/generic_tracepoints_suite/sched_process_exec.cpp +++ b/test/drivers/test_suites/generic_tracepoints_suite/sched_process_exec.cpp @@ -241,11 +241,9 @@ TEST(GenericTracepoints, sched_proc_exec_success_memfd) { /* Parameter 28: trusted_exepath (type: PT_FSPATH) */ /* In the kmod, we use the "d_path" helper while in BPF we reconstruct the path * by hand so the result is a little bit different. - * Please note that in the kernel module, we remove the " (deleted)" suffix while - * in BPF we don't add it at all. */ if(evt_test->is_kmod_engine()) { - evt_test->assert_charbuf_param(28, "/memfd:malware"); + evt_test->assert_charbuf_param(28, "/memfd:malware (deleted)"); } else { /* In BPF drivers we don't have the correct result but we can reconstruct part of it */ evt_test->assert_charbuf_param(28, "memfd:malware"); diff --git a/test/drivers/test_suites/syscall_exit_suite/execve_x.cpp b/test/drivers/test_suites/syscall_exit_suite/execve_x.cpp index 6ad4408c89..2998ea3320 100644 --- a/test/drivers/test_suites/syscall_exit_suite/execve_x.cpp +++ b/test/drivers/test_suites/syscall_exit_suite/execve_x.cpp @@ -912,11 +912,9 @@ TEST(SyscallExit, execveX_success_memfd) { /* Parameter 28: trusted_exepath (type: PT_FSPATH) */ /* In the kmod, we use the "d_path" helper while in BPF we reconstruct the path * by hand so the result is a little bit different. - * Please note that in the kernel module, we remove the " (deleted)" suffix while - * in BPF we don't add it at all. */ if(evt_test->is_kmod_engine()) { - evt_test->assert_charbuf_param(28, "/memfd:malware"); + evt_test->assert_charbuf_param(28, "/memfd:malware (deleted)"); } else { /* In BPF drivers we don't have the correct result but we can reconstruct part of it */ evt_test->assert_charbuf_param(28, "memfd:malware"); diff --git a/test/drivers/test_suites/syscall_exit_suite/execveat_x.cpp b/test/drivers/test_suites/syscall_exit_suite/execveat_x.cpp index 5f88a3b468..7af23d98a7 100644 --- a/test/drivers/test_suites/syscall_exit_suite/execveat_x.cpp +++ b/test/drivers/test_suites/syscall_exit_suite/execveat_x.cpp @@ -707,11 +707,9 @@ TEST(SyscallExit, execveatX_success_memfd) { /* Parameter 28: trusted_exepath (type: PT_FSPATH) */ /* In the kmod, we use the "d_path" helper while in BPF we reconstruct the path * by hand so the result is a little bit different. - * Please note that in the kernel module, we remove the " (deleted)" suffix while - * in BPF we don't add it at all. */ if(evt_test->is_kmod_engine()) { - evt_test->assert_charbuf_param(28, "/memfd:malware"); + evt_test->assert_charbuf_param(28, "/memfd:malware (deleted)"); } else { /* In BPF drivers we don't have the correct result but we can reconstruct part of it */ evt_test->assert_charbuf_param(28, "memfd:malware"); diff --git a/userspace/libsinsp/parsers.cpp b/userspace/libsinsp/parsers.cpp index 4c5e825e15..70e50709c5 100644 --- a/userspace/libsinsp/parsers.cpp +++ b/userspace/libsinsp/parsers.cpp @@ -1289,7 +1289,7 @@ void sinsp_parser::parse_clone_exit_caller(sinsp_evt *evt, int64_t child_tid) { /* Take some further info from the caller */ if(valid_caller) { /* We should trust the info we obtain from the caller, if it is valid */ - child_tinfo->m_exepath = caller_tinfo->m_exepath; + child_tinfo->set_exepath(std::string(caller_tinfo->m_exepath)); child_tinfo->m_exe_writable = caller_tinfo->m_exe_writable; @@ -1604,7 +1604,7 @@ void sinsp_parser::parse_clone_exit_child(sinsp_evt *evt) { * enrichment... */ - child_tinfo->m_exepath = lookup_tinfo->m_exepath; + child_tinfo->set_exepath(std::string(lookup_tinfo->m_exepath)); child_tinfo->m_exe_writable = lookup_tinfo->m_exe_writable; @@ -2089,8 +2089,7 @@ void sinsp_parser::parse_execve_exit(sinsp_evt *evt) { */ /* Parameter 28: trusted_exepath (type: PT_FSPATH) */ - parinfo = evt->get_param(27); - evt->get_tinfo()->m_exepath = parinfo->m_val; + evt->get_tinfo()->set_exepath(evt->get_param(27)->as()); } else { /* ONLY VALID FOR OLD SCAP-FILES: * In older event versions we can only rely on our userspace reconstruction @@ -2191,7 +2190,7 @@ void sinsp_parser::parse_execve_exit(sinsp_evt *evt) { fullpath = sinsp_utils::concatenate_paths(sdir, pathname); } } - evt->get_tinfo()->m_exepath = fullpath; + evt->get_tinfo()->set_exepath(std::move(fullpath)); } } diff --git a/userspace/libsinsp/test/classes/sinsp_threadinfo.cpp b/userspace/libsinsp/test/classes/sinsp_threadinfo.cpp index f63c7a034e..8b82dd4c65 100644 --- a/userspace/libsinsp/test/classes/sinsp_threadinfo.cpp +++ b/userspace/libsinsp/test/classes/sinsp_threadinfo.cpp @@ -128,3 +128,58 @@ TEST_F(sinsp_with_test_input, THRD_INFO_assign_children_to_a_nullptr) { ASSERT_THREAD_CHILDREN(p2_t1_tid, 0, 0); ASSERT_THREAD_INFO_PIDS(p3_t1_tid, p3_t1_pid, 0); } + +TEST(sinsp_threadinfo, set_exepath) { + auto tinfo = std::make_shared(); + + { + // Nothing changes + std::string path = "no_suffix (del)"; + size_t before_len = path.size(); + tinfo->set_exepath(std::move(path)); + ASSERT_EQ(tinfo->get_exepath().size(), before_len); + } + + { + // Truncate it + std::string path = "no_suffix (deleted)"; + size_t before_len = path.size(); + tinfo->set_exepath(std::move(path)); + ASSERT_NE(tinfo->get_exepath().size(), before_len); + ASSERT_EQ(tinfo->get_exepath(), "no_suffix"); + } + + { + // Nothing changes (this is not possible from the kernel) + std::string path = "no_suffix(deleted)"; + size_t before_len = path.size(); + tinfo->set_exepath(std::move(path)); + ASSERT_EQ(tinfo->get_exepath().size(), before_len); + } + + { + // Nothing changes (this is not possible from the kernel) + std::string path = "(deleted)"; + size_t before_len = path.size(); + tinfo->set_exepath(std::move(path)); + ASSERT_EQ(tinfo->get_exepath().size(), before_len); + } + + { + // Nothing changes (this is not possible from the kernel) + std::string path = " (deleted)"; + size_t before_len = path.size(); + tinfo->set_exepath(std::move(path)); + ASSERT_EQ(tinfo->get_exepath().size(), before_len); + } + + { + // Truncate it, please note that a double space from the kernel is not possible but here we + // just want to test it. + std::string path = "a (deleted)"; + size_t before_len = path.size(); + tinfo->set_exepath(std::move(path)); + ASSERT_NE(tinfo->get_exepath().size(), before_len); + ASSERT_EQ(tinfo->get_exepath(), "a "); + } +} diff --git a/userspace/libsinsp/test/filterchecks/proc.cpp b/userspace/libsinsp/test/filterchecks/proc.cpp index 7b936d3506..92fcedd8ad 100644 --- a/userspace/libsinsp/test/filterchecks/proc.cpp +++ b/userspace/libsinsp/test/filterchecks/proc.cpp @@ -67,14 +67,16 @@ TEST_F(sinsp_with_test_input, PROC_FILTER_exepath) { DEFAULT_TREE /* Now we call an execve on p6_t1 */ - auto evt = generate_execve_enter_and_exit_event(0, - p6_t1_tid, - p6_t1_tid, - p6_t1_pid, - p6_t1_ptid, - "/good-exe", - "good-exe", - "/usr/bin/bad-exe"); + auto evt = + generate_execve_enter_and_exit_event(0, + p6_t1_tid, + p6_t1_tid, + p6_t1_pid, + p6_t1_ptid, + "/good-exe", + "good-exe", + // Please note that the `deleted` will be removed. + "/usr/bin/bad-exe (deleted)"); ASSERT_EQ(get_field_as_string(evt, "proc.exepath"), "/usr/bin/bad-exe"); ASSERT_EQ(get_field_as_string(evt, "proc.name"), "good-exe"); diff --git a/userspace/libsinsp/threadinfo.cpp b/userspace/libsinsp/threadinfo.cpp index fffea17439..b7fe4a2d17 100644 --- a/userspace/libsinsp/threadinfo.cpp +++ b/userspace/libsinsp/threadinfo.cpp @@ -454,7 +454,7 @@ void sinsp_threadinfo::init(scap_threadinfo* pi) { m_comm = pi->comm; m_exe = pi->exe; /* The exepath is extracted from `/proc/pid/exe`. */ - m_exepath = pi->exepath; + set_exepath(std::string(pi->exepath)); m_exe_writable = pi->exe_writable; m_exe_upper_layer = pi->exe_upper_layer; m_exe_lower_layer = pi->exe_lower_layer; @@ -1248,6 +1248,17 @@ void sinsp_threadinfo::update_main_fdtable() { } } +void sinsp_threadinfo::set_exepath(std::string&& exepath) { + constexpr char suffix[] = " (deleted)"; + constexpr size_t suffix_len = sizeof(suffix) - 1; // Exclude null terminator + + m_exepath = exepath; + if(m_exepath.size() > suffix_len && + m_exepath.compare(m_exepath.size() - suffix_len, suffix_len, suffix) == 0) { + m_exepath.resize(m_exepath.size() - suffix_len); + } +} + static void fd_to_scap(scap_fdinfo* dst, sinsp_fdinfo* src) { dst->type = src->m_type; dst->ino = src->m_ino; diff --git a/userspace/libsinsp/threadinfo.h b/userspace/libsinsp/threadinfo.h index b046238f5d..f14a04f841 100644 --- a/userspace/libsinsp/threadinfo.h +++ b/userspace/libsinsp/threadinfo.h @@ -634,6 +634,8 @@ class SINSP_PUBLIC sinsp_threadinfo : public libsinsp::state::table_entry { void update_main_fdtable(); + void set_exepath(std::string&& exepath); + private: sinsp_threadinfo* get_cwd_root(); bool set_env_from_proc();