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

Peter Xu
2026-04-01T19:22:15Z

On Wed, Apr 01, 2026 at 07:34:47PM +0100, David CARLIER wrote:
> Hi Peter,
>
> On Tue, Apr 01, 2026 at 04:23:00PM +0300, Peter Xu wrote:
> > IMHO the flags is needed, consider a shared shmem vma remapped to a private
> > shmem vma. That needs to be covered in the fix.
>
> Right, I hadn't considered that case. Shared->private changes how
> the
> folio gets handled even with the same backing file. I'll keep the
> flags
> check.
>
> > Actually instead of reducing checks, maybe we also need to check
> the offset
> > of the mapping too, that is: vma->vm_pgoff can't change otherwise it may
> > also affect how the back store would behave on this UFFDIO_COPY
> request.
> >
> > For that, see the example of shmem_get_pgoff_policy() where it
> seems we can
> > apply different policies to different ranges of the back store.
>
> Good point. If vm_pgoff changes, linear_page_index() derives a
> different page cache offset for the same virtual address, and
> shmem_get_pgoff_policy() could apply a different NUMA policy to that
> range. So the folio could end up at the wrong offset or with the wrong
> placement.
>
> I'll add vm_pgoff to the snapshot. So the full set of checks after
> re-acquiring locks would be: vm_file, vm_flags, and vm_pgoff — ensuring
> the folio was allocated for the right backing file, at the right
> offset,
> with the right VMA type.

When caching the offset, we should likely use linear_page_index() with the
address provided rather than caching vma->vm_pgoff directly, then it'll
avoid same vm_pgoff while VMA mapping shifted like this:

VMA1: vm_pgoff=0x10000, vm_start=0x10000
VMA2: vm_pgoff=0x10000, vm_start=0xf000

So VMA1 unmapped, then the app mappped VMA2 at different VA but still cover
the address we're requesting for UFFDIO_COPY, even if the VMA will still
have the same vm_pgoff, the VA to access the same offset might change.
Using linear_page_index() will be accurate, IIUC.

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.

Thanks,

--
Peter Xu