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

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.

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.