[PATCH v2] perf/bpf: Fix data races in BPF perf event handling

[PATCH v2] perf/bpf: Fix data races in BPF perf event handling

[Deepanshu Kartikey]

Fix multiple data races involving event->prog field:

1. __perf_event_overflow() reads event->prog twice without
   synchronization creating a TOCTOU race. Fix by using READ_ONCE()
   to capture prog into a local variable and pass it directly to
   bpf_overflow_handler() to avoid a second read inside that function.

2. perf_event_set_bpf_handler() and perf_event_free_bpf_handler()
   perform plain writes to event->prog without WRITE_ONCE(), failing
   to pair with the READ_ONCE() in __perf_event_overflow(). Fix by
   using WRITE_ONCE() in all write paths including
   perf_event_detach_bpf_prog().

3. perf_event_alloc() reads parent_event->prog locklessly during
   fork() which can race with a concurrent detach clearing and freeing
   the prog, potentially causing a NULL pointer dereference or
   use-after-free in bpf_prog_inc(). Fix by holding bpf_event_mutex
   when inheriting the BPF program. Make bpf_event_mutex non-static
   and declare it extern in perf_event.h so it is accessible from
   kernel/events/core.c.

Reported-by: syzbot+651d2774bd1d8395595f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=651d2774bd1d8395595f
Link: https://lore.kernel.org/all/20260811235331.10044-1-kartikey406@gmail.com/T/ [v1]
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
v2:
- Pass validated prog pointer directly to bpf_overflow_handler()
  to avoid TOCTOU race from second READ_ONCE() inside the handler
- Add WRITE_ONCE() to perf_event_set_bpf_handler() and
  perf_event_free_bpf_handler() to pair with READ_ONCE() in overflow
- Fix lockless access in perf_event_alloc() during fork() by holding
  bpf_event_mutex to prevent concurrent detach UAF
- Make bpf_event_mutex non-static and export via perf_event.h
---
 include/linux/perf_event.h |  2 ++
 kernel/events/core.c       | 23 +++++++++++++----------
 kernel/trace/bpf_trace.c   |  4 ++--
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 48d851fbd8ea..5c6dabb6dccb 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -2136,4 +2136,6 @@ static inline void perf_lopwr_cb(bool mode)
 }
 #endif
 
+extern struct mutex bpf_event_mutex;
+
 #endif /* _LINUX_PERF_EVENT_H */
diff --git a/kernel/events/core.c b/kernel/events/core.c
index ba5bd6a78fe7..6f8d3b57fa01 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10652,20 +10652,19 @@ static inline bool sample_is_allowed(struct perf_event *event, struct pt_regs *r
 #ifdef CONFIG_BPF_SYSCALL
 static int bpf_overflow_handler(struct perf_event *event,
 				struct perf_sample_data *data,
-				struct pt_regs *regs)
+				struct pt_regs *regs,
+				struct bpf_prog *prog)
 {
 	struct bpf_perf_event_data_kern ctx = {
 		.data = data,
 		.event = event,
 	};
-	struct bpf_prog *prog;
 	int ret = 0;
 
 	ctx.regs = perf_arch_bpf_user_pt_regs(regs);
 	if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1))
 		goto out;
 	rcu_read_lock();
-	prog = READ_ONCE(event->prog);
 	if (prog) {
 		perf_prepare_sample(data, event, regs);
 		ret = bpf_prog_run(prog, &ctx);
@@ -10708,7 +10707,7 @@ static inline int perf_event_set_bpf_handler(struct perf_event *event,
 		return -EPROTO;
 	}
 
-	event->prog = prog;
+	WRITE_ONCE(event->prog, prog);
 	event->bpf_cookie = bpf_cookie;
 	return 0;
 }
@@ -10720,7 +10719,7 @@ static inline void perf_event_free_bpf_handler(struct perf_event *event)
 	if (!prog)
 		return;
 
-	event->prog = NULL;
+	WRITE_ONCE(event->prog, NULL);
 	bpf_prog_put(prog);
 }
 #else
