libbpf-tools: Fix memory leaks in ksnoop/gethostlatency

There are memory leaks when attach a BPF program to multiple
targets in these tools. This is because we misuse the
bpf_program__attach_kprobe function, the returned struct bpf_link
object is not freed after use. Closes #3664.

Signed-off-by: Hengqi Chen <chenhengqi@outlook.com>

diff --git a/libbpf-tools/gethostlatency.c b/libbpf-tools/gethostlatency.c
index 34859818c46d55f460529a73781a644ed78b3f84..d1019d18177086a45d4835b4f519d7a328a593ba 100644 (file)
--- a/libbpf-tools/gethostlatency.c
+++ b/libbpf-tools/gethostlatency.c
@@ -133,7 +133,7 @@ static int get_libc_path(char *path)
        return -1;
 }
 
-static int attach_uprobes(struct gethostlatency_bpf *obj)
+static int attach_uprobes(struct gethostlatency_bpf *obj, struct bpf_link *links[])
 {
        int err;
        char libc_path[PATH_MAX] = {};
@@ -150,18 +150,16 @@ static int attach_uprobes(struct gethostlatency_bpf *obj)
                warn("could not find getaddrinfo in %s\n", libc_path);
                return -1;
        }
-       obj->links.handle_entry =
-               bpf_program__attach_uprobe(obj->progs.handle_entry, false,
-                                          target_pid ?: -1, libc_path, func_off);
-       err = libbpf_get_error(obj->links.handle_entry);
+       links[0] = bpf_program__attach_uprobe(obj->progs.handle_entry, false,
+                                             target_pid ?: -1, libc_path, func_off);
+       err = libbpf_get_error(links[0]);
        if (err) {
                warn("failed to attach getaddrinfo: %d\n", err);
                return -1;
        }
-       obj->links.handle_return =
-               bpf_program__attach_uprobe(obj->progs.handle_return, true,
-                                          target_pid ?: -1, libc_path, func_off);
-       err = libbpf_get_error(obj->links.handle_return);
+       links[1] = bpf_program__attach_uprobe(obj->progs.handle_return, true,
+                                             target_pid ?: -1, libc_path, func_off);
+       err = libbpf_get_error(links[1]);
        if (err) {
                warn("failed to attach getaddrinfo: %d\n", err);
                return -1;
@@ -172,18 +170,16 @@ static int attach_uprobes(struct gethostlatency_bpf *obj)
                warn("could not find gethostbyname in %s\n", libc_path);
                return -1;
        }
-       obj->links.handle_entry =
-               bpf_program__attach_uprobe(obj->progs.handle_entry, false,
-                                          target_pid ?: -1, libc_path, func_off);
-       err = libbpf_get_error(obj->links.handle_entry);
+       links[2] = bpf_program__attach_uprobe(obj->progs.handle_entry, false,
+                                             target_pid ?: -1, libc_path, func_off);
+       err = libbpf_get_error(links[2]);
        if (err) {
                warn("failed to attach gethostbyname: %d\n", err);
                return -1;
        }
-       obj->links.handle_return =
-               bpf_program__attach_uprobe(obj->progs.handle_return, true,
-                                          target_pid ?: -1, libc_path, func_off);
-       err = libbpf_get_error(obj->links.handle_return);
+       links[3] = bpf_program__attach_uprobe(obj->progs.handle_return, true,
+                                             target_pid ?: -1, libc_path, func_off);
+       err = libbpf_get_error(links[3]);
        if (err) {
                warn("failed to attach gethostbyname: %d\n", err);
                return -1;
@@ -194,18 +190,16 @@ static int attach_uprobes(struct gethostlatency_bpf *obj)
                warn("could not find gethostbyname2 in %s\n", libc_path);
                return -1;
        }
