Re: [PATCH v3 02/15] userfaultfd: introduce struct mfill_state

[Mike Rapoport <rppt@kernel.org>]

On Wed, Apr 01, 2026 at 12:24:01AM +0900, Harry Yoo (Oracle) wrote:
> On Tue, Mar 31, 2026 at 05:32:28PM +0300, Mike Rapoport wrote:
> |         /*
> |          * Make sure the vma is not shared, that the dst range is
> |          * both valid and fully within a single existing vma.
> |          */
> |         dst_vma = uffd_mfill_lock(dst_mm, dst_start, len);
> 
> It acquires the vma lock (or mmap_lock) here, but doesn't set state.vma.
> 
> |         if (IS_ERR(dst_vma)) {
> |                 err = PTR_ERR(dst_vma);
> |                 goto out;
> |         }

...

> |         if (!vma_is_anonymous(dst_vma) && !vma_is_shmem(dst_vma))
> |                 goto out_unlock;
> |         if (!vma_is_shmem(dst_vma) &&
> |             uffd_flags_mode_is(flags, MFILL_ATOMIC_CONTINUE))
> |                 goto out_unlock;
> |
> |         state.vma = dst_vma;
> 
> It is set here. So if anything before this jumps to `out_unlock`
> label due to a sanity check,
> 
> [...]
> 
> |         while (state.src_addr < src_start + len) {                              
> |                 VM_WARN_ON_ONCE(state.dst_addr >= dst_start + len);             
> |                                                                                 
> |                 pmd_t dst_pmdval;         
> | [...]
> | 
> | out_unlock:
> |         up_read(&ctx->map_changing_lock);
> |         uffd_mfill_unlock(state.vma);
> 
> the `vma` parameter will be NULL?
> 
> If I'm not missing something this is introduced in patch 2 and
> fixed in patch 4.

You are right.
Here's a fixup (it causes a conflict in patch 4 though).
Andrew, I can send v4 if you prefer.

diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index fa9622ec7279..c4074b6f4aca 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -764,6 +764,7 @@ static __always_inline ssize_t mfill_atomic(struct userfaultfd_ctx *ctx,
 		err = PTR_ERR(dst_vma);
 		goto out;
 	}
+	state.vma = dst_vma;
 
 	/*
 	 * If memory mappings are changing because of non-cooperative
@@ -804,8 +805,6 @@ static __always_inline ssize_t mfill_atomic(struct userfaultfd_ctx *ctx,
 	    uffd_flags_mode_is(flags, MFILL_ATOMIC_CONTINUE))
 		goto out_unlock;
 
-	state.vma = dst_vma;
-
 	while (state.src_addr < src_start + len) {
 		VM_WARN_ON_ONCE(state.dst_addr >= dst_start + len);
 
 

-- 
Sincerely yours,
Mike.