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

[Peter Xu]

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