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? > /* CHECK 2 */ > if (unlikely(vma->vm_lock_seq == raw_read_seqcount(&mm->mm_lock_seq))) > /* > * vm_lock_seq(11) == mm_lock_seq(12)? > * NO! writer already finished and unlocked! > * mm_lock_seq incremented to 12 (even=unlocked) > * CHECK 2 MISSES the race! > */ > return vma; > /* > * returns split vma with vm_end=0x4ca000 > * but vm_refcnt=1 (lock held) > */ -- Cheers, Harry / Hyeonggon
