Re: [PATCH] sched/autogroup: Fix task_group UAF during thread creation

[Tejun Heo <tj@kernel.org>]

Hello, Peter.

On Mon, Nov 02, 2026 at 11:17:59AM +0100, Peter Zijlstra wrote:
> On Fri, Oct 30, 2026 at 04:28:54PM -1000, Tejun Heo wrote:
> > cgroup_threadgroup_rwsem is a global percpu rwsem and taking it for
> > write on a regular basis can be really expensive.
>
> Yeah, but setsid() isn't more critical than moving tasks between
> cgroups. Also, AFAIK nearly nobody is using autogroups these days,
> because systemd does the cgroup thing.

We don't want to take the write lock on every setsid(). It would have to be
limited to actual autogroup joins, which seems doable but likely adds a bit
of complication.

> > Either way, AFAICS, autogroup doesn't need cgroup_threadgroup_rwsem. The
> > cgroup side has to be settled early in fork, but the autogroup side
> > doesn't. Can't we just move the autogroup assignment later in fork, to a
> > point where it can't race against setsid()?
>
> That is more or less the proposal, but it rather creates more complexity
> rather than reducing it.
>
> Having two different rules for p->sched_task_group, depending on cgroup
> or autogroup isn't at all appealing. Not to mention adding even more
> fork callbacks.

I see your point. Moving the autogroup handling into the cpu controller
keeps it better contained though. sched_cgroup_fork() places the new task by
its cgroup only, and the cpu controller applies the autogroup once the task
is on the thread list. This isn't a hill I'm going to die on, but it looks
like the better direction to me. Can you please take a look at the patch
below?

diff --git a/kernel/sched/autogroup.h b/kernel/sched/autogroup.h
--- a/kernel/sched/autogroup.h
+++ b/kernel/sched/autogroup.h
@@ -27,12 +27,12 @@ static inline bool task_group_is_autogroup(struct task_group *tg)
 	return !!tg->autogroup;
 }
 
+extern unsigned int sysctl_sched_autogroup_enabled;
 extern bool task_wants_autogroup(struct task_struct *p, struct task_group *tg);
 
 static inline struct task_group *
 autogroup_task_group(struct task_struct *p, struct task_group *tg)
 {
-	extern unsigned int sysctl_sched_autogroup_enabled;
 	int enabled = READ_ONCE(sysctl_sched_autogroup_enabled);
 
 	if (enabled && task_wants_autogroup(p, tg))
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4958,7 +4958,6 @@ int sched_cgroup_fork(struct task_struct *p, struct kernel_clone_args *kargs)
 		struct task_group *tg;
 		tg = container_of(kargs->cset->subsys[cpu_cgrp_id],
 				  struct task_group, css);
-		tg = autogroup_task_group(p, tg);
 		p->sched_task_group = tg;
 	}
 #endif
@@ -9598,6 +9597,15 @@ static void cpu_cgroup_cancel_attach(struct cgroup_taskset *tset)
 	scx_cgroup_cancel_attach(tset);
 }
 
+#ifdef CONFIG_SCHED_AUTOGROUP
+static void cpu_cgroup_fork(struct task_struct *p)
+{
+	if (READ_ONCE(p->sched_task_group) == &root_task_group &&
+	    READ_ONCE(sysctl_sched_autogroup_enabled))
+		sched_move_task(p, true);
+}
+#endif
+
 #ifdef CONFIG_UCLAMP_TASK_GROUP
 static void cpu_util_update_eff(struct cgroup_subsys_state *css)
 {
@@ -10562,6 +10570,9 @@ struct cgroup_subsys cpu_cgrp_subsys = {
 	.can_attach	= cpu_cgroup_can_attach,
 	.attach		= cpu_cgroup_attach,
 	.cancel_attach	= cpu_cgroup_cancel_attach,
+#ifdef CONFIG_SCHED_AUTOGROUP
+	.fork		= cpu_cgroup_fork,
+#endif
 	.legacy_cftypes	= cpu_legacy_files,
 	.dfl_cftypes	= cpu_files,
 	.early_init	= true,

Thanks.

--
tejun