Linux Trace Kernel
 help / color / mirror / Atom feed
* Re: [PATCH] perf/bpf: Fix data race in __perf_event_overflow
       [not found] <20260811235331.10044-1-kartikey406@gmail.com>
@ 2026-08-12  0:08 ` sashiko-bot
  2026-08-12 10:35   ` Jiri Olsa
  2026-08-12  1:04 ` bot+bpf-ci
  2026-08-12 10:12 ` Peter Zijlstra
  2 siblings, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-08-12  0:08 UTC (permalink / raw)
  To: Deepanshu Kartikey; +Cc: bpf, linux-trace-kernel, linux-perf-users

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

Pre-existing issues:
- [High] Lockless TOCTOU race in perf_event_alloc() causes NULL pointer dereference or Use-After-Free during BPF program inheritance.
- [Medium] Incomplete fix for data races and missing memory barriers in BPF attach/detach paths.
- [Medium] Time-of-Check to Time-of-Use (TOCTOU) race drops valid perf events during concurrent BPF detach.
--

--- Patch [1]: [PATCH] perf/bpf: Fix data race in __perf_event_overflow ---
commit f63a2be5ceb79a2a8bb0263ba6723f7bf4ae763d
Author: Deepanshu Kartikey <kartikey406@gmail.com>

perf/bpf: Fix data race in __perf_event_overflow

This patch addresses a data race where __perf_event_overflow() concurrently
reads event->prog without lock protection while perf_event_detach_bpf_prog()
writes it to NULL. It fixes the race by introducing READ_ONCE() and
WRITE_ONCE() in the respective read and write paths.

Reported-by: syzbot+651d2774bd1d8395595f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=651d2774bd1d8395595f
Fixes: f11f10bfa1ca ("perf/bpf: Call BPF handler directly, not through overflow machinery")
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c

[ ... ]

