[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@xxxxxxxxx>
---
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