Re: [RFC PATCH 1/5] mm/damon/core: fix nr_accesses_bp underflow in damon_moving_sum

[SeongJae Park <sj@kernel.org>]

On Sat, 16 May 2026 22:29:51 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Unsigned integer overflow in DAMON's moving sum and access rate tracking for large intervals.
> --
> 
> commit 1d58ed9728ddbaf4579ec265e453a7b3a233285c
> Author: Ravi Jonnalagadda <ravis.opensrc@gmail.com>
> 
> mm/damon/core: fix nr_accesses_bp underflow in damon_moving_sum
> 
> Guard against unsigned integer underflow when nomvsum/len_window exceeds
> mvsum. When that subtraction wraps, the moving sum returns a near maximum
> value and corrupts nr_accesses_bp. If subtrahend > mvsum, return new_value.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 3a8725e400c6b..9975f3d9ebfe9 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -3449,7 +3449,11 @@ int damon_set_region_system_rams_default(struct damon_target *t,
> >  static unsigned int damon_moving_sum(unsigned int mvsum, unsigned int nomvsum,
> >  		unsigned int len_window, unsigned int new_value)
> >  {
> 
> This isn't a bug introduced by this patch, but could the nomvsum argument
> passed here be silently overflowing?
> 
> Looking at damon_update_region_access_rate(), damon_moving_sum() is called
> with r->last_nr_accesses * 10000:
> 
> mm/damon/core.c:damon_update_region_access_rate() {
> 	...
> 	if (attrs->sample_interval)
> 		len_window = damon_max_nr_accesses(attrs);
> 	r->nr_accesses_bp = damon_moving_sum(r->nr_accesses_bp,
> 			r->last_nr_accesses * 10000, len_window,
> 			accessed ? 10000 : 0);
> 	...
> }
> 
> DAMON allows configuring aggr_interval / sample_interval up to the maximum
> 32-bit unsigned integer (for example, 10ms sampling and 1.5 hours
> aggregation).
> 
> This means len_window, which bounds last_nr_accesses, can exceed 429,496.
> 
> If this happens, won't the multiplication r->last_nr_accesses * 10000
> silently overflow the 32-bit unsigned int limit of 4,294,967,295?
> 
> If this overflows, the corrupted, wrapped-around value is passed as nomvsum,
> which would cause subtrahend to be artificially small.
> 
> Additionally, would this cause r->nr_accesses_bp (which accumulates mvsum)
> to also continuously wrap around 32-bit limits?
> 
> Could these silent integer overflows corrupt memory access tracking and lead
> to erroneous memory management decisions, like reclaiming hot memory?
> 
> A proper fix might require bounding len_window to 429,496 or using 64-bit
> integers.

That could happen when user sets aggr_interval > sample_interval * 429,496.  I
don't think that's a common setup, and the sane user would do some testing
before using such arbitrary setup.  So I'd suggest revisiting this when we have
enough time, or if a real world issue is reported.


Thanks,
SJ

[...]