Re: [PATCH mm-hotfixes v3 2/4] x86/mm/pat: acquire mmap lock on page table free to avoid ptdump UAF

[Lorenzo Stoakes (ARM)]

On 2026-07-15 18:49 +0300, Mike Rapoport wrote:
> Hi Will,
>
> On Wed, Jul 15, 2026 at 04:24:59PM +0100, Will Deacon wrote:
> > Hi Lorenzo,
> >
> > I'm certainly no x86 expert (quite the opposite!), but I was looking at
> > this with Adrian and got myself confused. See below.
> >
> > On Tue, Jul 14, 2026 at 06:24:24PM +0100, Lorenzo Stoakes wrote:
> > > x86 implements page attribute modification using its Change Page
> > > Attributes (CPA) mechanism.
> > >
> > > This tracks properties of ranges such as cache mode through x86 page
> > > attributes, and as part of that logic manipulates kernel page tables.
> > >
> > > Since commit 41d88484c71c ("x86/mm/pat: restore large ROX pages after
> > > fragmentation") ranges of kernel page table entries can be collapsed into
> > > huge page table entries as part of this logic.
> > >
> > > As part of this collapse, it frees the page tables which the collapsed
> > > entries previously pointed to, and it does so without any relevant locks
> > > being held to preclude concurrent kernel page table walkers.
> > >
> > > The only way this code can be reached is if CPA_COLLAPSE is specified, and
> > > this is only set in set_memory_rox() via:
> > >
> > > set_memory_rox()
> > > -> change_page_attr_set_clr()
> > > -> cpa_flush()
> > > -> cpa_collapse_large_pages()
> > >
> > > Notable users of this are execmem and bpf when manipulating executable
> > > mappings.
> > >
> > > However, this is problematic for ptdump as it walks ranges it does not own
> > > and thus runs the risk of a use-after-free on page tables freed underneath
> > > it.
> > >
> > > Resolve the issue by acquiring the mmap read lock on init_mm which prevents
> > > a concurrent ptdump as it acquires the write lock.
> > >
> > > It is safe to acquire a sleeping lock as all the callers invoke
> > > set_memory_rox() from process context and in any case,
> > > change_page_attr_set_clr() calls vm_unmap_alias() which ultimately takes a
> > > mutex, disallowing atomic context here.
> > >
> > > Fixes: 41d88484c71c ("x86/mm/pat: restore large ROX pages after fragmentation")
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Reviewed-by: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
> > > Reviewed-by: Kiryl Shutsemau (Meta) <kas@xxxxxxxxxx>
> > > Signed-off-by: Lorenzo Stoakes <ljs@xxxxxxxxxx>
> > > ---
> > >  arch/x86/mm/pat/set_memory.c | 14 +++++++++++---
> > >  1 file changed, 11 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
> > > index d023a40a1e03..4c4b8244502f 100644
> > > --- a/arch/x86/mm/pat/set_memory.c
> > > +++ b/arch/x86/mm/pat/set_memory.c
> > > @@ -22,6 +22,7 @@
> > >  #include <linux/cc_platform.h>
> > >  #include <linux/set_memory.h>
> > >  #include <linux/memregion.h>
> > > +#include <linux/cleanup.h>
> > >
> > >  #include <asm/e820/api.h>
> > >  #include <asm/processor.h>
> > > @@ -436,9 +437,16 @@ static void cpa_collapse_large_pages(struct cpa_data *cpa)
> > >
> > >  	flush_tlb_all();
> > >
> > > -	list_for_each_entry_safe(ptdesc, tmp, &pgtables, pt_list) {
> > > -		list_del(&ptdesc->pt_list);
> > > -		pagetable_free(ptdesc);
> > > +	/*
> > > +	 * ptdump might read these page tables, so avoid a use-after-free by
> > > +	 * acquiring the mmap read lock on init_mm (ptdump acquires the mmap
> > > +	 * write lock).
> > > +	 */
> > > +	scoped_guard(mmap_read_lock, &init_mm) {
> > > +		list_for_each_entry_safe(ptdesc, tmp, &pgtables, pt_list) {
> > > +			list_del(&ptdesc->pt_list);
> > > +			pagetable_free(ptdesc);
> > > +		}
> >
> > As I understand it, the argument for taking the read lock is that we're
> > operating on a region that we "wholly own" and therefore we can happily
> > run concurrently with CPUs walking distinct parts of the page-table.
> > However, from what I can tell, the CPA collapse logic will operate on
> > regions outside of the address range being manipulated by its caller
> > because it rounds up to the PMD size.
> >
> > As a made-up example, imagine I have a 2MiB aligned region where the
> > first 1MiB is read-only and the second 1MiB is in the default r/w state.
> > If one CPU calls set_memory_rw() on the first 1MiB while another CPU is
> > walking the second 1MiB (via some other API that doesn't take cpa_lock),
> > it looks to me like the first CPU can collapse the page-table and free
> > the unused pages under the feet of the other CPU. What prevents that
> > from happening?
>
> Nothing, and there's a patch to fix that that synchronizes
> cpa_collapse_large_pages() using cpa_lock:
>
> https://lore.kernel.org/all/20260626163213.2284080-1-den@xxxxxxxxxx

Thanks Mike!

>
> This still won't be enough to sync with ptdump though.

Yeah, so this patch is still necessary.

But that patch conflicts with this one as it holds a spin lock over the page
table freeing, which prevents taking an rwsem... :/

Anyway I think this one is still fine as it seems there's not a consensus over
there as there was discussion about just removing the locking anyway ([0])?

So can kick that can down the road and just get these ptdump bugs fixed :)

>
> > If all concurrent walkers have interrupts disabled, I guess the TLB
> > invalidation logic would do it, but it would be good to call this out in
> > the commit message because it's not clear to me why the read_lock is
> > sufficient for the collapsing case.

Yeah, so this patch is _only_ fixing the ptdump case. Any existing bug must be
addressed separately.

> >
> > Cheers,
> >
> > Will
>
> --
> Sincerely yours,
> Mike.
>

Cheers, Lorenzo

[0]:https://lore.kernel.org/all/aab44f08-89f8-47fe-bee4-0ab6b25968c6@xxxxxxxxx/