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.
