[PATCH 1/2] mm/userfaultfd: fix stale ops and VMA type mismatch after copy retry

[PATCH 1/2] mm/userfaultfd: fix stale ops and VMA type mismatch after copy retry

[David Carlier]

In mfill_atomic_pte_copy(), ops is derived from the VMA once and passed
to __mfill_atomic_pte(). When the initial copy_from_user() fails under
pagefault_disable(), mfill_copy_folio_retry() drops all locks, performs
the copy with page faults enabled, then re-acquires locks via
mfill_get_vma(). During this window, the VMA can be replaced entirely
(e.g. munmap + mmap + UFFDIO_REGISTER by another thread), but ops is
never re-validated.

If a shared shmem VMA is replaced by an anonymous VMA, the stale
shmem_uffd_ops->filemap_add calls shmem_mfill_filemap_add() with an
anonymous VMA, causing a NULL pointer dereference at file_inode(vma->
vm_file) since vm_file is NULL for anonymous mappings.

The mmap_changing guard does not fully prevent this because
userfaultfd_unmap_prep() only increments mmap_changing when
UFFD_FEATURE_EVENT_UNMAP is enabled, which is optional. Without it,
munmap proceeds without any signal to the retry path.

The copy_from_user() in the retry runs with page faults enabled and can
block on slow backing stores (FUSE, NFS), significantly widening the
race window.

Fix this by:
 - Validating that the VMA's userfaultfd context matches state->ctx in
   mfill_get_vma() to detect cross-context VMA replacement.
 - Re-checking that vma_uffd_ops() still matches the frozen ops after
   the retry, and that the VMA is still VM_SHARED when ops expects it
   to be, returning -EAGAIN otherwise.

Signed-off-by: David Carlier <devnexen@gmail.com>
---
 mm/userfaultfd.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index 481ec7eb4442..2a6e034b15aa 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -225,8 +225,9 @@ static int mfill_get_vma(struct mfill_state *state)
 	 */
 	down_read(&ctx->map_changing_lock);
 	state->vma = dst_vma;
+
 	err = -EAGAIN;
-	if (atomic_read(&ctx->mmap_changing))
+	if (dst_vma->vm_userfaultfd_ctx.ctx != ctx || atomic_read(&ctx->mmap_changing))
 		goto out_unlock;
 
 	err = -EINVAL;
@@ -498,6 +499,12 @@ static int __mfill_atomic_pte(struct mfill_state *state,
 			ret = mfill_copy_folio_retry(state, folio);
 			if (ret)
 				goto err_folio_put;
+			if (vma_uffd_ops(state->vma) != ops ||
+			    (ops != &anon_uffd_ops &&
+			    !(state->vma->vm_flags & VM_SHARED))) {
+				ret = -EAGAIN;
+				goto err_folio_put;
+			}
 		}
 	} else if (uffd_flags_mode_is(flags, MFILL_ATOMIC_ZEROPAGE)) {
 		clear_user_highpage(&folio->page, state->dst_addr);
-- 
2.53.0

[PATCH 2/2] mm/userfaultfd: fix wrong likely() hint on mmap_changing check in move_pages()

[David Carlier]

The mmap_changing check in move_pages() uses likely() but the condition
being true (concurrent mapping changes during UFFDIO_MOVE) is the
exceptional case, not the common one. All other mmap_changing checks in
the same file correctly use no branch hint or use unlikely().

Replace likely() with unlikely() to match the expected branch behavior.

Signed-off-by: David Carlier <devnexen@gmail.com>
---
 mm/userfaultfd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index 2a6e034b15aa..dc1b3dd1aece 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -1872,7 +1872,7 @@ ssize_t move_pages(struct userfaultfd_ctx *ctx, unsigned long dst_start,
 	/* Re-check after taking map_changing_lock */
 	err = -EAGAIN;
 	down_read(&ctx->map_changing_lock);
-	if (likely(atomic_read(&ctx->mmap_changing)))
+	if (unlikely(atomic_read(&ctx->mmap_changing)))
 		goto out_unlock;
 	/*
 	 * Make sure the vma is not shared, that the src and dst remap
-- 
2.53.0