All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf/core: Close owner-list publication race with remove-on-exec
@ 2026-10-15 12:34 Daehyeon Ko
  2026-10-15 12:45 ` sashiko-bot
  2026-10-16  8:45 ` Peter Zijlstra
  0 siblings, 2 replies; 5+ messages in thread
From: Daehyeon Ko @ 2026-10-15 12:34 UTC (permalink / raw)
  To: peterz, mingo, acme, namhyung
  Cc: mark.rutland, alexander.shishkin, jolsa, irogers, adrian.hunter,
	james.clark, elver, linux-perf-users, linux-kernel, stable

perf_event_open() makes a user event visible in the target context before
adding owner_entry to the opener's perf_event_list.  For a remote task, it
also releases exec_update_lock before adding the list node.

A concurrent target exec can therefore process a remove_on_exec event in
this interval.  perf_remove_from_owner() sees the published non-NULL owner
and calls list_del_init() on the zero-allocated, but not initialized,
owner_entry.  DEBUG_LIST reports a NULL-link list corruption and can panic
the kernel.

If list corruption reporting is non-fatal, list_del_init() initializes the
node and clears the owner.  The open path then adds the ownerless event to
the opener's list.  Closing the returned file descriptor frees the event
while it remains linked, and the next owner-list insertion accesses the
freed object.

Initialize owner_entry when the event is allocated.  After taking the
opener's perf_event_mutex, add the node only if the exec side has not
cleared the owner.  The existing mutex then makes both orders safe: the
open side either publishes the list node before exec removes it, or skips
the late add after exec clears the owner.

An unprivileged remote-task reproducer triggered the NULL-link panic on an
unmodified hardened kernel.  With corruption reporting made non-fatal, the
same reproducer triggered a KASAN use-after-free in the next owner-list
insertion.  The patched hardened kernel completed 1,264,540 remote opens
during 60 seconds of concurrent exec without a list diagnostic or panic.

Fixes: 2e498d0a74e5 ("perf: Add support for event removal on exec")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
---
 kernel/events/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 7846d70be57f6c..5e191757a08ccc 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -13382,6 +13382,7 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
 	INIT_LIST_HEAD(&event->rb_entry);
 	INIT_LIST_HEAD(&event->active_entry);
 	INIT_LIST_HEAD(&event->addr_filters.list);
+	INIT_LIST_HEAD(&event->owner_entry);
 	INIT_HLIST_NODE(&event->hlist_entry);
 	INIT_LIST_HEAD(&event->pmu_list);
 
@@ -14252,7 +14253,8 @@ SYSCALL_DEFINE5(perf_event_open,
 	}
 
 	mutex_lock(&current->perf_event_mutex);
