Re: [PATCH RESEND] userfaultfd: snapshot VMA state across UFFDIO_COPY retry

[Liam R. Howlett]

On 26/05/20 04:38PM, David Hildenbrand (Arm) wrote:
> On 5/20/26 16:12, Mike Rapoport wrote:
> > On Wed, May 20, 2026 at 03:48:02PM +0200, David Hildenbrand (Arm) wrote:
> >> On 5/20/26 14:53, Mike Rapoport wrote:
> >>>
> >>> Do you mean per-VMA?
> >>> Per-MM counter would capture unrelated changes, e.g. an masvise() for
> >>> unrelated range.
> >>
> >> I was thinking about a per-MM thing for simplicity. If there were any changes,
> >> we'd retry (-EAGAIN).
> >>
> >> IOW, something like &mm->mm_lock_seq, which we have for per-vma locks already.
> >>
> >> Not sure if unrelated changes would really be a problem in practice (another
> >> thread gabbing the mmap_lock in write mode until we serviced the fault).
> > 
> > Let me reiterate:
> > 
> > A thread doing UFFDIO_COPY releases the VMA in mfill_copy_folio_retry(),
> > re-gets the VMA and checks if the per-MM counter stayed the same.
> > 
> > If another thread makes any change to VMA while mfill_copy_folio_retry()
> > waits to re-get the VMA, the counter would be incremented by another
> > thread. mfill_copy_folio_retry() will see the change after mfill_get_vma()
> > and will bail out with -EAGAIN.
> > 
> 
> Yeah.

This isn't bulletproof anyways.  The sequence count can wrap.  So, if
someone can replace the vma then cause the sequence counter wrap, then
you can be fooled into thinking it's okay (we had this problem years ago
with the vmacache using a 32 bit counter, iirc).

> 
> > So if another thread does, e.g. MADV_DONTFORK on completely unrelated VMA,
> > mfill_copy_folio_retry() will return -EAGAIN and I'm not sure we'll not
> > break the existing userspace by this.
> 
> Ah, you mean that -EAGAIN would not be handled by user space already even though
> documented in the man page ... because it relies on -EAGAIN not happening in
> specific situations (i.e., copying a single page).
> 
> That's very likely true and we'd have to retry internally, which makes it more
> complicated indeed.
> 
> -- 
> Cheers,
> 
> David