From d50f80e94bb61a06bb7bb3953790b3a071bac213 Mon Sep 17 00:00:00 2001 From: Federico Di Pierro Date: Wed, 18 Oct 2023 09:54:01 +0200 Subject: [PATCH 1/7] fix(userspace/libsinsp): avoid a possible source of segfault in libsinsp next. Signed-off-by: Federico Di Pierro --- userspace/libsinsp/sinsp.cpp | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/userspace/libsinsp/sinsp.cpp b/userspace/libsinsp/sinsp.cpp index d75d5046e8..ab2aa809f0 100644 --- a/userspace/libsinsp/sinsp.cpp +++ b/userspace/libsinsp/sinsp.cpp @@ -1220,6 +1220,8 @@ int32_t sinsp::next(OUT sinsp_evt **puevt) sinsp_evt* evt; int32_t res; + *puevt = NULL; + // // Check if there are fake cpu events to events // @@ -1293,8 +1295,6 @@ int32_t sinsp::next(OUT sinsp_evt **puevt) { m_external_event_processor->process_event(NULL, libsinsp::EVENT_RETURN_TIMEOUT); } - *puevt = NULL; - return res; } else if(res == SCAP_EOF) { @@ -1310,8 +1310,7 @@ int32_t sinsp::next(OUT sinsp_evt **puevt) // In this case, we restart the capture so that the internal states gets reset // and the blocks coming from the next appended file get consumed. restart_capture(); - return SCAP_TIMEOUT; - + res = SCAP_TIMEOUT; } else if(res == SCAP_FILTERED_EVENT) { @@ -1323,8 +1322,6 @@ int32_t sinsp::next(OUT sinsp_evt **puevt) if(m_external_event_processor) { m_external_event_processor->process_event(NULL, libsinsp::EVENT_RETURN_FILTERED); - *puevt = NULL; - return res; } } else @@ -1422,25 +1419,20 @@ int32_t sinsp::next(OUT sinsp_evt **puevt) // things like exit() or close() can be parsed. // uint32_t nfdr = (uint32_t)m_fds_to_remove->size(); - if(nfdr != 0) { /* This is a removal logic we shouldn't scan /proc. If we don't have the thread * to remove we are fine. */ sinsp_threadinfo* ptinfo = get_thread_ref(m_tid_of_fd_to_remove, false).get(); - if(!ptinfo) - { - ASSERT(false); - return res; - } - - for(uint32_t j = 0; j < nfdr; j++) + if(ptinfo) { - ptinfo->remove_fd(m_fds_to_remove->at(j)); + for(uint32_t j = 0; j < nfdr; j++) + { + ptinfo->remove_fd(m_fds_to_remove->at(j)); + } + m_fds_to_remove->clear(); } - - m_fds_to_remove->clear(); } #ifdef SIMULATE_DROP_MODE @@ -1520,7 +1512,6 @@ int32_t sinsp::next(OUT sinsp_evt **puevt) // mode and the category of this event is internal. if(!(m_isinternal_events_enabled && (cat & EC_INTERNAL))) { - *puevt = evt; return SCAP_FILTERED_EVENT; } } From 58e33758b2aa74ac181fe415ff5d4cea58f90db5 Mon Sep 17 00:00:00 2001 From: Federico Di Pierro Date: Wed, 18 Oct 2023 13:48:42 +0200 Subject: [PATCH 2/7] chore(userspace/libsinsp): always clear `m_fds_to_remove` even when no matching `ptinfo` is found. Signed-off-by: Federico Di Pierro Co-authored-by: Grzegorz Nosek --- 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 ab2aa809f0..5983cac0e6 100644 --- a/userspace/libsinsp/sinsp.cpp +++ b/userspace/libsinsp/sinsp.cpp @@ -1431,8 +1431,8 @@ int32_t sinsp::next(OUT sinsp_evt **puevt) { ptinfo->remove_fd(m_fds_to_remove->at(j)); } - m_fds_to_remove->clear(); } + m_fds_to_remove->clear(); } #ifdef SIMULATE_DROP_MODE From 0071d6e13b9eea8372f41313ce715f72ec31db8d Mon Sep 17 00:00:00 2001 From: Federico Di Pierro Date: Thu, 19 Oct 2023 14:51:42 +0200 Subject: [PATCH 3/7] chore(userspace/libsinsp): set puevt even when the event is fitlered out. Signed-off-by: Federico Di Pierro --- userspace/libsinsp/sinsp.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/userspace/libsinsp/sinsp.cpp b/userspace/libsinsp/sinsp.cpp index 5983cac0e6..963dae5b27 100644 --- a/userspace/libsinsp/sinsp.cpp +++ b/userspace/libsinsp/sinsp.cpp @@ -1504,6 +1504,9 @@ int32_t sinsp::next(OUT sinsp_evt **puevt) m_dumper->dump(evt); } + // Finally set output evt; + // From now on, any return must have the correct output being set. + *puevt = evt; if(evt->m_filtered_out) { ppm_event_category cat = evt->get_category(); @@ -1541,7 +1544,6 @@ int32_t sinsp::next(OUT sinsp_evt **puevt) // // Done // - *puevt = evt; return res; } From b733c0aac25293bbdbd562bf00ea4b34087c8b3e Mon Sep 17 00:00:00 2001 From: Andrea Terzolo Date: Wed, 11 Oct 2023 15:20:30 +0200 Subject: [PATCH 4/7] fix(modern): perform an exact check on `BPF_TRACE_RAW_TP` attach type Signed-off-by: Andrea Terzolo --- userspace/libpman/src/configuration.c | 130 ++++++++++++++++++++++++-- 1 file changed, 124 insertions(+), 6 deletions(-) diff --git a/userspace/libpman/src/configuration.c b/userspace/libpman/src/configuration.c index b0974bcf49..c48449ae56 100644 --- a/userspace/libpman/src/configuration.c +++ b/userspace/libpman/src/configuration.c @@ -17,6 +17,10 @@ limitations under the License. #include "state.h" #include +#include +#include +#include /* Definition of AT_* constants */ +#include static int setup_libbpf_print_verbose(enum libbpf_print_level level, const char* format, va_list args) { @@ -164,16 +168,127 @@ int pman_get_required_buffers() return g_state.n_required_buffers; } +static char byte_array[] = "BPF_TRACE_RAW_TP"; + +bool check_BPF_TRACE_RAW_TP(void) +{ + // These locations are taken from libbpf library: + // https://elixir.bootlin.com/linux/latest/source/tools/lib/bpf/btf.c#L4767 + const char *locations[] = { + /* try canonical vmlinux BTF through sysfs first */ + "/sys/kernel/btf/vmlinux", + /* fall back to trying to find vmlinux on disk otherwise */ + "/boot/vmlinux-%1$s", + "/lib/modules/%1$s/vmlinux-%1$s", + "/lib/modules/%1$s/build/vmlinux", + "/usr/lib/modules/%1$s/kernel/vmlinux", + "/usr/lib/debug/boot/vmlinux-%1$s", + "/usr/lib/debug/boot/vmlinux-%1$s.debug", + "/usr/lib/debug/lib/modules/%1$s/vmlinux", + }; + char path[PATH_MAX + 1]; + struct utsname buf; + char *data = NULL; + FILE *f = NULL; + uname(&buf); + + for (int i = 0; i < sizeof(locations) / sizeof(*locations); i++) + { + snprintf(path, PATH_MAX, locations[i], buf.release); + + // On success `faccessat` returns 0. + if(faccessat(0, path, R_OK, AT_EACCESS) != 0) + { + // The file doesn't exist, loop on the next one. + continue; + } + + f = fopen(path, "r"); + if(!f) + { + goto loop_again; + } + + // Seek to the end of file + if(fseek(f, 0, SEEK_END)) + { + goto loop_again; + } + + // Return the dimension of the file + long sz = ftell(f); + if (sz < 0) + { + goto loop_again; + } + + // Seek again to the beginning of the file + if(fseek(f, 0, SEEK_SET)) + { + goto loop_again; + } + + // pre-alloc memory to read all of BTF data + data = malloc(sz); + if (!data) + { + goto loop_again; + } + + // read all of BTF data + if(fread(data, 1, sz, f) < sz) + { + goto loop_again; + } + + // Search 'BPF_TRACE_RAW_TP' byte array + int z = 0; + for(int j = 0; j< sz; j++) + { + if(data[j] == byte_array[z]) + { + z++; + if(z == sizeof(byte_array) / sizeof(*byte_array)) + { + fclose(f); + free(data); + return true; + } + } + else + { + z = 0; + } + } +loop_again: + + if(f) + { + fclose(f); + f = NULL; + } + if(data) + { + free(data); + data = NULL; + } + } + + // Clear the errno for `pman_print_error` + errno = 0; + pman_print_error("prog 'BPF_TRACE_RAW_TP' is not supported"); + return false; +} + + /* * Probe the kernel for required dependencies, ring buffer maps and tracing * progs needs to be supported. */ bool pman_check_support() { - bool res; - - res = libbpf_probe_bpf_map_type(BPF_MAP_TYPE_RINGBUF, NULL) > 0; - if (!res) + bool res = libbpf_probe_bpf_map_type(BPF_MAP_TYPE_RINGBUF, NULL) > 0; + if(!res) { pman_print_error("ring buffer map type is not supported"); return res; @@ -182,8 +297,11 @@ bool pman_check_support() res = libbpf_probe_bpf_prog_type(BPF_PROG_TYPE_TRACING, NULL) > 0; if (!res) { - pman_print_error("tracing program type is not supported"); - return res; + // The above function checks for the `BPF_TRACE_FENTRY` attach type presence, while we need + // to check for the `BPF_TRACE_RAW_TP` one. If `BPF_TRACE_FENTRY` is defined we are + // sure `BPF_TRACE_RAW_TP` is defined as well, in all other cases, we need to search + // for it in the `vmlinux` file. + return check_BPF_TRACE_RAW_TP(); } /* Probe result depends on the success of map creation, no additional From f097b68427fefe0f1d57a8683d98f581779088f7 Mon Sep 17 00:00:00 2001 From: Andrea Terzolo Date: Thu, 12 Oct 2023 11:22:21 +0200 Subject: [PATCH 5/7] cleanup Signed-off-by: Andrea Terzolo Co-authored-by: Mauro Ezequiel Moltrasio --- userspace/libpman/src/configuration.c | 193 ++++++++++++++------------ 1 file changed, 106 insertions(+), 87 deletions(-) diff --git a/userspace/libpman/src/configuration.c b/userspace/libpman/src/configuration.c index c48449ae56..481b54ffab 100644 --- a/userspace/libpman/src/configuration.c +++ b/userspace/libpman/src/configuration.c @@ -170,14 +170,92 @@ int pman_get_required_buffers() static char byte_array[] = "BPF_TRACE_RAW_TP"; -bool check_BPF_TRACE_RAW_TP(void) +bool check_location(const char* path) +{ + bool res = false; + + // On success `faccessat` returns 0. + if(faccessat(0, path, R_OK, AT_EACCESS) != 0) + { + return false; + } + + char *file_content = NULL; + FILE *f = fopen(path, "r"); + if(!f) + { + return false; + } + + // Seek to the end of file + if(fseek(f, 0, SEEK_END)) + { + goto cleanup; + } + + // Return the dimension of the file + long sz = ftell(f); + if (sz < 0) + { + goto cleanup; + } + + // Seek again to the beginning of the file + if(fseek(f, 0, SEEK_SET)) + { + goto cleanup; + } + + // pre-alloc memory to read all of BTF data + file_content = malloc(sz); + if (!file_content) + { + goto cleanup; + } + + // read all of BTF data + if(fread(file_content, 1, sz, f) < sz) + { + goto cleanup; + } + + // Search 'BPF_TRACE_RAW_TP' byte array + int z = 0; + for(int j = 0; j< sz; j++) + { + if(file_content[j] == byte_array[z]) + { + z++; + if(z == sizeof(byte_array) / sizeof(*byte_array)) + { + res = true; + break; + } + } + else + { + z = 0; + } + } + +cleanup: + if(f) + { + fclose(f); + } + if(file_content) + { + free(file_content); + } + return res; +} + +bool probe_BPF_TRACE_RAW_TP_type(void) { // These locations are taken from libbpf library: // https://elixir.bootlin.com/linux/latest/source/tools/lib/bpf/btf.c#L4767 const char *locations[] = { - /* try canonical vmlinux BTF through sysfs first */ "/sys/kernel/btf/vmlinux", - /* fall back to trying to find vmlinux on disk otherwise */ "/boot/vmlinux-%1$s", "/lib/modules/%1$s/vmlinux-%1$s", "/lib/modules/%1$s/build/vmlinux", @@ -186,97 +264,31 @@ bool check_BPF_TRACE_RAW_TP(void) "/usr/lib/debug/boot/vmlinux-%1$s.debug", "/usr/lib/debug/lib/modules/%1$s/vmlinux", }; - char path[PATH_MAX + 1]; - struct utsname buf; - char *data = NULL; - FILE *f = NULL; - uname(&buf); - for (int i = 0; i < sizeof(locations) / sizeof(*locations); i++) + // Try canonical `vmlinux` BTF through `sysfs` first. + if(check_location(locations[0])) { - snprintf(path, PATH_MAX, locations[i], buf.release); - - // On success `faccessat` returns 0. - if(faccessat(0, path, R_OK, AT_EACCESS) != 0) - { - // The file doesn't exist, loop on the next one. - continue; - } - - f = fopen(path, "r"); - if(!f) - { - goto loop_again; - } - - // Seek to the end of file - if(fseek(f, 0, SEEK_END)) - { - goto loop_again; - } - - // Return the dimension of the file - long sz = ftell(f); - if (sz < 0) - { - goto loop_again; - } - - // Seek again to the beginning of the file - if(fseek(f, 0, SEEK_SET)) - { - goto loop_again; - } - - // pre-alloc memory to read all of BTF data - data = malloc(sz); - if (!data) - { - goto loop_again; - } + return true; + } - // read all of BTF data - if(fread(data, 1, sz, f) < sz) - { - goto loop_again; - } + // Fall back to trying to find `vmlinux` on disk otherwise + struct utsname buf = {}; + if(uname(&buf) == -1) + { + return false; + } - // Search 'BPF_TRACE_RAW_TP' byte array - int z = 0; - for(int j = 0; j< sz; j++) - { - if(data[j] == byte_array[z]) - { - z++; - if(z == sizeof(byte_array) / sizeof(*byte_array)) - { - fclose(f); - free(data); - return true; - } - } - else - { - z = 0; - } - } -loop_again: + char path[PATH_MAX + 1]; - if(f) - { - fclose(f); - f = NULL; - } - if(data) + // Skip vmlinux since we already tested it. + for (int i = 1; i < sizeof(locations) / sizeof(*locations); i++) + { + snprintf(path, PATH_MAX, locations[i], buf.release); + if(check_location(path)) { - free(data); - data = NULL; + return true; } } - - // Clear the errno for `pman_print_error` - errno = 0; - pman_print_error("prog 'BPF_TRACE_RAW_TP' is not supported"); return false; } @@ -301,7 +313,14 @@ bool pman_check_support() // to check for the `BPF_TRACE_RAW_TP` one. If `BPF_TRACE_FENTRY` is defined we are // sure `BPF_TRACE_RAW_TP` is defined as well, in all other cases, we need to search // for it in the `vmlinux` file. - return check_BPF_TRACE_RAW_TP(); + res = probe_BPF_TRACE_RAW_TP_type(); + if(!res) + { + // Clear the errno for `pman_print_error` + errno = 0; + pman_print_error("prog 'BPF_TRACE_RAW_TP' is not supported"); + return res; + } } /* Probe result depends on the success of map creation, no additional From dfc19293c1e465dea57e736876c3653577ef2c46 Mon Sep 17 00:00:00 2001 From: Andrea Terzolo Date: Wed, 18 Oct 2023 11:33:19 +0200 Subject: [PATCH 6/7] cleanup Signed-off-by: Andrea Terzolo Co-authored-by: Mauro Ezequiel Moltrasio --- userspace/libpman/src/configuration.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/userspace/libpman/src/configuration.c b/userspace/libpman/src/configuration.c index 481b54ffab..2a30f5dae9 100644 --- a/userspace/libpman/src/configuration.c +++ b/userspace/libpman/src/configuration.c @@ -168,10 +168,10 @@ int pman_get_required_buffers() return g_state.n_required_buffers; } -static char byte_array[] = "BPF_TRACE_RAW_TP"; - bool check_location(const char* path) { + static const char bpf_trace_raw_byte_array[] = "BPF_TRACE_RAW_TP"; + bool res = false; // On success `faccessat` returns 0. @@ -223,10 +223,10 @@ bool check_location(const char* path) int z = 0; for(int j = 0; j< sz; j++) { - if(file_content[j] == byte_array[z]) + if(file_content[j] == bpf_trace_raw_byte_array[z]) { z++; - if(z == sizeof(byte_array) / sizeof(*byte_array)) + if(z == sizeof(bpf_trace_raw_byte_array) / sizeof(*bpf_trace_raw_byte_array)) { res = true; break; From c485462e0b3b297f258325009e9e8182e4532cc7 Mon Sep 17 00:00:00 2001 From: Federico Di Pierro Date: Mon, 23 Oct 2023 10:31:47 +0200 Subject: [PATCH 7/7] fix(userspace/libsinsp): manually cherry picked 7326e56429b51dd1e72eb30e6794c3d8f2b13e96. Signed-off-by: Federico Di Pierro --- userspace/libsinsp/filterchecks.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/userspace/libsinsp/filterchecks.cpp b/userspace/libsinsp/filterchecks.cpp index 86d1fdacd2..e2aefe3c91 100644 --- a/userspace/libsinsp/filterchecks.cpp +++ b/userspace/libsinsp/filterchecks.cpp @@ -4494,6 +4494,7 @@ int32_t sinsp_filter_check_event::parse_field_name(const char* str, bool alloc_s res = extract_arg("evt.rawarg", val, &m_arginfo); m_customfield.m_type = m_arginfo->type; + m_customfield.m_print_format = m_arginfo->fmt; } else if(STR_MATCH("evt.around")) {