@@ -10753,6 +10752,7 @@ static int __perf_event_overflow(struct perf_event *event,
 {
 	int events = atomic_read(&event->event_limit);
 	int ret = 0;
+	struct bpf_prog *prog;
 
 	/*
 	 * Non-sampling counters might still use the PMI to fold short
@@ -10766,8 +10766,9 @@ 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 &&
-	    !bpf_overflow_handler(event, data, regs))
+	prog = READ_ONCE(event->prog);
+	if (prog && prog->type == BPF_PROG_TYPE_PERF_EVENT &&
+	    !bpf_overflow_handler(event, data, regs, prog))
 		goto out;
 
 	/*
@@ -13433,12 +13434,15 @@ 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;
-
+		struct bpf_prog *prog;
+
+		mutex_lock(&bpf_event_mutex);
+		prog = parent_event->prog;
+		if (prog) {
 			bpf_prog_inc(prog);
 			event->prog = prog;
 		}
+		mutex_unlock(&bpf_event_mutex);
 #endif
 	}
 
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 75495a5c3507..026b5c492115 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1944,7 +1944,7 @@ const struct bpf_verifier_ops perf_event_verifier_ops = {
 const struct bpf_prog_ops perf_event_prog_ops = {
 };
 
-static DEFINE_MUTEX(bpf_event_mutex);
+DEFINE_MUTEX(bpf_event_mutex);
 
 #define BPF_TRACE_MAX_PROGS 64
 
@@ -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:
 	mutex_unlock(&bpf_event_mutex);
-- 
2.43.0

Re: [PATCH v2] perf/bpf: Fix data races in BPF perf event handling

[sashiko-bot]

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

New issues:
- [High] Unconditional use of `bpf_event_mutex` in `kernel/events/core.c` causes a linker error in valid Kconfig combinations where `CONFIG_BPF_EVENTS` is disabled.

Pre-existing issues:
- [High] The fix for the data race in `perf_event_alloc()` is incomplete because `bpf_event_mutex` is not held during the detachment of non-tracing BPF programs, leaving the use-after-free race unfixed.
- [High] Dereferencing prog->type without rcu_read_lock_trace() exposes sleepable BPF programs to Use-After-Free.
- [Medium] The patch misses a plain write to event->prog in perf_event_attach_bpf_prog(), leaving a data race with READ_ONCE(event->prog) unfixed.
- [Medium] `perf_event_free_bpf_handler()` still contains a plain read of `event->prog` which can race with a concurrent `WRITE_ONCE()` in `perf_event_set_bpf_handler()`.
- [Medium] Missing release barrier in perf_event_set_bpf_handler() allows BPF programs to observe uninitialized or stale bpf_cookie values.
--

--- Patch [1]: [PATCH v2] perf/bpf: Fix data races in BPF perf event handling ---
commit f4518a73c55a1297eca26073838ec515d34cb05a
Author: Deepanshu Kartikey <kartikey406@gmail.com>
perf/bpf: Fix data races in BPF perf event handling

This commit attempts to fix data races around the event->prog field
by introducing READ_ONCE and WRITE_ONCE in various handlers, and acquiring
bpf_event_mutex during fork() in perf_event_alloc().

Link: https://lore.kernel.org/all/20260811235331.10044-1-kartikey406@gmail.com/T/

> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 48d851fbd8ea5..5c6dabb6dccb1 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -2136,4 +2136,6 @@ static inline void perf_lopwr_cb(bool mode)
>  }
>  #endif
>  
> +extern struct mutex bpf_event_mutex;
> +

[Severity: High]
Will this extern declaration lead to a linker error in valid Kconfig
combinations where CONFIG_BPF_EVENTS is disabled?

The bpf_event_mutex is defined in kernel/trace/bpf_trace.c, which is
only compiled when CONFIG_BPF_EVENTS=y. If a kernel is configured with
CONFIG_BPF_SYSCALL=y and CONFIG_EVENT_TRACING=y, but kprobe and uprobe
events are disabled, this might result in an unresolved external symbol.

>  #endif /* _LINUX_PERF_EVENT_H */
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index ba5bd6a78fe7b..1416066ef460e 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c

[ ... ]

> @@ -10708,7 +10707,7 @@ static inline int perf_event_set_bpf_handler(struct perf_event *event,
>  		return -EPROTO;
>  	}
>  
> -	event->prog = prog;
> +	WRITE_ONCE(event->prog, prog);
>  	event->bpf_cookie = bpf_cookie;
>  	return 0;
>  }

[Severity: Medium]
This is a pre-existing issue, but does publishing event->prog without a
release barrier allow concurrent readers in __perf_event_overflow() to
observe an uninitialized or stale bpf_cookie?

Without smp_store_release() or similar ordering, the CPU can reorder the
event->prog write and the plain store to event->bpf_cookie. Since the reader
uses READ_ONCE(), which lacks acquire semantics, a BPF program might read
the uninitialized cookie.

> @@ -10720,7 +10719,7 @@ static inline void perf_event_free_bpf_handler(struct perf_event *event)
>  	if (!prog)
>  		return;
>  
> -	event->prog = NULL;
> +	WRITE_ONCE(event->prog, NULL);
>  	bpf_prog_put(prog);
>  }

