[PATCH v2] x86/mm: Drop the page allocation from pud_free_pmd_page()

[PATCH v2] x86/mm: Drop the page allocation from pud_free_pmd_page()

[Mikhail Gavrilov]

On a box with a discrete GPU, lockdep reports a possible deadlock as soon
as kswapd shrinks the TTM page pool:

  WARNING: possible circular locking dependency detected
  7.3.0-rc3-f6e7b42bf05b+ #183 Tainted: G     U
  ------------------------------------------------------
  kswapd0/269 is trying to acquire lock:
  ((init_mm).mmap_lock){++++}-{4:4}, at: change_page_attr_set_clr+0x29a/0x4a0
  but task is already holding lock:
  (pool_shrink_rwsem){.+.+}-{4:4}, at: ttm_pool_shrink+0xb2/0x330 [ttm]
  Chain exists of:
    (init_mm).mmap_lock --> fs_reclaim --> pool_shrink_rwsem

The cycle is built from three edges:

  1) pool_shrink_rwsem -> (init_mm).mmap_lock

     The TTM shrinker restores the caching attribute of every page it
     frees, while holding pool_shrink_rwsem:

       ttm_pool_shrink()
       -> ttm_pool_dispose_list()
       -> ttm_pool_free_page()
       -> set_pages_wb()
       -> change_page_attr_set_clr()   [ init_mm mmap read lock ]

  2) fs_reclaim -> pool_shrink_rwsem

     The same shrinker, called from reclaim.

  3) (init_mm).mmap_lock -> fs_reclaim

     ioremap() installing a huge PUD mapping over an existing PMD table:

       ioremap_page_range()
       -> vmap_range_noflush()
       -> vmap_try_huge_pud()          [ init_mm mmap read lock ]
       -> pud_free_pmd_page()
       -> __get_free_page(GFP_KERNEL)  [ enters reclaim ]

Edge 3 is the one that should not exist.  Now that the attribute-change
path takes the init_mm mmap lock, reclaim can acquire it, so the lock
must not be held over an allocation which can enter reclaim.  CPA itself
follows this rule: split_large_page() drops the lock around
pagetable_alloc().  The huge vmap path, which has held the same lock
since commit 26444eb71465
("mm/vmalloc: acquire init_mm lock on huge vmap to avoid ptdump UAF"),
does not: pud_free_pmd_page() allocates a scratch page underneath it.

That page does not need to exist.  It only holds a copy of the PMD
entries, so that they can be cleared before the PUD is.  But the PMD
table itself is freed after pud_clear() and the flush, so the code
already relies on the table being out of reach of the page walker at
that point - and if it is safe to free it then, it is safe to read it
then.  Nobody else writes to it either: vmap_try_huge_pud() only gets
here for a range covering the whole PUD, and ptdump is kept out by the
init_mm lock the caller holds.

So clear the PUD, flush, and free the PTE tables straight from the
detached PMD table - the same order pmd_free_pte_page() uses one level
down.  With no allocation left the cycle is gone, and so is the only way
this function could fail.

The copy came with commit 5e0fb5df2ee8
("x86/mm: Add TLB purge to free pmd/pte page interfaces"), whose
changelog explains the flush but not the copy; the allocation itself was
already questioned in review back then [1].  The same lock cycle was
also reported from the i915 shrinker, with &vm->mutex in place of
pool_shrink_rwsem [2].

Fixes: d5d8b8662e6e ("x86/mm/pat: Acquire init_mm read lock on attribute changes to avoid UAF")
Suggested-by: Pedro Falcato <pfalcato@suse.de>
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/20180529144438.GM18595@8bytes.org # [1]
Link: https://lore.kernel.org/80993b70-352f-4069-84c7-39a04c061e98@intel.com # [2]
Link: https://lore.kernel.org/20260916062222.27347-1-mikhail.v.gavrilov@gmail.com
---

v2:
 - Drop the scratch page instead of allocating it with GFP_NOWAIT: the
   PMD table can be read after pud_clear() and the flush (Pedro Falcato)
 - Capitalise the subject per tip conventions
v1: https://lore.kernel.org/20260916062222.27347-1-mikhail.v.gavrilov@gmail.com

Tested on a Ryzen 9 7950X with a Radeon RX 7900 XTX (Navi 31), lockdep
and KASAN enabled.  Reproducer, on a lockdep kernel with a TTM driver
bound and non-zero wc/uc rows in /sys/kernel/debug/ttm/page_pool:

  # cat /sys/kernel/debug/ttm/page_pool_shrink

This runs the TTM shrinker with fs_reclaim held.  Unpatched
(7.3-rc3, f6e7b42bf05b) it produces the report above on demand.

With this patch (7.3-rc4, fe2ec83746e5): a boot-time kprobe on
pud_free_pmd_page() recorded one call from a udev worker during boot -
in the unpatched kernel's lockdep reports the same function is entered
from amdgpu_ttm_init() -> ioremap_page_range(), so this box reaches the
changed path without instrumentation.  In that same boot the reproducer
freed 354 write-combined pages through set_pages_wb(), with no report
and debug_locks still 1 afterwards.

 arch/x86/mm/pgtable.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index cb03f5a2b243..6a338d6e80e9 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -712,40 +712,31 @@ int pmd_clear_huge(pmd_t *pmd)
  *
  * Context: The PUD range has been unmapped and TLB purged.
  * Return: 1 if clearing the entry succeeded. 0 otherwise.
