Re: [PATCH 5/7] mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] 32-bit arithmetic overflow in sample count and basis points conversions leads to incorrect scaling for large interval ratios.
- [High] 32-bit integer overflow in `damon_age_for_new_attrs` when calculating region age during attribute updates on 32-bit architectures.
--

--- Patch [5]: [PATCH 5/7] mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/ ---
commit 7659397725f64a348908ab982da60d557f63b8c0
Author: SJ Park <sj@kernel.org>

mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/

This commit renames damon_nr_accesses_for_new_attrs() to
damon_nr_samples_for_new_attrs() to reflect its general use for any positive
sample count.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 718268df27fa7..800ff6b6d2473 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -890,22 +890,21 @@ static unsigned int damon_sample_count_to_bp(
>  	return mult_frac(count, 10000, damon_nr_samples_per_aggr(attrs));
>  }

[Severity: High]
This is a pre-existing issue, but does this code overflow 32-bit arithmetic
when the sample count is large?

In mm/damon/core.c:damon_sample_count_to_bp(), the mult_frac macro multiplies
the remainder by 10000, which can overflow 32-bit arithmetic.

Similarly, in damon_sample_bp_to_count(), the 32-bit multiplication overflows
since both operands are 32-bit unsigned integers:

mm/damon/core.c:damon_sample_bp_to_count() {
	return bp * damon_nr_samples_per_aggr(attrs) / 10000;
}

This occurs when the number of samples per aggregation interval exceeds
roughly 429,496 (e.g., sample_interval=1us and aggr_interval=1s, yielding
1,000,000 samples).

This could cause the region access tracking variables to be corrupted and
truncated during dynamic attribute updates (damon_set_attrs), rendering DAMOS
memory management schemes dysfunctional.

Both intervals are fully configurable by privileged users via DAMON
sysfs/debugfs interfaces, or dynamically updated by DAMON's auto-tuning
feature, making the overflow reachable in real-world scenarios.

[Severity: High]
This is also a pre-existing issue, but can the region age calculation
overflow on 32-bit architectures during attribute updates?

Looking at damon_age_for_new_attrs(), the multiplication is limited to the
architecture's native unsigned long size:

mm/damon/core.c:damon_age_for_new_attrs() {
	return age * old_attrs->aggr_interval / new_attrs->aggr_interval;
}

On 32-bit architectures, aggr_interval (unsigned long) and age (unsigned int)
are both 32-bit. The product exceeds ULONG_MAX (4,294,967,295) once the
stable time exceeds ~4,294 seconds.

This causes an integer overflow before the division, meaning any memory
region that remains stable for more than ~71.5 minutes will have its age
wrapped around and reset to an incorrectly small value when DAMON
attributes are dynamically updated.

This could break DAMOS schemes that target old pages for reclamation.

The bug is automatically triggered by DAMON's auto-tuning thread
(kdamond_tune_intervals), which periodically calls damon_set_attrs, or via
user-initiated sysfs attribute updates.

> -static unsigned int damon_nr_accesses_for_new_attrs(unsigned int nr_accesses,
> +static unsigned int damon_nr_samples_for_new_attrs(unsigned int nr,
>  		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs)
>  {
>  	return damon_sample_bp_to_count(
> -			damon_sample_count_to_bp(nr_accesses, old_attrs),
> -			new_attrs);
> +			damon_sample_count_to_bp(nr, old_attrs), new_attrs);
>  }

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