Re: [PATCH 2/7] mm/damon/reclaim: cover all system rams

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Massive single DAMON region spanning physical address holes causes CPU soft lockups and quota starvation.
- [High] Time-of-Check to Time-of-Use (TOCTOU) vulnerability in damon_set_region_system_rams_default() leads to infinite loop and OOM panic.
--

commit 179df0c96a01c49e5644b5cc1a9154fb63368993
Author: SeongJae Park <sj@kernel.org>

mm/damon/reclaim: cover all system rams

This commit changes DAMON_RECLAIM to monitor the entire system RAM by
default, rather than just the biggest System RAM region, to cover all
available memory.

> diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> index b330ff169590..a60ee800d63e 100644
> --- a/mm/damon/reclaim.c
> +++ b/mm/damon/reclaim.c

[ ... ]

> @@ -232,11 +234,9 @@ static int damon_reclaim_apply_parameters(void)
>  		damos_add_filter(scheme, filter);
>  	}
>  
> -	err = damon_set_region_biggest_system_ram_default(param_target,
> -					&monitor_region_start,
> -					&monitor_region_end,
> -					param_ctx->addr_unit,
> -					param_ctx->min_region_sz);
> +	err = damon_set_region_system_rams_default(param_target,
> +			&monitor_region_start, &monitor_region_end,
> +			param_ctx->addr_unit, param_ctx->min_region_sz);

Can monitor_region_start and monitor_region_end be modified concurrently
via sysfs module parameters while this code is executing?

Looking at damon_set_region_system_rams_default(), the pointers are
dereferenced multiple times:

mm/damon/core.c:damon_set_region_system_rams_default() {
	if (*start > *end)
		return -EINVAL;

	if (!*start && !*end &&
		!damon_find_system_rams_range(start, end, addr_unit))
		return -EINVAL;

	addr_range.start = *start;
	addr_range.end = *end;
	return damon_set_regions(t, &addr_range, 1, min_region_sz);
}

If a concurrent sysfs write alters the values between the initial check and
the subsequent assignment, could this result in an inverted range where start
is greater than end?

If an inverted range is passed to damon_set_regions(), the size calculation
(end - start) could underflow to a massive value. When the thread later
executes damon_apply_min_nr_regions(), it might enter an infinite loop trying
to split the underflowed region, endlessly allocating damon_region objects.

Additionally, could damon_find_system_rams_range() create a single bounding
box that spans massive unpopulated physical address holes on sparse memory
systems?

Since DAMON applies schemes by linearly looping over the region page-by-page,
would spinning over terabytes of holes without calling cond_resched() cause
CPU soft lockups?

Furthermore, because these holes are never accessed, they might perfectly match
PAGEOUT schemes. Could the massive region size (including holes) be incorrectly
charged against the reclaim quota, instantly depleting it and preventing actual
RAM from being reclaimed?

Would it be better to dynamically populate multiple regions (one for each
RAM block) instead of using a single bounding box?

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