All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Daehyeon Ko <4ncienth@gmail.com>
Cc: 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: Mon, 26 Oct 2026 12:57:07 +0100	[thread overview]
Message-ID: <20261026115707.GF2668200@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20261023013631.881730-1-4ncienth@gmail.com>

On Fri, Oct 23, 2026 at 10:36:31AM +0900, Daehyeon Ko wrote:
> sched_cgroup_fork() assigns a CLONE_THREAD child in the root CPU cgroup to
> p->signal->autogroup->tg before the child is visible on the thread list.
> Autogroup ownership is per signal_struct: CLONE_THREAD reuses the existing
> signal_struct and does not acquire an autogroup reference for the child.
> 
> ksys_setsid() releases tasklist_lock before calling
> sched_autogroup_create_attach(), so tasklist_lock does not serialize the
> autogroup move against this unpublished part of copy_process():
> 
>   clone(CLONE_THREAD) caller      sibling setsid() caller
>   -------------------------       -----------------------
>   sched_cgroup_fork()
>                                   sched_autogroup_create_attach()
>                                     autogroup_move_group()
>                                       lock(siglock)
>                                       signal->autogroup = new
>                                       for_each_thread()
>                                       unlock(siglock)
>                                       autogroup_kref_put(old)
>   lock(siglock)
>   list_add_tail_rcu(thread_node)
>   unlock(siglock)
>   wake_up_new_task()
> 
> The iteration cannot see the unpublished child. The final put can therefore
> drop the signal's last reference to the old autogroup. Two RCU callbacks
> then free its per-CPU cfs_rq and task_group while p->sched_task_group,
> p->se.cfs_rq, and p->se.parent still point into them.
> 
> At the last failure-free point in copy_process(), revalidate the child's
> scheduler group while holding the shared siglock and keep the lock held
> through thread-list publication. An autogroup move then either completes
> before revalidation, or blocks until publication and includes the child.
> 
> If the group changed, update p->sched_task_group under p->pi_lock, clear
> p->se.cfs_rq so set_task_rq_fair() cannot inspect the freed old runqueue,
> and call set_task_rq() to rebuild the fair and RT group links. The TASK_NEW
> child has no attached load to migrate.
> 
> An unprivileged clone()/setsid() reproducer running as uid/gid 65534 with
> all capability sets empty triggered a KASAN task_group slab use-after-free
> in wake_up_new_task() on an unmodified Linux v7.3 kernel. The allocation
> came from sched_create_group(), and the free ran from
> sched_free_group_rcu().
> The fix completed 1,024 iterations of both the original race sweep and a
> delayed-setsid stress schedule without a KASAN, lockdep, oops, or panic
> report.
> 
> An AI coding assistant helped identify and validate the race, construct
> the reproducer, and draft and test this fix.
> 
> Fixes: b1e8206582f9 ("sched: Fix yet more sched_fork() races")
> Cc: stable@vger.kernel.org
> Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>

> diff --git a/kernel/fork.c b/kernel/fork.c
> index 10f2d05d816a5f..6df4ff21d1b780 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2500,6 +2500,15 @@ __latent_entropy struct task_struct *copy_process(
>  
>  	/* No more failure paths after this point. */
>  
> +	/*
> +	 * A concurrent autogroup move can replace signal->autogroup after
> +	 * sched_cgroup_fork() but before this task is on the thread list.
> +	 * Recheck while holding siglock so the move either precedes this check or
> +	 * observes the new thread after publication below.
> +	 */
> +	if (clone_flags & CLONE_THREAD)
> +		sched_autogroup_fork_finalize(p, args);
> +
>  	/*
>  	 * Copy seccomp details explicitly here, in case they were changed
>  	 * before holding sighand lock.
> diff --git a/kernel/sched/autogroup.c b/kernel/sched/autogroup.c
> index e380cf9372bbd2..50ad43e7648fc5 100644
> --- a/kernel/sched/autogroup.c
> +++ b/kernel/sched/autogroup.c
> @@ -215,6 +215,36 @@ void sched_autogroup_fork(struct signal_struct *sig)
>  	sig->autogroup = autogroup_task_get(current);
>  }
>  
> +/**
> + * sched_autogroup_fork_finalize - recheck a new thread's scheduler group
> + * @p: new task
> + * @kargs: clone arguments holding the pinned cgroup set
> + *
> + * The caller must hold the shared sighand lock and keep it held through
> + * thread-list publication.  The task is still TASK_NEW and has no attached
> + * scheduler load.
> + */
> +void sched_autogroup_fork_finalize(struct task_struct *p,
> +				   struct kernel_clone_args *kargs)
> +{
> +	struct task_group *tg;
> +	unsigned long flags;
> +
> +	lockdep_assert_held(&p->sighand->siglock);
> +
> +	raw_spin_lock_irqsave(&p->pi_lock, flags);
> +	tg = container_of(kargs->cset->subsys[cpu_cgrp_id],
> +			  struct task_group, css);
> +	tg = autogroup_task_group(p, tg);
> +	if (tg != p->sched_task_group) {
> +		p->sched_task_group = tg;
> +		/* Do not let set_task_rq_fair() inspect the possibly freed cfs_rq. */
> +		p->se.cfs_rq = NULL;

I'm not sure this works; afaict this will result in a NULL deref
instead. 

> +		set_task_rq(p, task_cpu(p));
> +	}
> +	raw_spin_unlock_irqrestore(&p->pi_lock, flags);
> +}
> +
>  void sched_autogroup_exit(struct signal_struct *sig)
>  {
>  	autogroup_kref_put(sig->autogroup);


Can't we make ksys_setsid() more complicated instead? Have it create the
autogroup in advance, move under the tasklist_lock and destroy on
failure or something?


  reply	other threads:[~2026-10-26 11:57 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 [this message]
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

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=20261026115707.GF2668200@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.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=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.