Re: [PATCH v2] mm/vma: avoid spurious write locks when mm->mm_lock_seq overflows

[Pedro Falcato <pfalcato@suse.de>]

On Fri, Oct 16, 2026 at 12:34:33PM +0100, Lorenzo Stoakes (ARM) wrote:
> So far the assumption has been that mm->mm_lock_seq overflow is benign,
> resulting only in readers spuriously falling back to the mmap lock, however
> it turns out that this was an unsafe assumption.
> 
> If a VMA can be spuriously considered write locked that means read lock
> exclusion does not take place, and thus a VMA can end up write-locked with
> concurrent readers.
> 
> Since overflow only occurs every 2^31 write lock/unlock cycles, resolve the
> issue by resetting every VMA's vma->vm_lock_seq each time such an overflow
> occurs.
> 
> This is done in vma_end_write_all(), which is called when the mmap write
> lock is released, i.e. in mmap_write_unlock() and mmap_write_downgrade().
> 
> Care must be taken, as resetting the sequence count at overflow means
> changing the seqcount of VMAs which are already attached and observable.
> 
> The mm's seqcount is guaranteed to be odd when a write lock is held, and
> thus any write locked VMA will have an odd seqcount assigned.
> 
> Therefore resetting to an even value prevents spurious write locks, since
> these can only occur with the mmap write lock held, at which point the mmap
> seqcount will be odd.
> 
> However, vma_start_read() also checks vma->vm_lock_seq == mm->mm_lock_seq,
> bailing if so (indicating a write lock).
> 
> This would cause any even reset value chosen to prevent VMA read locks from
> being taken - avoid this by also checking whether the VMA seqcount is odd
> before bailing on the read lock.
> 
> So far, vma->vm_lock_seq has defaulted to UINT_MAX, but this has been
> unnecessary since commit e5e7fb278e59 ("mm: convert mm_lock_seq to a proper
> seqcount"), since this is assigned when the VMA is still detached, and
> cannot be observed.
> 
> Therefore perform both reset and initialisation of vma->vm_lock_seq by
> simply setting vma->vm_lock_seq to zero.
> 
> The reset, performed in __mm_reset_vma_seqcounts(), only needs to
> WRITE_ONCE() the value.
> 
> However the read of vma->vm_lock_seq in vma_start_read() needs to have
> acquire semantics to pair with the smp_wmb() in do_raw_write_seqcount_end()
> so a reader reading a reset value also observes VMA modifications made
> under the write lock.
> 
> Since all VMA write locks were just released and the mmap write lock is
> still held, there are no concurrent VMA writers.
> 
> Concurrent VMA readers are unaffected. New VMA readers observe either zero,
> never a write lock, or the prior seqcount, behaving as before the reset.
> 
> Since this only occurs every 2^31 write lock/unlock cycles, the time taken
> to update all VMAs is considered to be acceptable.
> 
> Also update comments, documentation and VMA userland tests to reflect the
> change.
> 
> User facing effects are - dangerous races that could cause memory
> corruption or NULL pointer derefences (especially on page table traversal)
> are avoided.
> 
> Reported-by: Daehyeon Ko <4ncienth@gmail.com>
> Closes: https://lore.kernel.org/all/20261015143735.1630957-1-4ncienth@gmail.com/
> Fixes: 5e31275cc997 ("mm: add per-VMA lock and helper functions to control it")
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> ---
> Andrew - no stable tag please.
> 
> I will do the stable backports manually, they can be solved with a different
> approach.
> 
> Also, since, we're so close to the merge window I'm applying this against
> mm-unstable.
> 
> So I think it's fine to just send it (early :) in the merge window.
> ---
> v2:
> * Updated comment in vma_seqcount_is_write_locked() to mention the VMA
>   being attached.
> * Renamed reset_vma_seqcounts() to __mm_reset_vma_seqcounts().
> * Used acquire semantics in vma_start_reads() to avoid race with reset -
>   there is another acquire immediately after this one so it should just be
>   noise perf-wise.
> * Removed stable tag, will do the backports manually (just make the value
>   64 bits...)

I guess you're fitting this where __vm_anon_pgoff_lo now sits? If so, then
this patch probably wants backporting to 7.3.

> * Updated comments to reflect changes.
> * Added user-facing effects as per Vlasta ;)
> 
> v1:
> https://lore.kernel.org/r/20261015-fix-vma-lock-overflow-v1-1-01beb245d9da@kernel.org
> ---
>  
> +/*
> + * Called with mmap write lock held after mm->mm_lock_seq just overflowed (and
> + * therefore zero), with all VMA write locks now released.
> + *
> + * The stores below follow the smp_wmb() in do_raw_write_seqcount_end(), so
> + * a reader that observes a reset value also observes this cycle's VMA
> + * modifications (ordered by the ACQUIRE on vma->vm_lock_seq in
> + * vma_start_read()).
> + */
> +void __mm_reset_vma_seqcounts(struct mm_struct *mm)
> +{
> +	struct vm_area_struct *vma;
> +	VMA_ITERATOR(vmi, mm, 0);
> +
> +	mmap_assert_write_locked(mm);
> +	VM_WARN_ON_ONCE(mm->mm_lock_seq.sequence);
> +
> +	/*
> +	 * Failing to reset means stale vma->vm_lock_seq values could result in
> +	 * those VMAs becoming spuriously write locked when mm->mm_lock_seq
> +	 * happens to match their values.
> +	 *
> +	 * This is problematic, as it causes the VMA write lock to not exclude
> +	 * readers, which can result in concurrent readers while a VMA write
> +	 * lock is held.
> +	 *
> +	 * Avoid this by resetting to an even value (indicates no write lock
> +	 * held).
> +	 *
> +	 * Readers will either observe zero (not write locked) or the previous
> +	 * value, so this is safe.
> +	 */
> +	for_each_vma(vmi, vma)
> +		WRITE_ONCE(vma->vm_lock_seq, 0);

I'm concerned by the possible latency when doing this to many VMAs. Do you
have any numbers?

-- 
Pedro