Re: [PATCH 07/19] mm/damon/core: use abs_diff() instead of abs()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Unchecked memory allocation failure in `damon_apply_min_nr_regions()` leads to heap corruption and infinite loops
--

--- Patch [7]: [PATCH 07/19] mm/damon/core: use abs_diff() instead of abs() ---
commit 168e2738a1f4077551ffee47ed4ecb0a582831f3
Author: SJ Park <sj@kernel.org>

mm/damon/core: use abs_diff() instead of abs()

Replaces abs() with abs_diff() in damon_merge_regions_of() to prevent
silent integer overflow when casting unsigned int to signed int.

Link: https://lore.kernel.org/20260705213817.100841-1-sj@kernel.org/

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index fea97399b5754..4c7e8a12a9b67 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -3269,7 +3269,7 @@ static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
>  			r->age++;
>  
>  		if (prev && prev->ar.end == r->ar.start &&
> -		    abs(prev->nr_accesses - r->nr_accesses) <= thres &&
> +		    abs_diff(prev->nr_accesses, r->nr_accesses) <= thres &&
>  		    damon_sz_region(prev) + damon_sz_region(r) <= sz_limit)
>  			damon_merge_two_regions(t, prev, r);
>  		else

[Severity: Critical]
This is a pre-existing issue and was not introduced by the patch under review,
but I noticed a potential memory corruption issue in
damon_apply_min_nr_regions() when a memory allocation fails.

If damon_split_region_at() fails to allocate memory, it aborts silently and
leaves r->ar.end unchanged.

The loop then unconditionally advances r:

mm/damon/core.c:damon_apply_min_nr_regions() {
	damon_for_each_target(t, ctx) {
		damon_for_each_region_safe(r, next, t) {
			while (damon_sz_region(r) > max_region_sz) {
				damon_split_region_at(t, r, max_region_sz);
				r = damon_next_region(r);
			}
		}
	}
}

Can this corrupt memory or cause infinite loops if the allocation fails on
the last region in the list?

If this occurs on the last region in the target's list,
r = damon_next_region(r) resolves to the list head (&t->regions_list).

Because the list head is embedded inside struct damon_target, container_of()
returns a bogus struct damon_region pointer pointing before the damon_target
object.

The next iteration of the while loop would read r->ar.start and r->ar.end
from this invalid memory. If the computed difference exceeds max_region_sz,
it enters the loop again. A successful damon_split_region_at() could then
write the new region's start address to r->ar.end, corrupting memory before
the damon_target structure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710134651.18084-1-sj@kernel.org?part=7