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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Memory leak of struct perf_event_context in perf_get_task_ctxp()
- [High] Cross-session interference due to flawed inheritance matching in perf_get_task_ctxp()
--

commit 1659d24a7199e1068822505af2a6e5f2de5e9e1a
Author: Minwoo Ahn <mwahn402@gmail.com>

perf/core: Fix sampling period inconsistency across CPU migration

This commit introduces struct perf_task_context to preserve the sampling
state of per-task software events when task migration occurs. It allocates
a shared context across per-CPU perf_event instances to transport the
period_left value between CPUs.

[ ... ]

> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c

[ ... ]

> @@ -13259,6 +13291,41 @@ 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;
> +	struct perf_event *iter;
> +	unsigned long flags;
> +
> +	ctx = perf_lock_task_context(task, &flags);
> +	if (ctx) {
> +		list_for_each_entry(iter, &ctx->event_list, event_entry) {
> +			if (iter->perf_task_ctxp &&
> +			    (iter->owner == current ||
> +			     (inherit && !iter->owner)) &&

Could this cause independent tracing sessions to interfere with each other?

If two separate sessions monitor the same process with identical
configurations, their inherited child events will both have an owner of NULL.

Since perf_event_equal_task_ctx() only compares the config and sample_period,
the second session's child event might incorrectly match the first session's
event.

Would this lead to independent sessions sharing the same perf_task_ctxp and
clobbering each other's period_left state during task migrations?

> +			    perf_event_equal_task_ctx(&iter->attr,
> +						     &event->attr)) {
> +				ctxp = iter->perf_task_ctxp;
> +				refcount_inc(&ctxp->refcount);
> +				break;
> +			}
> +		}
> +		raw_spin_unlock_irqrestore(&ctx->lock, flags);

Does this leak the perf_event_context?

perf_lock_task_context() returns the context with its reference count
incremented, so the caller is typically responsible for dropping the reference
by calling put_ctx() after unlocking it.

Because put_ctx() is missing here, it seems this could leak the context and
the task_struct it pins each time a per-task software sampling event is
successfully created.

> +	}
> +
> +	if (!ctxp) {
> +		ctxp = kzalloc_obj(struct perf_task_context);
> +		if (!ctxp)
> +			return NULL;
> +		refcount_set(&ctxp->refcount, 1);
> +	}
> +
> +	return ctxp;
> +}

[ ... ]

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