-	list_add_tail(&event->owner_entry, &current->perf_event_list);
+	if (event->owner)
+		list_add_tail(&event->owner_entry, &current->perf_event_list);
 	mutex_unlock(&current->perf_event_mutex);
 
 	/*

base-commit: 0fcb05b72344f40806cbe810de8872f1698bf053

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf/core: Close owner-list publication race with remove-on-exec
  2026-10-15 12:34 [PATCH] perf/core: Close owner-list publication race with remove-on-exec Daehyeon Ko
@ 2026-10-15 12:45 ` sashiko-bot
  2026-10-16  8:45 ` Peter Zijlstra
  1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-10-15 12:45 UTC (permalink / raw)
  To: Daehyeon Ko; +Cc: linux-perf-users

> perf_event_open() makes a user event visible in the target context before
> adding owner_entry to the opener's perf_event_list.  For a remote task, it
> also releases exec_update_lock before adding the list node.
> 
> A concurrent target exec can therefore process a remove_on_exec event in
> this interval.  perf_remove_from_owner() sees the published non-NULL owner
> and calls list_del_init() on the zero-allocated, but not initialized,
> owner_entry.  DEBUG_LIST reports a NULL-link list corruption and can panic
> the kernel.
> 
> If list corruption reporting is non-fatal, list_del_init() initializes the
> node and clears the owner.  The open path then adds the ownerless event to
> the opener's list.  Closing the returned file descriptor frees the event
> while it remains linked, and the next owner-list insertion accesses the
> freed object.
> [ ... ]
> 
> Fixes: 2e498d0a74e5 ("perf: Add support for event removal on exec")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] perf/core: Close owner-list publication race with remove-on-exec
  2026-10-15 12:34 [PATCH] perf/core: Close owner-list publication race with remove-on-exec Daehyeon Ko
  2026-10-15 12:45 ` sashiko-bot
@ 2026-10-16  8:45 ` Peter Zijlstra
  2026-10-17 15:07   ` [PATCH v2] " Daehyeon Ko
  1 sibling, 1 reply; 5+ messages in thread
From: Peter Zijlstra @ 2026-10-16  8:45 UTC (permalink / raw)
  To: Daehyeon Ko
  Cc: mingo, acme, namhyung, mark.rutland, alexander.shishkin, jolsa,
	irogers, adrian.hunter, james.clark, elver, linux-perf-users,
	linux-kernel, stable

On Thu, Oct 15, 2026 at 09:34:16PM +0900, Daehyeon Ko wrote:
> perf_event_open() makes a user event visible in the target context before
> adding owner_entry to the opener's perf_event_list.  For a remote task, it
> also releases exec_update_lock before adding the list node.
> 
> A concurrent target exec can therefore process a remove_on_exec event in
> this interval.  perf_remove_from_owner() sees the published non-NULL owner
> and calls list_del_init() on the zero-allocated, but not initialized,
> owner_entry.  DEBUG_LIST reports a NULL-link list corruption and can panic
> the kernel.
> 
> If list corruption reporting is non-fatal, list_del_init() initializes the
> node and clears the owner.  The open path then adds the ownerless event to
> the opener's list.  Closing the returned file descriptor frees the event
> while it remains linked, and the next owner-list insertion accesses the
> freed object.
> 
> Initialize owner_entry when the event is allocated.  After taking the
> opener's perf_event_mutex, add the node only if the exec side has not
> cleared the owner.  The existing mutex then makes both orders safe: the
> open side either publishes the list node before exec removes it, or skips
> the late add after exec clears the owner.
> 
> An unprivileged remote-task reproducer triggered the NULL-link panic on an
> unmodified hardened kernel.  With corruption reporting made non-fatal, the
> same reproducer triggered a KASAN use-after-free in the next owner-list
> insertion.  The patched hardened kernel completed 1,264,540 remote opens
> during 60 seconds of concurrent exec without a list diagnostic or panic.
> 
> Fixes: 2e498d0a74e5 ("perf: Add support for event removal on exec")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
> ---
>  kernel/events/core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 7846d70be57f6c..5e191757a08ccc 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -13382,6 +13382,7 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
>  	INIT_LIST_HEAD(&event->rb_entry);
>  	INIT_LIST_HEAD(&event->active_entry);
>  	INIT_LIST_HEAD(&event->addr_filters.list);
> +	INIT_LIST_HEAD(&event->owner_entry);
>  	INIT_HLIST_NODE(&event->hlist_entry);
>  	INIT_LIST_HEAD(&event->pmu_list);
>  
> @@ -14252,7 +14253,8 @@ SYSCALL_DEFINE5(perf_event_open,
>  	}
>  
>  	mutex_lock(&current->perf_event_mutex);
> -	list_add_tail(&event->owner_entry, &current->perf_event_list);
> +	if (event->owner)
> +		list_add_tail(&event->owner_entry, &current->perf_event_list);
>  	mutex_unlock(&current->perf_event_mutex);


So per the locking rules (in the comment above
perf_event_ctx_lock_nested()), we can take perf_event_mutex inside
exec_update_lock.

Would not moving this list_add into that section also cure the problem
and with less races?

That is..

---
diff --git a/kernel/events/core.c b/kernel/events/core.c
index db7b76d6b68a..4e8508194ee8 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -14286,15 +14286,15 @@ SYSCALL_DEFINE5(perf_event_open,
 
 	mutex_unlock(&ctx->mutex);
 
+	mutex_lock(&current->perf_event_mutex);
+	list_add_tail(&event->owner_entry, &current->perf_event_list);
+	mutex_unlock(&current->perf_event_mutex);
+
 	if (task) {
 		up_read(&task->signal->exec_update_lock);
 		put_task_struct(task);
 	}
 
-	mutex_lock(&current->perf_event_mutex);
-	list_add_tail(&event->owner_entry, &current->perf_event_list);
-	mutex_unlock(&current->perf_event_mutex);
-
 	/*
 	 * File reference in group guarantees that group_leader has been
 	 * kept alive until we place the new event on the sibling_list.

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2] perf/core: Close owner-list publication race with remove-on-exec
  2026-10-16  8:45 ` Peter Zijlstra
@ 2026-10-17 15:07   ` Daehyeon Ko
  2026-10-17 15:25     ` sashiko-bot
  0 siblings, 1 reply; 5+ messages in thread
From: Daehyeon Ko @ 2026-10-17 15:07 UTC (permalink / raw)
  To: peterz, mingo, acme, namhyung
  Cc: mark.rutland, alexander.shishkin, jolsa, irogers, adrian.hunter,
	james.clark, elver, linux-perf-users, linux-kernel

perf_event_open() makes a user event visible in the target context before
adding owner_entry to the opener's perf_event_list. For a remote task, it
also releases exec_update_lock before adding the list node.

A concurrent target exec can therefore process a remove_on_exec event in
this interval. perf_remove_from_owner() sees the published non-NULL owner
and calls list_del_init() on the zero-allocated owner_entry, triggering a
list corruption diagnostic. With BUG_ON_DATA_CORRUPTION, this panics the
kernel.

If the diagnostic is non-fatal, the open path subsequently links the
ownerless event. Closing its file descriptor then frees an event that
remains on the owner list, leading to a use-after-free on the next list
insertion.

Keep the target exec_update_lock held until owner_entry is linked. The
documented lock order permits exec_update_lock to nest outside
perf_event_mutex, and target exec can no longer observe the partially
published event.

An unprivileged remote-task reproducer triggered the NULL-link panic on an
unmodified hardened kernel. With corruption reporting made non-fatal, it
reached a KASAN use-after-free in the next owner-list insertion. With this
change, the patched hardened kernel completed 1,229,779 remote opens during
60 seconds of concurrent exec without a list diagnostic, oops, or panic.

Fixes: 2e498d0a74e5 ("perf: Add support for event removal on exec")
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
---
Changes in v2:
- Keep exec_update_lock held through the existing owner-list insertion
  instead of repairing the race after partial publication, as suggested by
  Peter Zijlstra.
- Rebase onto current mainline commit 58161ead29e5.

 kernel/events/core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 7846d70be57f6..2fdd8d71dcd55 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -14246,15 +14246,15 @@ SYSCALL_DEFINE5(perf_event_open,
 
 	mutex_unlock(&ctx->mutex);
 
+	mutex_lock(&current->perf_event_mutex);
+	list_add_tail(&event->owner_entry, &current->perf_event_list);
+	mutex_unlock(&current->perf_event_mutex);
+
 	if (task) {
 		up_read(&task->signal->exec_update_lock);
 		put_task_struct(task);
 	}
 
-	mutex_lock(&current->perf_event_mutex);
-	list_add_tail(&event->owner_entry, &current->perf_event_list);
-	mutex_unlock(&current->perf_event_mutex);
-
 	/*
 	 * File reference in group guarantees that group_leader has been
 	 * kept alive until we place the new event on the sibling_list.

base-commit: 58161ead29e50d70a317ea26b472e0c7508e69da
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] perf/core: Close owner-list publication race with remove-on-exec
  2026-10-17 15:07   ` [PATCH v2] " Daehyeon Ko
@ 2026-10-17 15:25     ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-10-17 15:25 UTC (permalink / raw)
  To: Daehyeon Ko; +Cc: linux-perf-users

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] perf/core: unpinning ctx before owner_entry link allows fork/swap exec race
--

commit 32e21b1610ca28e8fe922d31bdbd5f0a4a31607c
Author: Daehyeon Ko <4ncienth@gmail.com>

perf/core: Close owner-list publication race with remove-on-exec

This commit keeps the target task's exec_update_lock held in perf_event_open()
until event->owner_entry is added to current->perf_event_list, preventing a
concurrent target exec from running perf_remove_from_owner() on an unlinked
owner_entry.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 7846d70be57f6..2fdd8d71dcd55 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -14246,15 +14246,15 @@ SYSCALL_DEFINE5(perf_event_open,
>  
>  	mutex_unlock(&ctx->mutex);
>  
> +	mutex_lock(&current->perf_event_mutex);
> +	list_add_tail(&event->owner_entry, &current->perf_event_list);
> +	mutex_unlock(&current->perf_event_mutex);
> +
>  	if (task) {
>  		up_read(&task->signal->exec_update_lock);
>  		put_task_struct(task);
>  	}
>  
> -	mutex_lock(&current->perf_event_mutex);
> -	list_add_tail(&event->owner_entry, &current->perf_event_list);
> -	mutex_unlock(&current->perf_event_mutex);
> -
>  	/*
>  	 * File reference in group guarantees that group_leader has been
>  	 * kept alive until we place the new event on the sibling_list.

[Severity: Critical]
Can the same owner_entry race still happen if the target task forks and the
child execs before list_add_tail() runs?

In SYSCALL_DEFINE5(perf_event_open) in kernel/events/core.c, ctx is unpinned
and ctx->mutex is unlocked before event->owner_entry is linked onto
current->perf_event_list:

	event->owner = current;

	perf_install_in_context(ctx, event, event->cpu);
	perf_unpin_context(ctx);

	mutex_unlock(&ctx->mutex);

	mutex_lock(&current->perf_event_mutex);
	list_add_tail(&event->owner_entry, &current->perf_event_list);
	mutex_unlock(&current->perf_event_mutex);

Holding task->signal->exec_update_lock only blocks execve() on task's own
signal_struct. It does not prevent task from calling fork(), which runs
copy_process() -> perf_event_init_task() -> perf_event_init_context() without
taking task->signal->exec_update_lock and allocates a new signal_struct with a
separate unlocked exec_update_lock for the child.

When an event is opened with attr.remove_on_exec = 1 and attr.inherit = 1,
perf_event_init_context() acquires ctx->mutex, inherits the event into
child_ctx, and marks child_ctx as a clone of ctx. Because perf_unpin_context()
already dropped ctx->pin_count to 0, context_equiv() allows the context swap
optimization:

kernel/events/core.c:context_equiv() {
	/* Pinning disables the swap optimization */
	if (ctx1->pin_count || ctx2->pin_count)
		return 0;
}

