Re: [PATCH 11/12] mm: make userland page table freeing RCU-safe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] try_collapse_pte_mapped_thp accesses page tables under mmap_read_lock without excluding per-VMA locks, allowing concurrent MADV_DONTNEED to clear PMDs, leading to a UAF/Panic.
- [High] Page walk callbacks return ACTION_AGAIN infinitely when encountering migration entries.
--

commit 2921a41442d23274b78e2a0ab1dcfe01ec2fff5f
Author: Lorenzo Stoakes (ARM) <ljs@kernel.org>
mm: make userland page table freeing RCU-safe

This commit removes the configuration option for RCU table freeing, as all
architectures now support it, and cleans up dead code. It establishes a
kernel policy that all page tables are freed after an RCU grace period,
ensuring safe unconditional page table walks under RCU.

> With this change in place the kernel policy is now that all page tables are
> freed after an RCU grace period, and thus it is now safe to unconditionally
> perform page table walks under RCU, safe in the knowledge that page tables
> will not be freed underneath the walker.
> 
> This is all that is guaranteed, however, so naturally it is still incumbent
> upon page table walkers to ensure that the page table entries are as
> expected.

[Severity: High]
This is a pre-existing issue, but will these page walk callbacks spin
forever if they encounter a migration entry?

For example, in fs/proc/task_mmu.c:clear_refs_pte_range():

	pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl);
	if (!pte) {
		walk->action = ACTION_AGAIN;
		return 0;
	}

Since pte_offset_map_lock() returns NULL non-transiently for migration
entries, walk_pmd_range() handles ACTION_AGAIN by looping back to again:
immediately without a scheduling point. Could an unprivileged user
triggering THP migration concurrently with a /proc/pid/clear_refs read
cause a CPU soft lockup here?

The same pattern appears to exist in smaps_pte_range() in the same file,
and queue_folios_pte_range() in mm/mempolicy.c.

[Severity: Critical]
This is a pre-existing issue, but does try_collapse_pte_mapped_thp() in
mm/khugepaged.c properly exclude per-VMA locks to prevent concurrent
MADV_DONTNEED from clearing PMDs?

When checking the page tables under mmap_read_lock(), the pmd_same() check
is nested inside the if (ptl != pml) block:

mm/khugepaged.c:try_collapse_pte_mapped_thp() {
	...
	/* step 4: remove empty page table */
	if (!pml) {
		pml = pmd_lock(mm, pmd);
		if (ptl != pml) {
			spin_lock_nested(ptl, SINGLE_DEPTH_NESTING);
			if (unlikely(!pmd_same(pgt_pmd, pmdp_get_lockless(pmd)))) {
				flush_tlb_mm(mm);
				goto unlock;
			}
		}
	}
	...
}

If the system has ptl == pml, this bypasses the pmd_same() check entirely.
A concurrent MADV_DONTNEED running under the VMA lock could trigger
PT_RECLAIM and clear the PMD, freeing the PTE page.

Later in the same function:

mm/khugepaged.c:try_collapse_pte_mapped_thp() {
	...
	pgt_pmd = pmdp_collapse_flush(vma, haddr, pmd);
	pmdp_get_lockless_sync();
	pte_unmap_unlock(start_pte, ptl);
	if (ptl != pml)
		spin_unlock(pml);

	mmu_notifier_invalidate_range_end(&range);

	mm_dec_nr_ptes(mm);
	page_table_check_pte_clear_range(mm, haddr, pgt_pmd);
	pte_free_defer(mm, pmd_pgtable(pgt_pmd));
	...
}

Does this result in pmdp_collapse_flush() being called on a cleared PMD,
passing a garbage PFN to pte_free_defer() and potentially causing a kernel
panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-rcu-pagetable-freeing-v1-0-5456a81c8212@kernel.org?part=11