[PATCH 0/2] mm/userfaultfd: prevent poison marker loss

[PATCH 0/2] mm/userfaultfd: prevent poison marker loss

[David Hildenbrand (Arm)]

Now that Lorenzo's atomic vma flag fixes are in, we can also use it
for userfaultfd POISON markers.

In patch #1, we use a new VM_MAYBE_UFFD_POISON to remember that we had
POISON markers installed by uffd. We then use that to fix marker
handling during fork(), during hugetlb PMD table sharing, and during
retract_page_tables().

In patch #2, we perform a similar rework to retract_page_tables()[1] that
we did to try_collapse_pte_mapped_thp(), to make the code simpler and
less error-prone.

Once review finished, I will take this through core.git, where it
depends on a related try_collapse_pte_mapped_thp() cleanup/fix.

This is not urgent, just something that should be fixed properly.

[1] https://lore.kernel.org/linux-mm/20261031-fixes_core-v3-18-d6437171ecc3@kernel.org/

Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
---
David Hildenbrand (Arm) (2):
      mm/userfaultfd: prevent poison marker loss
      mm/khugepaged: check for empty PTE tables before retracting

 Documentation/filesystems/proc.rst |   1 +
 fs/proc/task_mmu.c                 |   3 +
 include/linux/mm.h                 |  45 ++++++++-----
 include/linux/userfaultfd_k.h      |   7 ++-
 include/trace/events/mmflags.h     |   9 ++-
 mm/Kconfig                         |   1 +
 mm/khugepaged.c                    | 126 +++++++++++++------------------------
 mm/userfaultfd.c                   |   3 +
 tools/testing/vma/include/dup.h    |  20 ++++--
 9 files changed, 110 insertions(+), 105 deletions(-)

---

base-commit: df3c65887715e66e2cba69116924020c37996cd8

change-id: 20261106-b4-uffd_marker_loss-f0f56ec12ce6

--

Cheers,

David

[PATCH 1/2] mm/userfaultfd: prevent poison marker loss

[David Hildenbrand (Arm)]

Unregistering userfaultfd leaves poison markers in the page tables.
However, once unregistered, hugetlb PMD tables can be shared again.

Further, shmem page-table retraction can discard poison markers even
while registered in missing mode, and fork() can skip copying page
tables containing poison markers.

To fix it, add VM_MAYBE_UFFD_POISON and set it before installing poison
markers. Preserve it across unregistration, splits, merges and fork(),
just like we do with guard markers.

Use the new atomic VMA flag to disable PMD sharing and page-table
retraction, and to force page-table copying during fork().

Move VM_UFFD_WP to bit 44 and make HAVE_ARCH_USERFAULTFD_WP depend on
64BIT, freeing bit 12 for the new flag on both 32-bit and 64-bit systems.
Expose the flag as "up" in smaps.

Losing poison markers, while unpleasant, is not really fatal, and the
use case is rather a niche thing. In addition, we rely on the atomic
VMA flags infrastructure ... so let's not CC stable.

Fixes: fc71884a5f59 ("mm: userfaultfd: add new UFFDIO_POISON ioctl")
Fixes: 8a13897fb0da ("mm: userfaultfd: support UFFDIO_POISON for hugetlbfs")
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
---
 Documentation/filesystems/proc.rst |  1 +
 fs/proc/task_mmu.c                 |  3 +++
 include/linux/mm.h                 | 45 ++++++++++++++++++++++++--------------
 include/linux/userfaultfd_k.h      |  7 ++++--
 include/trace/events/mmflags.h     |  9 +++++++-
 mm/Kconfig                         |  1 +
 mm/khugepaged.c                    |  9 ++++----
 mm/userfaultfd.c                   |  3 +++
 tools/testing/vma/include/dup.h    | 20 ++++++++++++-----
 9 files changed, 70 insertions(+), 28 deletions(-)

diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst
index fa7a468e8edf2..bcbcb41814d96 100644
--- a/Documentation/filesystems/proc.rst
+++ b/Documentation/filesystems/proc.rst
@@ -614,6 +614,7 @@ encoded manner. The codes are the following:
     lf    lock on fault pages
     dp    always lazily freeable mapping
     gu    maybe contains guard regions (if not set, definitely doesn't)
+    up    maybe contains poison markers installed by userfaultfd
     ==    =============================================================
 
 Note that there is no guarantee that every flag and associated mnemonic will
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index e3d894ed2b913..d7f9934d0e499 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1106,6 +1106,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
 		[ilog2(VM_GROWSDOWN)]	= "gd",
 		[ilog2(VM_PFNMAP)]	= "pf",
 		[ilog2(VM_MAYBE_GUARD)]	= "gu",
+		[ilog2(VM_MAYBE_UFFD_POISON)]	= "up",
 		[ilog2(VM_LOCKED)]	= "lo",
 		[ilog2(VM_IO)]		= "io",
 		[ilog2(VM_SEQ_READ)]	= "sr",
@@ -1131,7 +1132,9 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
 		[ilog2(VM_NOHUGEPAGE)]	= "nh",
 		[ilog2(VM_MERGEABLE)]	= "mg",
 		[ilog2(VM_UFFD_MISSING)]= "um",
+#ifdef CONFIG_HAVE_ARCH_USERFAULTFD_WP
 		[ilog2(VM_UFFD_WP)]	= "uw",
+#endif
 #ifdef CONFIG_ARM64_MTE
 		[ilog2(VM_MTE)]		= "mt",
 		[ilog2(VM_MTE_ALLOWED)]	= "",
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 6a34dd3800f56..6cd9d86f7adac 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -302,7 +302,7 @@ enum {
 	/* Page-ranges managed without "struct page", just pure PFN */
 	DECLARE_VMA_BIT(PFNMAP, 10),
 	DECLARE_VMA_BIT(MAYBE_GUARD, 11),
-	DECLARE_VMA_BIT(UFFD_WP, 12),	/* wrprotect pages tracking */
+	DECLARE_VMA_BIT(MAYBE_UFFD_POISON, 12),
 	DECLARE_VMA_BIT(LOCKED, 13),
 	DECLARE_VMA_BIT(IO, 14),	/* Memory mapped I/O or similar */
 	DECLARE_VMA_BIT(SEQ_READ, 15),	/* App will access data sequentially */
@@ -346,6 +346,7 @@ enum {
 	DECLARE_VMA_BIT(UFFD_MINOR, 41),
 	DECLARE_VMA_BIT(SEALED, 42),
 	DECLARE_VMA_BIT(UFFD_RWP, 43),
+	DECLARE_VMA_BIT(UFFD_WP, 44),	/* wrprotect pages tracking */
 	/* Flags that reuse flags above. */
 	DECLARE_VMA_BIT_ALIAS(PKEY_BIT0, HIGH_ARCH_0),
 	DECLARE_VMA_BIT_ALIAS(PKEY_BIT1, HIGH_ARCH_1),
@@ -406,7 +407,12 @@ enum {
 #endif
 #define VM_PFNMAP	INIT_VM_FLAG(PFNMAP)
 #define VM_MAYBE_GUARD	INIT_VM_FLAG(MAYBE_GUARD)
+#define VM_MAYBE_UFFD_POISON	INIT_VM_FLAG(MAYBE_UFFD_POISON)
+#ifdef CONFIG_HAVE_ARCH_USERFAULTFD_WP
 #define VM_UFFD_WP	INIT_VM_FLAG(UFFD_WP)
+#else
+#define VM_UFFD_WP	VM_NONE
+#endif
 #define VM_LOCKED	INIT_VM_FLAG(LOCKED)
 #define VM_IO		INIT_VM_FLAG(IO)
 #define VM_SEQ_READ	INIT_VM_FLAG(SEQ_READ)
@@ -502,13 +508,17 @@ enum {
 #endif
 
 /*
- * vma_flags_t masks for the userfaultfd VMA flags. The two high-bit modes are
- * gated on the same configs as their VM_* flags above -- both of which imply
+ * vma_flags_t masks for the userfaultfd VMA flags. The three high-bit modes are
+ * gated on the same configs as their VM_* flags above -- all of which imply
  * 64BIT -- so an out-of-range bit is never fed to mk_vma_flags() on a build
  * whose bitmap cannot hold it.
  */
 #define VMA_UFFD_MISSING	mk_vma_flags(VMA_UFFD_MISSING_BIT)
+#ifdef CONFIG_HAVE_ARCH_USERFAULTFD_WP
 #define VMA_UFFD_WP		mk_vma_flags(VMA_UFFD_WP_BIT)
+#else
+#define VMA_UFFD_WP		EMPTY_VMA_FLAGS
+#endif
 #ifdef CONFIG_HAVE_ARCH_USERFAULTFD_MINOR
 #define VMA_UFFD_MINOR		mk_vma_flags(VMA_UFFD_MINOR_BIT)
 #else
@@ -602,7 +612,8 @@ enum {
 #define VMA_LOCKED_MASK	mk_vma_flags(VMA_LOCKED_BIT, VMA_LOCKONFAULT_BIT)
 
 /* These flags can be updated atomically via VMA/mmap read lock. */
-#define VMA_ATOMIC_SET_ALLOWED mk_vma_flags(VMA_MAYBE_GUARD_BIT)
+#define VMA_ATOMIC_SET_ALLOWED mk_vma_flags(VMA_MAYBE_GUARD_BIT, \
+					    VMA_MAYBE_UFFD_POISON_BIT)
 
 /* Arch-specific flags to clear when updating VM flags on protection change */
 #ifndef VM_ARCH_CLEAR
@@ -620,16 +631,16 @@ enum {
  *                       merged VMA should be considered soft-dirty also as it
  *                       operates at a VMA granularity.
  *
- * VMA_MAYBE_GUARD_BIT - If a VMA may have guard regions in place it implies
- *                       that mapped page tables may contain metadata not
- *                       described by the VMA and thus any merged VMA may also
- *                       contain this metadata, and thus we must make this flag
- *                       sticky.
+ * VMA_MAYBE_GUARD_BIT / VMA_MAYBE_UFFD_POISON_BIT - Page tables may contain guard
+ *                       or poison markers not described by the VMA. Any
+ *                       merged VMA may also contain this metadata, so these
+ *                       flags must be sticky.
  */
 #ifdef CONFIG_MEM_SOFT_DIRTY
-#define VMA_STICKY_FLAGS mk_vma_flags(VMA_SOFTDIRTY_BIT, VMA_MAYBE_GUARD_BIT)
+#define VMA_STICKY_FLAGS mk_vma_flags(VMA_SOFTDIRTY_BIT, VMA_MAYBE_GUARD_BIT, \
+				     VMA_MAYBE_UFFD_POISON_BIT)
 #else
-#define VMA_STICKY_FLAGS mk_vma_flags(VMA_MAYBE_GUARD_BIT)
+#define VMA_STICKY_FLAGS mk_vma_flags(VMA_MAYBE_GUARD_BIT, VMA_MAYBE_UFFD_POISON_BIT)
 #endif
 
 /*
@@ -667,13 +678,15 @@ enum {
  *                           retrieve from page cache, and skip copying
  *                           will lose those info.
  *
- *          VM_MAYBE_GUARD - Could contain page guard region markers which
- *                           by design are a property of the page tables
- *                           only and thus cannot be reconstructed on page
- *                           fault.
+ *          VM_MAYBE_GUARD - Could contain guard markers which cannot be
+ *                           reconstructed on page fault.
+ *
+ *   VM_MAYBE_UFFD_POISON - Could contain poison markers which cannot be
+ *                           reconstructed on page fault, including after
+ *                           userfaultfd unregister.
  */
 #define VM_COPY_ON_FORK (VM_PFNMAP | VM_MIXEDMAP | VM_UFFD_WP | VM_UFFD_RWP | \
-			 VM_MAYBE_GUARD)
+			 VM_MAYBE_GUARD | VM_MAYBE_UFFD_POISON)
 
 /*
  * mapping from the currently active vm_flags protection bits (the
diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
index dff9a5c9a0dfa..b8b7cc2f00388 100644
--- a/include/linux/userfaultfd_k.h
+++ b/include/linux/userfaultfd_k.h
@@ -167,11 +167,14 @@ static inline bool is_mergeable_vm_userfaultfd_ctx(struct vm_area_struct *vma,
 
 /*
  * Never enable huge pmd sharing on userfaultfd registered VMAs, because
- * userfaultfd state can be tracked per VMA or per page table entry.
+ * userfaultfd state can be tracked per VMA or per page table entry. Similarly,
+ * handle poison markers that stick around even after userfaultfd was
+ * unregistered.
  */
 static inline bool uffd_disable_huge_pmd_share(struct vm_area_struct *vma)
 {
-	return vma->vm_userfaultfd_ctx.ctx;
+	return vma->vm_userfaultfd_ctx.ctx ||
+	       vma_test_atomic_flag(vma, VMA_MAYBE_UFFD_POISON_BIT);
 }
 
 /*
diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
index 3c153b3ad8450..942746e4de15b 100644
--- a/include/trace/events/mmflags.h
+++ b/include/trace/events/mmflags.h
@@ -184,6 +184,12 @@ IF_HAVE_PG_ARCH_3(arch_3)
 #define IF_HAVE_VM_SOFTDIRTY(flag,name)
 #endif
 
+#ifdef CONFIG_HAVE_ARCH_USERFAULTFD_WP
+# define IF_HAVE_UFFD_WP(flag, name) {flag, name},
+#else
+# define IF_HAVE_UFFD_WP(flag, name)
+#endif
+
 #ifdef CONFIG_HAVE_ARCH_USERFAULTFD_MINOR
 # define IF_HAVE_UFFD_MINOR(flag, name) {flag, name},
 #else
@@ -247,7 +253,8 @@ IF_HAVE_PG_ARCH_3(arch_3)
 IF_HAVE_UFFD_MINOR(VM_UFFD_MINOR,	"uffd_minor"	)		\
 	{VM_PFNMAP,			"pfnmap"	},		\
 	{VM_MAYBE_GUARD,		"maybe_guard"	},		\
-	{VM_UFFD_WP,			"uffd_wp"	},		\
+	{VM_MAYBE_UFFD_POISON,		"maybe_uffd_poison"	},		\
+IF_HAVE_UFFD_WP(VM_UFFD_WP,		"uffd_wp")			\
 IF_HAVE_UFFD_RWP(VM_UFFD_RWP,		"uffd_rwp"	)		\
 	{VM_LOCKED,			"locked"	},		\
 	{VM_IO,				"io"		},		\
diff --git a/mm/Kconfig b/mm/Kconfig
index afff1b25b5438..ad9236933aab3 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -1357,6 +1357,7 @@ config ANON_VMA_NAME
 
 config HAVE_ARCH_USERFAULTFD_WP
 	bool
+	depends on 64BIT
 	help
 	  Arch has userfaultfd write protection support
 
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 42d9eb26d5fab..2a5ee8fc42335 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -2107,16 +2107,17 @@ static bool file_backed_vma_is_retractable(struct vm_area_struct *vma)
 		return false;
 
 	/*
-	 * If the VMA contains guard regions then we can't collapse it.
+	 * Preserve guard and poison markers when retracting page tables.
 	 *
-	 * This is set atomically on guard marker installation under mmap/VMA
+	 * These flags are set atomically on marker installation under mmap/VMA
 	 * read lock, and here we may not hold any VMA or mmap lock at all.
 	 *
 	 * This is therefore serialised on the PTE page table lock, which is
-	 * obtained on guard region installation after the flag is set, so this
+	 * obtained on marker installation after the flag is set, so this
 	 * check being performed under this lock excludes races.
 	 */
-	if (vma_test_atomic_flag(vma, VMA_MAYBE_GUARD_BIT))
+	if (vma_test_atomic_flag(vma, VMA_MAYBE_GUARD_BIT) ||
+	    vma_test_atomic_flag(vma, VMA_MAYBE_UFFD_POISON_BIT))
 		return false;
 
 	return true;
diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index 169f681f9849b..bb6069320b347 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -963,6 +963,9 @@ static __always_inline ssize_t mfill_atomic(struct userfaultfd_ctx *ctx,
 	if (err)
 		goto out;
 
+	if (uffd_flags_mode_is(flags, MFILL_ATOMIC_POISON))
+		vma_set_atomic_flag(state.vma, VMA_MAYBE_UFFD_POISON_BIT);
+
 	/*
 	 * If this is a HUGETLB vma, pass off to appropriate routine
 	 */
diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
index dc4927d26296c..f9bafe4e20158 100644
--- a/tools/testing/vma/include/dup.h
+++ b/tools/testing/vma/include/dup.h
@@ -117,7 +117,7 @@ enum {
 	/* Page-ranges managed without "struct page", just pure PFN */
 	DECLARE_VMA_BIT(PFNMAP, 10),
 	DECLARE_VMA_BIT(MAYBE_GUARD, 11),
-	DECLARE_VMA_BIT(UFFD_WP, 12),	/* wrprotect pages tracking */
+	DECLARE_VMA_BIT(MAYBE_UFFD_POISON, 12),
 	DECLARE_VMA_BIT(LOCKED, 13),
 	DECLARE_VMA_BIT(IO, 14),	/* Memory mapped I/O or similar */
 	DECLARE_VMA_BIT(SEQ_READ, 15),	/* App will access data sequentially */
@@ -160,6 +160,7 @@ enum {
 #endif
 	DECLARE_VMA_BIT(UFFD_MINOR, 41),
 	DECLARE_VMA_BIT(SEALED, 42),
+	DECLARE_VMA_BIT(UFFD_WP, 44),	/* wrprotect pages tracking */
 	/* Flags that reuse flags above. */
 	DECLARE_VMA_BIT_ALIAS(PKEY_BIT0, HIGH_ARCH_0),
 	DECLARE_VMA_BIT_ALIAS(PKEY_BIT1, HIGH_ARCH_1),
@@ -218,7 +219,12 @@ enum {
 #endif
 #define VM_PFNMAP	INIT_VM_FLAG(PFNMAP)
 #define VM_MAYBE_GUARD	INIT_VM_FLAG(MAYBE_GUARD)
+#define VM_MAYBE_UFFD_POISON	INIT_VM_FLAG(MAYBE_UFFD_POISON)
+#ifdef CONFIG_HAVE_ARCH_USERFAULTFD_WP
 #define VM_UFFD_WP	INIT_VM_FLAG(UFFD_WP)
+#else
+#define VM_UFFD_WP	VM_NONE
+#endif
 #define VM_LOCKED	INIT_VM_FLAG(LOCKED)
 #define VM_IO		INIT_VM_FLAG(IO)
 #define VM_SEQ_READ	INIT_VM_FLAG(SEQ_READ)
@@ -367,7 +373,8 @@ enum {
 #define VMA_LOCKED_MASK	mk_vma_flags(VMA_LOCKED_BIT, VMA_LOCKONFAULT_BIT)
 
 /* These flags can be updated atomically via VMA/mmap read lock. */
-#define VMA_ATOMIC_SET_ALLOWED mk_vma_flags(VMA_MAYBE_GUARD_BIT)
+#define VMA_ATOMIC_SET_ALLOWED mk_vma_flags(VMA_MAYBE_GUARD_BIT, \
+					    VMA_MAYBE_UFFD_POISON_BIT)
 
 #define RLIMIT_STACK		3	/* max stack size */
 #define RLIMIT_MEMLOCK		8	/* max locked-in-memory address space */
@@ -375,14 +382,17 @@ enum {
 #define CAP_IPC_LOCK         14
 
 #ifdef CONFIG_MEM_SOFT_DIRTY
-#define VMA_STICKY_FLAGS mk_vma_flags(VMA_SOFTDIRTY_BIT, VMA_MAYBE_GUARD_BIT)
+#define VMA_STICKY_FLAGS mk_vma_flags(VMA_SOFTDIRTY_BIT, VMA_MAYBE_GUARD_BIT, \
+				      VMA_MAYBE_UFFD_POISON_BIT)
 #else
-#define VMA_STICKY_FLAGS mk_vma_flags(VMA_MAYBE_GUARD_BIT)
+#define VMA_STICKY_FLAGS mk_vma_flags(VMA_MAYBE_GUARD_BIT, \
+				      VMA_MAYBE_UFFD_POISON_BIT)
 #endif
 
 #define VMA_IGNORE_MERGE_FLAGS VMA_STICKY_FLAGS
 
-#define VM_COPY_ON_FORK (VM_PFNMAP | VM_MIXEDMAP | VM_UFFD_WP | VM_MAYBE_GUARD)
+#define VM_COPY_ON_FORK (VM_PFNMAP | VM_MIXEDMAP | VM_UFFD_WP | \
+			 VM_MAYBE_GUARD | VM_MAYBE_UFFD_POISON)
 
 #define pgprot_val(x)		((x).pgprot)
 #define __pgprot(x)		((pgprot_t) { (x) } )

-- 
2.43.0

[PATCH 2/2] mm/khugepaged: check for empty PTE tables before retracting

[David Hildenbrand (Arm)]

retract_page_tables() uses elaborate (error-prone) logic to avoid scanning
whether a page table is empty.

Let's prevent more future bugs and reduce the complexity by doing it
just like try_collapse_pte_mapped_thp(): scan if the page table is
empty. Keep the check for anon_vma in place.

Scan under the PTE lock only and try to grab the PMD lock afterwards.
If that fails, drop the PTE lock, take both locks in the right order
and check again. This avoids holding the PMD lock during the scan in
the common case.

This will now retract in more cases where the VMA flags were an
over-indication, with the cost of scanning more frequently under lock.

We could scan lockless first, but it doesn't really seem worth it for
now. If this ever becomes a problem, we could do that, or alternatively
refuse again more of the VMA flags early.

The structure of the retraction code is now similar to
try_collapse_pte_mapped_thp(), with some slight differences (e.g.,
initial empty scan) that make this harder to generalize. But likely we
should look in the future into merging more of that code.

Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
---
 mm/khugepaged.c | 127 ++++++++++++++++++++------------------------------------
 1 file changed, 45 insertions(+), 82 deletions(-)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 2a5ee8fc42335..f39ea3d076554 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1884,8 +1884,9 @@ static enum scan_result try_collapse_pte_mapped_thp(struct mm_struct *mm, unsign
 		return SCAN_VMA_CHECK;
 
 	/*
-	 * Keep pmd pgtable while the uffd bit is in use; see comment in
-	 * retract_page_tables().
+	 * Keep the PTE table for uffd-wp and RWP registrations: it may
+	 * contain per-PTE protection state that cannot be reconstructed
+	 * from the page cache.
 	 */
 	if (userfaultfd_protected(vma))
 		return SCAN_PTE_UFFD;
@@ -2081,48 +2082,6 @@ void collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
 	try_collapse_pte_mapped_thp(mm, addr, install_pmd);
 }
 
-/* Can we retract page tables for this file-backed VMA? */
-static bool file_backed_vma_is_retractable(struct vm_area_struct *vma)
-{
-	/*
-	 * Check vma->anon_vma to exclude MAP_PRIVATE mappings that
-	 * got written to. These VMAs are likely not worth removing
-	 * page tables from, as PMD-mapping is likely to be split later.
-	 *
-	 * Can't use vma_has_anon_rmap() here as the VMA may be stabilised
-	 * by the file rmap lock.
-	 */
-	if (READ_ONCE(vma->anon_vma))
-		return false;
-
-	/*
-	 * When a vma is registered with uffd-wp or RWP, we cannot recycle
-	 * the page table because there may be pte markers installed.
-	 * VM_UFFD_RWP ranges similarly rely on per-PTE uffd state
-	 * and cannot be recycled to a shared PMD. Other vmas can still
-	 * have the same file mapped hugely, but skip this one: it will
-	 * always be mapped in small page size for these registrations.
-	 */
-	if (userfaultfd_protected(vma))
-		return false;
-
-	/*
-	 * Preserve guard and poison markers when retracting page tables.
-	 *
-	 * These flags are set atomically on marker installation under mmap/VMA
-	 * read lock, and here we may not hold any VMA or mmap lock at all.
-	 *
-	 * This is therefore serialised on the PTE page table lock, which is
-	 * obtained on marker installation after the flag is set, so this
-	 * check being performed under this lock excludes races.
-	 */
-	if (vma_test_atomic_flag(vma, VMA_MAYBE_GUARD_BIT) ||
-	    vma_test_atomic_flag(vma, VMA_MAYBE_UFFD_POISON_BIT))
-		return false;
-
-	return true;
-}
-
 static void retract_page_tables(struct address_space *mapping, pgoff_t pgoff)
 {
 	struct vm_area_struct *vma;
@@ -2133,10 +2092,18 @@ static void retract_page_tables(struct address_space *mapping, pgoff_t pgoff)
 		struct mm_struct *mm;
 		unsigned long addr;
 		pmd_t *pmd, pgt_pmd;
-		spinlock_t *pml;
-		spinlock_t *ptl;
+		pte_t *pte;
+		spinlock_t *pml = NULL, *ptl;
 		bool success = false;
 
+		/*
+		 * Skip private mappings that have been written to: we'll likely
+		 * split them as soon as we CoW later. Use READ_ONCE() because
+		 * we are neither holding the VMA lock nor the mmap_lock.
+		 */
+		if (READ_ONCE(vma->anon_vma))
+			continue;
+
 		addr = vma->vm_start +
 			((pgoff - vma_start_pgoff(vma)) << PAGE_SHIFT);
 		if (addr & ~HPAGE_PMD_MASK ||
@@ -2150,52 +2117,48 @@ static void retract_page_tables(struct address_space *mapping, pgoff_t pgoff)
 		if (collapse_test_exit(mm))
 			continue;
 
-		if (!file_backed_vma_is_retractable(vma))
-			continue;
-
 		/* PTEs were notified when unmapped; but now for the PMD? */
 		mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, mm,
 					addr, addr + HPAGE_PMD_SIZE);
 		mmu_notifier_invalidate_range_start(&range);
 
-		pml = pmd_lock(mm, pmd);
-		/*
-		 * The lock of new_folio is still held, we will be blocked in
-		 * the page fault path, which prevents the pte entries from
-		 * being set again. So even though the old empty PTE page may be
-		 * concurrently freed and a new PTE page is filled into the pmd
-		 * entry, it is still empty and can be removed.
-		 *
-		 * So here we only need to recheck if the state of pmd entry
-		 * still meets our requirements, rather than checking pmd_same()
-		 * like elsewhere.
-		 */
-		if (check_pmd_state(pmd) != SCAN_SUCCEED)
-			goto drop_pml;
-		ptl = pte_lockptr(mm, pmd);
-		if (ptl != pml)
-			spin_lock_nested(ptl, SINGLE_DEPTH_NESTING);
+		pte = pte_offset_map_lock(mm, pmd, addr, &ptl);
+		if (!pte)
+			goto unlock;
 
 		/*
-		 * Huge page lock is still held, so normally the page table must
-		 * remain empty; and we have already skipped vmas with an anon
-		 * rmap and userfaultfd_wp() vmas.  But since the mmap_lock is not
-		 * held, it is still possible for a racing userfaultfd_ioctl() or
-		 * madvise() to have inserted ptes or markers.  Now that we hold
-		 * ptlock, repeating the retractable checks protects us from
-		 * races against the prior checks.
+		 * The folio lock prevents file faults from modifying the
+		 * page table, but other things (userfaultfd, CoW, guard
+		 * markers) could have modified the page table until we lock it
+		 * and check.
 		 */
-		if (likely(file_backed_vma_is_retractable(vma))) {
-			pgt_pmd = pmdp_collapse_flush(vma, addr, pmd);
-			pmdp_get_lockless_sync();
-			success = true;
-		}
+		if (!pte_table_empty(pte))
+			goto unlock;
 
-		if (ptl != pml)
-			spin_unlock(ptl);
-drop_pml:
-		spin_unlock(pml);
+		pml = pmd_lockptr(mm, pmd);
+		if (ptl != pml && !spin_trylock(pml)) {
+			pte_unmap_unlock(pte, ptl);
+			ptl = NULL;
+
+			pml = pmd_lock(mm, pmd);
+			pte = pte_offset_map_rw_nolock(mm, pmd, addr, &pgt_pmd, &ptl);
+			if (!pte)
+				goto unlock;
+			if (ptl != pml)
+				spin_lock_nested(ptl, SINGLE_DEPTH_NESTING);
 
+			if (!pte_table_empty(pte))
+				goto unlock;
+		}
+
+		pgt_pmd = pmdp_collapse_flush(vma, addr, pmd);
+		pmdp_get_lockless_sync();
+		success = true;
+unlock:
+		if (pte)
+			pte_unmap_unlock(pte, ptl);
+		if (pml && pml != ptl)
+			spin_unlock(pml);
 		mmu_notifier_invalidate_range_end(&range);
 
 		if (success) {

-- 
2.43.0

Re: [PATCH 2/2] mm/khugepaged: check for empty PTE tables before retracting

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] mm/khugepaged: race between retract_page_tables() and
  copy_page_range()
- [Medium] mm/khugepaged: retract_page_tables() breaks uffd RWP on file
  and shmem VMAs
--

commit 15ea4d4b6e2586260561871f9acd4c3bff3278ec
Author: David Hildenbrand (Arm) <david@kernel.org>

mm/khugepaged: check for empty PTE tables before retracting

This commit replaces the VMA flag checks in retract_page_tables() with a
scan under the PTE lock to verify that the PTE table is empty before
retracting it.

> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 2a5ee8fc42335..f39ea3d076554 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
[ ... ]
> @@ -2081,48 +2082,6 @@ void collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
>  	try_collapse_pte_mapped_thp(mm, addr, install_pmd);
>  }
>  
> -/* Can we retract page tables for this file-backed VMA? */
> -static bool file_backed_vma_is_retractable(struct vm_area_struct *vma)
> -{
> -	/*
> -	 * Check vma->anon_vma to exclude MAP_PRIVATE mappings that
> -	 * got written to. These VMAs are likely not worth removing
> -	 * page tables from, as PMD-mapping is likely to be split later.
> -	 *
> -	 * Can't use vma_has_anon_rmap() here as the VMA may be stabilised
> -	 * by the file rmap lock.
> -	 */
> -	if (READ_ONCE(vma->anon_vma))
> -		return false;
> -
> -	/*
> -	 * When a vma is registered with uffd-wp or RWP, we cannot recycle
> -	 * the page table because there may be pte markers installed.
> -	 * VM_UFFD_RWP ranges similarly rely on per-PTE uffd state
> -	 * and cannot be recycled to a shared PMD. Other vmas can still
> -	 * have the same file mapped hugely, but skip this one: it will
> -	 * always be mapped in small page size for these registrations.
> -	 */
> -	if (userfaultfd_protected(vma))
> -		return false;
> -
> -	/*
> -	 * Preserve guard and poison markers when retracting page tables.
> -	 *
> -	 * These flags are set atomically on marker installation under mmap/VMA
> -	 * read lock, and here we may not hold any VMA or mmap lock at all.
> -	 *
> -	 * This is therefore serialised on the PTE page table lock, which is
> -	 * obtained on marker installation after the flag is set, so this
> -	 * check being performed under this lock excludes races.
> -	 */
> -	if (vma_test_atomic_flag(vma, VMA_MAYBE_GUARD_BIT) ||
> -	    vma_test_atomic_flag(vma, VMA_MAYBE_UFFD_POISON_BIT))
> -		return false;
> -
> -	return true;
> -}
> -
>  static void retract_page_tables(struct address_space *mapping, pgoff_t pgoff)
>  {
>  	struct vm_area_struct *vma;
> @@ -2133,10 +2092,18 @@ static void retract_page_tables(struct address_space *mapping, pgoff_t pgoff)
>  		struct mm_struct *mm;
>  		unsigned long addr;
>  		pmd_t *pmd, pgt_pmd;
> -		spinlock_t *pml;
> -		spinlock_t *ptl;
> +		pte_t *pte;
> +		spinlock_t *pml = NULL, *ptl;
>  		bool success = false;
>  
> +		/*
> +		 * Skip private mappings that have been written to: we'll likely
> +		 * split them as soon as we CoW later. Use READ_ONCE() because
> +		 * we are neither holding the VMA lock nor the mmap_lock.
> +		 */
> +		if (READ_ONCE(vma->anon_vma))
> +			continue;

[Severity: Medium]
Can removing the VM_COPY_ON_FORK flag checks here cause retract_page_tables()
to race with copy_page_range() during fork()?

In dup_mmap(), the child VMA is linked into mapping->i_mmap and
i_mmap_unlock_write(mapping) is dropped before copy_page_range() runs.
vma_needs_copy() copies page tables not only when vma_has_anon_rmap() is set,
but also when VM_COPY_ON_FORK flags (VM_MAYBE_GUARD, VM_MAYBE_UFFD_POISON,
VM_UFFD_WP, or VM_UFFD_RWP) are set on a file or shmem VMA whose anon_vma is
still NULL.

mm/memory.c:copy_pte_range() {
	/*
	 * copy_pmd_range()'s prior pmd_none_or_clear_bad(src_pmd), and the
	 * error handling here, assume that exclusive mmap_lock on dst and src
	 * protects anon from unexpected THP transitions; with shmem and file
	 * protected by mmap_lock-less collapse skipping areas with an anon rmap
	 * (whereas vma_needs_copy() skips areas without one).  A rework
	 * can remove such assumptions later, but this is good enough for now.
	 */
	dst_pte = pte_alloc_map_lock(dst_mm, dst_pmd, addr, &dst_ptl);
	if (!dst_pte) {
		ret = -ENOMEM;
		goto out;
	}
}

Because retract_page_tables() no longer skips VMAs with VM_COPY_ON_FORK flags
set, it can retract the newly allocated empty dst_pmd in the child between
__pte_alloc() and pte_offset_map_lock() (or across cond_resched() before a
non-empty PTE is copied):

CPU0                                      CPU1
dup_mmap()
  mapping_rmap_tree_insert_after(dst_vma, ...)
  i_mmap_unlock_write(mapping)
  copy_page_range(dst_vma, src_vma)
    ...
    copy_pte_range()
      pte_alloc_map_lock(dst_mm, dst_pmd, ...)
        __pte_alloc(dst_mm, dst_pmd)
                                          collapse_file()
                                            retract_page_tables()
                                              pte_table_empty(pte) == true
                                              pmdp_collapse_flush(..., dst_pmd)
        pte_offset_map_lock(dst_mm, dst_pmd, ...)
          --> returns NULL
      ret = -ENOMEM;  <-- fork() fails spuriously

On the parent side, retracting an empty src_pmd concurrently with
copy_pmd_range() can also clear *src_pmd between pmd_none(*src_pmd) and
pmd_bad(*src_pmd) inside pmd_none_or_clear_bad(src_pmd), triggering a false
pmd_clear_bad() report.

> +
>  		addr = vma->vm_start +
>  			((pgoff - vma_start_pgoff(vma)) << PAGE_SHIFT);
>  		if (addr & ~HPAGE_PMD_MASK ||
> @@ -2150,52 +2117,48 @@ static void retract_page_tables(struct address_space *mapping, pgoff_t pgoff)
>  		if (collapse_test_exit(mm))
>  			continue;
>  
> -		if (!file_backed_vma_is_retractable(vma))
> -			continue;
> -
>  		/* PTEs were notified when unmapped; but now for the PMD? */
>  		mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, mm,
>  					addr, addr + HPAGE_PMD_SIZE);
>  		mmu_notifier_invalidate_range_start(&range);
>  
> -		pml = pmd_lock(mm, pmd);
[ ... ]
> +		pte = pte_offset_map_lock(mm, pmd, addr, &ptl);
> +		if (!pte)
> +			goto unlock;
>  
>  		/*
> -		 * Huge page lock is still held, so normally the page table must
> -		 * remain empty; and we have already skipped vmas with an anon
> -		 * rmap and userfaultfd_wp() vmas.  But since the mmap_lock is not
> -		 * held, it is still possible for a racing userfaultfd_ioctl() or
> -		 * madvise() to have inserted ptes or markers.  Now that we hold
> -		 * ptlock, repeating the retractable checks protects us from
> -		 * races against the prior checks.
> +		 * The folio lock prevents file faults from modifying the
> +		 * page table, but other things (userfaultfd, CoW, guard
> +		 * markers) could have modified the page table until we lock it
> +		 * and check.
>  		 */
> -		if (likely(file_backed_vma_is_retractable(vma))) {
> -			pgt_pmd = pmdp_collapse_flush(vma, addr, pmd);
> -			pmdp_get_lockless_sync();
> -			success = true;
> -		}
> +		if (!pte_table_empty(pte))
> +			goto unlock;

[Severity: Medium]
Does checking pte_table_empty() instead of userfaultfd_protected(vma) break
userfaultfd RWP (and WP) on file and shmem VMAs?

Unlike VM_UFFD_WP on write-protected PTEs, try_to_unmap_one() does not
install PTE markers for VM_UFFD_RWP (or for VM_UFFD_WP PTEs that are not
currently write-protected) when collapse_file() unmaps the small folios.
pte_table_empty(pte) is then true and retract_page_tables() clears the PMD to
pmd_none, even though try_collapse_pte_mapped_thp() still refuses
userfaultfd_protected(vma).

Once the PMD is pmd_none, the next fault in finish_fault() installs a huge
PMD before checking userfaultfd_armed(vma):

mm/memory.c:finish_fault() {
	if (pmd_none(*vmf->pmd)) {
		if (!needs_fallback && folio_test_pmd_mappable(folio)) {
			ret = do_set_pmd(vmf, folio, page);
			if (ret != VM_FAULT_FALLBACK)
				return ret;
		}
    ...
	/* Using per-page fault to maintain the uffd semantics */
	if (unlikely(userfaultfd_armed(vma)) || unlikely(needs_fallback)) {
		nr_pages = 1;
}

When userspace later issues UFFDIO_RWPROTECT on that file or shmem VMA,
change_pmd_range() sees pgtable_split_needed() is true and calls
__split_huge_pmd(), which zaps the file huge PMD back to pmd_none without
populating a PTE table (since pgtable_populate_needed() is false for
MM_CP_UFFD_RWP), so RWP protection is lost and subsequent accesses fault the
unprotected huge PMD back in without delivering RWP faults.  Installing a
huge PMD also causes PAGEMAP_SCAN to report all 512 pages as accessed or
written after a single 4KB page fault.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107-b4-uffd_marker_loss-v1-0-82926b22704c@kernel.org?part=2

Re: [PATCH 2/2] mm/khugepaged: check for empty PTE tables before retracting

[David Hildenbrand (Arm)]

On 11/8/26 00:51, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issues to consider:
> - [Medium] mm/khugepaged: race between retract_page_tables() and
>   copy_page_range()
> - [Medium] mm/khugepaged: retract_page_tables() breaks uffd RWP on file
>   and shmem VMAs
> --
> 
> commit 15ea4d4b6e2586260561871f9acd4c3bff3278ec
> Author: David Hildenbrand (Arm) <david@kernel.org>
> 
> mm/khugepaged: check for empty PTE tables before retracting
> 
> This commit replaces the VMA flag checks in retract_page_tables() with a
> scan under the PTE lock to verify that the PTE table is empty before
> retracting it.
> 
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 2a5ee8fc42335..f39ea3d076554 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
> [ ... ]
>> @@ -2081,48 +2082,6 @@ void collapse_pte_mapped_thp(struct mm_struct *mm, unsigned long addr,
>>  	try_collapse_pte_mapped_thp(mm, addr, install_pmd);
>>  }
>>  
>> -/* Can we retract page tables for this file-backed VMA? */
>> -static bool file_backed_vma_is_retractable(struct vm_area_struct *vma)
>> -{
>> -	/*
>> -	 * Check vma->anon_vma to exclude MAP_PRIVATE mappings that
>> -	 * got written to. These VMAs are likely not worth removing
>> -	 * page tables from, as PMD-mapping is likely to be split later.
>> -	 *
>> -	 * Can't use vma_has_anon_rmap() here as the VMA may be stabilised
>> -	 * by the file rmap lock.
>> -	 */
>> -	if (READ_ONCE(vma->anon_vma))
>> -		return false;
>> -
>> -	/*
>> -	 * When a vma is registered with uffd-wp or RWP, we cannot recycle
>> -	 * the page table because there may be pte markers installed.
>> -	 * VM_UFFD_RWP ranges similarly rely on per-PTE uffd state
>> -	 * and cannot be recycled to a shared PMD. Other vmas can still
>> -	 * have the same file mapped hugely, but skip this one: it will
>> -	 * always be mapped in small page size for these registrations.
>> -	 */
>> -	if (userfaultfd_protected(vma))
>> -		return false;
>> -
>> -	/*
>> -	 * Preserve guard and poison markers when retracting page tables.
>> -	 *
>> -	 * These flags are set atomically on marker installation under mmap/VMA
>> -	 * read lock, and here we may not hold any VMA or mmap lock at all.
>> -	 *
>> -	 * This is therefore serialised on the PTE page table lock, which is
>> -	 * obtained on marker installation after the flag is set, so this
>> -	 * check being performed under this lock excludes races.
>> -	 */
>> -	if (vma_test_atomic_flag(vma, VMA_MAYBE_GUARD_BIT) ||
>> -	    vma_test_atomic_flag(vma, VMA_MAYBE_UFFD_POISON_BIT))
>> -		return false;
>> -
>> -	return true;
>> -}
>> -
>>  static void retract_page_tables(struct address_space *mapping, pgoff_t pgoff)
>>  {
>>  	struct vm_area_struct *vma;
>> @@ -2133,10 +2092,18 @@ static void retract_page_tables(struct address_space *mapping, pgoff_t pgoff)
>>  		struct mm_struct *mm;
>>  		unsigned long addr;
>>  		pmd_t *pmd, pgt_pmd;
>> -		spinlock_t *pml;
>> -		spinlock_t *ptl;
>> +		pte_t *pte;
>> +		spinlock_t *pml = NULL, *ptl;
>>  		bool success = false;
>>  
>> +		/*
>> +		 * Skip private mappings that have been written to: we'll likely
>> +		 * split them as soon as we CoW later. Use READ_ONCE() because
>> +		 * we are neither holding the VMA lock nor the mmap_lock.
>> +		 */
>> +		if (READ_ONCE(vma->anon_vma))
>> +			continue;
> 
> [Severity: Medium]
> Can removing the VM_COPY_ON_FORK flag checks here cause retract_page_tables()
> to race with copy_page_range() during fork()?
> 
> In dup_mmap(), the child VMA is linked into mapping->i_mmap and
> i_mmap_unlock_write(mapping) is dropped before copy_page_range() runs.
> vma_needs_copy() copies page tables not only when vma_has_anon_rmap() is set,
> but also when VM_COPY_ON_FORK flags (VM_MAYBE_GUARD, VM_MAYBE_UFFD_POISON,
> VM_UFFD_WP, or VM_UFFD_RWP) are set on a file or shmem VMA whose anon_vma is
> still NULL.

Ah, try_collapse_pte_mapped_thp() runs withe the mmap lock, whereby
retract_page_tables() only holds the rmap lock.



> 
> mm/memory.c:copy_pte_range() {
> 	/*
> 	 * copy_pmd_range()'s prior pmd_none_or_clear_bad(src_pmd), and the
> 	 * error handling here, assume that exclusive mmap_lock on dst and src
> 	 * protects anon from unexpected THP transitions; with shmem and file
> 	 * protected by mmap_lock-less collapse skipping areas with an anon rmap
> 	 * (whereas vma_needs_copy() skips areas without one).  A rework
> 	 * can remove such assumptions later, but this is good enough for now.
> 	 */
> 	dst_pte = pte_alloc_map_lock(dst_mm, dst_pmd, addr, &dst_ptl);
> 	if (!dst_pte) {
> 		ret = -ENOMEM;
> 		goto out;
> 	}
> }
> 
> Because retract_page_tables() no longer skips VMAs with VM_COPY_ON_FORK flags
> set, it can retract the newly allocated empty dst_pmd in the child between
> __pte_alloc() and pte_offset_map_lock() (or across cond_resched() before a
> non-empty PTE is copied):
> 
> CPU0                                      CPU1
> dup_mmap()
>   mapping_rmap_tree_insert_after(dst_vma, ...)
>   i_mmap_unlock_write(mapping)
>   copy_page_range(dst_vma, src_vma)
>     ...
>     copy_pte_range()
>       pte_alloc_map_lock(dst_mm, dst_pmd, ...)
>         __pte_alloc(dst_mm, dst_pmd)
>                                           collapse_file()
>                                             retract_page_tables()
>                                               pte_table_empty(pte) == true
>                                               pmdp_collapse_flush(..., dst_pmd)
>         pte_offset_map_lock(dst_mm, dst_pmd, ...)
>           --> returns NULL
>       ret = -ENOMEM;  <-- fork() fails spuriously
> 
> On the parent side, retracting an empty src_pmd concurrently with
> copy_pmd_range() can also clear *src_pmd between pmd_none(*src_pmd) and
> pmd_bad(*src_pmd) inside pmd_none_or_clear_bad(src_pmd), triggering a false
> pmd_clear_bad() report.

Yeah, that would likely need a different handling.

> 
>> +
>>  		addr = vma->vm_start +
>>  			((pgoff - vma_start_pgoff(vma)) << PAGE_SHIFT);
>>  		if (addr & ~HPAGE_PMD_MASK ||
>> @@ -2150,52 +2117,48 @@ static void retract_page_tables(struct address_space *mapping, pgoff_t pgoff)
>>  		if (collapse_test_exit(mm))
>>  			continue;
>>  
>> -		if (!file_backed_vma_is_retractable(vma))
>> -			continue;
>> -
>>  		/* PTEs were notified when unmapped; but now for the PMD? */
>>  		mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, mm,
>>  					addr, addr + HPAGE_PMD_SIZE);
>>  		mmu_notifier_invalidate_range_start(&range);
>>  
>> -		pml = pmd_lock(mm, pmd);
> [ ... ]
>> +		pte = pte_offset_map_lock(mm, pmd, addr, &ptl);
>> +		if (!pte)
>> +			goto unlock;
>>  
>>  		/*
>> -		 * Huge page lock is still held, so normally the page table must
>> -		 * remain empty; and we have already skipped vmas with an anon
>> -		 * rmap and userfaultfd_wp() vmas.  But since the mmap_lock is not
>> -		 * held, it is still possible for a racing userfaultfd_ioctl() or
>> -		 * madvise() to have inserted ptes or markers.  Now that we hold
>> -		 * ptlock, repeating the retractable checks protects us from
>> -		 * races against the prior checks.
>> +		 * The folio lock prevents file faults from modifying the
>> +		 * page table, but other things (userfaultfd, CoW, guard
>> +		 * markers) could have modified the page table until we lock it
>> +		 * and check.
>>  		 */
>> -		if (likely(file_backed_vma_is_retractable(vma))) {
>> -			pgt_pmd = pmdp_collapse_flush(vma, addr, pmd);
>> -			pmdp_get_lockless_sync();
>> -			success = true;
>> -		}
>> +		if (!pte_table_empty(pte))
>> +			goto unlock;
> 
> [Severity: Medium]
> Does checking pte_table_empty() instead of userfaultfd_protected(vma) break
> userfaultfd RWP (and WP) on file and shmem VMAs?
> 
> Unlike VM_UFFD_WP on write-protected PTEs, try_to_unmap_one() does not
> install PTE markers for VM_UFFD_RWP (or for VM_UFFD_WP PTEs that are not
> currently write-protected) when collapse_file() unmaps the small folios.
> pte_table_empty(pte) is then true and retract_page_tables() clears the PMD to
> pmd_none, even though try_collapse_pte_mapped_thp() still refuses
> userfaultfd_protected(vma).
> 
> Once the PMD is pmd_none, the next fault in finish_fault() installs a huge
> PMD before checking userfaultfd_armed(vma):
> 
> mm/memory.c:finish_fault() {
> 	if (pmd_none(*vmf->pmd)) {
> 		if (!needs_fallback && folio_test_pmd_mappable(folio)) {
> 			ret = do_set_pmd(vmf, folio, page);
> 			if (ret != VM_FAULT_FALLBACK)
> 				return ret;
> 		}
>     ...
> 	/* Using per-page fault to maintain the uffd semantics */
> 	if (unlikely(userfaultfd_armed(vma)) || unlikely(needs_fallback)) {
> 		nr_pages = 1;
> }
> 
> When userspace later issues UFFDIO_RWPROTECT on that file or shmem VMA,
> change_pmd_range() sees pgtable_split_needed() is true and calls
> __split_huge_pmd(), which zaps the file huge PMD back to pmd_none without
> populating a PTE table (since pgtable_populate_needed() is false for
> MM_CP_UFFD_RWP), so RWP protection is lost and subsequent accesses fault the
> unprotected huge PMD back in without delivering RWP faults.  Installing a
> huge PMD also causes PAGEMAP_SCAN to report all 512 pages as accessed or
> written after a single 4KB page fault.

"Since pgtable_populate_needed() is false for MM_CP_UFFD_RWP" I think RWP also
needs proper uffd marker support, independent of the change here.
-- 
Cheers,

David

Re: [PATCH 2/2] mm/khugepaged: check for empty PTE tables before retracting

[Lorenzo Stoakes (ARM)]

On Mon, Nov 09, 2026 at 04:08:23PM +0100, David Hildenbrand (Arm) wrote:
> On 11/8/26 00:51, sashiko-bot@kernel.org wrote:
> > [Severity: Medium]
> > Can removing the VM_COPY_ON_FORK flag checks here cause retract_page_tables()
> > to race with copy_page_range() during fork()?
> >
> > In dup_mmap(), the child VMA is linked into mapping->i_mmap and
> > i_mmap_unlock_write(mapping) is dropped before copy_page_range() runs.
> > vma_needs_copy() copies page tables not only when vma_has_anon_rmap() is set,
> > but also when VM_COPY_ON_FORK flags (VM_MAYBE_GUARD, VM_MAYBE_UFFD_POISON,
> > VM_UFFD_WP, or VM_UFFD_RWP) are set on a file or shmem VMA whose anon_vma is
> > still NULL.
>
> Ah, try_collapse_pte_mapped_thp() runs withe the mmap lock, whereby
> retract_page_tables() only holds the rmap lock.

[ ... ]

> > Once the PMD is pmd_none, the next fault in finish_fault() installs a huge
> > PMD before checking userfaultfd_armed(vma):
> >
> > mm/memory.c:finish_fault() {
> > 	if (pmd_none(*vmf->pmd)) {
> > 		if (!needs_fallback && folio_test_pmd_mappable(folio)) {
> > 			ret = do_set_pmd(vmf, folio, page);
> > 			if (ret != VM_FAULT_FALLBACK)
> > 				return ret;
> > 		}
> >     ...
> > 	/* Using per-page fault to maintain the uffd semantics */
> > 	if (unlikely(userfaultfd_armed(vma)) || unlikely(needs_fallback)) {
> > 		nr_pages = 1;
> > }
> >
> > When userspace later issues UFFDIO_RWPROTECT on that file or shmem VMA,
> > change_pmd_range() sees pgtable_split_needed() is true and calls
> > __split_huge_pmd(), which zaps the file huge PMD back to pmd_none without
> > populating a PTE table (since pgtable_populate_needed() is false for
> > MM_CP_UFFD_RWP), so RWP protection is lost and subsequent accesses fault the
> > unprotected huge PMD back in without delivering RWP faults.  Installing a
> > huge PMD also causes PAGEMAP_SCAN to report all 512 pages as accessed or
> > written after a single 4KB page fault.
>
> "Since pgtable_populate_needed() is false for MM_CP_UFFD_RWP" I think RWP also
> needs proper uffd marker support, independent of the change here.

Hey,

Wondered if you planned to do a respin on this? Want to know whether to delay
review for that or not, thanks!

--
Cheers, Lorenzo

Re: [PATCH 2/2] mm/khugepaged: check for empty PTE tables before retracting

[David Hildenbrand (Arm)]

On 11/10/26 18:08, Lorenzo Stoakes (ARM) wrote:
> On Mon, Nov 09, 2026 at 04:08:23PM +0100, David Hildenbrand (Arm) wrote:
>> On 11/8/26 00:51, sashiko-bot@kernel.org wrote:
>>> [Severity: Medium]
>>> Can removing the VM_COPY_ON_FORK flag checks here cause retract_page_tables()
>>> to race with copy_page_range() during fork()?
>>>
>>> In dup_mmap(), the child VMA is linked into mapping->i_mmap and
>>> i_mmap_unlock_write(mapping) is dropped before copy_page_range() runs.
>>> vma_needs_copy() copies page tables not only when vma_has_anon_rmap() is set,
>>> but also when VM_COPY_ON_FORK flags (VM_MAYBE_GUARD, VM_MAYBE_UFFD_POISON,
>>> VM_UFFD_WP, or VM_UFFD_RWP) are set on a file or shmem VMA whose anon_vma is
>>> still NULL.
>>
>> Ah, try_collapse_pte_mapped_thp() runs withe the mmap lock, whereby
>> retract_page_tables() only holds the rmap lock.
> 
> [ ... ]
> 
>>> Once the PMD is pmd_none, the next fault in finish_fault() installs a huge
>>> PMD before checking userfaultfd_armed(vma):
>>>
>>> mm/memory.c:finish_fault() {
>>> 	if (pmd_none(*vmf->pmd)) {
>>> 		if (!needs_fallback && folio_test_pmd_mappable(folio)) {
>>> 			ret = do_set_pmd(vmf, folio, page);
>>> 			if (ret != VM_FAULT_FALLBACK)
>>> 				return ret;
>>> 		}
>>>     ...
>>> 	/* Using per-page fault to maintain the uffd semantics */
>>> 	if (unlikely(userfaultfd_armed(vma)) || unlikely(needs_fallback)) {
>>> 		nr_pages = 1;
>>> }
>>>
>>> When userspace later issues UFFDIO_RWPROTECT on that file or shmem VMA,
>>> change_pmd_range() sees pgtable_split_needed() is true and calls
>>> __split_huge_pmd(), which zaps the file huge PMD back to pmd_none without
>>> populating a PTE table (since pgtable_populate_needed() is false for
>>> MM_CP_UFFD_RWP), so RWP protection is lost and subsequent accesses fault the
>>> unprotected huge PMD back in without delivering RWP faults.  Installing a
>>> huge PMD also causes PAGEMAP_SCAN to report all 512 pages as accessed or
>>> written after a single 4KB page fault.
>>
>> "Since pgtable_populate_needed() is false for MM_CP_UFFD_RWP" I think RWP also
>> needs proper uffd marker support, independent of the change here.
> 
> Hey,
> 
> Wondered if you planned to do a respin on this? Want to know whether to delay
> review for that or not, thanks!

Let's move forward with patch #1 for now. Patch #2 is mostly a cleanup on top
that can wai.

-- 
Cheers,

David