Re: [PATCH] perf/core: Close owner-list publication race with remove-on-exec

[Peter Zijlstra <peterz@infradead.org>]

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.