On Mon, Mar 16, 2026 at 2:06 PM Harry Yoo <[email protected]> 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 <[email protected]> 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/[email protected] > > > > > > > > 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
