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

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

[David Hildenbrand (Arm)]

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.

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;
+		}
 
 		_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;
 			} else {
 				ret = change_huge_pmd(tlb, vma, pmd,
 						addr, newprot, cp_flags);
@@ -554,7 +528,7 @@ static inline long change_pmd_range(struct mmu_gather *tlb,
 					goto next;
 				}
 			}
-			/* fall through, the trans huge pmd just split */
+			/* The PMD might have been split. */
 		}
 
 		ret = change_pte_range(tlb, vma, pmd, addr, next, newprot,

---

base-commit: a3d27154a205f6b32000e7e2734b2a6a4d22c65b

change-id: 20261107-uffd_shmem_marker-a0b9869d783c

--

Cheers,

David

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

[Lance Yang]

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