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

[Lorenzo Stoakes (Oracle)]

On Wed, Apr 01, 2026 at 10:49:30AM +0300, Mike Rapoport wrote:
> Hi Andrew,
>
> On Tue, Mar 31, 2026 at 08:01:48PM -0700, Andrew Morton wrote:
> > On Tue, 31 Mar 2026 14:41:58 +0100 David Carlier <devnexen@xxxxxxxxx> wrote:
> >
> > > In mfill_copy_folio_retry(), all locks are dropped to retry
> > > copy_from_user() with page faults enabled. During this window, the VMA
> > > can be replaced entirely (e.g. munmap + mmap + UFFDIO_REGISTER by
> > > another thread), but the caller proceeds with a folio allocated from the
> > > original VMA's backing store.
>
> What does "folio allocated from the original VMA's backing store" exactly
> mean? Why is this a problem?
>
> > > Checking ops alone is insufficient: the replacement VMA could be the
> > > same type (e.g. shmem -> shmem) with identical flags but a different
> > > backing inode. Take a snapshot of the VMA's file and flags before
> > > dropping locks, and compare after re-acquiring them. If anything
> > > changed, bail out with -EINVAL.
> > >
> > > Use get_file()/fput() rather than ihold()/iput() to hold the file
> > > reference across the lock-dropped window, avoiding potential deadlocks
> > > from filesystem eviction under mmap_lock.
> >
> > Thanks, I've queued this as a squashable fix against mm-unstable's
> > "shmem, userfaultfd: implement shmem uffd operations using vm_uffd_ops
> > ongoing".
>
> First, this a pre-existing and TBH quite theoretical bug and it was there
> since the very beginning, so it should not be added as a fixup for the
> uffd+guestmemfd series.
>
> Second, I have reservations about vma_snapshot implementation. What
> invariant does it exactly enforce?

Yeah me too.

Unfortunately my bandwidth is a bit limited at the moment, but if you're
comparing VMAs like this it seems something is fundamentally broken.

We should definitely at least delay this until next cycle for consideration I
think until we can figure out a sensible approach.

>
> > I've fumbled the ball on your [2/2] unlikely() fix ;).  Please resend that
> > after -rc1.
>
> This one should go the same route IMO.

Agreed, let's delay until next cycle please.

>
> --
> Sincerely yours,
> Mike.

Thanks, Lorenzo