When switching from task to the newly forked child,
perf_event_context_sched_out() can then swap the unpinned original ctx onto the
child task:

kernel/events/core.c:perf_event_context_sched_out() {
			WRITE_ONCE(ctx->task, next);
			WRITE_ONCE(next_ctx->task, task);

			perf_ctx_sched_task_cb(ctx, task, false);

			perf_ctx_enable(ctx, 0);

			RCU_INIT_POINTER(task->perf_event_ctxp, next_ctx);
			RCU_INIT_POINTER(next->perf_event_ctxp, ctx);
}

At that point, the original ctx containing the parent event (with
event->owner == current and event->owner_entry still {NULL, NULL}) belongs to
the child process. If the child immediately calls execve() under its own
child->signal->exec_update_lock, begin_new_exec() -> perf_event_exec() ->
perf_event_remove_on_exec() iterates the swapped ctx->event_list:

kernel/events/core.c:perf_event_remove_on_exec() {
	list_for_each_entry_safe(event, next, &ctx->event_list, event_entry) {
		if (!event->attr.remove_on_exec)
			continue;

		if (!is_kernel_event(event))
			perf_remove_from_owner(event);
}

Wouldn't this allow perf_remove_from_owner(event) to run before the opener
executes list_add_tail(&event->owner_entry, &current->perf_event_list), hitting
the same list_del_init() corruption or use-after-free on
current->perf_event_list?

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-17 15:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-15 12:34 [PATCH] perf/core: Close owner-list publication race with remove-on-exec Daehyeon Ko
2026-10-15 12:45 ` sashiko-bot
2026-10-16  8:45 ` Peter Zijlstra
2026-10-17 15:07   ` [PATCH v2] " Daehyeon Ko
2026-10-17 15:25     ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.