All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Phat Dinh <phatdt1116@gmail.com>
Cc: mingo@redhat.com, acme@kernel.org, namhyung@kernel.org,
	security@kernel.org, linux-perf-users@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] perf/core: Do not resurrect cpuctx->cgrp after last cgroup event exit
Date: Fri, 6 Nov 2026 11:40:43 +0100	[thread overview]
Message-ID: <20261106104043.GF1779021@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20261106101642.494674-1-phatdt1116@gmail.com>

On Fri, Nov 06, 2026 at 05:16:42PM +0700, Phat Dinh wrote:
> perf_cgroup_switch() relies on:
> 
>     /*
>      * cpuctx->cgrp is set when the first cgroup event enabled,
>      * and is cleared when the last cgroup event disabled.
>      */
> 
> It computes the next task's cgroup, schedules out the CPU context and
> then assigns cpuctx->cgrp unconditionally:
> 
>     ctx_sched_out(&cpuctx->ctx, NULL, EVENT_ALL|EVENT_CGROUP);
>     /*
>      * must not be done before ctxswout due
>      * to update_cgrp_time_from_cpuctx() in
>      * ctx_sched_out()
>      */
>     cpuctx->cgrp = cgrp;
> 
> However, ctx_sched_out() can disable events itself. If a cgroup event
> has a pending disable from perf_event_disable_inatomic() (e.g. its
> PERF_EVENT_IOC_REFRESH limit ran out) that the irq_work has not
> handled yet, event_sched_out() consumes it inline:
> 
>     if (event->pending_disable) {
>         event->pending_disable = 0;
>         perf_cgroup_event_disable(event, ctx);
>         state = PERF_EVENT_STATE_OFF;
>     }
> 
> If that was the last cgroup event, perf_cgroup_event_disable() clears
> cpuctx->cgrp:
> 
>     if (--ctx->nr_cgroups)
>         return;
> 
>     cpuctx->cgrp = NULL;
> 
> and perf_cgroup_switch() then sets it again. cpuctx->cgrp is left
> non-NULL with ctx.nr_cgroups == 0, so the NULL check in
> perf_cgroup_switch() no longer catches it and the next switch trips:
> 
>     WARN_ON_ONCE(cpuctx->ctx.nr_cgroups == 0);
> 
> cpuctx->cgrp does not hold a reference on the cgroup. Once that cgroup
> is removed, the next ctx_sched_in() dereferences the stale pointer:
> 
>   BUG: KASAN: slab-use-after-free in ctx_sched_in+0xb1f/0xfa0
>   Read of size 8 at addr ffff8881008972d0 by task swapper/3/0
> 
>   Tainted: [W]=WARN
>   Call Trace:
>    <IRQ>
>    ctx_sched_in+0xb1f/0xfa0
>    perf_event_sched_in+0x1a/0x70
>    ctx_resched+0x32b/0xac0
>    __perf_install_in_context+0x475/0xab0
>    remote_function+0x105/0x200
>    __flush_smp_call_function_queue+0x179/0x8d0
>    __sysvec_call_function_single+0x1b/0x1c0
>    sysvec_call_function_single+0x66/0x80
>    </IRQ>
>    <TASK>
>    asm_sysvec_call_function_single+0x1a/0x20
> 
> Only assign cpuctx->cgrp when cgroup events remain after the context
> has been scheduled out, so that it stays NULL as the invariant
> requires.
> 
> Fixes: e19cd0b6fa59 ("perf/core: Always set cpuctx cgrp when enable cgroup event")
> Cc: stable@vger.kernel.org
> Signed-off-by: Phat Dinh <phatdt1116@gmail.com>

Indeed, thanks!

> ---
>  kernel/events/core.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index f36a3bbaa33c..54b87584c752 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -1072,8 +1072,13 @@ static void perf_cgroup_switch(struct task_struct *task)
>  	 * must not be done before ctxswout due
>  	 * to update_cgrp_time_from_cpuctx() in
>  	 * ctx_sched_out()
> +	 *
> +	 * ctx_sched_out() can consume a pending disable and drop the last cgroup
> +	 * event, which clear the cpuctx->cgrp. Cgroup tracking follow active cgroup
> +	 * event, so do not resurrect it
>  	 */
> -	cpuctx->cgrp = cgrp;
> +	if (cpuctx->ctx.nr_cgroups)
> +		cpuctx->cgrp = cgrp;
>  	/*
>  	 * set cgrp before ctxsw in to allow
>  	 * perf_cgroup_set_timestamp() in ctx_sched_in()
> 
> base-commit: d095321349222961813333277881b3c74f10b17b
> -- 
> 2.56.0
> 

  parent reply	other threads:[~2026-11-06 10:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-06 10:16 [PATCH] perf/core: Do not resurrect cpuctx->cgrp after last cgroup event exit Phat Dinh
2026-11-06 10:28 ` sashiko-bot
2026-11-06 10:40 ` Peter Zijlstra [this message]
2026-11-09  8:37 ` [tip: perf/urgent] " tip-bot2 for Phat Dinh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261106104043.GF1779021@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=acme@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=phatdt1116@gmail.com \
    --cc=security@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.