Re: [PATCH 01/12] userfaultfd: use anonymous memory for UFFDIO_ZEROPAGE on private file mappings

[Lorenzo Stoakes (ARM)]

On Wed, Nov 11, 2026 at 12:55:09PM -0500, Tal Zussman wrote:
> If the mm cannot map the shared zero page, UFFDIO_ZEROPAGE falls back to
> mfill_atomic_pte_zeroed_folio(), which allocates a zeroed folio and
> installs it through vma_uffd_ops(), no matter what kind of mapping it
> is. An s390 KVM guest with storage keys or protected virtualization is
> such an mm.
>
> For a MAP_PRIVATE shmem mapping, that adds the zeroed folio to the page
> cache of the file. That is wrong: a private mapping must never modify
> the file it maps, and the file might even be mapped from a read-only fd.
>
> The ioctl can also fail where it should succeed. A missing fault is only
> delivered for a hole in the file, but UFFDIO_ZEROPAGE can be called on
> an address that did not fault, or can race with a write to the file. If
> the file has a page at that offset by then, inserting the zeroed folio
> fails and the ioctl returns -EEXIST although nothing is mapped at the
> destination address.
>
> What we want is what UFFDIO_ZEROPAGE does on a private mapping when the
> shared zero page can be used, and what UFFDIO_COPY does: only check that
> nothing is mapped at the destination address, map zeroed memory there,
> and leave the page cache alone. The zeroed folio path did that before it
> was moved to vm_uffd_ops.
>
> To fix it, pick the ops in the common folio path, as
> mfill_atomic_pte_copy() did for UFFDIO_COPY: anon_uffd_ops for a private
> mapping, the ops of the VMA otherwise. That makes
> mfill_atomic_pte_copy() and mfill_atomic_pte_zeroed_folio() redundant,
> so remove them and rename __mfill_atomic_pte() to
> mfill_atomic_pte_folio().
>
> UFFDIO_ZEROPAGE no longer modifies the file, and only fails with -EEXIST
> if something is already mapped at the destination address.
>
> Fixes: f74991b4e383 ("shmem, userfaultfd: implement shmem uffd operations using vm_uffd_ops")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Tal Zussman <tz2294@xxxxxxxxxxxx>

Looks sensible (also another cleanup in disguise which I like ;):

Acked-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>

> ---
>  mm/userfaultfd.c | 46 ++++++++++++++++------------------------------
>  1 file changed, 16 insertions(+), 30 deletions(-)
>
> diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> index d65184d9a480..61a712e1f9f5 100644
> --- a/mm/userfaultfd.c
> +++ b/mm/userfaultfd.c
> @@ -531,15 +531,27 @@ static int mfill_copy_folio_retry(struct mfill_state *mfill_state,
>  	return 0;
>  }
>
> -static int __mfill_atomic_pte(struct mfill_state *state,
> -			      const struct vm_uffd_ops *ops)
> +static int mfill_atomic_pte_folio(struct mfill_state *state)
>  {
> +	const struct vm_uffd_ops *ops = vma_uffd_ops(state->vma);
>  	unsigned long dst_addr = state->dst_addr;
>  	unsigned long src_addr = state->src_addr;
>  	uffd_flags_t flags = state->flags;
>  	struct folio *folio;
>  	int ret;
>
> +	/*
> +	 * The normal page fault path for a MAP_PRIVATE mapping in a
> +	 * file-backed VMA will invoke the fault, fill the hole in the file and
> +	 * COW it right away. The result generates plain anonymous memory.
> +	 * So when we are asked to fill a hole in a MAP_PRIVATE mapping, we'll
> +	 * generate anonymous memory directly without actually filling the
> +	 * hole. For the MAP_PRIVATE case the robustness check only happens in
> +	 * the pagetable (to verify it's still none) and not in the page cache.
> +	 */
> +	if (!(state->vma->vm_flags & VM_SHARED))
> +		ops = &anon_uffd_ops;
> +
>  	if (!ops) {
>  		VM_WARN_ONCE(1, "UFFDIO_COPY for unsupported VMA");
>  		return -EOPNOTSUPP;
> @@ -597,32 +609,6 @@ static int __mfill_atomic_pte(struct mfill_state *state,
>  	return ret;
>  }
>
> -static int mfill_atomic_pte_copy(struct mfill_state *state)
> -{
> -	const struct vm_uffd_ops *ops = vma_uffd_ops(state->vma);
> -
> -	/*
> -	 * The normal page fault path for a MAP_PRIVATE mapping in a
> -	 * file-backed VMA will invoke the fault, fill the hole in the file and
> -	 * COW it right away. The result generates plain anonymous memory.
> -	 * So when we are asked to fill a hole in a MAP_PRIVATE mapping, we'll
> -	 * generate anonymous memory directly without actually filling the
> -	 * hole. For the MAP_PRIVATE case the robustness check only happens in
> -	 * the pagetable (to verify it's still none) and not in the page cache.
> -	 */
> -	if (!(state->vma->vm_flags & VM_SHARED))
> -		ops = &anon_uffd_ops;
> -
> -	return __mfill_atomic_pte(state, ops);
> -}
> -
> -static int mfill_atomic_pte_zeroed_folio(struct mfill_state *state)
> -{
> -	const struct vm_uffd_ops *ops = vma_uffd_ops(state->vma);
> -
> -	return __mfill_atomic_pte(state, ops);
> -}
> -
>  static int mfill_atomic_pte_zeropage(struct mfill_state *state)
>  {
>  	struct vm_area_struct *dst_vma = state->vma;
> @@ -634,7 +620,7 @@ static int mfill_atomic_pte_zeropage(struct mfill_state *state)
>
>  	if (mm_forbids_zeropage(dst_vma->vm_mm) ||
>  	    (dst_vma->vm_flags & VM_SHARED))
> -		return mfill_atomic_pte_zeroed_folio(state);
> +		return mfill_atomic_pte_folio(state);
>
>  	_dst_pte = pte_mkspecial(pfn_pte(zero_pfn(dst_addr),
>  					 dst_vma->vm_page_prot));
> @@ -927,7 +913,7 @@ static __always_inline ssize_t mfill_atomic_pte(struct mfill_state *state)
>  	if (uffd_flags_mode_is(flags, MFILL_ATOMIC_POISON))
>  		return mfill_atomic_pte_poison(state);
>  	if (uffd_flags_mode_is(flags, MFILL_ATOMIC_COPY))
> -		return mfill_atomic_pte_copy(state);
> +		return mfill_atomic_pte_folio(state);
>  	if (uffd_flags_mode_is(flags, MFILL_ATOMIC_ZEROPAGE))
>  		return mfill_atomic_pte_zeropage(state);
>
>
> --
> 2.39.5
>

--
Cheers, Lorenzo