[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <23b583fc-e98e-48f8-bc8d-fbf7b47a188c@lucifer.local>
Date: Mon, 28 Jul 2025 05:33:34 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Harry Yoo <harry.yoo@...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 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
> > ---
>
> Oops, I'm late to the party.
Isn't this the fashionable time to turn up? :P
>
> 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)
This won't impact this issue that much AFAICT, as we only reuse an anon_vma
if it has a refcount == 1 and for that to be the case it has to be have
children >= 1.
We'd then have to rely on this bug triggering by this firing when the child
no longer references it but then it is dereffed, but we're seeing the bug
now so presumably it's not required.
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.
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.
Cheers, Lorenzo
Powered by blists - more mailing lists