[PATCH] perf/core: Prevent duplicate sigtrap task work enqueue

[PATCH] perf/core: Prevent duplicate sigtrap task work enqueue

[Tengda Wu]

The sigtrap handling in __perf_event_overflow() checks
event->pending_work before calling task_work_add(), but only sets
pending_work after the task work has been queued.

This leaves a window where an NMI can interrupt task_work_add()
and re-enter __perf_event_overflow() for the same event. Since
pending_work is still zero, the nested invocation can enqueue
the same pending_task again.

The re-entrant call path is:

  __perf_event_overflow()
    task_work_add()
      kasan_record_aux_stack()
        <NMI>
          __perf_event_overflow()
            task_work_add()

The nested invocation can enqueue pending_task before the
outer invocation does. When the outer invocation resumes, it
inserts the same task_work entry again, potentially creating
a cycle in the task work list.

This causes perf_pending_task() to be executed repeatedly
and leads to unbalanced event reference counting.

Use cmpxchg() to atomically claim pending_work before calling
task_work_add(). This ensures that only one invocation can
enqueue the pending_task, even when interrupted by an NMI.

If task_work_add() fails, clear the pending_work claim so that
a subsequent overflow can retry.

Also use READ_ONCE() when checking pending_work in the
exclude_kernel warning path to avoid an unsynchronized read.

Fixes: c5d93d23a260 ("perf: Enqueue SIGTRAP always via task_work.")
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
 kernel/events/core.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 457d75ea7142..f1e4e153c49a 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10925,16 +10925,18 @@ static int __perf_event_overflow(struct perf_event *event,
 
 		notify_mode = in_nmi() ? TWA_NMI_CURRENT : TWA_RESUME;
 
-		if (!event->pending_work &&
-		    !task_work_add(current, &event->pending_task, notify_mode)) {
-			event->pending_work = pending_id;
-			local_inc(&event->ctx->nr_no_switch_fast);
-			WARN_ON_ONCE(!atomic_long_inc_not_zero(&event->refcount));
-
-			event->pending_addr = 0;
-			if (valid_sample && (data->sample_flags & PERF_SAMPLE_ADDR))
-				event->pending_addr = data->addr;
-
+		if (cmpxchg(&event->pending_work, 0, pending_id) == 0) {
+			if (!task_work_add(current, &event->pending_task, notify_mode)) {
+				event->pending_work = pending_id;
+				local_inc(&event->ctx->nr_no_switch_fast);
+				WARN_ON_ONCE(!atomic_long_inc_not_zero(&event->refcount));
+
+				event->pending_addr = 0;
+				if (valid_sample && (data->sample_flags & PERF_SAMPLE_ADDR))
+					event->pending_addr = data->addr;
+			} else {
+				cmpxchg(&event->pending_work, pending_id, 0);
+			}
 		} else if (event->attr.exclude_kernel && valid_sample) {
 			/*
 			 * Should not be able to return to user space without
@@ -10948,7 +10950,7 @@ static int __perf_event_overflow(struct perf_event *event,
 			 *     To approximate progress (with false negatives),
 			 *     check 32-bit hash of the current IP.
 			 */
-			WARN_ON_ONCE(event->pending_work != pending_id);
+			WARN_ON_ONCE(READ_ONCE(event->pending_work) != pending_id);
 		}
 	}
 
-- 
2.34.1

Re: [PATCH] perf/core: Prevent duplicate sigtrap task work enqueue

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] perf/core: redundant store to event->pending_work in __perf_event_overflow()
--

commit badcfdb790f5048e93691fc4bd20053f8cc9e2d2
Author: Tengda Wu <wutengda@huaweicloud.com>

perf/core: Prevent duplicate sigtrap task work enqueue

This commit uses cmpxchg() to atomically claim event->pending_work before
calling task_work_add() in __perf_event_overflow() so that a nested NMI
cannot enqueue the same sigtrap pending_task twice.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 457d75ea71420..f1e4e153c49a4 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -10925,16 +10925,18 @@ static int __perf_event_overflow(struct perf_event *event,
>  
>  		notify_mode = in_nmi() ? TWA_NMI_CURRENT : TWA_RESUME;
>  
> -		if (!event->pending_work &&
> -		    !task_work_add(current, &event->pending_task, notify_mode)) {
> -			event->pending_work = pending_id;
> -			local_inc(&event->ctx->nr_no_switch_fast);
> -			WARN_ON_ONCE(!atomic_long_inc_not_zero(&event->refcount));
> -
> -			event->pending_addr = 0;
> -			if (valid_sample && (data->sample_flags & PERF_SAMPLE_ADDR))
> -				event->pending_addr = data->addr;
> -
> +		if (cmpxchg(&event->pending_work, 0, pending_id) == 0) {
> +			if (!task_work_add(current, &event->pending_task, notify_mode)) {
> +				event->pending_work = pending_id;

[Severity: Low]
Is this store to event->pending_work in __perf_event_overflow() in
kernel/events/core.c still needed when task_work_add() succeeds?

The outer cmpxchg(&event->pending_work, 0, pending_id) == 0 check already
sets event->pending_work to pending_id before calling task_work_add(), and
nothing can clear or modify event->pending_work while inside this branch
(interrupts are disabled, perf_pending_task() cannot run until return to
user space or task exit, and any nested NMI sees a non-zero value in
cmpxchg()).

Leaving this assignment also performs an unmarked plain write to
event->pending_work, whereas the rest of this change accesses it via
cmpxchg() and READ_ONCE(). Can this redundant assignment be dropped?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261031114520.642476-1-wutengda@huaweicloud.com?part=1