On Wed, Aug 12, 2026 at 4:05 PM Jiri Olsa <[email protected]> wrote: > > On Wed, Aug 12, 2026 at 12:08:58AM +0000, [email protected] 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 <[email protected]> > > > > 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: [email protected] > > 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 <[email protected]> > > > > > 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/[email protected]?part=1 > >
Thank you for the thorough review! On the three issues raised: [1] TOCTOU in bpf_overflow_handler(): You are correct. The second READ_ONCE() inside bpf_overflow_handler() creates a TOCTOU race. If a concurrent detach sets event->prog to NULL between the two reads, bpf_overflow_handler() returns 0, causing __perf_event_overflow() to drop the perf sample via goto out. I will fix this by passing the already-validated prog pointer directly to bpf_overflow_handler() instead of having it re-read event->prog. [2] Incomplete fix - other write paths: Thanks Jirka for pointing out that perf_event_free_bpf_handler() is used for handler-based events (tracepoint/kprobe/uprobe). You are right that my fix is incomplete. I will extend the fix to also add WRITE_ONCE() in: - perf_event_free_bpf_handler() - perf_event_set_bpf_handler() - perf_event_attach_bpf_prog() [3] Lockless access in perf_event_alloc() - UAF: Agreed. The lockless access to parent_event->prog in perf_event_alloc() during fork can lead to NULL pointer dereference or use-after-free if a concurrent detach clears and frees the prog. I will fix this by using READ_ONCE() to capture the pointer atomically before the NULL check. I will send a v2 patch addressing all three issues. Thanks Deepanshu
