Linux RCU subsystem development
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: acme@kernel.org, adrian.hunter@intel.com, ddhariwal@google.com,
	eranian@google.com, james.clark@linaro.org, jolsa@kernel.org,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	mingo@redhat.com, peterz@infradead.org, rcu@vger.kernel.org
Subject: Re: [PATCH v6 01/11] perf bpf-filter: Fix bpf_link leak by using PERF_EVENT_IOC_SET_BPF
Date: Thu, 22 Oct 2026 15:06:23 -0700	[thread overview]
Message-ID: <atqI3_hai_P5d80q@google.com> (raw)
In-Reply-To: <20261022203003.1779152-2-irogers@google.com>

On Thu, Oct 22, 2026 at 01:29:53PM -0700, Ian Rogers wrote:
> In perf_bpf_filter__prepare(), the unpinned filter path attaches
> skel->progs.perf_sample_filter to each FD(evsel, x, y) using
> bpf_program__attach_perf_event_opts() and ignores the returned
> struct bpf_link *, leaking the allocated struct bpf_link_perf and its
> BPF_PERF_EVENT link fd.
> 
> Furthermore, bpf_program__attach_perf_event_opts() is designed for
> callers that transfer ownership of the perf_event fd to the returned
> bpf_link: when bpf_link__destroy() is called, bpf_link_perf_detach()
> unconditionally disables and closes perf_link->perf_event_fd. Because
> evsel->core.fd owns FD(evsel, x, y) and closes them in evlist__close(),
> destroying the returned bpf_link would double-close the perf_event file
> descriptors.
> 
> Fix this by attaching the program directly with
> ioctl(FD(evsel, x, y), PERF_EVENT_IOC_SET_BPF, fd), matching the pinned
> filter path right above it in perf_bpf_filter__prepare(). In the
> kernel, PERF_EVENT_IOC_SET_BPF takes a reference on the BPF program and
> _free_event() -> perf_event_free_bpf_prog() automatically detaches and
> releases it when the perf_event fd is closed. This also avoids enabling
> the event prematurely, allowing pe_opts (.dont_enable = true) and the
> LIBBPF_CURRENT_VERSION_GEQ(1, 7) guard to be dropped.
> 
> Before (built with EXTRA_CFLAGS="-fsanitize=address"):
>   # perf record -e cycles --filter 'period > 1000' -- true
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.024 MB perf.data ]
> 
>   =================================================================
>   ==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)
>       #1 0x625df7 in bpf_program__attach_perf_event_opts (tools/perf/perf+0x625df7)
>       #2 0xbeddc0 in perf_bpf_filter__prepare util/bpf-filter.c:507
>       #3 0x8f3def in evlist__apply_filters util/evlist.c:1421
>       #4 0x44bbcf in record__open tools/perf/builtin-record.c:1469
>       #5 0x453f9e in __cmd_record tools/perf/builtin-record.c:2594
>       #6 0x45dadb in cmd_record tools/perf/builtin-record.c:4427
>       #7 0x5df56b in run_builtin tools/perf/perf.c:383
>       #8 0x5dfa0e in handle_internal_command tools/perf/perf.c:436
>       #9 0x5dfddc in run_argv tools/perf/perf.c:480
>       #10 0x5e0333 in main tools/perf/perf.c:581
> 
>   SUMMARY: AddressSanitizer: 1568 byte(s) leaked in 28 allocation(s).
> 
> After (built with EXTRA_CFLAGS="-fsanitize=address"):
>   # perf record -e cycles --filter 'period > 1000' -- true
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.024 MB perf.data ]
> 
> Reported-by: Namhyung Kim <namhyung@kernel.org>
> Closes: https://lore.kernel.org/linux-perf-users/20261020053226.55936-1-namhyung@kernel.org/
> Fixes: 56ec9457a4a2 ("perf bpf filter: Implement event sample filtering")
> Signed-off-by: Ian Rogers <irogers@google.com>

