Re: [PATCH v2] perf/bpf: Fix lockless access to parent_event->prog in perf_event_alloc()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Missing RCU read lock around the lockless read and increment of `parent_event->prog` allows a Use-After-Free.
- [High] The BPF program reference is leaked on error paths of `perf_event_alloc()`.
--

--- Patch [1]: [PATCH v2] perf/bpf: Fix lockless access to parent_event->prog in perf_event_alloc() ---
commit 14870f355d1ed182bbc1e31accf43918dde6051a
Author: Deepanshu Kartikey <kartikey406@gmail.com>

perf/bpf: Fix lockless access to parent_event->prog in perf_event_alloc()

This commit attempts to fix a race condition during fork() where
perf_event_alloc() locklessly reads parent_event->prog. It updates the code
to use READ_ONCE() and bpf_prog_inc_not_zero() to safely increment the
reference count without locks.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index a6c8e38a31104..9d456b03c4969 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -13465,11 +13465,13 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
>  		overflow_handler = parent_event->overflow_handler;
>  		context = parent_event->overflow_handler_context;
>  #if defined(CONFIG_BPF_SYSCALL) && defined(CONFIG_EVENT_TRACING)
> -		if (parent_event->prog) {
> -			struct bpf_prog *prog = parent_event->prog;
>  
> -			bpf_prog_inc(prog);
> -			event->prog = prog;
> +		struct bpf_prog *prog;
>  
> +		prog = READ_ONCE(parent_event->prog);
> +		if (prog) {
> +			prog = bpf_prog_inc_not_zero(prog);

[Severity: Critical]
This isn't a bug introduced by this patch, but since it aims to fix a race
here, does this code still leave a use-after-free window?

Because perf_event_alloc() executes in a sleepable context, the RCU grace
period could expire between reading parent_event->prog and calling
bpf_prog_inc_not_zero().

If a concurrent thread detaches the BPF program by closing the bpf_link fd,
it can locklessly release the program before the atomic increment happens.

Should this section be protected by an rcu_read_lock() and rcu_read_unlock()
block to ensure the memory isn't freed before the atomic increment
completes?

> +			if (!IS_ERR(prog))
> +				event->prog = prog;

[Severity: High]
This is a pre-existing issue, but does this increment leak the BPF program
reference if perf_event_alloc() fails later on?

If a subsequent initialization step like attach_perf_ctx_data() or
perf_cgroup_connect() fails, perf_event_alloc() returns an error pointer
and triggers the cleanup via __free_event().

Unlike _free_event() which calls perf_event_free_bpf_prog(), the
__free_event() error cleanup path doesn't appear to release the acquired
bpf_prog reference.

Could an unprivileged user systematically exhaust memory by forcing failures
during fork() on an inherited perf event?

>  		}
>  #endif
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825044507.15217-1-kartikey406@gmail.com?part=1