Skip to content

Commit

Permalink
libbpf-tools: Fix memory leaks in ksnoop/gethostlatency
Browse files Browse the repository at this point in the history
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 iovisor#3664.

Signed-off-by: Hengqi Chen <[email protected]>
  • Loading branch information
chenhengqi committed Oct 23, 2021
1 parent bced75a commit e9f140f
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 47 deletions.
51 changes: 24 additions & 27 deletions libbpf-tools/gethostlatency.c
Original file line number Diff line number Diff line change
Expand Up @@ -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] = {};
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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)
Expand All @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
43 changes: 25 additions & 18 deletions libbpf-tools/ksnoop.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -696,6 +696,7 @@ static int cmd_info(int argc, char **argv)
func->nr_args - MAX_ARGS);
printf(");\n");
}
free(traces);
return 0;
}

Expand Down Expand Up @@ -815,25 +816,26 @@ 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));
return ret;
}
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));
Expand All @@ -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)
Expand All @@ -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;
Expand All @@ -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");
Expand All @@ -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);

Expand Down
5 changes: 3 additions & 2 deletions libbpf-tools/ksnoop.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit e9f140f

Please sign in to comment.