Re: [PATCH] perf/core: Fix sampling period inconsistency across CPU migration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Use-After-Free of perf_event_context in perf_get_task_ctxp() due to lockless task->perf_event_ctxp access.
- [High] IRQ inversion deadlock in perf_get_task_ctxp() due to raw_spin_lock() without disabling interrupts.
- [High] State corruption and logic flaw due to missing event type validation in perf_event_equal_task_ctx().
--

commit 9f612ffa1ab8888251bf71b52ebf7bdaadacbb11
Author: Minwoo Ahn <mwahn402@gmail.com>

perf/core: Fix sampling period inconsistency across CPU migration

This commit introduces a per-task container to preserve period_left across
CPU migrations for software sampling events. The context is shared across
multiple per-CPU perf_event instances originating from the same caller via
refcounting.

> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 48d851fbd8ea..84827f81cc9c 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
[ ... ]
> @@ -1148,6 +1151,21 @@ struct perf_cpu_context {
>  	struct perf_event		*heap_default[2];
>  };
>  
> +#define perf_event_equal_task_ctx(a1, a2)	\
> +	((a1)->config == (a2)->config &&	\
> +	 (a1)->sample_period == (a2)->sample_period)

Should this macro also compare attr.type? Since it only compares config
and sample_period, unrelated events with small config IDs (such as an
inherited hardware event and a software CPU-clock event that both have a
config of 0) could erroneously match. This would cause independent sampling
sessions to share state and overwrite each other's period_left.

> +
> +/**
> + * struct perf_task_context - per-task software event context
> + *
> + * Shared across per-CPU perf_event instances of the same task to
> + * preserve period_left across CPU migrations.
> + */
> +struct perf_task_context {
> +	refcount_t			refcount;
> +	local64_t			period_left;
> +};
[ ... ]
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 6d1f8bad7e1c..bd106e0b854a 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
[ ... ]
> @@ -13259,6 +13291,40 @@ static void account_event(struct perf_event *event)
>  	account_pmu_sb_event(event);
>  }
>  
> +static struct perf_task_context *
> +perf_get_task_ctxp(struct perf_event *event, struct task_struct *task,
> +		   bool inherit)
> +{
> +	struct perf_task_context *ctxp = NULL;
> +	struct perf_event_context *ctx = task->perf_event_ctxp;

Does this code risk a use-after-free of the perf_event_context?

The task->perf_event_ctxp pointer is read directly into the local ctx
variable without holding rcu_read_lock() or using the safe
perf_lock_task_context() helper.

Although the target task_struct is pinned by get_task_struct(), the
perf_event_context it points to is dynamically managed. If the target task
exits concurrently, perf_event_exit_task() could set task->perf_event_ctxp
to NULL and schedule the context for destruction.

Because this accesses the context outside an RCU read-side critical section,
the allocating thread could be preempted while the RCU grace period elapses,
allowing the context memory to be freed before the lock is acquired below.

> +	struct perf_event *iter;
> +
> +	if (ctx) {
> +		raw_spin_lock(&ctx->lock);

Could this cause an IRQ inversion deadlock?

perf_get_task_ctxp() is called from perf_event_alloc() during the
sys_perf_event_open syscall, which executes in a sleepable process context
with hardware interrupts enabled.

ctx->lock is an IRQ-safe raw spinlock heavily used in hardirq contexts.
If a timer or PMU interrupt fires on the local CPU while this lock is held,
the interrupt handler could attempt to acquire the same ctx->lock, resulting
in an immediate IRQ inversion deadlock that hangs the CPU.

> +		list_for_each_entry(iter, &ctx->event_list, event_entry) {
> +			if (iter->perf_task_ctxp &&
> +			    (iter->owner == current ||
> +			     (inherit && !iter->owner)) &&
> +			    perf_event_equal_task_ctx(&iter->attr,
> +						     &event->attr)) {

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