Tested-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> ---
>  tools/perf/util/bpf-filter.c | 18 +++++-------------
>  1 file changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/perf/util/bpf-filter.c b/tools/perf/util/bpf-filter.c
> index 72e83ac1d6a4..506414e5c407 100644
> --- a/tools/perf/util/bpf-filter.c
> +++ b/tools/perf/util/bpf-filter.c
> @@ -40,6 +40,7 @@
>   * The 'dropped' map is to keep how many samples it dropped by the filter and
>   * it will be reported as lost samples.
>   */
> +#include <errno.h>
>  #include <stdlib.h>
>  #include <fcntl.h>
>  #include <sys/ioctl.h>
> @@ -423,16 +424,8 @@ int perf_bpf_filter__prepare(struct evsel *evsel, struct target *target)
>  {
>  	int i, x, y, fd, ret;
>  	struct sample_filter_bpf *skel = NULL;
> -	struct bpf_program *prog;
> -	struct bpf_link *link;
>  	struct perf_bpf_filter_entry *entry;
>  	bool needs_idx_hash = !target__has_cpu(target);
> -#if LIBBPF_CURRENT_VERSION_GEQ(1, 7)
> -	DECLARE_LIBBPF_OPTS(bpf_perf_event_opts, pe_opts,
> -			    .dont_enable = true);
> -#else
> -	DECLARE_LIBBPF_OPTS(bpf_perf_event_opts, pe_opts);
> -#endif
>  
>  	entry = calloc(MAX_FILTERS, sizeof(*entry));
>  	if (entry == NULL)
> @@ -501,14 +494,13 @@ int perf_bpf_filter__prepare(struct evsel *evsel, struct target *target)
>  		goto err;
>  	}
>  
> -	prog = skel->progs.perf_sample_filter;
> +	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++) {
> -			link = bpf_program__attach_perf_event_opts(prog, FD(evsel, x, y),
> -								   &pe_opts);
> -			if (IS_ERR(link)) {
> +			ret = ioctl(FD(evsel, x, y), PERF_EVENT_IOC_SET_BPF, fd);
> +			if (ret < 0) {
> +				ret = -errno;
>  				pr_err("Failed to attach perf sample-filter program\n");
> -				ret = PTR_ERR(link);
>  				goto err;
>  			}
>  		}
> -- 
> 2.56.0.307.g5fe7bd680c-goog
> 

  reply	other threads:[~2026-10-22 22:06 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-17  5:45 [PATCH v1 0/5] perf bpf-filter / lock contention: Add lock_held() sample filter and RCU/SRCU support Ian Rogers
2026-10-17  5:45 ` [PATCH v1 1/5] perf lock contention: Share common BPF and lock type definitions in lock_data.h Ian Rogers
2026-10-17  5:45 ` [PATCH v1 2/5] perf bpf-filter: Add lock_held() sample filter Ian Rogers
2026-10-17  5:45 ` [PATCH v1 3/5] perf test: Add tests for lock_held() BPF " Ian Rogers
2026-10-17  5:45 ` [PATCH v1 4/5] perf bpf-filter: Extend lock_held() to support RCU, Tasks Trace RCU, and SRCU Ian Rogers
2026-10-17  5:45 ` [PATCH v1 5/5] perf lock contention: Extend BPF lock contention to RCU " Ian Rogers
2026-10-17 17:14 ` [PATCH v2 0/6] perf bpf-filter / lock contention: Add lock_held() sample filter and RCU/SRCU support Ian Rogers
2026-10-17 17:14   ` [PATCH v2 1/6] perf lock contention: Share common BPF and lock type definitions in lock_data.h Ian Rogers
2026-10-17 17:14   ` [PATCH v2 2/6] perf bpf-filter: Add lock_held() sample filter Ian Rogers
2026-10-17 17:14   ` [PATCH v2 3/6] perf test: Add tests for lock_held() BPF " Ian Rogers
2026-10-17 17:14   ` [PATCH v2 4/6] perf bpf-filter: Extend lock_held() to support RCU, Tasks Trace RCU, and SRCU Ian Rogers
2026-10-17 17:14   ` [PATCH v2 5/6] perf lock contention: Extend BPF lock contention to RCU " Ian Rogers
2026-10-17 17:14   ` [PATCH v2 6/6] perf lock contention: Filter events before creating lock_stat Ian Rogers
2026-10-18  5:37   ` [PATCH v3 0/6] perf bpf-filter / lock contention: Add lock_held() sample filter and RCU/SRCU support Ian Rogers
2026-10-18  5:37     ` [PATCH v3 1/6] perf lock contention: Share common BPF and lock type definitions in lock_data.h Ian Rogers
2026-10-18  5:37     ` [PATCH v3 2/6] perf bpf-filter: Add lock_held() sample filter Ian Rogers
2026-10-18  5:37     ` [PATCH v3 3/6] perf test: Add tests for lock_held() BPF " Ian Rogers
2026-10-18  5:37     ` [PATCH v3 4/6] perf bpf-filter: Extend lock_held() to support RCU, Tasks Trace RCU, and SRCU Ian Rogers
2026-10-18  5:37     ` [PATCH v3 5/6] perf lock contention: Extend BPF lock contention to RCU " Ian Rogers
2026-10-18  5:37     ` [PATCH v3 6/6] perf lock contention: Filter events before creating lock_stat Ian Rogers
2026-10-19  7:00     ` [PATCH v3 0/6] perf bpf-filter / lock contention: Add lock_held() sample filter and RCU/SRCU support Namhyung Kim
2026-10-19 19:08       ` Ian Rogers
2026-10-19 22:09     ` [PATCH v4 0/7] " Ian Rogers
2026-10-19 22:09       ` [PATCH v4 1/7] perf lock contention: Filter events before creating lock_stat Ian Rogers
2026-10-20  4:34         ` Namhyung Kim
2026-10-21 21:39           ` Arnaldo Carvalho de Melo
2026-10-19 22:09       ` [PATCH v4 2/7] perf bpf-filter: Support BPF sample filters on tracepoint events Ian Rogers
2026-10-20  5:41         ` Namhyung Kim
2026-10-20 15:30           ` Ian Rogers
2026-10-20 22:29             ` Namhyung Kim
2026-10-19 22:09       ` [PATCH v4 3/7] perf lock contention: Share common BPF and lock type definitions in lock_data.h Ian Rogers
2026-10-19 22:09       ` [PATCH v4 4/7] perf bpf-filter: Add lock_held() sample filter Ian Rogers
2026-10-19 22:09       ` [PATCH v4 5/7] perf test: Add tests for lock_held() BPF " Ian Rogers
2026-10-19 22:09       ` [PATCH v4 6/7] perf bpf-filter: Extend lock_held() to support RCU, Tasks Trace RCU, and SRCU Ian Rogers
2026-10-19 22:09       ` [PATCH v4 7/7] perf lock contention: Extend BPF lock contention to RCU " Ian Rogers
2026-10-20 20:58       ` [PATCH v5 0/8] perf bpf-filter / lock contention: Add lock_held() sample filter and RCU/SRCU support Ian Rogers
2026-10-20 20:58         ` [PATCH v5 1/8] perf lock contention: Filter events before creating lock_stat Ian Rogers
2026-10-20 20:58         ` [PATCH v5 2/8] perf bpf-filter: Fix bpf_link leak by using PERF_EVENT_IOC_SET_BPF Ian Rogers
2026-10-20 20:58         ` [PATCH v5 3/8] perf bpf-filter: Support BPF sample filters on tracepoint events Ian Rogers
2026-10-20 20:58         ` [PATCH v5 4/8] perf lock contention: Share common BPF and lock type definitions in lock_data.h Ian Rogers
2026-10-20 22:42           ` Namhyung Kim
2026-10-20 20:58         ` [PATCH v5 5/8] perf bpf-filter: Add lock_held() sample filter Ian Rogers
2026-10-20 22:49           ` Namhyung Kim
2026-10-20 20:58         ` [PATCH v5 6/8] perf test: Add tests for lock_held() BPF " Ian Rogers
2026-10-20 20:58         ` [PATCH v5 7/8] perf bpf-filter: Extend lock_held() to support RCU, Tasks Trace RCU, and SRCU Ian Rogers
2026-10-20 20:58         ` [PATCH v5 8/8] perf lock contention: Extend BPF lock contention to RCU " Ian Rogers
2026-10-22 20:29         ` [PATCH v6 00/11] perf bpf-filter / lock contention: Add lock_held() sample filter and RCU/SRCU support Ian Rogers
2026-10-22 20:29           ` [PATCH v6 01/11] perf bpf-filter: Fix bpf_link leak by using PERF_EVENT_IOC_SET_BPF Ian Rogers
2026-10-22 22:06             ` Namhyung Kim [this message]
2026-10-22 20:29           ` [PATCH v6 02/11] perf bpf-filter: Support BPF sample filters on tracepoint events Ian Rogers
2026-10-22 20:29           ` [PATCH v6 03/11] perf lock contention: Share common BPF and lock type definitions in lock_data.h Ian Rogers
2026-10-22 20:29           ` [PATCH v6 04/11] perf lock contention: Extend get_lock_owner() for rt_mutex and mutex handoff Ian Rogers
2026-10-22 20:29           ` [PATCH v6 05/11] perf bpf-filter: Add lock_held() sample filter for global mutexes Ian Rogers
2026-10-22 22:55             ` Namhyung Kim
2026-10-23  5:16               ` Ian Rogers
2026-10-22 20:29           ` [PATCH v6 06/11] perf bpf-filter: Extend lock_held() to other kernel lock types Ian Rogers
2026-10-22 22:56             ` Namhyung Kim
2026-10-23  5:19               ` Ian Rogers
2026-10-22 20:29           ` [PATCH v6 07/11] perf bpf-filter: Support per-process and semi-global locks in lock_held() Ian Rogers
2026-10-22 20:30           ` [PATCH v6 08/11] perf bpf-filter: Support slab cache locks (&<name>) " Ian Rogers
2026-10-22 20:30           ` [PATCH v6 09/11] perf test: Add tests for lock_held() BPF sample filter Ian Rogers
2026-10-22 20:30           ` [PATCH v6 10/11] perf bpf-filter: Extend lock_held() to support RCU, Tasks Trace RCU, and SRCU Ian Rogers
2026-10-22 20:30           ` [PATCH v6 11/11] perf lock contention: Extend BPF lock contention to RCU " Ian Rogers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=atqI3_hai_P5d80q@google.com \
    --to=namhyung@kernel.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=ddhariwal@google.com \
    --cc=eranian@google.com \
    --cc=irogers@google.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rcu@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox