Re: [RFC PATCH 4/6] mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/

[SJ Park <sj@kernel.org>]

On Tue, 07 Jul 2026 04:50:18 +0000 sashiko-bot@kernel.org wrote:

> 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?

Known low priority issue.

> 
> > -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


Thanks,
SJ

[...]