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 01:25:06PM -0700, Tim Chen wrote:
> On Tue, 2026-09-01 at 04:28 +0900, Hyunwoo Kim wrote:
> > 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?
> 
> Thanks to Kyunwoo to show this potential use-after-free problem.

It is a real, triggerable issue.

> 
> While synchronize_rcu() avoids rq lock, it could introduce a long delay
> of a full grace period that will be undesirable.
> 
> 
> I think a better fix is to decouple sc_stat from mm and make it its own structure.
> Then we can do proper refcounting and rcu management on sc_stat itself.
> We will let task points to sc_stat with proper ref count and change the access
> as task->sc_stat directly nstead of via task->mm->sc_stat.
> 
> Then we will know sc_stat exists in account_mm_sched() execution
> as long as a task points to it and do not have to worry that sc_stat is
> released due to the mm life cycle.
> 
> In the first 2 patches of the prctl control series for cache aware scheduling
> we posted, we introduced the change I described above and should 
> fix this issue without additional rq locking or synchronize_rcu() overhead.
>  https://lore.kernel.org/lkml/cover.1787955777.git.tim.c.chen@linux.intel.com/
> 
> We should probably prioritize to merge the first couple of patches from
> that series now in light of this issue.  It will make maintenance and future
> modification of the cache aware scheduling code easier.

That is a fairly large change. 

If you add a Reported-by: tag for this UAF to the patch, I am fine with 
handling it that way.


Best regards,
Hyunwoo Kim