[Severity: Medium]
This is a pre-existing issue, but does perf_event_free_bpf_handler() still
contain a plain read of event->prog?

At the beginning of this function:
	struct bpf_prog *prog = event->prog;

Could this read race with a concurrent WRITE_ONCE() from
perf_event_set_bpf_handler() during ioctl(PERF_EVENT_IOC_SET_BPF)?

[Severity: High]
This is a pre-existing issue, but does perf_event_free_bpf_handler() need to
acquire bpf_event_mutex to safely detach non-tracing BPF programs?

Because this function clears event->prog and drops the program reference
locklessly, a concurrent fork() calling perf_event_alloc() could acquire the
mutex, read the program, and increment a zero-refcount object.

>  #else
> @@ -10753,6 +10752,7 @@ static int __perf_event_overflow(struct perf_event *event,
>  {
>  	int events = atomic_read(&event->event_limit);
>  	int ret = 0;
> +	struct bpf_prog *prog;
>  
>  	/*
>  	 * Non-sampling counters might still use the PMI to fold short
> @@ -10766,8 +10766,9 @@ 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 &&
> -	    !bpf_overflow_handler(event, data, regs))
> +	prog = READ_ONCE(event->prog);
> +	if (prog && prog->type == BPF_PROG_TYPE_PERF_EVENT &&
> +	    !bpf_overflow_handler(event, data, regs, prog))

[Severity: High]
This is a pre-existing issue, but does dereferencing prog->type here
without rcu_read_lock_trace() expose sleepable BPF programs to a
use-after-free?

RCU Tasks Trace grace periods do not wait for NMIs or preemption-disabled
sections (Classic RCU context). If a sleepable program is detached and freed
via call_rcu_tasks_trace(), couldn't it be freed while __perf_event_overflow()
is still executing and evaluating prog->type?

>  		goto out;
>  
>  	/*
> @@ -13433,12 +13434,15 @@ 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;
>  
> +		struct bpf_prog *prog;
>  
> +		mutex_lock(&bpf_event_mutex);
> +		prog = parent_event->prog;
> +		if (prog) {
>  			bpf_prog_inc(prog);
>  			event->prog = prog;
>  		}
> +		mutex_unlock(&bpf_event_mutex);
>  #endif

[Severity: High]
As mentioned above, since bpf_event_mutex is only compiled when
CONFIG_BPF_EVENTS=y, will unconditionally locking it here cause a build
failure on kernels where that configuration is disabled?

>  	}
>  
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 75495a5c35079..026b5c4921158 100644
> --- 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:
>  	mutex_unlock(&bpf_event_mutex);

[Severity: Medium]
This isn't a bug introduced by this patch, but doesn't
perf_event_attach_bpf_prog() still contain a plain write to event->prog?

Since this patch fixes data races against the READ_ONCE() in the fast
path, should the plain write:
    event->prog = prog;

in the attach path also be converted to a WRITE_ONCE()?

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

Re: [PATCH v2] perf/bpf: Fix data races in BPF perf event handling

[bot+bpf-ci]

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

> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 48d851fbd8ea5..5c6dabb6dccb1 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -2136,4 +2136,6 @@ static inline void perf_lopwr_cb(bool mode)
>  }
>  #endif
>
> +extern struct mutex bpf_event_mutex;
> +
>  #endif /* _LINUX_PERF_EVENT_H */

bpf_event_mutex is defined in kernel/trace/bpf_trace.c, which is only
built when CONFIG_BPF_EVENTS=y:

  kernel/trace/Makefile: obj-$(CONFIG_BPF_EVENTS) += bpf_trace.o

CONFIG_BPF_EVENTS depends on BPF_SYSCALL and also requires either
KPROBE_EVENTS or UPROBE_EVENTS (kernel/trace/Kconfig:853-855). However,
the new reference in perf_event_alloc() is guarded by the weaker condition
'#if defined(CONFIG_BPF_SYSCALL) && defined(CONFIG_EVENT_TRACING)'.

CONFIG_EVENT_TRACING is selected by CONFIG_TRACING and has no dependency
on KPROBE_EVENTS or UPROBE_EVENTS. This means a config with
CONFIG_PERF_EVENTS=y, CONFIG_BPF_SYSCALL=y, CONFIG_TRACING=y,
CONFIG_KPROBES=n, and CONFIG_UPROBE_EVENTS=n will have CONFIG_BPF_EVENTS=n
and produce an undefined reference to bpf_event_mutex at link time.

Should the declaration be moved to include/linux/trace_events.h with
appropriate CONFIG_BPF_EVENTS guards, similar to how
perf_event_attach_bpf_prog() and perf_event_detach_bpf_prog() are
declared there?

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index ba5bd6a78fe7b..1416066ef460e 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -10652,20 +10652,19 @@ static inline bool sample_is_allowed(struct perf_event *event, struct pt_regs *r
>  #ifdef CONFIG_BPF_SYSCALL
>  static int bpf_overflow_handler(struct perf_event *event,
>  				struct perf_sample_data *data,
> -				struct pt_regs *regs)
> +				struct pt_regs *regs,
> +				struct bpf_prog *prog)
>  {
>  	struct bpf_perf_event_data_kern ctx = {
>  		.data = data,
>  		.event = event,
>  	};
> -	struct bpf_prog *prog;
>  	int ret = 0;
>
>  	ctx.regs = perf_arch_bpf_user_pt_regs(regs);
>  	if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1))
>  		goto out;
>  	rcu_read_lock();
> -	prog = READ_ONCE(event->prog);
>  	if (prog) {
>  		perf_prepare_sample(data, event, regs);
>  		ret = bpf_prog_run(prog, &ctx);

The function signature was updated to add the fourth parameter, but the
!CONFIG_BPF_SYSCALL stub at the end of this file was not updated:

  #else
  static inline int bpf_overflow_handler(struct perf_event *event,
                                         struct perf_sample_data *data,
                                         struct pt_regs *regs)
  {
      return 1;
  }

The call site at __perf_event_overflow() passes four arguments and is not
inside any CONFIG_BPF_SYSCALL guard. Since kernel/events/core.c is built
unconditionally for CONFIG_PERF_EVENTS=y, any CONFIG_PERF_EVENTS=y,
CONFIG_BPF_SYSCALL=n build will fail with "error: too many arguments to
function bpf_overflow_handler".

Should the stub also take the fourth 'struct bpf_prog *prog' parameter?

> @@ -10708,7 +10707,7 @@ static inline int perf_event_set_bpf_handler(struct perf_event *event,
>  		return -EPROTO;
>  	}
>
> -	event->prog = prog;
> +	WRITE_ONCE(event->prog, prog);
>  	event->bpf_cookie = bpf_cookie;
>  	return 0;
>  }
> @@ -10720,7 +10719,7 @@ static inline void perf_event_free_bpf_handler(struct perf_event *event)
>  	if (!prog)
>  		return;
>
> -	event->prog = NULL;
> +	WRITE_ONCE(event->prog, NULL);
>  	bpf_prog_put(prog);
>  }
>  #else
> @@ -10753,6 +10752,7 @@ static int __perf_event_overflow(struct perf_event *event,
>  {
>  	int events = atomic_read(&event->event_limit);
>  	int ret = 0;
> +	struct bpf_prog *prog;
>
>  	/*
>  	 * Non-sampling counters might still use the PMI to fold short
> @@ -10766,8 +10766,9 @@ 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 &&
> -	    !bpf_overflow_handler(event, data, regs))
> +	prog = READ_ONCE(event->prog);
> +	if (prog && prog->type == BPF_PROG_TYPE_PERF_EVENT &&
> +	    !bpf_overflow_handler(event, data, regs, prog))
>  		goto out;
>
>  	/*

[ ... ]

> @@ -13433,12 +13434,15 @@ 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;
> +		struct bpf_prog *prog;
>
> +		mutex_lock(&bpf_event_mutex);
> +		prog = parent_event->prog;
> +		if (prog) {
>  			bpf_prog_inc(prog);
>  			event->prog = prog;
>  		}
> +		mutex_unlock(&bpf_event_mutex);
>  #endif
>  	}
>

bpf_event_mutex is taken to serialize against concurrent detach during
fork, but does this actually protect against all writers?

event->prog has two sets of writers:
(a) tracing events: perf_event_attach_bpf_prog() and
    perf_event_detach_bpf_prog() in kernel/trace/bpf_trace.c - these hold
    bpf_event_mutex
(b) non-tracing events (BPF_PROG_TYPE_PERF_EVENT):
    perf_event_set_bpf_handler() and perf_event_free_bpf_handler() in
    kernel/events/core.c - these do not hold bpf_event_mutex

For case (b), the concurrent path remains unprotected:

  bpf_perf_link_release()  (kernel/bpf/syscall.c, bpf_link fd close)
    -> perf_event_free_bpf_prog()
       -> perf_event_free_bpf_handler()
            struct bpf_prog *prog = event->prog;
            WRITE_ONCE(event->prog, NULL);
            bpf_prog_put(prog);

versus the fork path here:

  perf_event_init_context()  (holds parent_ctx->mutex only)
    -> inherit_task_group -> inherit_group -> inherit_event()
       -> perf_event_alloc()
            mutex_lock(&bpf_event_mutex);
            prog = parent_event->prog;
            if (prog) bpf_prog_inc(prog);

Timeline:
  CPU0: mutex_lock(&bpf_event_mutex); prog = parent_event->prog  -> P
  CPU0: preempted
  CPU1: perf_event_free_bpf_handler(): WRITE_ONCE(event->prog, NULL);
        bpf_prog_put(P); -> refcnt -> 0, free deferred by call_rcu()
  CPU0: bpf_prog_inc(P) on an object already at zero and queued for
        freeing

bpf_perf_link_release() takes neither ctx->mutex nor bpf_event_mutex, so
the NULL-deref/use-after-free window remains open for overflow-handler
progs.

Should perf_event_free_bpf_handler() also take bpf_event_mutex, or should
the inherit be serialized differently (e.g., under the event's ctx->mutex)?

Also, the read stayed a plain load rather than READ_ONCE(), so it still
data-races with the WRITE_ONCE(event->prog, NULL) added in this same
patch.

> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 29260951aa871..e91676ae6cfb4 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1944,7 +1944,7 @@ const struct bpf_verifier_ops perf_event_verifier_ops = {
>  const struct bpf_prog_ops perf_event_prog_ops = {
>  };
>
> -static DEFINE_MUTEX(bpf_event_mutex);
> +DEFINE_MUTEX(bpf_event_mutex);
>
>  #define BPF_TRACE_MAX_PROGS 64

[ ... ]

> @@ -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:
>  	mutex_unlock(&bpf_event_mutex);

The commit message states "Fix by using WRITE_ONCE() in all write paths
including perf_event_detach_bpf_prog()", but the matching attach function
in the same file was left as a plain store:

  kernel/trace/bpf_trace.c perf_event_attach_bpf_prog():
      event->prog = prog;

perf_event_attach_bpf_prog() is a writer of the same field that
__perf_event_overflow() now reads with READ_ONCE(). The attach path can
be called on already-enabled tracepoint events that are concurrently
firing (via ioctl(PERF_EVENT_IOC_SET_BPF) or BPF_LINK_CREATE), so KCSAN
can still report 'data-race in __perf_event_overflow /
perf_event_attach_bpf_prog' (marked READ_ONCE vs plain write).

Similarly, several other plain reads remain:
- perf_event_free_bpf_handler(): 'struct bpf_prog *prog = event->prog;'
- perf_event_free_bpf_prog() read before dispatching
- perf_event_set_bpf_handler() checks before writing
- perf_event_detach_bpf_prog() reads before the WRITE_ONCE
- perf_event_alloc() read in the new mutex-protected section

Should the remaining plain writes be converted to WRITE_ONCE() and the
plain reads to READ_ONCE() to complete the annotation coverage?

---

The commit message describes the changes and their intent:

>     Fix multiple data races involving event->prog field:
>     1. __perf_event_overflow() reads event->prog twice without
>        synchronization creating a TOCTOU race. Fix by using READ_ONCE()
>        to capture prog into a local variable and pass it directly to
>        bpf_overflow_handler() to avoid a second read inside that function.

The fix does eliminate the double-read and the dropped-sample bug. However,
moving the READ_ONCE() out of bpf_overflow_handler() also moves it outside
the rcu_read_lock() that previously ensured the prog couldn't be freed
before bpf_prog_run().

Pre-patch: bpf_overflow_handler() did 'rcu_read_lock(); prog =
READ_ONCE(event->prog);' and then used that pointer. Since
perf_event_free_bpf_handler() stores NULL before bpf_prog_put(), and
freeing is RCU-deferred (call_rcu() in __bpf_prog_put_noref()), observing
a non-NULL prog inside the RCU section guaranteed the object couldn't be
freed before rcu_read_unlock().

Post-patch: the pointer is loaded in __perf_event_overflow() before
rcu_read_lock(), so in principle bpf_prog_run() could be handed an
already-freed prog if the grace period ends between the READ_ONCE() and
the rcu_read_lock().

Looking at the call paths, __perf_event_overflow() is reached with
preemption and IRQs disabled (perf_event_overflow() asserts
lockdep_assert_irqs_disabled()), which is an implicit RCU read-side
critical section covering the whole call, so the object is protected.

Should the commit message mention this trade-off, or should the code
document why the moved load is still safe despite being outside the
explicit rcu_read_lock()?

>     2. perf_event_set_bpf_handler() and perf_event_free_bpf_handler()
>        perform plain writes to event->prog without WRITE_ONCE(), failing
>        to pair with the READ_ONCE() in __perf_event_overflow().

Before this patch there was no READ_ONCE() in __perf_event_overflow() at
all - the only READ_ONCE(event->prog) was in bpf_overflow_handler(), and
this patch deletes it. The sentence describes the pre-existing code as
failing to pair with something the same patch introduces.

The substantive point (an unpaired marked read existed) is accurate, but
it was in bpf_overflow_handler(), not __perf_event_overflow().

Should this be clarified to say the plain writes failed to pair with the
READ_ONCE() that was in bpf_overflow_handler()?

>     3. perf_event_alloc() reads parent_event->prog locklessly during
>        fork() which can race with a concurrent detach clearing and freeing
>        the prog, potentially causing a NULL pointer dereference or
>        use-after-free in bpf_prog_inc(). Fix by holding bpf_event_mutex
>        when inheriting the BPF program.

As noted above, bpf_event_mutex is only held by the tracing event
attach/detach paths in kernel/trace/bpf_trace.c. It is not taken by
perf_event_free_bpf_handler(), which is the detach path for non-tracing
events (BPF_PROG_TYPE_PERF_EVENT).

perf_event_free_bpf_prog() dispatches based on perf_event_is_tracing()
and routes BPF_PROG_TYPE_PERF_EVENT progs to perf_event_free_bpf_handler(),
which does 'WRITE_ONCE(event->prog, NULL); bpf_prog_put(prog);' with no
lock at all.

A BPF_PROG_TYPE_PERF_EVENT prog can be attached to an inheritable sampling
event, and the inherit block in perf_event_alloc() is not restricted by
event type. For that case the NULL-deref/refcount-resurrection race
described above remains open.

Holding bpf_event_mutex does correctly close the race against
perf_event_detach_bpf_prog() (which clears event->prog under the mutex and
only calls bpf_prog_put() after unlocking).

Should the commit message clarify that the fix is partial and only covers
tracing events, or should perf_event_free_bpf_handler() also take
bpf_event_mutex?

>     Make bpf_event_mutex non-static and declare it extern in perf_event.h
>     so it is accessible from kernel/events/core.c.

The declaration is added to include/linux/perf_event.h outside any CONFIG
guard, but bpf_event_mutex is only defined when CONFIG_BPF_EVENTS=y
(kernel/trace/bpf_trace.o is only built for CONFIG_BPF_EVENTS).

The new mutex_lock/unlock reference in kernel/events/core.c is guarded by
'#if defined(CONFIG_BPF_SYSCALL) && defined(CONFIG_EVENT_TRACING)', which
is strictly weaker than CONFIG_BPF_EVENTS (CONFIG_BPF_EVENTS additionally
requires KPROBE_EVENTS || UPROBE_EVENTS). A valid config with
CONFIG_BPF_SYSCALL=y, CONFIG_TRACING=y, CONFIG_KPROBES=n, and
CONFIG_UPROBE_EVENTS=n yields CONFIG_BPF_EVENTS=n and an undefined
reference at link time.

This creates both a build failure and a layering concern (exposing a
bpf_trace.c-internal mutex through the global perf_event.h). Other
bpf_trace.c functions (perf_event_attach_bpf_prog,
perf_event_detach_bpf_prog, perf_event_query_prog_array) are declared in
include/linux/trace_events.h under CONFIG_BPF_EVENTS with !BPF_EVENTS
stubs.

Should bpf_event_mutex follow the same pattern?

---

This commit fixes data races but lacks a Fixes: tag. The bugs were
introduced when BPF perf event support was first added. Suggested:

  Fixes: aa6a5f3cb2b2 ("perf, bpf: add perf events core support for BPF_PROG_TYPE_PERF_EVENT programs")


---
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/31673322805

Re: [PATCH v2] perf/bpf: Fix data races in BPF perf event handling

[Peter Zijlstra]

On Thu, Aug 13, 2026 at 11:35:28AM +0530, Deepanshu Kartikey wrote:
> Fix multiple data races involving event->prog field:
> 
> 1. __perf_event_overflow() reads event->prog twice without
>    synchronization creating a TOCTOU race. Fix by using READ_ONCE()
>    to capture prog into a local variable and pass it directly to
>    bpf_overflow_handler() to avoid a second read inside that function.

Well, first you have to show there is concurrency where this matters,
since I still don't believe in your next point.

Also, there's actually worse issues when you consider ->prog and
->bpf_cookie form a pair.

> 2. perf_event_set_bpf_handler() and perf_event_free_bpf_handler()
>    perform plain writes to event->prog without WRITE_ONCE(), failing
>    to pair with the READ_ONCE() in __perf_event_overflow(). Fix by
>    using WRITE_ONCE() in all write paths including
>    perf_event_detach_bpf_prog().

As I said yesterday, how can perf_event_detach_bpf_prog() run
concurrently with __perf_event_overflow()? Unless you answer that, this
patch ain't moving nowhere.

> 3. perf_event_alloc() reads parent_event->prog locklessly during
>    fork() which can race with a concurrent detach clearing and freeing
>    the prog, potentially causing a NULL pointer dereference or
>    use-after-free in bpf_prog_inc(). Fix by holding bpf_event_mutex
>    when inheriting the BPF program. Make bpf_event_mutex non-static
>    and declare it extern in perf_event.h so it is accessible from
>    kernel/events/core.c.

This seems like a separate issue and should thus be a separate patch.