[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aIg0z3ItBtlMZQAI@hyeyoo>
Date: Tue, 29 Jul 2025 11:41:19 +0900
From: Harry Yoo <harry.yoo@...cle.com>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: Jann Horn <jannh@...gle.com>, Andrew Morton <akpm@...ux-foundation.org>,
David Hildenbrand <david@...hat.com>, Rik van Riel <riel@...riel.com>,
"Liam R. Howlett" <Liam.Howlett@...cle.com>,
Vlastimil Babka <vbabka@...e.cz>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] mm/rmap: Add anon_vma lifetime debug check
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@...hat.com>
> > > Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> > > Signed-off-by: Jann Horn <jannh@...gle.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
Powered by blists - more mailing lists