-       obj->links.handle_entry =
-               bpf_program__attach_uprobe(obj->progs.handle_entry, false,
-                                          target_pid ?: -1, libc_path, func_off);
-       err = libbpf_get_error(obj->links.handle_entry);
+       links[4] = bpf_program__attach_uprobe(obj->progs.handle_entry, false,
+                                             target_pid ?: -1, libc_path, func_off);
+       err = libbpf_get_error(links[4]);
        if (err) {
                warn("failed to attach gethostbyname2: %d\n", err);
                return -1;
        }
-       obj->links.handle_return =
-               bpf_program__attach_uprobe(obj->progs.handle_return, true,
-                                          target_pid ?: -1, libc_path, func_off);
-       err = libbpf_get_error(obj->links.handle_return);
+       links[5] = bpf_program__attach_uprobe(obj->progs.handle_return, true,
+                                             target_pid ?: -1, libc_path, func_off);
+       err = libbpf_get_error(links[5]);
        if (err) {
                warn("failed to attach gethostbyname2: %d\n", err);
                return -1;
@@ -223,8 +217,9 @@ int main(int argc, char **argv)
        };
        struct perf_buffer_opts pb_opts;
        struct perf_buffer *pb = NULL;
+       struct bpf_link *links[6] = {};
        struct gethostlatency_bpf *obj;
-       int err;
+       int i, err;
 
        err = argp_parse(&argp, argc, argv, 0, NULL, NULL);
        if (err)
@@ -250,7 +245,7 @@ int main(int argc, char **argv)
                goto cleanup;
        }
 
-       err = attach_uprobes(obj);
+       err = attach_uprobes(obj, links);
        if (err)
                goto cleanup;
 
@@ -285,6 +280,8 @@ int main(int argc, char **argv)
 
 cleanup:
        perf_buffer__free(pb);
+       for (i = 0; i < 6; i++)
+               bpf_link__destroy(links[i]);
        gethostlatency_bpf__destroy(obj);
 
        return err != 0;

diff --git a/libbpf-tools/ksnoop.c b/libbpf-tools/ksnoop.c
index 896b25a6f4e62ca89201e8e797f40a6e9cfe41fa..ce8b240bcd5b7237d85278274601f5fb59507cb9 100644 (file)
--- a/libbpf-tools/ksnoop.c
+++ b/libbpf-tools/ksnoop.c
@@ -669,7 +669,7 @@ static int parse_traces(int argc, char **argv, struct trace **traces)
 
 static int cmd_info(int argc, char **argv)
 {
-       struct trace *traces;
+       struct trace *traces = NULL;
        char str[MAX_STR];
        int nr_traces;
        __u8 i, j;
@@ -696,6 +696,7 @@ static int cmd_info(int argc, char **argv)
                               func->nr_args - MAX_ARGS);
                printf(");\n");
        }
+       free(traces);
        return 0;
 }
 
