Commit 4b9ce671 authored by Peter Zijlstra's avatar Peter Zijlstra
Browse files

perf: Make sure to use pmu_ctx->pmu for groups 



Oliver reported that x86_pmu_del() ended up doing an out-of-bound memory access
when group_sched_in() fails and needs to roll back.

This *should* be handled by the transaction callbacks, but he found that when
the group leader is a software event, the transaction handlers of the wrong PMU
are used. Despite the move_group case in perf_event_open() and group_sched_in()
using pmu_ctx->pmu.

Turns out, inherit uses event->pmu to clone the events, effectively undoing the
move_group case for all inherited contexts. Fix this by also making inherit use
pmu_ctx->pmu, ensuring all inherited counters end up in the same pmu context.

Similarly, __perf_event_read() should use equally use pmu_ctx->pmu for the
group case.

Fixes: bd275681 ("perf: Rewrite core context handling")
Reported-by: default avatarOliver Rosenberg <olrose55@gmail.com>
Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: default avatarIan Rogers <irogers@google.com>
Link: https://patch.msgid.link/20260309133713.GB606826@noisy.programming.kicks-ass.net
parent f1cac6ac

kernel/events/core.c

+8 −11
Original line number Diff line number Diff line
@@ -4813,7 +4813,7 @@ static void __perf_event_read(void *info) 
	struct perf_event *sub, *event = data->event;

	struct perf_event_context *ctx = event->ctx;

	struct perf_cpu_context *cpuctx = this_cpu_ptr(&perf_cpu_context);

	struct pmu *pmu = event->pmu;

	struct pmu *pmu;



	/*

	 * If this is a task context, we need to check whether it is
@@ -4825,7 +4825,7 @@ static void __perf_event_read(void *info) 
	if (ctx->task && cpuctx->task_ctx != ctx)

		return;



	raw_spin_lock(&ctx->lock);

	guard(raw_spinlock)(&ctx->lock);

	ctx_time_update_event(ctx, event);



	perf_event_update_time(event);
@@ -4833,25 +4833,22 @@ static void __perf_event_read(void *info) 
		perf_event_update_sibling_time(event);



	if (event->state != PERF_EVENT_STATE_ACTIVE)

		goto unlock;

		return;



	if (!data->group) {

		pmu->read(event);

		perf_pmu_read(event);

		data->ret = 0;

		goto unlock;

		return;

	}



	pmu = event->pmu_ctx->pmu;

	pmu->start_txn(pmu, PERF_PMU_TXN_READ);



	pmu->read(event);



	perf_pmu_read(event);

	for_each_sibling_event(sub, event)

		perf_pmu_read(sub);



	data->ret = pmu->commit_txn(pmu);



unlock:

	raw_spin_unlock(&ctx->lock);

}



static inline u64 perf_event_count(struct perf_event *event, bool self)
@@ -14744,7 +14741,7 @@ inherit_event(struct perf_event *parent_event, 
	get_ctx(child_ctx);

	child_event->ctx = child_ctx;



	pmu_ctx = find_get_pmu_context(child_event->pmu, child_ctx, child_event);

	pmu_ctx = find_get_pmu_context(parent_event->pmu_ctx->pmu, child_ctx, child_event);

	if (IS_ERR(pmu_ctx)) {

		free_event(child_event);

		return ERR_CAST(pmu_ctx);