Re: [PATCH v2] mm/rmap: Add anon_vma lifetime debug check

[Harry Yoo <harry.yoo@oracle.com>]

On Mon, Jul 28, 2025 at 05:33:34AM +0100, Lorenzo Stoakes wrote:
> On Mon, Jul 28, 2025 at 01:05:54PM +0900, Harry Yoo wrote:
> > On Fri, Jul 25, 2025 at 02:16:24PM +0200, Jann Horn wrote:
> > > If an anon folio is mapped into userspace, its anon_vma must be alive,
> > > otherwise rmap walks can hit UAF.
> > >
> > > There have been syzkaller reports a few months ago[1][2] of UAF in rmap
> > > walks that seems to indicate that there can be pages with elevated mapcount
> > > whose anon_vma has already been freed, but I think we never figured out
> > > what the cause is; and syzkaller only hit these UAFs when memory pressure
> > > randomly caused reclaim to rmap-walk the affected pages, so it of course
> > > didn't manage to create a reproducer.
> > >
> > > Add a VM_WARN_ON_FOLIO() when we add/remove mappings of anonymous folios to
> > > hopefully catch such issues more reliably.
> > >
> > > [1] https://lore.kernel.org/r/67abaeaf.050a0220.110943.0041.GAE@google.com
> > > [2] https://lore.kernel.org/r/67a76f33.050a0220.3d72c.0028.GAE@google.com
> > >
> > > Acked-by: David Hildenbrand <david@redhat.com>
> > > Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > Signed-off-by: Jann Horn <jannh@google.com>
> > > ---
> > > Changes in v2:
> > > - applied akpm's fixup (use FOLIO_MAPPING_ANON, ...)
> > > - remove CONFIG_DEBUG_VM check and use folio_test_* helpers (David)
> > > - more verbose comment (Lorenzo)
> > > - replaced "page" mentions with "folio" in commit message
> > > - Link to v1: https://lore.kernel.org/r/20250724-anonvma-uaf-debug-v1-1-29989ddc4e2a@google.com
> > > ---
> >
> > A question; does it make sense to disable reuse of anon_vmas during
> > anon_vma_clone() to increase chances of detecting this? (of course,
> > for debugging-purpose only)
> 
> On the other hand, it would obviously cause more anon_vma's to get to
> refcount 0, so maybe it'd increase the prevelance of it.
> 
> However, we might actually be seeing the bug _because_ of anon_vma reuse :)
> at which point obviously it would not help increase prevelance... so we
> should keep behaviour as close to 'reality' as possible IMO.

That's fair enough. Agree with you that adding a new config option that
introduces behavior diverging from reality for debugging doesn't really
add much value - it may even prevent some bugs from being reported.

> Finally, I'm not in favour of introducing some special debug mode for this
> or changing this code to be arbitrarily disabled in existing debug modes -
> let's keep this change simple.

Sure. Thanks for the answer!

> Cheers, Lorenzo

-- 
Cheers,
Harry / Hyeonggon