Re: [PATCH] mm/damon/vaddr: drop redundant mmap_read_lock from damon_va_three_regions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Unsafe lockless VMA iteration exposes `SLAB_TYPESAFE_BY_RCU` freelist pointer corruption

Pre-existing issues:
- [High] Missing `check_stable_address_space()` validation during external `mm_struct` VMA iteration
--

commit fc1954fba1bc736b5f26aad51449f15c4774293c
Author: Cunlong Li <shenxiaogll@gmail.com>

mm/damon/vaddr: drop redundant mmap_read_lock from damon_va_three_regions

__damon_va_three_regions() already walks VMAs under rcu_read_lock(), so
the outer mmap_read_lock() is no longer needed.

> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index b069dbc7e3d25..27e34596c43b8 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
> @@ -77,7 +77,7 @@ static int __damon_va_three_regions(struct mm_struct *mm,
>  	struct damon_addr_range first_gap = {0}, second_gap = {0};
>  	VMA_ITERATOR(vmi, mm, 0);

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to iterate over
the VMAs of an external mm_struct here without first checking
check_stable_address_space(mm)?

If the target address space becomes unstable (for example, due to an OOM
reap or a failed dup_mmap()), the maple tree may contain XA_ZERO_ENTRY
markers and the mm is flagged MMF_UNSTABLE.

Without validating the address space, could the maple tree iterator
return XA_ZERO_ENTRY as a VMA pointer and lead to a kernel panic when
vma->vm_start is dereferenced?

>  	struct vm_area_struct *vma, *prev = NULL;
> -	unsigned long start;
> +	unsigned long start = 0, last_vma_end = 0;
>  
>  	/*
>  	 * Find the two biggest gaps so that first_gap > second_gap > others.

[ ... ]

> @@ -140,9 +141,7 @@ static int damon_va_three_regions(struct damon_target *t,
>  	if (!mm)
>  		return -EINVAL;
>  
> -	mmap_read_lock(mm);
>  	rc = __damon_va_three_regions(mm, regions);
> -	mmap_read_unlock(mm);

[Severity: High]
Does removing this mmap_read_lock() expose the VMA iteration to
SLAB_TYPESAFE_BY_RCU freelist pointer corruption?

Since the VMA slab cache is created with SLAB_TYPESAFE_BY_RCU, a VMA can
be freed while an RCU reader is still accessing it. When this happens,
the SLUB allocator overwrites the first word of the structure with its
freelist pointer (vm_freeptr).

Because vm_freeptr is in a union with vm_start and vm_end, locklessly
reading vma->vm_start from a recently freed VMA in
__damon_va_three_regions() will yield an unvalidated kernel freelist
pointer instead of a valid address.

Could subtracting prev->vm_end from this corrupted pointer cause massive
arithmetic underflows, tricking DAMON into tracking bogus memory regions?

>  
>  	mmput(mm);
>  	return rc;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260607-rcu-v1-1-3c9f5995a3a0@gmail.com?part=1