Re: [PATCH v2] perf/core: Don't send SIGTRAP after exec removed the event

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing READ_ONCE() on concurrent lockless access to event->state
--

commit a39b84d0fe4531dad91da56d867a84ee0e8c6cf6
Author: Danish Khateeb <danishkhateeb03@gmail.com>

perf/core: Don't send SIGTRAP after exec removed the event

This commit prevents sending a SIGTRAP to a process after an exec() call has
removed the perf event. It adds a state check in perf_sigtrap() to ensure
signals are skipped if the event state indicates it has already been removed.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 634d2ccbab82d..eabe6cdf7a888 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c

[ ... ]

> @@ -7631,6 +7631,15 @@ static void perf_sigtrap(struct perf_event *event)
>  	if (current->flags & PF_EXITING)
>  		return;
>  
> +	/*
> +	 * The event was removed after this signal was queued, e.g. by exec()
> +	 * (remove_on_exec), which may also have closed its fd (close-on-exec).
> +	 * The new program has default signal handlers, so a SIGTRAP would
> +	 * kill it.
> +	 */
> +	if (event->state <= PERF_EVENT_STATE_EXIT)

[Severity: Low]
Does this lockless read of event->state need a READ_ONCE() annotation?

If another thread sharing the file descriptor calls close(), perf_release()
can run concurrently on another CPU and modify event->state using
WRITE_ONCE(). Under the kernel memory model, reading a variable that can be
concurrently modified typically requires READ_ONCE() to prevent compiler
optimizations like tearing or rematerialization, and to avoid data race
warnings.

> +		return;
> +
>  	/*
>  	 * We'd expect this to only occur if the irq_work is delayed and either
>  	 * ctx->task or current has changed in the meantime. This can be the

[ ... ]

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