linux-kernel - Re: [BUG] hard-to-hit mm_struct UAF due to insufficiently careful vma_refcount_put() wrt SLAB_TYPESAFE_BY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAJuCfpEMhGe_eZuFm__4CDstM9=OG2kTUTziNL-f=M3BYQor2A@mail.gmail.com>
Date: Mon, 28 Jul 2025 10:14:33 -0700
From: Suren Baghdasaryan <surenb@...gle.com>
To: Jann Horn <jannh@...gle.com>
Cc: Vlastimil Babka <vbabka@...e.cz>, Andrew Morton <akpm@...ux-foundation.org>, 
	"Liam R. Howlett" <Liam.Howlett@...cle.com>, Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, 
	Pedro Falcato <pfalcato@...e.de>, Linux-MM <linux-mm@...ck.org>, 
	kernel list <linux-kernel@...r.kernel.org>
Subject: Re: [BUG] hard-to-hit mm_struct UAF due to insufficiently careful
 vma_refcount_put() wrt SLAB_TYPESAFE_BY_RCU

On Thu, Jul 24, 2025 at 9:36 AM Suren Baghdasaryan <surenb@...gle.com> wrote:
>
> On Thu, Jul 24, 2025 at 2:45 PM Jann Horn <jannh@...gle.com> wrote:
> >
> > On Thu, Jul 24, 2025 at 10:38 AM Vlastimil Babka <vbabka@...e.cz> wrote:
> > > On 7/24/25 04:30, Suren Baghdasaryan wrote:
> > > > So, I think vma_refcount_put() can mmgrab(vma->mm) before calling
> > > > __refcount_dec_and_test(), to stabilize that mm and then mmdrop()
> > > > after it calls rcuwait_wake_up(). What do you think about this
> > > > approach, folks?
> > >
> > > Yeah except it would be wasteful to do for all vma_refcount_put(). Should be
> > > enough to have this version (as Jann suggested) for inval_end_read: part of
> > > lock_vma_under_rcu. I think we need it also for the vma_refcount_put() done
> > > in vma_start_read() when we fail the seqcount check? I think in that case
> > > the same thing can be happening too, just with different race windows?
> > >
> > > Also as Jann suggested, maybe it's not great (or even safe) to perform
> > > __mmdrop() under rcu? And maybe some vma_start_read() users are even more
> > > restricted? Maybe then we'd need to make __mmdrop_delayed() not RT-only, and
> > > use that.
> >
> > FWIW, I think I have been mixing things up in my head - mmdrop_async()
> > exists, but this comment in free_signal_struct() explains that it's
> > because __mmdrop() is not softirq-safe because x86's pgd_lock spinlock
> > does not disable IRQs.
> >
> > /*
> > * __mmdrop is not safe to call from softirq context on x86 due to
> > * pgd_dtor so postpone it to the async context
> > */
> >
> > So I guess using mmdrop() here might actually be fine, since we're
> > just in atomic context, not in softirq.
>
> Thanks for looking more into this. Even if it's safe, I would still
> prefer to make mmdrop() outside of RCU read section. The code might
> actually end-up cleaner that way too.

Sorry for the delay, I got some time over the weekend to work on this.
Unfortunately vma_start_read() is used in one more place in
mm-unstable and it uses vma iterators for the lookup, so combining
lookup with vma_start_read() is not as clean as we thought. After
trying couple of ways to fix this I decided to follow KISS principle.
The fix is posted at
https://lore.kernel.org/all/20250728170950.2216966-1-surenb@google.com/.
Reviews and feedback is appreciated.