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

[Will Deacon]

On Thu, Jul 16, 2026 at 11:06:51AM +0100, Lorenzo Stoakes (ARM) wrote:
> On 2026-07-15 18:49 +0300, Mike Rapoport wrote:
> > On Wed, Jul 15, 2026 at 04:24:59PM +0100, Will Deacon wrote:
> > > > 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

Hmm, but that relies on all concurrent walkers taking cpa_lock, no? Surely
that's not generally the case.

> 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.

Up to you folks, but if you took the write lock during collapse wouldn't
that fix it for all concurrent walkers and we wouldn't need another patch?

I would argue that using the read lock here directly contradicts the
rationale for why this is safe, because this is an occurence of code
that operates on a range that it doesn't wholly own. It seems bizarre
to me to go to the effort of adding locking, but then not adding
sufficient locking for all concurrent walkers.

Will