- *
- * NOTE: Callers must allow a single page allocation.
  */
 int pud_free_pmd_page(pud_t *pud, unsigned long addr)
 {
-	pmd_t *pmd, *pmd_sv;
+	pmd_t *pmd;
 	struct ptdesc *pt;
 	int i;
 
 	pmd = pud_pgtable(*pud);
-	pmd_sv = (pmd_t *)__get_free_page(GFP_KERNEL);
-	if (!pmd_sv)
-		return 0;
-
-	for (i = 0; i < PTRS_PER_PMD; i++) {
-		pmd_sv[i] = pmd[i];
-		if (!pmd_none(pmd[i]))
-			pmd_clear(&pmd[i]);
-	}
 
 	pud_clear(pud);
 
 	/* INVLPG to clear all paging-structure caches */
 	flush_tlb_kernel_range(addr, addr + PAGE_SIZE-1);
 
+	/*
+	 * The PMD table can no longer be walked, but it is still allocated:
+	 * free the PTE tables straight from it, then the table itself.
+	 */
 	for (i = 0; i < PTRS_PER_PMD; i++) {
-		if (!pmd_none(pmd_sv[i])) {
-			pt = page_ptdesc(pmd_page(pmd_sv[i]));
+		if (!pmd_none(pmd[i])) {
+			pt = page_ptdesc(pmd_page(pmd[i]));
 			pagetable_dtor_free(pt);
 		}
 	}
 
-	free_page((unsigned long)pmd_sv);
-
 	pmd_free(&init_mm, pmd);
 
 	return 1;
-- 
2.55.0

Re: [PATCH v2] x86/mm: Drop the page allocation from pud_free_pmd_page()

[Dave Hansen]

On 9/23/26 15:31, Mikhail Gavrilov wrote:
> So clear the PUD, flush, and free the PTE tables straight from the
> detached PMD table - the same order pmd_free_pte_page() uses one level
> down.  With no allocation left the cycle is gone, and so is the only way
> this function could fail.

Yeah, I basically agree with your analysis and fix.

There are a few things we need to figure out like having
pmd_free_pte_page() return 'int' when it doesn't ever fail, the
comments, and the reverse Christmas tree.

I'll dig into it a bit tomorrow and see what I come up with.

Re: [PATCH v2] x86/mm: Drop the page allocation from pud_free_pmd_page()

[Mikhail Gavrilov]

On 9/23/26 16:07, Dave Hansen wrote:
> There are a few things we need to figure out like having
> pmd_free_pte_page() return 'int' when it doesn't ever fail, the
> comments, and the reverse Christmas tree.

One data point for the return value: the x86-PAE pmd_free_pte_page()
does fail.  It returns pmd_none(*pmd), and vmap_try_huge_pmd() relies
on that to keep ioremap() from replacing an existing PTE table there
(the comment above it is about the sync'd PMD entries).  Huge PMD vmap
is enabled on PAE whenever the CPU has PSE.

pud_free_pmd_page() has no such case: the PUD level is 64-bit only on
x86, and with v2 applied the x86-64 version, like the arm64, riscv and
powerpc ones, always returns 1.

> I'll dig into it a bit tomorrow and see what I come up with.

Thanks.  This box reaches pud_free_pmd_page() on every boot I have
checked, so I can run whatever you come up with through the same
boot-time kprobe and reproducer check.

Re: [PATCH v2] x86/mm: Drop the page allocation from pud_free_pmd_page()

[Dave Hansen]

On 9/23/26 22:10, Mikhail Gavrilov wrote:
> On 9/23/26 16:07, Dave Hansen wrote:
>> There are a few things we need to figure out like having
>> pmd_free_pte_page() return 'int' when it doesn't ever fail, the
>> comments, and the reverse Christmas tree.
> 
> One data point for the return value: the x86-PAE pmd_free_pte_page()
> does fail.  It returns pmd_none(*pmd), and vmap_try_huge_pmd() relies
> on that to keep ioremap() from replacing an existing PTE table there
> (the comment above it is about the sync'd PMD entries).  Huge PMD vmap
> is enabled on PAE whenever the CPU has PSE.

Oh, that's just kinda silly. We should just zap that version and huge
PMD vmap on 32-bit.

> pud_free_pmd_page() has no such case: the PUD level is 64-bit only on
> x86, and with v2 applied the x86-64 version, like the arm64, riscv and
> powerpc ones, always returns 1.

Yeah, all the ones for CPUs from this millennium are OK.

>> I'll dig into it a bit tomorrow and see what I come up with.
> 
> Thanks.  This box reaches pud_free_pmd_page() on every boot I have
> checked, so I can run whatever you come up with through the same
> boot-time kprobe and reproducer check.

I think I'll just take your version since you actually, you know,
compiled it and all. ;)

Re: [PATCH v2] x86/mm: Drop the page allocation from pud_free_pmd_page()

[Pedro Falcato]

On Thu, Sep 24, 2026 at 03:31:16AM +0500, Mikhail Gavrilov wrote:
> On a box with a discrete GPU, lockdep reports a possible deadlock as soon
> as kswapd shrinks the TTM page pool:
> 
>   WARNING: possible circular locking dependency detected
>   7.3.0-rc3-f6e7b42bf05b+ #183 Tainted: G     U
>   ------------------------------------------------------
>   kswapd0/269 is trying to acquire lock:
>   ((init_mm).mmap_lock){++++}-{4:4}, at: change_page_attr_set_clr+0x29a/0x4a0
>   but task is already holding lock:
>   (pool_shrink_rwsem){.+.+}-{4:4}, at: ttm_pool_shrink+0xb2/0x330 [ttm]
>   Chain exists of:
>     (init_mm).mmap_lock --> fs_reclaim --> pool_shrink_rwsem
> 
> The cycle is built from three edges:
> 
>   1) pool_shrink_rwsem -> (init_mm).mmap_lock
> 
>      The TTM shrinker restores the caching attribute of every page it
>      frees, while holding pool_shrink_rwsem:
> 
>        ttm_pool_shrink()
>        -> ttm_pool_dispose_list()
>        -> ttm_pool_free_page()
>        -> set_pages_wb()
>        -> change_page_attr_set_clr()   [ init_mm mmap read lock ]
> 
>   2) fs_reclaim -> pool_shrink_rwsem
> 
>      The same shrinker, called from reclaim.
> 
>   3) (init_mm).mmap_lock -> fs_reclaim
> 
>      ioremap() installing a huge PUD mapping over an existing PMD table:
> 
>        ioremap_page_range()
>        -> vmap_range_noflush()
>        -> vmap_try_huge_pud()          [ init_mm mmap read lock ]
>        -> pud_free_pmd_page()
>        -> __get_free_page(GFP_KERNEL)  [ enters reclaim ]
> 
> Edge 3 is the one that should not exist.  Now that the attribute-change
> path takes the init_mm mmap lock, reclaim can acquire it, so the lock
> must not be held over an allocation which can enter reclaim.  CPA itself
> follows this rule: split_large_page() drops the lock around
> pagetable_alloc().  The huge vmap path, which has held the same lock
> since commit 26444eb71465
> ("mm/vmalloc: acquire init_mm lock on huge vmap to avoid ptdump UAF"),
> does not: pud_free_pmd_page() allocates a scratch page underneath it.
> 
> That page does not need to exist.  It only holds a copy of the PMD
> entries, so that they can be cleared before the PUD is.  But the PMD
> table itself is freed after pud_clear() and the flush, so the code
> already relies on the table being out of reach of the page walker at
> that point - and if it is safe to free it then, it is safe to read it
> then.  Nobody else writes to it either: vmap_try_huge_pud() only gets
> here for a range covering the whole PUD, and ptdump is kept out by the
> init_mm lock the caller holds.
> 
> So clear the PUD, flush, and free the PTE tables straight from the
> detached PMD table - the same order pmd_free_pte_page() uses one level
> down.  With no allocation left the cycle is gone, and so is the only way
> this function could fail.
> 
> The copy came with commit 5e0fb5df2ee8
> ("x86/mm: Add TLB purge to free pmd/pte page interfaces"), whose
> changelog explains the flush but not the copy; the allocation itself was
> already questioned in review back then [1].  The same lock cycle was
> also reported from the i915 shrinker, with &vm->mutex in place of
> pool_shrink_rwsem [2].
> 
> Fixes: d5d8b8662e6e ("x86/mm/pat: Acquire init_mm read lock on attribute changes to avoid UAF")
> Suggested-by: Pedro Falcato <pfalcato@suse.de>
> Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>

