Re: [PATCH v10 12/18] mm: replace vm_lock and detached flag with a reference count

[Heiko Carstens <hca@linux.ibm.com>]

On Thu, Feb 13, 2025 at 02:46:49PM -0800, Suren Baghdasaryan wrote:
...
> While this vm_lock replacement does not yet result in a smaller
> vm_area_struct (it stays at 256 bytes due to cacheline alignment), it
> allows for further size optimization by structure member regrouping
> to bring the size of vm_area_struct below 192 bytes.
> 
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> ---
> Changes since v9 [1]:
> - Use __refcount_inc_not_zero_limited_acquire() in vma_start_read(),
> per Hillf Danton
> - Refactor vma_assert_locked() to avoid vm_refcnt read when CONFIG_DEBUG_VM=n,
> per Mateusz Guzik
> - Update changelog, per Wei Yang
> - Change vma_start_read() to return EAGAIN if vma got isolated and changed
> lock_vma_under_rcu() back to detect this condition, per Wei Yang
> - Change VM_BUG_ON_VMA() to WARN_ON_ONCE() when checking vma detached state,
> per Lorenzo Stoakes
> - Remove Vlastimil's Reviewed-by since code is changed

This causes crashes (NULL pointer deref) with linux-next when running
the ltp test suite; mtest06 (mmap1) test case.

The bug seems to be quite obvious:

> @@ -6424,15 +6492,18 @@ struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm,
>  	if (!vma)
>  		goto inval;
>  
> -	if (!vma_start_read(vma))
> -		goto inval;
> +	vma = vma_start_read(vma);
> +	if (IS_ERR_OR_NULL(vma)) {
            ^^^^^^^^^^^^^^^^^^^
> +		/* Check if the VMA got isolated after we found it */
> +		if (PTR_ERR(vma) == -EAGAIN) {
> +			vma_end_read(vma);
                        ^^^^^^^^^^^^^^^^