Re: [PATCH 07/12] mm/mremap: resolve uffd state when the context is dropped, not during the move

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] userfaultfd: missing vma_start_write() in
  userfaultfd_clear_vma() [1]
--

commit 4d41e88f9230b819ec9df4cf29791deea1bb44b7
Author: Tal Zussman <tz2294@columbia.edu>

mm/mremap: resolve uffd state when the context is dropped, not during the move

This commit leaves userfaultfd PTE and PMD state untouched while moving
page table entries during mremap() and instead resolves it via
userfaultfd_clear_ptes() in mremap_userfaultfd_prep() once the move
succeeds.

> diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> index ad9d243dc64b0..bf42fabb5afd5 100644
> --- a/mm/userfaultfd.c
> +++ b/mm/userfaultfd.c
> @@ -3161,6 +3161,7 @@ void mremap_userfaultfd_prep(struct vm_area_struct *vma,
>  		up_write(&ctx->map_changing_lock);
>  	} else {
>  		/* Drop uffd context if remap feature not enabled */
> +		userfaultfd_clear_ptes(vma, vma->vm_start, vma->vm_end);
>  		userfaultfd_reset_ctx(vma);
>  	}
>  }

[Severity: High]
This problem wasn't introduced by this patch, but in the existing
unregister and release paths (userfaultfd_clear_vma() and
userfaultfd_release_new()), can calling userfaultfd_clear_ptes() before
vma_start_write() allow a concurrent UFFDIO_COPY, UFFDIO_CONTINUE, or
UFFDIO_MOVE to leave stale uffd PTE bits in the unregistered VMA [1]?

CPU0                                  CPU1
userfaultfd_clear_vma()
  userfaultfd_clear_ptes()
    change_protection()
      // scans past addr
                                      mfill_atomic()
                                        uffd_lock_vma()
                                          lock_vma_under_rcu()
                                        mfill_atomic_install_pte()
                                          // sees VM_UFFD_WP
                                          // installs pte_mkuffd() at addr
                                        uffd_unlock_vma()
  vma_modify_flags_uffd()
  userfaultfd_reset_ctx()
    userfaultfd_set_ctx()
      vma_start_write(vma)
      // clears VM_UFFD_* and ctx

Neither userfaultfd_unregister() nor userfaultfd_release() increments
ctx->mmap_changing, and lock_vma_under_rcu() succeeds until
vma_start_write() is called in userfaultfd_set_ctx() (or inside
vma_modify_flags_uffd() when the VMA is split or merged).  When a later
write fault reuses the exclusive page via wp_page_reuse() and makes the
PTE writable while the uffd bit is still set, page_table_check_pte_flags()
triggers WARN_ON_ONCE(pte_uffd(pte) && pte_write(pte)).

[1] https://sashiko.dev/#/bug/linux-5cddfd13-146a-47fc-97f8-feca4b2339b4

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-uffd-state-fixes-v1-0-00713d368976@columbia.edu?part=7