From fcd0c83e65abc1a426959a44d06351099d284bb9 Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Thu, 23 Nov 2023 21:34:52 +0000 Subject: [PATCH 1/3] fix(userspace/libsinsp): assign inspector platform before importing system info Signed-off-by: Jason Dellaluce --- userspace/libsinsp/sinsp.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/userspace/libsinsp/sinsp.cpp b/userspace/libsinsp/sinsp.cpp index bd9e6c0074..7934500389 100644 --- a/userspace/libsinsp/sinsp.cpp +++ b/userspace/libsinsp/sinsp.cpp @@ -399,6 +399,7 @@ void sinsp::open_common(scap_open_args* oargs, const struct scap_vtable* vtable, throw scap_open_exception(error, scap_rc); } + m_platform = platform; scap_rc = scap_platform_init(platform, m_platform_lasterr, m_h->m_engine, oargs); if(scap_rc != SCAP_SUCCESS) { @@ -409,7 +410,6 @@ void sinsp::open_common(scap_open_args* oargs, const struct scap_vtable* vtable, throw scap_open_exception(m_platform_lasterr, scap_rc); } - m_platform = platform; init(); From 286ecbdfce045a14970951c90fa3af90a53ae5f4 Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Fri, 24 Nov 2023 15:13:10 +0000 Subject: [PATCH 2/3] fix(userspace/libscap): return error when silently skipping unknown or invalid proc infos Signed-off-by: Jason Dellaluce --- userspace/libscap/linux/scap_procs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/userspace/libscap/linux/scap_procs.c b/userspace/libscap/linux/scap_procs.c index 74d42e3f94..f037669bd9 100644 --- a/userspace/libscap/linux/scap_procs.c +++ b/userspace/libscap/linux/scap_procs.c @@ -585,7 +585,7 @@ static int32_t scap_proc_add_from_proc(struct scap_linux_platform* linux_platfor f = fopen(filename, "r"); if(f == NULL) { - return SCAP_SUCCESS; + return scap_errprintf(error, errno, "can't find valid proc dir in %s", dir_name); } ASSERT(sizeof(line) >= SCAP_MAX_PATH_SIZE); @@ -593,7 +593,7 @@ static int32_t scap_proc_add_from_proc(struct scap_linux_platform* linux_platfor if(fgets(line, SCAP_MAX_PATH_SIZE, f) == NULL) { fclose(f); - return SCAP_SUCCESS; + return scap_errprintf(error, errno, "can't read cmdline file %s", filename); } else { From ef7fa5408e96600efc17247aaa2702a69a2cc56a Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Fri, 24 Nov 2023 15:17:39 +0000 Subject: [PATCH 3/3] fix(usespace/libsinsp): check for null inspector before accessing stats Signed-off-by: Jason Dellaluce --- userspace/libsinsp/container.cpp | 4 ++-- userspace/libsinsp/fdinfo.cpp | 12 ++++++------ userspace/libsinsp/parsers.cpp | 18 +++++++++--------- userspace/libsinsp/threadinfo.cpp | 12 ++++++------ 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/userspace/libsinsp/container.cpp b/userspace/libsinsp/container.cpp index df4f357d39..25b30f15ba 100644 --- a/userspace/libsinsp/container.cpp +++ b/userspace/libsinsp/container.cpp @@ -88,7 +88,7 @@ bool sinsp_container_manager::remove_inactive_containers() }); auto containers = m_containers.lock(); - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_missing_container_images = 0; // Will include pod sanboxes, but that's ok @@ -97,7 +97,7 @@ bool sinsp_container_manager::remove_inactive_containers() for(auto it = containers->begin(); it != containers->end();) { sinsp_container_info::ptr_t container = it->second; - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { auto container_info = container.get(); if (!container_info || (container_info && !container_info->m_is_pod_sandbox && container_info->m_image.empty())) diff --git a/userspace/libsinsp/fdinfo.cpp b/userspace/libsinsp/fdinfo.cpp index 090f28e8f6..106b337079 100644 --- a/userspace/libsinsp/fdinfo.cpp +++ b/userspace/libsinsp/fdinfo.cpp @@ -286,7 +286,7 @@ sinsp_fdinfo_t* sinsp_fdtable::find(int64_t fd) // if(m_last_accessed_fd != -1 && fd == m_last_accessed_fd) { - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_cached_fd_lookups++; } @@ -300,7 +300,7 @@ sinsp_fdinfo_t* sinsp_fdtable::find(int64_t fd) if(fdit == m_table.end()) { - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_failed_fd_lookups++; } @@ -308,7 +308,7 @@ sinsp_fdinfo_t* sinsp_fdtable::find(int64_t fd) } else { - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_noncached_fd_lookups++; } @@ -340,7 +340,7 @@ sinsp_fdinfo_t* sinsp_fdtable::add(int64_t fd, sinsp_fdinfo_t* fdinfo) // No entry in the table, this is the normal case // m_last_accessed_fd = -1; - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_added_fds++; } @@ -412,7 +412,7 @@ void sinsp_fdtable::erase(int64_t fd) // keep going. // ASSERT(false); - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_failed_fd_lookups++; } @@ -420,7 +420,7 @@ void sinsp_fdtable::erase(int64_t fd) else { m_table.erase(fdit); - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_noncached_fd_lookups++; m_inspector->m_sinsp_stats_v2->m_n_removed_fds++; diff --git a/userspace/libsinsp/parsers.cpp b/userspace/libsinsp/parsers.cpp index 10d7317eaf..c07ecc7a44 100644 --- a/userspace/libsinsp/parsers.cpp +++ b/userspace/libsinsp/parsers.cpp @@ -677,7 +677,7 @@ bool sinsp_parser::reset(sinsp_evt *evt) etype == PPME_SYSCALL_VFORK_20_X || etype == PPME_SYSCALL_CLONE3_X) { - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_failed_thread_lookups--; } @@ -826,7 +826,7 @@ void sinsp_parser::store_event(sinsp_evt *evt) // we won't be able to parse the corresponding exit event and we'll have // to drop the information it carries. // - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_store_evts_drops++; } @@ -862,7 +862,7 @@ void sinsp_parser::store_event(sinsp_evt *evt) memcpy(tinfo->m_lastevent_data, evt->m_pevt, elen); tinfo->m_lastevent_cpuid = evt->get_cpuid(); - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_stored_evts++; } @@ -887,7 +887,7 @@ bool sinsp_parser::retrieve_enter_event(sinsp_evt *enter_evt, sinsp_evt *exit_ev // This happen especially at the beginning of trace files, where events // can be truncated // - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_retrieve_evts_drops++; } @@ -910,7 +910,7 @@ bool sinsp_parser::retrieve_enter_event(sinsp_evt *enter_evt, sinsp_evt *exit_ev && enter_evt->get_type() == PPME_SYSCALL_EXECVEAT_E) { - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_retrieved_evts++; } @@ -925,13 +925,13 @@ bool sinsp_parser::retrieve_enter_event(sinsp_evt *enter_evt, sinsp_evt *exit_ev { //ASSERT(false); exit_evt->m_tinfo->set_lastevent_data_validity(false); - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_retrieve_evts_drops++; } return false; } - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_retrieved_evts++; } @@ -3654,13 +3654,13 @@ void sinsp_parser::parse_close_exit(sinsp_evt *evt) // It is normal when a close fails that the fd lookup failed, so we revert the // increment of m_n_failed_fd_lookups (for the enter event too if there's one). // - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_failed_fd_lookups--; } if(evt->m_tinfo && evt->m_tinfo->is_lastevent_data_valid()) { - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_failed_fd_lookups--; } diff --git a/userspace/libsinsp/threadinfo.cpp b/userspace/libsinsp/threadinfo.cpp index dec8e4b78d..3bc74ae41e 100644 --- a/userspace/libsinsp/threadinfo.cpp +++ b/userspace/libsinsp/threadinfo.cpp @@ -1473,7 +1473,7 @@ bool sinsp_thread_manager::add_thread(sinsp_threadinfo *threadinfo, bool from_sc #endif ) { - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { // rate limit messages to avoid spamming the logs if (m_inspector->m_sinsp_stats_v2->m_n_drops_full_threadtable % m_max_thread_table_size == 0) @@ -1505,7 +1505,7 @@ bool sinsp_thread_manager::add_thread(sinsp_threadinfo *threadinfo, bool from_sc tinfo_shared_ptr->compute_program_hash(); m_threadtable.put(std::move(tinfo_shared_ptr)); - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_added_threads++; } @@ -1777,7 +1777,7 @@ void sinsp_thread_manager::remove_thread(int64_t tid) * the cache just to be sure. */ m_last_tid = -1; - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_removed_threads++; } @@ -2112,7 +2112,7 @@ threadinfo_map_t::ptr_t sinsp_thread_manager::find_thread(int64_t tid, bool look thr = m_last_tinfo.lock(); if (thr) { - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_cached_thread_lookups++; } @@ -2130,7 +2130,7 @@ threadinfo_map_t::ptr_t sinsp_thread_manager::find_thread(int64_t tid, bool look if(thr) { - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_noncached_thread_lookups++; } @@ -2145,7 +2145,7 @@ threadinfo_map_t::ptr_t sinsp_thread_manager::find_thread(int64_t tid, bool look } else { - if (m_inspector->m_sinsp_stats_v2) + if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2) { m_inspector->m_sinsp_stats_v2->m_n_failed_thread_lookups++; }