[PATCH] perf/core: Fix double put of the system-wide perf_ctx_data reference

[PATCH] perf/core: Fix double put of the system-wide perf_ctx_data reference

[vineash]

The reference on a task's perf_ctx_data that belongs to the system-wide
events (cd->global) can be put twice. __detach_global_ctx_data() puts it
when the last system-wide event goes away, and perf_event_exit_task()
puts it again when the task exits. perf_event_exit_task() runs without
global_ctx_data_rwsem and never clears cd->global, so nothing stops both
paths from putting the same reference:

  CPU0                               CPU1 (task T exiting)
  __detach_global_ctx_data()
    cd->global = 0;
    detach_task_ctx_data(T)
      refcount_dec_and_test() == true
                                     perf_event_exit_task(T)
                                       detach_task_ctx_data(T)
                                         cd = T->perf_ctx_data;
                                         // still set: underflow
                                         refcount_dec_and_test()
      try_cmpxchg(&T->perf_ctx_data,
                  &cd, NULL)

This shows up as:

  refcount_t: underflow; use-after-free.
  WARNING: lib/refcount.c:28 at refcount_warn_saturate+0xc9/0x120
   detach_task_ctx_data+0x1db/0x4a0
   do_exit+0x6a9/0x2bb0
   do_group_exit+0xd3/0x2a0
   get_signal+0x266a/0x26d0

When the exit path puts first, the warning comes from
__detach_global_ctx_data(), via perf_release() or via the error path of
perf_event_open().

An unprivileged user can open this window with the default
perf_event_paranoid=2. For a CPU-wide event that requests a branch call
stack, perf_event_alloc() calls attach_perf_ctx_data() before
find_get_context() rejects the event with -EACCES, and the error path
then runs detach_global_ctx_data().

Treat the global reference as a one-shot token. Claim cd->global with
xchg() in both __detach_global_ctx_data() and perf_event_exit_task(), and
put the reference only if the claim succeeded. Also take the reference
before setting cd->global in attach_global_ctx_data() and
perf_event_alloc_task_data(). Otherwise the exit path could put a
reference that has not been taken yet.

perf_event_exit_task() no longer puts the reference of a non-global
perf_ctx_data. That reference belongs to the per-task events, and they
put it when they are freed.

Fixes: 506e64e710ff ("perf: attach/detach PMU specific data")
Assisted-by: Claude:claude-opus-5-5 syzkaller
Signed-off-by: vineash <vineash30640@gmail.com>
---
Notes (not for the commit log):

Found with syzkaller in a QEMU/KVM guest with "-cpu host" (Intel Xeon Gold
5118 host). The guest has a vPMU but no LBR. The trigger works without LBR:
for a counting event with PERF_SAMPLE_READ, intel_pmu_hw_config() skips the
LBR setup, but x86_pmu_hw_config() has already set PERF_ATTACH_TASK_DATA
because of PERF_SAMPLE_BRANCH_CALL_STACK.

Tested on next-20261002 (x86_64, syzbot's upstream-apparmor-kasan config):
- without this patch: the syzkaller reproducer (root) hit the underflow
  in 2 of 2 runs (after 103 s and 288 s). A small reproducer running as
  uid 65534 with perf_event_paranoid=2 hit it in 2 of 3 runs (after 333 s
  and 376 s). The third run lasted 900 s and made ~2M perf_event_open()
  calls without a hit.
- with a debug patch that records the last decrement, the second put came
  from do_exit() 0.49 ms after __detach_global_ctx_data() on another
  task had put the same reference (old refcount 1).
- with this patch: no warning in 7 runs per reproducer (3 x 15 min and
  4 x 20 min). The unprivileged runs made 0.84-0.93M rejected
  perf_event_open() calls each, ~6.2M in total.
- not tested: hardware with real LBR call stacks, and kmemleak.

Unrelated to the fix, but this is what opens the window for unprivileged
users: attach_global_ctx_data() takes global_ctx_data_rwsem for write and
allocates perf_ctx_data for every thread in the system before the
perf_allow_cpu() check in find_get_context() rejects the event. It may be
worth doing that check before perf_event_alloc() for CPU-wide events.

A C reproducer is available on request.

 kernel/events/core.c | 39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index a34ff4cb4..63122c3b8 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5498,8 +5498,9 @@ attach_global_ctx_data(struct kmem_cache *ctx_cache)
 				continue;
 			cd = rcu_dereference(p->perf_ctx_data);
 			if (cd && !cd->global) {
-				cd->global = 1;
-				if (!refcount_inc_not_zero(&cd->refcount))
+				if (refcount_inc_not_zero(&cd->refcount))
+					cd->global = 1;
+				else
 					cd = NULL;
 			}
 			if (!cd) {
@@ -5569,19 +5570,33 @@ detach_task_ctx_data(struct task_struct *p)
 		perf_free_ctx_data_rcu(cd);
 }
 
+/*
+ * Drop the reference owned by the system-wide events, if @p still holds one.
+ *
+ * Both __detach_global_ctx_data() and perf_event_exit_task() may try to drop
+ * it concurrently; claiming ->global with xchg() makes sure only one of them
+ * does, otherwise the refcount is decremented twice.
+ */
+static void detach_task_global_ctx_data(struct task_struct *p)
+{
+	struct perf_ctx_data *cd;
+
+	scoped_guard (rcu) {
+		cd = rcu_dereference(p->perf_ctx_data);
+		if (!cd || !xchg(&cd->global, 0))
+			return;
+	}
+
+	detach_task_ctx_data(p);
+}
+
 static void __detach_global_ctx_data(void)
 {
 	struct task_struct *g, *p;
-	struct perf_ctx_data *cd;
 
 	scoped_guard (rcu) {
-		for_each_process_thread(g, p) {
-			cd = rcu_dereference(p->perf_ctx_data);
-			if (cd && cd->global) {
-				cd->global = 0;
-				detach_task_ctx_data(p);
-			}
-		}
+		for_each_process_thread(g, p)
+			detach_task_global_ctx_data(p);
 	}
 }
 
@@ -9459,8 +9474,8 @@ perf_event_alloc_task_data(struct task_struct *child,
 		}
 
 		if (!cd->global) {
-			cd->global = 1;
 			refcount_inc(&cd->refcount);
+			cd->global = 1;
 		}
 	}
 
