* Re: [PATCH] perf bpf-filter: Fix memory leak on bpf_link
2026-10-20 5:32 [PATCH] perf bpf-filter: Fix memory leak on bpf_link Namhyung Kim
@ 2026-10-20 8:33 ` sashiko-bot
2026-10-20 15:47 ` Ian Rogers
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-10-20 8:33 UTC (permalink / raw)
To: Namhyung Kim; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] perf bpf-filter: double close of perf_event fd in bpf_link__destroy()
- [Medium] perf bpf-filter: incorrect IS_ERR() check in perf_bpf_filter__prepare()
--
commit 0bafd46f68639fd52d99226001cedebcf8838c32
Author: Namhyung Kim <namhyung@kernel.org>
perf bpf-filter: Fix memory leak on bpf_link
This commit stores the bpf_link pointers returned when attaching BPF sample
filters in an xyarray on the evsel and destroys them on cleanup and error
paths to fix a memory leak.
> diff --git a/tools/perf/util/bpf-filter.c b/tools/perf/util/bpf-filter.c
> index bcd81084e3420..664484703fc3d 100644
> --- a/tools/perf/util/bpf-filter.c
> +++ b/tools/perf/util/bpf-filter.c
> @@ -66,6 +66,7 @@
> #include "bpf_skel/sample_filter.skel.h"
>
> #define FD(e, x, y) (*(int *)xyarray__entry(e->core.fd, x, y))
> +#define LINK(e, x, y) (*(struct bpf_link **)xyarray__entry(e->bpf_links, x, y))
>
> #define __PERF_SAMPLE_TYPE(tt, st, opt) { tt, #st, opt }
> #define PERF_SAMPLE_TYPE(_st, opt) __PERF_SAMPLE_TYPE(PBF_TERM_##_st, PERF_SAMPLE_##_st, opt)
> @@ -526,6 +527,15 @@ int perf_bpf_filter__prepare(struct evsel *evsel, struct target *target)
> goto err;
> }
>
> + evsel->bpf_links = xyarray__new(xyarray__max_x(evsel->core.fd),
> + xyarray__max_y(evsel->core.fd),
> + sizeof(link));
> + if (evsel->bpf_links == NULL) {
> + ret = -errno;
> + pr_err("Failed to allocate bpf links\n");
> + goto err;
> + }
> +
> prog = skel->progs.perf_sample_filter;
> for (x = 0; x < xyarray__max_x(evsel->core.fd); x++) {
> for (y = 0; y < xyarray__max_y(evsel->core.fd); y++) {
> @@ -536,6 +546,8 @@ int perf_bpf_filter__prepare(struct evsel *evsel, struct target *target)
> ret = PTR_ERR(link);
> goto err;
> }
> +
> + LINK(evsel, x, y) = link;
> }
> }
> free(entry);
> @@ -553,6 +565,15 @@ int perf_bpf_filter__prepare(struct evsel *evsel, struct target *target)
> free(pfi);
> }
> }
> + if (evsel->bpf_links) {
> + for (x = 0; x < xyarray__max_x(evsel->bpf_links); x++) {
> + for (y = 0; y < xyarray__max_y(evsel->bpf_links); y++) {
> + bpf_link__destroy(LINK(evsel, x, y));
> + }
> + }
> + xyarray__delete(evsel->bpf_links);
> + evsel->bpf_links = NULL;
> + }
[Severity: Medium]
Can this error cleanup block ever be reached when attaching fails?
In perf_bpf_filter__prepare(), the attachment loop checks the return value
of bpf_program__attach_perf_event_opts() with IS_ERR(link):
tools/perf/util/bpf-filter.c:perf_bpf_filter__prepare() {
...
for (x = 0; x < xyarray__max_x(evsel->core.fd); x++) {
for (y = 0; y < xyarray__max_y(evsel->core.fd); y++) {
link = bpf_program__attach_perf_event_opts(prog, FD(evsel, x, y),
&pe_opts);
if (IS_ERR(link)) {
pr_err("Failed to attach perf sample-filter program\n");
ret = PTR_ERR(link);
goto err;
}
LINK(evsel, x, y) = link;
}
}
}
However, in tools/lib/bpf/libbpf.c, bpf_program__attach_perf_event_opts()
returns libbpf_err_ptr(err) on failure, which sets errno and returns NULL
rather than an error pointer:
tools/lib/bpf/libbpf.c:bpf_program__attach_perf_event_opts() {
...
return &link->link;
err_out:
if (link_fd >= 0)
close(link_fd);
free(link);
return libbpf_err_ptr(err);
}
Because IS_ERR(NULL) is false, an attachment failure stores NULL into
LINK(evsel, x, y) and continues instead of jumping to err, leaving the
evsel->bpf_links cleanup at err unreachable and returning 0.
> sample_filter_bpf__destroy(skel);
> return ret;
> }
> @@ -566,6 +587,17 @@ int perf_bpf_filter__destroy(struct evsel *evsel)
> list_del(&expr->list);
> free(expr);
> }
> +
> + if (evsel->bpf_links) {
> + for (int x = 0; x < xyarray__max_x(evsel->bpf_links); x++) {
> + for (int y = 0; y < xyarray__max_y(evsel->bpf_links); y++) {
> + bpf_link__destroy(LINK(evsel, x, y));
> + }
> + }
> + xyarray__delete(evsel->bpf_links);
> + evsel->bpf_links = NULL;
> + }
> +
[Severity: Medium]
Does calling bpf_link__destroy() here and in the error path of
perf_bpf_filter__prepare() cause a double close on the perf_event file
descriptors in evsel->core.fd?
In perf_bpf_filter__prepare(), FD(evsel, x, y) is passed directly to
bpf_program__attach_perf_event_opts(), which stores that descriptor in
perf_link->perf_event_fd and sets the detach callback to
bpf_link_perf_detach().
When bpf_link__destroy() is later called, bpf_link_perf_detach() disables
and closes perf_link->perf_event_fd in addition to link->fd:
tools/lib/bpf/libbpf.c:bpf_link_perf_detach() {
...
if (ioctl(perf_link->perf_event_fd, PERF_EVENT_IOC_DISABLE, 0) < 0)
err = -errno;
if (perf_link->perf_event_fd != link->fd)
close(perf_link->perf_event_fd);
close(link->fd);
...
}
During normal teardown in evlist__put(), evlist__close() closes all
FD(evsel, x, y) descriptors before evlist__purge() -> evsel__put() ->
evsel__exit() -> perf_bpf_filter__destroy() runs:
tools/perf/util/evlist.c:evlist__put() {
...
out_delete:
evlist__free_stats(evlist);
evlist__do_munmap(evlist);
evlist__close(evlist);
evlist__purge(evlist);
evlist__exit(evlist);
RC_CHK_FREE(evlist);
}
As a result, bpf_link__destroy() in perf_bpf_filter__destroy() performs
ioctl() and close() on FD(evsel, x, y) after it has already been closed.
Similarly, if perf_bpf_filter__prepare() fails partway through attaching
and jumps to err, bpf_link__destroy() closes the attached FD(evsel, x, y)
descriptors while leaving their positive FD numbers in evsel->core.fd, so
a subsequent evlist__close() closes them a second time.
> sample_filter_bpf__destroy(evsel->bpf_skel);
>
> list_for_each_entry_safe(pfi, pos, &pinned_filters, list) {
[ ... ]
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261020053226.55936-1-namhyung@kernel.org?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] perf bpf-filter: Fix memory leak on bpf_link
2026-10-20 5:32 [PATCH] perf bpf-filter: Fix memory leak on bpf_link Namhyung Kim
2026-10-20 8:33 ` sashiko-bot
@ 2026-10-20 15:47 ` Ian Rogers
1 sibling, 0 replies; 3+ messages in thread
From: Ian Rogers @ 2026-10-20 15:47 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter,
Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users
On Mon, Oct 19, 2026 at 10:32 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Attaching BPF program returns bpf_link but it leaked like following.
>
> ==51328==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 1568 byte(s) in 28 object(s) allocated from:
> #0 0x7ffbe76ef24f in calloc (/lib64/libasan.so.8+0xef24f) (BuildId: 61e8165ca90f7a4f06b73f064a938c76deaae079)
> #1 0x000000625df7 in bpf_program__attach_perf_event_opts (tools/perf/perf+0x625df7) (BuildId: 5e6309fda155e58708e92f1c76d44cadd585d0eb)
> #2 0x000000beddc0 in perf_bpf_filter__prepare util/bpf-filter.c:507
> #3 0x0000008f3def in evlist__apply_filters util/evlist.c:1421
> #4 0x00000044bbcf in record__open tools/perf/builtin-record.c:1469
> #5 0x000000453f9e in __cmd_record tools/perf/builtin-record.c:2594
> #6 0x00000045dadb in cmd_record tools/perf/builtin-record.c:4427
> #7 0x0000005df56b in run_builtin tools/perf/perf.c:383
> #8 0x0000005dfa0e in handle_internal_command tools/perf/perf.c:436
> #9 0x0000005dfddc in run_argv tools/perf/perf.c:480
> #10 0x0000005e0333 in main tools/perf/perf.c:581
> #11 0x7ffbdd450680 in __libc_start_call_main (/lib64/libc.so.6+0x3680) (BuildId: 85075e2ae2e13352a1413d41c327dacf6461526c)
> #12 0x7ffbdd450797 in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x3797) (BuildId: 85075e2ae2e13352a1413d41c327dacf6461526c)
> #13 0x000000409c54 in _start (tools/perf/perf+0x409c54) (BuildId: 5e6309fda155e58708e92f1c76d44cadd585d0eb)
>
> SUMMARY: AddressSanitizer: 1568 byte(s) leaked in 28 allocation(s).
>
> Let's add an xyarray of bpf_link for the filters and release them at the
> end to fix the leaks.
As Sashiko noted, libbpf's bpf_program__attach_perf_event_opts() is
designed for callers that transfer ownership of the perf_event fd to
the returned bpf_link (such as bpf_program__attach_kprobe(), which
opens its own perf_event fd internally). When bpf_link__destroy() is
called, bpf_link_perf_detach() unconditionally disables and closes
perf_link->perf_event_fd:
static int bpf_link_perf_detach(struct bpf_link *link)
{
struct bpf_link_perf *perf_link = container_of(link,
struct bpf_link_perf, link);
int err = 0;
if (ioctl(perf_link->perf_event_fd,
PERF_EVENT_IOC_DISABLE, 0) < 0)
err = -errno;
if (perf_link->perf_event_fd != link->fd)
close(perf_link->perf_event_fd);
close(link->fd);
...
Since evsel->core.fd owns FD(evsel, x, y) and evlist__close() closes
those descriptors before evsel__exit() -> perf_bpf_filter__destroy()
runs, calling bpf_link__destroy() ends up closing FD(evsel, x, y) a
second time (and my v4 patch 2 has the same issue on FD(evsel, 0, 0)
by assigning skel->links.perf_tp_sample_filter = link).
Instead of using bpf_program__attach_perf_event_opts() and tracking
bpf_links, how about attaching the program directly with
ioctl(FD(evsel, x, y), PERF_EVENT_IOC_SET_BPF, fd), just like the
pinned filter path right above it in perf_bpf_filter__prepare()
already does?
fd = bpf_program__fd(skel->progs.perf_sample_filter);
for (x = 0; x < xyarray__max_x(evsel->core.fd); x++) {
for (y = 0; y < xyarray__max_y(evsel->core.fd); y++) {
ret = ioctl(FD(evsel, x, y),
PERF_EVENT_IOC_SET_BPF, fd);
if (ret < 0) {
pr_err("Failed to attach perf
sample-filter\n");
goto err;
}
}
}
That avoids allocating struct bpf_link_perf and extra BPF_PERF_EVENT
link fds altogether (fixing the leak without needing
evsel->bpf_links), avoids the double-close on FD(evsel, x, y) since
the kernel's _free_event() -> perf_event_free_bpf_prog() detaches the
program when evlist__close() closes the perf_event fd, and also lets
us drop pe_opts (.dont_enable = true) and the
LIBBPF_CURRENT_VERSION_GEQ(1, 7) guard.
For simplicity, I will include a patch doing this in v5 of my series.
Thanks,
Ian
^ permalink raw reply [flat|nested] 3+ messages in thread