Re: [PATCH] mm/mprotect: don't skip uffd marker install when racing with PTE table reclaim

[Lance Yang <lance.yang@linux.dev>]

On Sat, Nov 07, 2026 at 12:28:55AM +0100, David Hildenbrand (Arm) wrote:
>change_pmd_range() populates PTE tables before installing uffd-wp
>markers. An empty PTE table can be removed after change_pmd_prepare()
>returns but before the pmd_none() check.
>
>That check then skips the range, allowing UFFDIO_WRITEPROTECT to report
>success without installing the required markers. Subsequent writes by
>userspace to these areas will not trigger uffd events.
>
>PTE table reclaim during MADV_DONTNEED and page table retraction during
>khugepaged can trigger this.
>
>Retry page-table allocation in that case. Simplify the code, getting
>rid of change_pmd_prepare() by routing all code through the "again" label.

Cool, nice work1

>It's a small race though, so likely this is not very urgent.
>
>Fixes: fe2567eb5521 ("mm/shmem: allow uffd wr-protect none pte for file-backed mem")
>Fixes: 6375e95f381e ("mm: pgtable: reclaim empty PTE page in madvise(MADV_DONTNEED)")
>Cc: stable@vger.kernel.org
>Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
>---
>I expect Andrew to pick this up.
>---
> mm/mprotect.c | 48 +++++++++++-------------------------------------
> 1 file changed, 11 insertions(+), 37 deletions(-)
>
>diff --git a/mm/mprotect.c b/mm/mprotect.c
>index e18d4e12fc341..ef4e7188e24da 100644
>--- a/mm/mprotect.c
>+++ b/mm/mprotect.c
>@@ -470,25 +470,8 @@ pgtable_populate_needed(struct vm_area_struct *vma, unsigned long cp_flags)
> }
> 
> /*
>- * Populate the pgtable underneath for whatever reason if requested.
>- * When {pte|pmd|...}_alloc() failed we treat it the same way as pgtable
>- * allocation failures during page faults by kicking OOM and returning
>- * error.
>- */
>-#define  change_pmd_prepare(vma, pmd, cp_flags)				\
>-	({								\
>-		long err = 0;						\
>-		if (unlikely(pgtable_populate_needed(vma, cp_flags))) {	\
>-			if (pte_alloc(vma->vm_mm, pmd))			\
>-				err = -ENOMEM;				\
>-		}							\
>-		err;							\
>-	})
>-
>-/*
>- * This is the general pud/p4d/pgd version of change_pmd_prepare(). We need to
>- * have separate change_pmd_prepare() because pte_alloc() returns 0 on success,
>- * while {pmd|pud|p4d}_alloc() returns the valid pointer on success.
>+ * Populate the lower-level page table if marker installation requires it.
>+ * Return -ENOMEM on allocation failure.
>  */
> #define  change_prepare(vma, high, low, addr, cp_flags)			\
> 	  ({								\
>@@ -517,30 +500,21 @@ static inline long change_pmd_range(struct mmu_gather *tlb,
> again:
> 		next = pmd_addr_end(addr, end);
> 
>-		ret = change_pmd_prepare(vma, pmd, cp_flags);
>-		if (ret) {
>-			pages = ret;
>-			break;
>-		}
>-
>-		if (pmd_none(*pmd))
>+		if (unlikely(pgtable_populate_needed(vma, cp_flags))) {
>+			if (pte_alloc(vma->vm_mm, pmd)) {
>+				pages = -ENOMEM;
>+				break;
>+			}
>+		} else if (pmd_none(*pmd)) {
> 			goto next;
>+		}

Previously, reclaim could clear the PMD after allocation and we'd just
skip the range without installing the markers ...

Now it's change_pte_range() -> -EAGAIN -> goto again (retry the same range)

And yeah, that should do the trick ;)

> 
> 		_pmd = pmdp_get_lockless(pmd);
> 		if (pmd_is_huge(_pmd)) {
> 			if ((next - addr != HPAGE_PMD_SIZE) ||
> 			    pgtable_split_needed(vma, cp_flags)) {
> 				__split_huge_pmd(vma, pmd, addr, false);
>-				/*
>-				 * For file-backed, the pmd could have been
>-				 * cleared; make sure pmd populated if
>-				 * necessary, then fall-through to pte level.
>-				 */
>-				ret = change_pmd_prepare(vma, pmd, cp_flags);
>-				if (ret) {
>-					pages = ret;
>-					break;
>-				}
>+				goto again;

And, after the split, we just go back to again -> allocate a PTE table
if needed -> recheck the PMD.

So, no need for that second change_pmd_prepare() ANYMORE :D

LGTM!

Reviewed-by: Lance Yang <lance.yang@linux.dev>

Cheers, Lance