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

[Peter Xu <peterx@redhat.com>]

On Thu, Apr 09, 2026 at 02:20:15PM +0300, Mike Rapoport wrote:
> On Thu, Apr 02, 2026 at 09:29:56AM -0400, Peter Xu wrote:
> > Hi, Mike,
> > 
> > On Thu, Apr 02, 2026 at 07:02:40AM +0300, Mike Rapoport wrote:
> > > On Wed, Apr 01, 2026 at 03:22:03PM -0400, Peter Xu wrote:
> > > >
> > > > The other thing is I just noticed the err code was changed to -EINVAL for
> > > > snapshot changed cases, sorry I didn't follow previously as closely on the
> > > > discussion.  I think it should be -EAGAIN.  It's because the userapp can't
> > > > resolve -EINVAL failures and app will crash.  In a VMA change use case, we
> > > > should return -EAGAIN to imply the app to retry, rather than crashing.
> > > 
> > > No. The return value should express that the VMA is invalid. -EINVAL could
> > > work, but looking now at the manual -ENOENT would be even better:
> > > 
> > >        ENOENT (since Linux 4.11)
> > >               The faulting process has changed its virtual memory layout
> > >               simultaneously with an outstanding UFFDIO_COPY operation.
> > 
> > The VMA changed, but it doesn't mean the UFFDIO_COPY becomes illegal, am I
> > right?
> 
> I don't think that "munmap + mmap + userfault_register"
> during an outstanding UFFDIO_COPY to the same range is, hmm, the smartest
> thing to do, and I think aborting the outstanding UFFDIO_COPY in such case
> is better than allowing it to continue.

It doesn't need to be unmap+map+register.  As mentioned below, I believe
writting 4 to clear_refs will already change VMA flags.  There're also many
other ways to change, IIUC, like mprotect() on top of uffd MISSING
registered ranges.

Meanwhile, I also don't think it's about whether it's a smart move.. I
agree most apps shouldn't do complex operations on VMAs when having
userfaultfd involved.  Said that, IMHO the whole point of kernel uAPI is to
make sure it works with every (even malicious) userapps, and it shouldn't
crash kernel.  So even if the reproducer will require complex VMA setups,
we should still close the gap.

>  
> > For example, I wonder if it's possible someone runs soft-dirty concurrently
> > with userfaultfd, we shouldn't fail the userapp if there's a concurrent
> > thread collecting dirty information, which IIUC can cause VMA flag changes,
> > and should be benign, and I think there can be other things causing the
> > interruption too.
> 
> Right, we shouldn't fail if some of the VMA flags changed, but we are
> talking about of complete change of the mapping, with potentially
> completely different backing store.

I don't know how to define "complete change of the mapping".  Here, IMHO
what we should do is to be strict on vma checks, either using the vma
snapshot or anything that can achieve the same goal, then returning -EAGAIN
is the safest because it won't crash a good citizen userapp.  The
re-evaluation will only be done later.

Thanks,

-- 
Peter Xu