All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf bpf-filter: Fix memory leak on bpf_link
@ 2026-10-20  5:32 Namhyung Kim
  2026-10-20  8:33 ` sashiko-bot
  2026-10-20 15:47 ` Ian Rogers
  0 siblings, 2 replies; 3+ messages in thread
From: Namhyung Kim @ 2026-10-20  5:32 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Ian Rogers, James Clark
  Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
	linux-perf-users

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.

Fixes: 56ec9457a4a20c5e ("perf bpf filter: Implement event sample filtering")
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/bpf-filter.c | 32 ++++++++++++++++++++++++++++++++
 tools/perf/util/evsel.h      |  1 +
 2 files changed, 33 insertions(+)

diff --git a/tools/perf/util/bpf-filter.c b/tools/perf/util/bpf-filter.c
index 72e83ac1d6a4b075..2fec6004b1aabc1e 100644
--- a/tools/perf/util/bpf-filter.c
+++ b/tools/perf/util/bpf-filter.c
@@ -67,6 +67,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)
@@ -501,6 +502,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++) {
@@ -511,6 +521,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);
@@ -528,6 +540,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;
+	}
 	sample_filter_bpf__destroy(skel);
 	return ret;
 }
@@ -541,6 +562,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;
+	}
+
 	sample_filter_bpf__destroy(evsel->bpf_skel);
 
 	list_for_each_entry_safe(pfi, pos, &pinned_filters, list) {
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 53f3641b66e18674..c4639e6aeac2ffce 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -174,6 +174,7 @@ struct evsel {
 
 	struct list_head	bpf_counter_list; /* for perf-stat -b */
 	struct list_head	bpf_filters; /* for perf-record --filter */
+	struct xyarray		*bpf_links;
 
 	/* for perf-stat --use-bpf */
 	int			bperf_leader_prog_fd;
-- 
2.56.0


^ permalink raw reply related	[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: 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

end of thread, other threads:[~2026-10-20 15:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.