Re: [PATCH] perf_events: fix array-index-out-of-bounds in x86_pmu_del

[Ian Rogers]

On Mon, Mar 9, 2026 at 6:37 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> On Mon, Mar 09, 2026 at 01:05:43PM +0100, Peter Zijlstra wrote:
> > Now, let me go audit the code to see if this same problem exists in more
> > shapes...
>
> I've ended up with the below.
>
> ---
> Subject: perf: Make sure to use pmu_ctx->pmu for groups
> From: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Date: Mon Mar 9 13:55:46 CET 2026
>
> 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: bd2756811766 ("perf: Rewrite core context handling")
> Reported-by: Oliver Rosenberg <olrose55@xxxxxxxxx>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>

Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>

Thanks,
Ian

> ---
>  kernel/events/core.c |   14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
>
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -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,14 +4833,15 @@ 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);
>                 data->ret = 0;
> -               goto unlock;
> +               return;
>         }
>
> +       pmu = event->pmu_ctx->pmu;
>         pmu->start_txn(pmu, PERF_PMU_TXN_READ);
>
>         pmu->read(event);
> @@ -4849,9 +4850,6 @@ static void __perf_event_read(void *info
>                 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)
> @@ -14743,7 +14741,7 @@ inherit_event(struct perf_event *parent_
>         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);