@@ -14901,7 +14916,7 @@ void perf_event_exit_task(struct task_struct *task)
 	 * attach_global_ctx_data() will skip over this task, but otherwise
 	 * attach_task_ctx_data() will observe PF_EXITING.
 	 */
-	detach_task_ctx_data(task);
+	detach_task_global_ctx_data(task);
 }
 
 /*
-- 
2.25.1

Re: [PATCH] perf/core: Fix double put of the system-wide perf_ctx_data reference

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] perf/core: memory leak of perf_ctx_data due to TOCTOU race in attach_global_ctx_data()
- [Medium] perf/core: data race on cd->global leading to undefined behavior
--

commit 6c40093668f0aa28205d435b7cecaca312682e03
Author: vineash <vineash30640@gmail.com>

perf/core: Fix double put of the system-wide perf_ctx_data reference

The reference on a task's perf_ctx_data that belongs to the system-wide
events (cd->global) can be put twice. __detach_global_ctx_data() puts it
when the last system-wide event goes away, and perf_event_exit_task()
puts it again when the task exits.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index a34ff4cb410da..63122c3b872ef 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -5498,8 +5498,9 @@ attach_global_ctx_data(struct kmem_cache *ctx_cache)
>  				continue;
>  			cd = rcu_dereference(p->perf_ctx_data);
>  			if (cd && !cd->global) {
> -				cd->global = 1;
> -				if (!refcount_inc_not_zero(&cd->refcount))
> +				if (refcount_inc_not_zero(&cd->refcount))
> +					cd->global = 1;

[Severity: High]
Does this introduce a TOCTOU race condition that can leak the perf_ctx_data
reference?

Earlier in attach_global_ctx_data(), the code checks if the task is exiting
without holding locks that exclude do_exit():

    if (p->flags & PF_EXITING)
        continue;

Could a task concurrently set PF_EXITING and call perf_event_exit_task(),
which then calls detach_task_global_ctx_data()?

If so, the exiting task would observe cd->global == 0 via xchg(), miss the
cleanup, and return. Then this code continues, increments cd->refcount, and
sets cd->global = 1. Because the exiting task has already bypassed the
cleanup phase, it will never drop this reference. Once the task is unhashed,
__detach_global_ctx_data() will not find it, resulting in a permanent
memory leak of the perf_ctx_data structure.

A local user with perf_event_paranoid=2 might trigger this by racing
CPU-wide perf event creation with task exits.

[Severity: Medium]
Does this plain store to cd->global race with the atomic xchg() operation in
detach_task_global_ctx_data() from an exiting task?

Because perf_event_exit_task() executes without holding global_ctx_data_rwsem,
these accesses appear unsynchronized, which violates the Linux Kernel Memory
Model and can lead to undefined behavior.

> +				else
>  					cd = NULL;
>  			}
>  			if (!cd) {

[ ... ]

> @@ -5569,19 +5570,33 @@ detach_task_ctx_data(struct task_struct *p)
>  		perf_free_ctx_data_rcu(cd);
>  }
>  
> +/*
> + * Drop the reference owned by the system-wide events, if @p still holds one.
> + *
> + * Both __detach_global_ctx_data() and perf_event_exit_task() may try to drop
> + * it concurrently; claiming ->global with xchg() makes sure only one of them
> + * does, otherwise the refcount is decremented twice.
> + */
> +static void detach_task_global_ctx_data(struct task_struct *p)
> +{
> +	struct perf_ctx_data *cd;
> +
> +	scoped_guard (rcu) {
> +		cd = rcu_dereference(p->perf_ctx_data);
> +		if (!cd || !xchg(&cd->global, 0))
> +			return;
> +	}
> +
> +	detach_task_ctx_data(p);
> +}

[ ... ]

> @@ -9459,8 +9474,8 @@ perf_event_alloc_task_data(struct task_struct *child,
>  		}
>  
>  		if (!cd->global) {
> -			cd->global = 1;
>  			refcount_inc(&cd->refcount);
> +			cd->global = 1;

[Severity: Medium]
Similar to the concern in attach_global_ctx_data(), does this plain store
to cd->global race with the atomic xchg() operation from an exiting task in
detach_task_global_ctx_data()?

>  		}
>  	}
>  

[ ... ]

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