> @@ -10766,7 +10767,8 @@ static int __perf_event_overflow(struct perf_event *event,
>  	if (event->attr.aux_pause)
>  		perf_event_aux_pause(event->aux_event, true);
>  
> -	if (event->prog && event->prog->type == BPF_PROG_TYPE_PERF_EVENT &&
> +	prog = READ_ONCE(event->prog);
> +	if (prog && prog->type == BPF_PROG_TYPE_PERF_EVENT &&
>  	    !bpf_overflow_handler(event, data, regs))
>  		goto out;

[Severity: Medium]
This isn't a bug introduced by this patch, but does the independent second
read in bpf_overflow_handler() create a time-of-check to time-of-use race?

If a concurrent detach sets event->prog to NULL between the READ_ONCE() here
in __perf_event_overflow() and the second READ_ONCE() inside
bpf_overflow_handler():

bpf_overflow_handler() {
    prog = READ_ONCE(event->prog);
    if (prog) {
        ...
    }
    return 0;
}

Wouldn't bpf_overflow_handler() evaluate prog as NULL and return 0? The
caller __perf_event_overflow() evaluates !0 as true and executes goto out,
which unconditionally drops the perf sample.

Should the already-validated prog pointer be passed directly to
bpf_overflow_handler()?


> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2018,7 +2018,7 @@ void perf_event_detach_bpf_prog(struct perf_event *event)
>  
>  put:
>  	prog = event->prog;
> -	event->prog = NULL;
> +	WRITE_ONCE(event->prog, NULL);
>  
>  unlock:

[Severity: Medium]
This is a pre-existing issue, but does this incomplete fix leave other data
races in the attach and detach paths?

Other functions like perf_event_set_bpf_handler(),
perf_event_free_bpf_handler(), and perf_event_attach_bpf_prog() still
perform plain writes to event->prog without WRITE_ONCE().

Additionally, does this need release and acquire semantics? If the writes to
event->prog and event->bpf_cookie in the attach paths are reordered by the
CPU, could the NMI handler read a valid event->prog but a stale or
uninitialized event->bpf_cookie?

[Severity: High]
This is also a pre-existing issue, but does the lockless access in
perf_event_alloc() create a use-after-free or NULL pointer dereference
during BPF program inheritance?

During fork, perf_event_alloc() inherits the BPF program:

perf_event_alloc() {
    ...
    if (parent_event->prog) {
        struct bpf_prog *prog = parent_event->prog;
        bpf_prog_inc(prog);
        ...
    }
}

Since this accesses parent_event->prog locklessly, can a concurrent
bpf_perf_link_release() clear the pointer? If the compiler reloads the
pointer after the check, bpf_prog_inc() would dereference NULL.

Even if it is optimized to a single read, perf_event_alloc() is not inside
an RCU read-side critical section. Could a concurrent detach finish its RCU
grace period and free the BPF program, causing bpf_prog_inc() to write to
freed memory?

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf/bpf: Fix data race in __perf_event_overflow
       [not found] <20260811235331.10044-1-kartikey406@gmail.com>
  2026-08-12  0:08 ` [PATCH] perf/bpf: Fix data race in __perf_event_overflow sashiko-bot
@ 2026-08-12  1:04 ` bot+bpf-ci
  2026-08-12 10:12 ` Peter Zijlstra
  2 siblings, 0 replies; 4+ messages in thread
From: bot+bpf-ci @ 2026-08-12  1:04 UTC (permalink / raw)
  To: kartikey406, peterz, mingo, acme, namhyung, mark.rutland,
	alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark,
	song, ast, daniel, andrii, eddyz87, memxor, martin.lau,
	yonghong.song, emil, kpsingh, mattbobrowski, rostedt, mhiramat,
	mathieu.desnoyers, me
  Cc: linux-perf-users, linux-kernel, bpf, linux-trace-kernel,
	kartikey406, syzbot+651d2774bd1d8395595f, ast, andrii, daniel,
	martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 1931 bytes --]

> commit ffa161d3558e4937793a0c3e98e19e4b22e58bd9
> Author: Deepanshu Kartikey <kartikey406@gmail.com>
> Date:   Wed Aug 12 05:23:31 2026 +0530
>
>     perf/bpf: Fix data race in __perf_event_overflow
>
>     perf_event_detach_bpf_prog() writes event->prog = NULL
>     under bpf_event_mutex, while __perf_event_overflow() reads
>     event->prog concurrently without any lock protection. This
>     causes a data race detected by KCSAN.
>
>     __perf_event_overflow() can be called from interrupt/NMI
>     context, so it cannot acquire bpf_event_mutex. Fix the race
>     by using WRITE_ONCE() in perf_event_detach_bpf_prog() and
>     READ_ONCE() in __perf_event_overflow(). Also store the result
>     of READ_ONCE() in a local variable to avoid reading event->prog
>     twice which could result in a NULL pointer dereference if the
>     pointer becomes NULL between the two reads.
>
>     Reported-by: syzbot+651d2774bd1d8395595f@syzkaller.appspotmail.com
>     Closes: https://syzkaller.appspot.com/bug?extid=651d2774bd1d8395595f
>     Fixes: f11f10bfa1ca ("perf/bpf: Call BPF handler directly, not through overflow machinery")
>     Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
>     Reported-by: syzbot+651d2774bd1d8395595f@syzkaller.appspotmail.com
>     Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>

The trailer block contains duplicate tags: both Reported-by: and
Signed-off-by: appear twice. Also, the second Reported-by: is placed
after the first Signed-off-by:, which breaks the conventional trailer
ordering (Reported-by/Closes/Fixes should come before Signed-off-by).

Should the duplicates be collapsed to one of each, in proper trailer order?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31550171807

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf/bpf: Fix data race in __perf_event_overflow
       [not found] <20260811235331.10044-1-kartikey406@gmail.com>
  2026-08-12  0:08 ` [PATCH] perf/bpf: Fix data race in __perf_event_overflow sashiko-bot
  2026-08-12  1:04 ` bot+bpf-ci
@ 2026-08-12 10:12 ` Peter Zijlstra
  2 siblings, 0 replies; 4+ messages in thread
From: Peter Zijlstra @ 2026-08-12 10:12 UTC (permalink / raw)
  To: Deepanshu Kartikey
  Cc: mingo, acme, namhyung, mark.rutland, alexander.shishkin, jolsa,
	irogers, adrian.hunter, james.clark, song, ast, daniel, andrii,
	eddyz87, memxor, martin.lau, yonghong.song, emil, kpsingh,
	mattbobrowski, rostedt, mhiramat, mathieu.desnoyers, me,
	linux-perf-users, linux-kernel, bpf, linux-trace-kernel,
	syzbot+651d2774bd1d8395595f

On Wed, Aug 12, 2026 at 05:23:31AM +0530, Deepanshu Kartikey wrote:
> perf_event_detach_bpf_prog() writes event->prog = NULL
> under bpf_event_mutex, while __perf_event_overflow() reads
> event->prog concurrently without any lock protection. This
> causes a data race detected by KCSAN.

IIRC perf_event_free_pbf_prog() was supposed to be called after the
event is shut down, so there is no possible concurrency.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf/bpf: Fix data race in __perf_event_overflow
  2026-08-12  0:08 ` [PATCH] perf/bpf: Fix data race in __perf_event_overflow sashiko-bot
@ 2026-08-12 10:35   ` Jiri Olsa
  0 siblings, 0 replies; 4+ messages in thread
From: Jiri Olsa @ 2026-08-12 10:35 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: Deepanshu Kartikey, bpf, linux-trace-kernel, linux-perf-users

On Wed, Aug 12, 2026 at 12:08:58AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Lockless TOCTOU race in perf_event_alloc() causes NULL pointer dereference or Use-After-Free during BPF program inheritance.
> - [Medium] Incomplete fix for data races and missing memory barriers in BPF attach/detach paths.
> - [Medium] Time-of-Check to Time-of-Use (TOCTOU) race drops valid perf events during concurrent BPF detach.
> --
> 
> --- Patch [1]: [PATCH] perf/bpf: Fix data race in __perf_event_overflow ---
> commit f63a2be5ceb79a2a8bb0263ba6723f7bf4ae763d
> Author: Deepanshu Kartikey <kartikey406@gmail.com>
> 
> perf/bpf: Fix data race in __perf_event_overflow
> 
> This patch addresses a data race where __perf_event_overflow() concurrently
> reads event->prog without lock protection while perf_event_detach_bpf_prog()
> writes it to NULL. It fixes the race by introducing READ_ONCE() and
> WRITE_ONCE() in the respective read and write paths.
> 
> Reported-by: syzbot+651d2774bd1d8395595f@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=651d2774bd1d8395595f
> Fixes: f11f10bfa1ca ("perf/bpf: Call BPF handler directly, not through overflow machinery")
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> 
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> 
> [ ... ]
> 
> > @@ -10766,7 +10767,8 @@ static int __perf_event_overflow(struct perf_event *event,
> >  	if (event->attr.aux_pause)
> >  		perf_event_aux_pause(event->aux_event, true);
> >  
> > -	if (event->prog && event->prog->type == BPF_PROG_TYPE_PERF_EVENT &&
> > +	prog = READ_ONCE(event->prog);
> > +	if (prog && prog->type == BPF_PROG_TYPE_PERF_EVENT &&
> >  	    !bpf_overflow_handler(event, data, regs))
> >  		goto out;
> 
> [Severity: Medium]
> This isn't a bug introduced by this patch, but does the independent second
> read in bpf_overflow_handler() create a time-of-check to time-of-use race?
> 
> If a concurrent detach sets event->prog to NULL between the READ_ONCE() here
> in __perf_event_overflow() and the second READ_ONCE() inside
> bpf_overflow_handler():
> 
> bpf_overflow_handler() {
>     prog = READ_ONCE(event->prog);
>     if (prog) {
>         ...
>     }
>     return 0;
> }
> 
> Wouldn't bpf_overflow_handler() evaluate prog as NULL and return 0? The
> caller __perf_event_overflow() evaluates !0 as true and executes goto out,
> which unconditionally drops the perf sample.
> 
> Should the already-validated prog pointer be passed directly to
> bpf_overflow_handler()?

seems legit

> 
> 
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -2018,7 +2018,7 @@ void perf_event_detach_bpf_prog(struct perf_event *event)
> >  
> >  put:
> >  	prog = event->prog;
> > -	event->prog = NULL;
> > +	WRITE_ONCE(event->prog, NULL);
> >  
> >  unlock:
> 
> [Severity: Medium]
> This is a pre-existing issue, but does this incomplete fix leave other data
> races in the attach and detach paths?
> 
> Other functions like perf_event_set_bpf_handler(),
> perf_event_free_bpf_handler(), and perf_event_attach_bpf_prog() still
> perform plain writes to event->prog without WRITE_ONCE().

was wondering the same, the perf_event_detach_bpf_prog is called for
tracing events (tracepoint/kprobe/uprobe) the perf event release goes
through perf_event_free_bpf_handler, check perf_event_free_bpf_prog

jirka

> 
> Additionally, does this need release and acquire semantics? If the writes to
> event->prog and event->bpf_cookie in the attach paths are reordered by the
> CPU, could the NMI handler read a valid event->prog but a stale or
> uninitialized event->bpf_cookie?
> 
> [Severity: High]
> This is also a pre-existing issue, but does the lockless access in
> perf_event_alloc() create a use-after-free or NULL pointer dereference
> during BPF program inheritance?
> 
> During fork, perf_event_alloc() inherits the BPF program:
> 
> perf_event_alloc() {
>     ...
>     if (parent_event->prog) {
>         struct bpf_prog *prog = parent_event->prog;
>         bpf_prog_inc(prog);
>         ...
>     }
> }
> 
> Since this accesses parent_event->prog locklessly, can a concurrent
> bpf_perf_link_release() clear the pointer? If the compiler reloads the
> pointer after the check, bpf_prog_inc() would dereference NULL.
> 
> Even if it is optimized to a single read, perf_event_alloc() is not inside
> an RCU read-side critical section. Could a concurrent detach finish its RCU
> grace period and free the BPF program, causing bpf_prog_inc() to write to
> freed memory?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260811235331.10044-1-kartikey406@gmail.com?part=1
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-12 10:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260811235331.10044-1-kartikey406@gmail.com>
2026-08-12  0:08 ` [PATCH] perf/bpf: Fix data race in __perf_event_overflow sashiko-bot
2026-08-12 10:35   ` Jiri Olsa
2026-08-12  1:04 ` bot+bpf-ci
2026-08-12 10:12 ` Peter Zijlstra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox