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

[Mike Rapoport]

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.