Re: [PATCH v4] mm/userfaultfd: detect VMA replacement after copy retry in mfill_copy_folio_retry()

Mike Rapoport
2026-04-02T03:58:44Z

Hi David,

It feels that you use an LLM for correspondence. Please tune it down to
produce more laconic and to the point responses.

On Wed, Apr 01, 2026 at 09:06:36AM +0100, David CARLIER wrote:
> On Tue, Apr 01, 2026 at 08:49:00AM +0300, Mike Rapoport wrote:
> > What does "folio allocated from the original VMA's backing store" exactly
> > mean? Why is this a problem?
>
> Fair point, the commit message was vague here. What I meant is:
>
> mfill_atomic_pte_copy() captures ops = vma_uffd_ops(state->vma) and
> passes it to __mfill_atomic_pte(). There, ops->alloc_folio() allocates
> a folio for the original VMA's inode (e.g. a shmem folio for that
> specific shmem inode).

I wouldn't say ->alloc_folio() allocates a folio _for_ the inode, it
allocates it with inode's memory policy. Worst can happen without any
changes is that the allocated folio will end up in a wrong node.

This is still a footgun, but I don't see it as a big deal.
Let's revisit it after -rc1 and please make sure to cc "MEMORY MAPPING"
folks for insights about how to better track VMA changes or their absence.

> Then mfill_copy_folio_retry() drops all locks for
> the copy_from_user retry. After mfill_get_vma() re-acquires them,
> state->vma may now point to a replacement VMA, but ops is still the
> stale pointer from before the drop.

And this is a bug in my uffd refactoring, and it needs to be fixed ASAP
with a simple comparison of old ops and new ops.

> > Second, I have reservations about vma_snapshot implementation. What
> > invariant does it exactly enforce?
>
> The invariant I was going for: "the folio we allocated is still
> compatible with the VMA we're about to install it into." Since
> alloc_folio() allocates from the VMA's backing file (inode), checking
> that vm_file is still the same after re-acquiring locks ensures the
> folio matches the inode.

Again, it's not that folio matches the inode, but folio is allocated using
the correct mempolicy.

> The vm_flags comparison was a secondary guard against permission/type
> changes during the window.

Permissions should be fine, they are checked in userfaultfd_register.
Some other flags that don't matter to uffd operation may change during the
window, though and then a comparison of vm_flags will give a false
positive.

--
Sincerely yours,
Mike.