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

On Thu, Jul 16, 2026 at 11:06:51AM +0100, Lorenzo Stoakes (ARM) wrote:

On 2026-07-15 18:49 +0300, Mike Rapoport wrote:

quoted

On Wed, Jul 15, 2026 at 04:24:59PM +0100, Will Deacon wrote:

quoted

quoted

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@openvz.org (local)

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

Thanks Mike!

quoted

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 :)

quoted

quoted

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