Re: [RFC PATCH v1.3 05/18] mm/damon/core: use damon_nr_accesses_mvsum() in __damos_valid_target()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] 32-bit integer overflow in damon_nr_accesses_mvsum() leads to incorrect region access frequency scores.
--

commit 31c0062dc1352032d1779ac0035b6e2cd5a7461a
Author: SeongJae Park <sj@kernel.org>

mm/damon/core: use damon_nr_accesses_mvsum() in __damos_valid_target()

This commit changes __damos_valid_target() to use damon_nr_accesses_mvsum()
instead of reading the nr_accesses_bp field directly. This provides a more
error-tolerant moving sum access score for DAMOS scheme logic.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 8a7351b4a27c2..b9d47599f5cbc 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2128,10 +2128,11 @@ static void kdamond_tune_intervals(struct damon_ctx *c)
>  	damon_set_attrs(c, &new_attrs);
>  }
>  
> -static bool __damos_valid_target(struct damon_region *r, struct damos *s)
> +static bool __damos_valid_target(struct damon_region *r, struct damos *s,
> +		struct damon_ctx *c)
>  {
>  	unsigned long sz;
> -	unsigned int nr_accesses = r->nr_accesses_bp / 10000;
> +	unsigned int nr_accesses = damon_nr_accesses_mvsum(r, c);

[Severity: High]
Does this introduction of damon_nr_accesses_mvsum() expose an integer overflow
on 32-bit systems?

When calculating the access frequency score, damon_nr_accesses_mvsum() calls
mult_frac() which performs an intermediate multiplication:

mm/damon/core.c:damon_nr_accesses_mvsum() {
    ...
	left_window_bp = mult_frac(left_window, 10000, window_len);
    ...
}

If a large tracking window is configured, for instance with an aggr_interval
of 1,000,000 us and a sample_interval of 1 us, window_len becomes 1,000,000
and left_window can reach up to 999,999.

The intermediate multiplication for mult_frac() uses the type of left_window,
which is unsigned long. On 32-bit architectures, this intermediate value
(e.g., 999,999 * 10000 = 9,999,990,000) exceeds the 32-bit maximum of
ULONG_MAX (4,294,967,295), causing a silent wrap-around.

This results in a completely incorrect left_window_bp value and drastically
undervalues the access frequency of memory regions.

Since __damos_valid_target() directly ingests this faulty access score, could
this regression lead DAMOS schemes to inappropriately skip highly active memory
regions and break memory reclamation or migration mechanisms on 32-bit systems?

>  
>  	sz = damon_sz_region(r);
>  	return s->pattern.min_sz_region <= sz &&

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