Re: [PATCH] sched/cache: Fix use-after-free of the mm replaced by exec

[Hyunwoo Kim <imv4bel@gmail.com>]

On Mon, Aug 31, 2026 at 12:22:45PM +0800, Chen, Yu C wrote:
> Hi Hyunwoo,
> 
> On 8/30/2026 3:30 PM, Hyunwoo Kim wrote:
> > When the waker cannot use the wakelist, ttwu_queue() takes the target rq
> > lock and goes down into update_curr(). If the target rq belongs to another
> > CPU, the task handed to account_mm_sched() is the one running on that CPU,
> > not the task being woken.
> > 
> > account_mm_sched() reads p->mm and updates mm->sc_stat. Nothing keeps that
> > mm alive. The rq lock and rq->cpu_epoch_lock it holds have nothing to do
> > with the lifetime of the mm.
> > 
> > If that task happens to be in execve(), exec_mmap() points tsk->mm and
> > tsk->active_mm at the new mm, and exec_mm_put_old() from setup_new_exec()
> > drops the old one. free_bprm() does the same when exec fails. On the way
> > from mmput() down to __mmdrop(), mm_destroy_sched() calls free_percpu() on
> > sc_stat.pcpu_sched and free_mm() returns the mm_struct.
> > 
> > Whoever already read the old pointer keeps using it. It adds to runtime in
> > the freed per-cpu area and reads sc_stat in the freed mm_struct. Depending
> > on the condition it also writes sc_stat.cpu. That is a use-after-free.
> > 
> > Commit 9f23469401b0 ("sched/cache: Fix potential NULL mm pointer access")
> > changed the remaining p->mm dereference to the local variable, and said the
> > active_mm reference keeps the structure allocated. That holds for the other
> > paths that detach an mm, since they take an mmgrab_lazy_tlb() reference.
> > exec reassigns active_mm to the new mm as well, so that reference is gone.
> > What is left is the mm_users reference in bprm->old_mm, and dropping it is
> > the free.
> > 
> >               CPU0                                CPU1
> > 
> >                                        write(pipe)
> >                                        try_to_wake_up()
> >                                          ttwu_queue()      // takes rq0 lock
> >                                            enqueue_task_fair()
> >                                              update_curr()
> >                                                update_se()
> >                                                  account_mm_sched()
> >                                                    mm = rq0->curr->mm
> >                                                          // old mm
> >    execve()
> >      exec_mmap()                       // tsk->mm = new mm
> >      setup_new_exec()
> >        exec_mm_put_old()
> >          mmput() -> ... -> __mmdrop()
> >            mm_destroy_sched()          // free_percpu()
> >            free_mm()
> >                                                    read mm->sc_stat.epoch
> >                                                          // use-after-free
> > 
> 
> Ah, thanks for catching this.
> 
> > ---
> >   fs/exec.c             |  1 +
> >   include/linux/sched.h |  4 ++++
> >   kernel/events/core.c  |  2 ++
> >   kernel/sched/fair.c   | 16 ++++++++++++++++
> >   4 files changed, 23 insertions(+)
> > 
> > diff --git a/fs/exec.c b/fs/exec.c
> > index 745f6eb5279e6..6194c38807980 100644
> > --- a/fs/exec.c
> > +++ b/fs/exec.c
> > @@ -916,6 +916,7 @@ static void exec_mm_put_old(struct mm_struct *old_mm)
> >   {
> >   	setmax_mm_hiwater_rss(&current->signal->maxrss, old_mm);
> >   	mm_update_next_owner(old_mm);
> > +	sched_cache_exec_done();
> >   	mmput(old_mm);
> >   }
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index 8b3d47a325cca..6ae31bffe049e 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -2415,10 +2415,14 @@ struct sched_cache_stat {
> >   	int cpu;
> >   } ____cacheline_aligned_in_smp;
> > +void sched_cache_exec_done(void);
> > +
> >   #else
> >   struct sched_cache_stat { };
> > +static inline void sched_cache_exec_done(void) { }
> > +
> >   #endif
> >   #ifndef MODULE
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index a6c8e38a31104..2f29cbccf03f1 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -5427,6 +5427,8 @@ attach_task_ctx_data(struct task_struct *task, struct kmem_cache *ctx_cache,
> >   	if (!cd)
> >   		return -ENOMEM;
> > +	/* @old, loaded by the try_cmpxchg() below, is only stable under RCU. */
> > +	guard(rcu)();
> 
> Is this change related to this UAF issue?

Duh.. that one is unrelated. My mistake.

> 
> > +/* exec() has switched to the new mm and is about to drop the old one. */
> > +void sched_cache_exec_done(void)
> > +{
> > +	struct rq_flags rf;
> > +	struct rq *rq;
> > +
> > +	/*
> > +	 * account_mm_sched() dereferences rq->curr->mm under this rq's lock,
> > +	 * so a remote CPU can still be using the old mm. The lock cycle waits
> > +	 * for it, and the store to tsk->mm cannot be reordered past the
> > +	 * release, so later acquirers see the new mm.
> > +	 */
> > +	rq = this_rq_lock_irq(&rf);
> 
> A smart fix, learnt! It behaves like a synchronize_rcu() to protect against
> the read in account_mm_sched(). Small open: since the context of invoking
> account_mm_sched() is preemption-disabled, I wonder if we can simply use
> synchronize_rcu() directly instead of this_rq_lock_irq() - just to avoid
> contention for rq-lock in heavy system?

Yeah, that works too. I think rcu is nicer here as well. I'll do some
testing and send a v2.


Best regards,
Hyunwoo Kim