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

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Sat, Nov 07, 2026 at 12:51:05AM +0100, David Hildenbrand (Arm) wrote:
> 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")

Are we looking to cc: stable or this something for newer kernels only?

> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>

Logic LGTM, some small nits below. Overall:

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@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

As per below, please move the VMA_UFFD_WP decls here.

Also, could you also do:

static inline bool userfaultfd_wp(struct vm_area_struct *vma)
{
-	return vma_test_any_mask(vma, VMA_UFFD_WP);
+	return vma_test_single_mask(vma, VMA_UFFD_WP);
}

To match convention for using vma_test_single_mask() for flags that aren't
always defined. Thanks! :)


>  #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

Could you just move these up to the block above to match the convention here?

>  #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

This is just saying out loud what was already the case effectively, which is
nice to do as a side-effect! :)

>  	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
>

--
Cheers, Lorenzo