Reviewed-by: Pedro Falcato <pfalcato@suse.de>

Thanks for the fix!

-- 
Pedro

Re: [PATCH v2] x86/mm: Drop the page allocation from pud_free_pmd_page()

[Dave Hansen]

OK, I've got a version of Mikhail's fix with a changelog and comments
I've munged a bit:

> https://git.kernel.org/pub/scm/linux/kernel/git/daveh/devel.git/log/?h=pmdfree

Does that look OK to everyone?

Re: [PATCH v2] x86/mm: Drop the page allocation from pud_free_pmd_page()

[Pedro Falcato]

On Thu, Sep 24, 2026 at 10:21:03AM -0700, Dave Hansen wrote:
> OK, I've got a version of Mikhail's fix with a changelog and comments
> I've munged a bit:
> 
> > https://git.kernel.org/pub/scm/linux/kernel/git/daveh/devel.git/log/?h=pmdfree
> 
> Does that look OK to everyone?


LGTM. This is where we find out if we have a solid understanding of x86
paging caching guarantees :)

-- 
Pedro

Re: [PATCH v2] x86/mm: Drop the page allocation from pud_free_pmd_page()

[Mikhail Gavrilov]

On 9/24/26 10:21, Dave Hansen wrote:
> OK, I've got a version of Mikhail's fix with a changelog and comments
> I've munged a bit:
>
>> https://git.kernel.org/pub/scm/linux/kernel/git/daveh/devel.git/log/?h=pmdfree
>
> Does that look OK to everyone?

Looks good to me, thanks.  The code is v2 with your comments and the
declarations reordered, so the testing done for v2 still applies.

Two small things, both optional:

 - "The new mmap_read_lock() is held over a GFP_KERNEL allocation":
   the lock held over the allocation is the same init_mm lock, but
   taken in vmap_try_huge_pud(), not the new one in CPA.  Maybe "The
   huge vmap code holds the same lock over a GFP_KERNEL allocation,
   which is a no-no now that reclaim can take it."

 - The kernel-doc lost its "Return:" line while the function still
   returns int, so kernel-doc -Wreturn will complain about it.