Re: [RFC PATCH 4/6] 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 1 potential issue(s) to consider:

Pre-existing issues:
- [High] 32-bit integer overflow in damon_sample_bp_to_count() and damon_sample_count_to_bp() corrupts memory access tracking.
--

--- Patch [4]: [RFC PATCH 4/6] mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/ ---
commit cb8a54b34f772f70862ab31ccf30eea4a5cb2263
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 clarify its general use for any
positive sample count.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 718268df27fa7..cda318806b65b 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -890,7 +890,7 @@ 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 can this math silently overflow when the
sample count is large?

In mm/damon/core.c:damon_sample_count_to_bp(), the mult_frac() macro inherits
the 32-bit precision of 'count'. If the user configures sysfs such that max
samples exceeds ~429,496, the intermediate remainder multiplication can wrap
around UINT_MAX.

For instance, 450,000 accesses out of 500,000 maximum samples will silently
overflow during mult_frac() calculation, resulting in a basis point of 410
(4.1% hot) instead of 9000 (90% hot).

Similarly, in mm/damon/core.c:damon_sample_bp_to_count():

    static unsigned int damon_sample_bp_to_count(
            unsigned int bp, struct damon_attrs *attrs)
    {
        return bp * damon_nr_samples_per_aggr(attrs) / 10000;
    }

The multiplication occurs in 32-bit precision before the division by 10000.
If damon_nr_samples_per_aggr() exceeds ~429,496, this will also overflow.

This drastic corruption could cause DAMOS to incorrectly evaluate region
hotness, potentially swapping out highly accessed memory. Would it make sense
to cast these to a 64-bit type before multiplication to avoid the overflow?

> -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_accesses,
>  		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs)
>  {
>  	return damon_sample_bp_to_count(

[ ... ]

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