Re: [PATCH v2 4/15] userfaultfd: introduce mfill_get_vma() and mfill_put_vma()

Deepanshu Kartikey
2026-03-16T08:52:58Z

On Mon, Mar 16, 2026 at 2:06 PM Harry Yoo <harry.yoo@oracle.com> wrote:
>
> On Mon, Mar 16, 2026 at 01:35:38PM +0530, Deepanshu Kartikey wrote:
> > On Mon, Mar 16, 2026 at 1:19 PM Harry Yoo <harry.yoo@oracle.com> wrote:
> > >
> > > > It seems there's another attempt to fix the syzbot report from
> > > > Deepanshu Kartikey [2], which I didn't take a deeper look.
> > > >
> > > > At first look [2] looks a bit wrong way to fix to me though,
> > > > because it allows operating only on a single VMA nothing should really split
> > > > or shrink the VMA if somebody is holding the VMA lock in read mode
> > > > (and the validation of the range is done while holding the lock).
> > > >
> > > > [2] https://lore.kernel.org/linux-mm/20260316070039.549506-1-kartikey406@gmail.com
> > > >
> >
> > Harry,
> >
> > You are correct that once vm_refcnt > 0, nobody can split the VMA.
> > However the split can happen in the race window BEFORE vm_refcnt++
> > in vma_start_read(), and CHECK 2 can miss this if mmap_write_unlock()
> > completes before CHECK 2 runs.
> >
> > Here is the exact race:
> >
> > vma_start_read():
> >
> > /* CHECK 1 */
> > if (READ_ONCE(vma->vm_lock_seq) == READ_ONCE(mm->mm_lock_seq.sequence))
> > goto err;
> >
> > /*
> > * RACE WINDOW: vm_refcnt is still 0 here!
> > * UFFDIO_UNREGISTER can run:
> > *
> > * mmap_write_lock() -> mm_lock_seq = 11
> > * vma_start_write(vma) -> vm_lock_seq = 11
> > * __split_vma() -> vma->vm_end = 0x4ca000
> > * mmap_write_unlock() -> mm_lock_seq = 12
> > *
> > * writer completes entirely before vm_refcnt++!
> > */
> >
> > __refcount_inc_not_zero_limited_acquire(&vma->vm_refcnt, ...);
> > /* vm_refcnt = 1 now, but vma->vm_end already modified! */
>
> It is true that vma->vm_end might have changed before acquiring the vma lock,
> but it doesn't matter as long as you verify the range after acquiring
> the lock, no? (that's what uffd_mfill_lock() does)
>
> You're not really supposed to read vma->vm_end before acquiring
> the vma lock and use the value because nothing guarantees that
> the VMA is stable until the lock is acquired.
>
> Or am I still missing something?
>

Harry, you are right. The real bug is state.len = 0.
I withdraw my fix.
Thank you for the explanation