[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