@@ -815,14 +816,14 @@ static int add_traces(struct bpf_map *func_map, struct trace *traces,
 static int attach_traces(struct ksnoop_bpf *skel, struct trace *traces,
                         int nr_traces)
 {
-       struct bpf_link *link;
        int i, ret;
 
        for (i = 0; i < nr_traces; i++) {
-               link = bpf_program__attach_kprobe(skel->progs.kprobe_entry,
-                                                 false,
-                                                 traces[i].func.name);
-               ret = libbpf_get_error(link);
+               traces[i].links[0] =
+                       bpf_program__attach_kprobe(skel->progs.kprobe_entry,
+                                                  false,
+                                                  traces[i].func.name);
+               ret = libbpf_get_error(traces[i].links[0]);
                if (ret) {
                        p_err("Could not attach kprobe to '%s': %s",
                              traces[i].func.name, strerror(-ret));
@@ -830,10 +831,11 @@ static int attach_traces(struct ksnoop_bpf *skel, struct trace *traces,
                        }
                p_debug("Attached kprobe for '%s'", traces[i].func.name);
 
-               link = bpf_program__attach_kprobe(skel->progs.kprobe_return,
-                                                 true,
-                                                 traces[i].func.name);
-               ret = libbpf_get_error(link);
+               traces[i].links[1] =
+                       bpf_program__attach_kprobe(skel->progs.kprobe_return,
+                                                  true,
+                                                  traces[i].func.name);
+               ret = libbpf_get_error(traces[i].links[1]);
                if (ret) {
                        p_err("Could not attach kretprobe to '%s': %s",
                              traces[i].func.name, strerror(-ret));
@@ -848,10 +850,10 @@ static int cmd_trace(int argc, char **argv)
 {
        struct perf_buffer_opts pb_opts = {};
        struct bpf_map *perf_map, *func_map;
-       struct perf_buffer *pb;
+       struct perf_buffer *pb = NULL;
        struct ksnoop_bpf *skel;
-       int nr_traces, ret = 0;
-       struct trace *traces;
+       int i, nr_traces, ret = -1;
+       struct trace *traces = NULL;
 
        nr_traces = parse_traces(argc, argv, &traces);
        if (nr_traces < 0)
@@ -866,22 +868,22 @@ static int cmd_trace(int argc, char **argv)
        perf_map = skel->maps.ksnoop_perf_map;
        if (!perf_map) {
                p_err("Could not find '%s'", "ksnoop_perf_map");
-               return 1;
+               goto cleanup;
        }
        func_map = bpf_object__find_map_by_name(skel->obj, "ksnoop_func_map");
        if (!func_map) {
                p_err("Could not find '%s'", "ksnoop_func_map");
-               return 1;
+               goto cleanup;
        }
 
        if (add_traces(func_map, traces, nr_traces)) {
                p_err("Could not add traces to '%s'", "ksnoop_func_map");
-               return 1;
+               goto cleanup;
        }
 
        if (attach_traces(skel, traces, nr_traces)) {
                p_err("Could not attach %d traces", nr_traces);
-               return 1;
+               goto cleanup;
        }
 
        pb_opts.sample_cb = trace_handler;
@@ -890,7 +892,7 @@ static int cmd_trace(int argc, char **argv)
        if (libbpf_get_error(pb)) {
                p_err("Could not create perf buffer: %s",
                      libbpf_errstr(pb));
-               return 1;
+               goto cleanup;
        }
 
        printf("%16s %4s %8s %s\n", "TIME", "CPU", "PID", "FUNCTION/ARGS");
@@ -912,6 +914,11 @@ static int cmd_trace(int argc, char **argv)
        }
 
 cleanup:
+       for (i = 0; i < nr_traces; i++) {
+               bpf_link__destroy(traces[i].links[0]);
+               bpf_link__destroy(traces[i].links[1]);
+       }
+       free(traces);
        perf_buffer__free(pb);
        ksnoop_bpf__destroy(skel);
 

diff --git a/libbpf-tools/ksnoop.h b/libbpf-tools/ksnoop.h
index 6c55b0efe4a40a409d9fc16a637c0ef6314b47ba..6b33df4ec7fdd3b786f2851f19afd6c2884a5465 100644 (file)
--- a/libbpf-tools/ksnoop.h
+++ b/libbpf-tools/ksnoop.h
@@ -24,8 +24,8 @@ enum arg {
  */
 #define KSNOOP_ID_UNKNOWN              0xffffffff
 
-#define MAX_NAME                       96      
-#define MAX_STR                                256     
+#define MAX_NAME                       96
+#define MAX_STR                                256
 #define MAX_PATH                       512
 #define MAX_VALUES                     6
 #define MAX_ARGS                       (MAX_VALUES - 1)
@@ -112,6 +112,7 @@ struct trace {
        __u16 buf_len;
        char buf[MAX_TRACE_BUF];
        char buf_end[0];
+       struct bpf_link *links[2];
 };
 
 #define PAGES_DEFAULT  16