All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Daehyeon Ko <4ncienth@gmail.com>, Ingo Molnar <mingo@redhat.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sched/autogroup: Fix task_group UAF during thread creation
Date: Tue, 03 Nov 2026 07:42:24 -1000	[thread overview]
Message-ID: <477c7b868652b4d9152ef61c4c94fe16@kernel.org> (raw)
In-Reply-To: <20261103114557.GJ1813469@noisy.programming.kicks-ass.net>

Hello, Peter.

On Tue, Nov 03, 2026 at 12:45:57PM +0100, Peter Zijlstra wrote:
> Now, using cgroup_post_fork() seems weird (also, I'm not sure if this
> calls cgroup_subsys::fork for *all* controllers, or only the enabled
> ones).

It calls ->fork for every built-in controller that has one, whether or
not it's bound to a hierarchy or disabled with cgroup_disable=.

> But we can easily move this into sched_post_fork(); all we really need
> to ensure is this happens before scx_post_fork(), right?

Yeah, that works. AFAICS there's no ordering requirement against
scx_post_fork() though. sched_move_task(p, true) skips
scx_cgroup_move_task(), so the autogroup move doesn't invoke any sched_ext
op. tg_cgrp() maps both root_task_group and autogroup task_groups to the
root cgroup, so sched_ext sees the same cgroup either way. What matters is
that it runs after the task is on the thread list and before
wake_up_new_task().

> +	if (READ_ONCE(sysctl_sched_autogroup_enabled) &&
> +	    task_wants_autogroup(p, p->sched_task_group))
> +		sched_move_task(p, true);

The p->sched_task_group read races with setsid() and cgroup migration.
Maybe READ_ONCE()?

Separately, there's a pre-existing misplacement. If setsid() lands after
the new thread is on the thread list but before cgroup_post_fork() links
it to its css_set, sched_change_group() sees init_css_set and puts the
thread in the autogroup, or in root_task_group with autogroup disabled,
even when its cgroup isn't root, and it stays there. I think this should
be fixed from the setsid() side.

Thanks.

--
tejun

      reply	other threads:[~2026-11-03 17:42 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-23  1:36 [PATCH] sched/autogroup: Fix task_group UAF during thread creation Daehyeon Ko
2026-10-26 11:57 ` Peter Zijlstra
2026-10-30  3:45   ` Daehyeon Ko
2026-10-30 15:49     ` Peter Zijlstra
2026-10-31  2:28       ` Tejun Heo
2026-11-02 10:17         ` Peter Zijlstra
2026-11-02 19:21           ` Tejun Heo
2026-11-03 11:45             ` Peter Zijlstra
2026-11-03 17:42               ` Tejun Heo [this message]

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=477c7b868652b4d9152ef61c4c94fe16@kernel.org \
    --to=tj@kernel.org \
    --cc=4ncienth@gmail.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=vincent.guittot@linaro.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.