Re: [PATCH v2 5/5] mm/damon/vaddr: implement DAMOS_SPLIT handler

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Hardcoded 2MB chunk stepping and alignment skips folios, evaluates out-of-bounds addresses, and inflates DAMON quota accounting.
- [High] Missing VMA validation allows `folio_walk_start` to interact with driver-allocated non-folio compound pages and hugetlb pages.
--

commit 5ba0b9e1345c425f2c1d50b6fcfb6b47ff6ed394
Author: Wang Lian <lianux.mm@gmail.com>

mm/damon/vaddr: implement DAMOS_SPLIT handler

This commit implements the vaddr operations layer handler for DAMOS_SPLIT.
It walks the target region and attempts to split any folios larger than
the scheme's target_order using split_folio_to_order().

> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index eeaa403eb79e4..2dd268f037a4f 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c

[ ... ]

> @@ -932,6 +932,71 @@ static unsigned long damos_va_collapse(struct damon_target *target,
>  	return applied;
>  }
>  
> +static unsigned long damos_va_split(struct damon_target *target,
> +		struct damon_region *r, struct damos *s,
> +		unsigned long *sz_filter_passed)
> +{
> +	unsigned long addr, end, chunk_sz;
> +	unsigned int target_order = s->target_order;
> +	unsigned long applied = 0;
> +	struct mm_struct *mm;
> +	struct vm_area_struct *vma;
> +	struct folio *folio;
> +	struct folio_walk fw;
> +
> +	mm = damon_get_mm(target);
> +	if (!mm)
> +		return 0;
> +
> +	chunk_sz = PAGE_SIZE << HPAGE_PMD_ORDER;
> +	addr = ALIGN_DOWN(r->ar.start, chunk_sz);
> +	end = ALIGN(r->ar.end, chunk_sz);

[Severity: High]
Does this alignment safely handle regions that do not start on 2MB
boundaries? By rounding down the start and rounding up the end to chunk_sz,
the loop in damos_va_split() may evaluate addresses outside the intended
damon_region bounds.

> +
> +	while (addr < end) {
> +		mmap_read_lock(mm);
> +		vma = find_vma(mm, addr);
> +		/*
> +		 * split_folio_to_order() supports both anon and shmem
> +		 * folios, so we accept any VMA that has a folio at @addr.
> +		 * This covers important use cases like tmpfs THP-backed
> +		 * KVM guest memory where cold and hot pages are bundled
> +		 * together in a single PMD THP.
> +		 */
> +		if (!vma || addr < vma->vm_start)
> +			goto unlock;

[Severity: High]
Are we missing VMA type validation here? This check accepts any VMA, which
can include VM_HUGETLB or driver-allocated VM_MIXEDMAP VMAs. 

If the VMA is VM_HUGETLB, folio_walk_start() will use standard page table
accessors which can return garbage or crash. If it is a driver-mapped
compound page, folio_walk_start() will return a non-folio page, and
subsequent calls to folio_trylock() and split_folio_to_order() can dereference
uninitialized fields and corrupt driver state.

> +
> +		folio = folio_walk_start(&fw, vma, addr, 0);
> +		if (!folio)
> +			goto unlock;
> +
> +		if (folio_order(folio) > target_order) {
> +			if (!folio_trylock(folio)) {
> +				folio_walk_end(&fw, vma);
> +				goto unlock;
> +			}
> +			folio_get(folio);
> +			folio_walk_end(&fw, vma);
> +
> +			if (!split_folio_to_order(folio, target_order))
> +				applied += chunk_sz;

[Severity: High]
Could this over-account the applied bytes? By unconditionally adding chunk_sz
(2MB) here, successfully splitting a smaller folio (like a 64KB mTHP) will
increment the applied counter by 2MB, artificially inflating the DAMON quota
accounting.

> +
> +			folio_unlock(folio);
> +			folio_put(folio);
> +		} else {
> +			folio_walk_end(&fw, vma);
> +		}
> +
> +unlock:
> +		*sz_filter_passed += chunk_sz;
> +		addr += chunk_sz;

[Severity: High]
Does this loop inadvertently skip folios? By unconditionally stepping addr by
chunk_sz (2MB) on every iteration, any smaller folios inside this 2MB span
will be completely skipped by the loop in damos_va_split(). Should this step
by the actual folio size instead to ensure we process all folios?

> +		mmap_read_unlock(mm);
> +		cond_resched();
> +	}
> +
> +	mmput(mm);
> +	return applied;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701114716.56503-1-lianux.mm@gmail.com?part=5