diff options
author | thiagoftsm <thiagoftsm@gmail.com> | 2020-07-14 16:42:45 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-07-14 16:42:45 +0000 |
commit | 664d180dc7a5e34334a4f94399e5663eb5c0be8a (patch) | |
tree | d00c58b81114c7993703c4b262e67e7764c5cf59 /collectors | |
parent | 4732d30e9545b6d3e56822f156a8f0218126fb2f (diff) |
Fix potential memory leak in ebpf.plugin (#9484)
Fix reported bugs with ebpf.plugin.
Diffstat (limited to 'collectors')
-rw-r--r-- | collectors/ebpf.plugin/Makefile.am | 13 | ||||
-rw-r--r-- | collectors/ebpf.plugin/README.md | 5 | ||||
-rw-r--r-- | collectors/ebpf.plugin/ebpf.c | 19 | ||||
-rw-r--r-- | collectors/ebpf.plugin/ebpf.h | 4 | ||||
-rw-r--r-- | collectors/ebpf.plugin/ebpf_apps.c | 2 | ||||
-rw-r--r-- | collectors/ebpf.plugin/ebpf_process.c | 23 | ||||
-rw-r--r-- | collectors/ebpf.plugin/ebpf_socket.c | 6 | ||||
-rw-r--r-- | collectors/ebpf.plugin/reset_netdata_trace.sh.in | 9 |
8 files changed, 53 insertions, 28 deletions
diff --git a/collectors/ebpf.plugin/Makefile.am b/collectors/ebpf.plugin/Makefile.am index 719c236367..497f1f3a2c 100644 --- a/collectors/ebpf.plugin/Makefile.am +++ b/collectors/ebpf.plugin/Makefile.am @@ -3,11 +3,22 @@ AUTOMAKE_OPTIONS = subdir-objects MAINTAINERCLEANFILES = $(srcdir)/Makefile.in +CLEANFILES = \ + reset_netdata_trace.sh \ + $(NULL) + +include $(top_srcdir)/build/subst.inc +SUFFIXES = .in + +dist_plugins_SCRIPTS = \ + reset_netdata_trace.sh \ + $(NULL) + dist_noinst_DATA = \ + reset_netdata_trace.sh.in \ README.md \ $(NULL) dist_libconfig_DATA = \ ebpf.conf \ $(NULL) - diff --git a/collectors/ebpf.plugin/README.md b/collectors/ebpf.plugin/README.md index 7e3b794041..d8743eacba 100644 --- a/collectors/ebpf.plugin/README.md +++ b/collectors/ebpf.plugin/README.md @@ -338,4 +338,9 @@ shows how the lockdown module impacts `ebpf.plugin` based on the selected option If you or your distribution compiled the kernel with the last combination, your system cannot load shared libraries required to run `ebpf.plugin`. +## Cleaning `kprobe_events` +The eBPF collector adds entries to the file `/sys/kernel/debug/tracing/kprobe_events`, and cleans them on exit, unless +another process prevents it. If you need to clean the eBPF entries safely, you can manually run the script +`/usr/libexec/netdata/plugins.d/reset_netdata_trace.sh`. + [![analytics](https://www.google-analytics.com/collect?v=1&aip=1&t=pageview&_s=1&ds=github&dr=https%3A%2F%2Fgithub.com%2Fnetdata%2Fnetdata&dl=https%3A%2F%2Fmy-netdata.io%2Fgithub%2Fcollectors%2Febpf.plugin%2FREADME&_u=MAC~&cid=5792dfd7-8dc4-476b-af31-da2fdb9f93d2&tid=UA-64295674-3)](<>) diff --git a/collectors/ebpf.plugin/ebpf.c b/collectors/ebpf.plugin/ebpf.c index caa00e997f..8350438e3b 100644 --- a/collectors/ebpf.plugin/ebpf.c +++ b/collectors/ebpf.plugin/ebpf.c @@ -86,13 +86,14 @@ netdata_ebpf_events_t process_probes[] = { }; netdata_ebpf_events_t socket_probes[] = { - { .type = 'r', .name = "tcp_sendmsg" }, { .type = 'p', .name = "tcp_cleanup_rbuf" }, { .type = 'p', .name = "tcp_close" }, { .type = 'p', .name = "udp_recvmsg" }, { .type = 'r', .name = "udp_recvmsg" }, { .type = 'r', .name = "udp_sendmsg" }, { .type = 'p', .name = "do_exit" }, + { .type = 'p', .name = "tcp_sendmsg" }, + { .type = 'r', .name = "tcp_sendmsg" }, { .type = 0, .name = NULL } }; @@ -115,6 +116,15 @@ ebpf_process_stat_t *global_process_stat = NULL; * *****************************************************************/ +static void change_events() +{ + if (ebpf_modules[0].mode == MODE_ENTRY) + change_process_event(); + + if (ebpf_modules[1].mode == MODE_ENTRY) + change_socket_event(); +} + /** * Clean Loaded Events * @@ -166,8 +176,8 @@ static void ebpf_exit(int sig) int sid = setsid(); if(sid >= 0) { - sleep(1); debug(D_EXIT, "Wait for father %d die", getpid()); + sleep_usec(200000); //Sleep 200 miliseconds to father dies. clean_loaded_events(); } else { error("Cannot become session id leader, so I won't try to clean kprobe_events.\n"); @@ -816,10 +826,10 @@ static void parse_args(int argc, char **argv) } if (load_collector_config(ebpf_user_config_dir, &disable_apps)) { - error("Does not have a configuration file inside `%s/ebpf.conf. It will try to load stock file.", + info("Does not have a configuration file inside `%s/ebpf.conf. It will try to load stock file.", ebpf_user_config_dir); if (load_collector_config(ebpf_stock_config_dir, &disable_apps)) { - error("Does not have a stock file. It is starting with default options."); + info("Does not have a stock file. It is starting with default options."); } else { enabled = 1; } @@ -920,6 +930,7 @@ int main(int argc, char **argv) {NULL, NULL, NULL, 0, NULL, NULL, NULL} }; + change_events(); clean_loaded_events(); int i; diff --git a/collectors/ebpf.plugin/ebpf.h b/collectors/ebpf.plugin/ebpf.h index ad0c025fa4..1fd6e8801e 100644 --- a/collectors/ebpf.plugin/ebpf.h +++ b/collectors/ebpf.plugin/ebpf.h @@ -202,4 +202,8 @@ extern int update_every; # define EBPF_MAX_SYNCHRONIZATION_TIME 300 +//External functions +extern void change_socket_event(); +extern void change_process_event(); + #endif diff --git a/collectors/ebpf.plugin/ebpf_apps.c b/collectors/ebpf.plugin/ebpf_apps.c index a0cb8b873b..964f8fe642 100644 --- a/collectors/ebpf.plugin/ebpf_apps.c +++ b/collectors/ebpf.plugin/ebpf_apps.c @@ -764,7 +764,7 @@ static inline void link_all_processes_to_their_parents(void) { } else { p->parent = NULL; - error("pid %d %s states parent %d, but the later does not exist.", p->pid, p->comm, p->ppid); + debug_log("pid %d %s states parent %d, but the later does not exist.", p->pid, p->comm, p->ppid); } } } diff --git a/collectors/ebpf.plugin/ebpf_process.c b/collectors/ebpf.plugin/ebpf_process.c index 4a2ee47a55..8ff596c9b2 100644 --- a/collectors/ebpf.plugin/ebpf_process.c +++ b/collectors/ebpf.plugin/ebpf_process.c @@ -877,18 +877,6 @@ static void process_collector(usec_t step, ebpf_module_t *em) *****************************************************************/ /** - * Clean the allocated process stat structure - */ -static void clean_process_stat() -{ - size_t i; - for (i = 0 ; i < all_pids_count ; i++) { - ebpf_process_stat_t *w = local_process_stats[pid_index[i]]; - freez(w); - } -} - -/** * Clean up the main thread. * * @param ptr thread data. @@ -901,7 +889,6 @@ static void ebpf_process_cleanup(void *ptr) freez(process_publish_aggregated); freez(process_hash_values); - clean_process_stat(); freez(local_process_stats); if (process_functions.libnetdata) { @@ -936,7 +923,7 @@ static void ebpf_process_allocate_global_vectors(size_t length) { prev_apps_data = callocz((size_t)pid_max, sizeof(ebpf_process_publish_apps_t *)); } -static void change_collector_event() { +void change_process_event() { int i; if (running_on_kernel < NETDATA_KERNEL_V5_3) process_probes[EBPF_SYS_CLONE_IDX].name = NULL; @@ -956,7 +943,7 @@ static void change_syscalls() { * Set local variables * */ -static void set_local_pointers(ebpf_module_t *em) { +static void set_local_pointers() { #ifndef STATIC bpf_map_lookup_elem = process_functions.bpf_map_lookup_elem; @@ -965,10 +952,6 @@ static void set_local_pointers(ebpf_module_t *em) { map_fd = process_functions.map_fd; - if (em->mode == MODE_ENTRY) { - change_collector_event(); - } - if (process_functions.isrh >= NETDATA_MINIMUM_RH_VERSION && process_functions.isrh < NETDATA_RH_8) change_syscalls(); } @@ -1032,7 +1015,7 @@ void *ebpf_process_thread(void *ptr) goto endprocess; } - set_local_pointers(em); + set_local_pointers(); if (ebpf_load_program(ebpf_plugin_dir, em->thread_id, em->mode, kernel_string, em->thread_name, process_functions.map_fd, process_functions.load_bpf_file) ) { pthread_mutex_unlock(&lock); diff --git a/collectors/ebpf.plugin/ebpf_socket.c b/collectors/ebpf.plugin/ebpf_socket.c index 7b6b2ea2d7..369e8ea454 100644 --- a/collectors/ebpf.plugin/ebpf_socket.c +++ b/collectors/ebpf.plugin/ebpf_socket.c @@ -515,9 +515,11 @@ static void ebpf_socket_allocate_global_vectors(size_t length) { bandwidth_vector = callocz((size_t) ebpf_nprocs, sizeof(ebpf_bandwidth_t)); } -static void change_collector_event() { +void change_socket_event() { socket_probes[0].type = 'p'; + socket_probes[4].type = 'p'; socket_probes[5].type = 'p'; + socket_probes[7].name = NULL; } /** @@ -533,7 +535,7 @@ static void set_local_pointers(ebpf_module_t *em) { map_fd = socket_functions.map_fd; if (em->mode == MODE_ENTRY) { - change_collector_event(); + change_socket_event(); } } diff --git a/collectors/ebpf.plugin/reset_netdata_trace.sh.in b/collectors/ebpf.plugin/reset_netdata_trace.sh.in new file mode 100644 index 0000000000..51d981ee3d --- /dev/null +++ b/collectors/ebpf.plugin/reset_netdata_trace.sh.in @@ -0,0 +1,9 @@ +#!/bin/bash + +KPROBE_FILE="/sys/kernel/debug/tracing/kprobe_events" + +DATA="$(grep _netdata_ $KPROBE_FILE| cut -d' ' -f1 | cut -d: -f2)" + +for I in $DATA; do + echo "-:$I" > $KPROBE_FILE 2>/dev/null; +done |