Re: [RFC PATCH v1.2 01/17] mm/damon: introduce damon_nr_accesses_mvsum()

[SeongJae Park <sj@kernel.org>]

On Sun, 21 Jun 2026 16:10:38 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Integer overflow in left_window_bp calculation on 32-bit architectures
> --
> 
> commit c5893684a7c356bd0f4693c2ef6895c46e83d332
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon: introduce damon_nr_accesses_mvsum()
> 
> Introduce a new DAMON core function, damon_nr_accesses_mvsum(), to return a
> pseudo moving sum value of a given region's nr_accesses for the last
> aggregation interval. This value acts as a relative access frequency score
> that is updated only when requested.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> > +unsigned int damon_nr_accesses_mvsum(struct damon_region *r,
> > +		struct damon_ctx *ctx)
> > +{
> > +	unsigned long sample_interval, aggr_interval;
> > +	unsigned long window_len, left_window, left_window_bp;
> > +
> > +	sample_interval = ctx->attrs.sample_interval ? : 1;
> > +	aggr_interval = ctx->attrs.aggr_interval ? : 1;
> > +	window_len = aggr_interval / sample_interval;
> > +	if (time_after_eq(ctx->passed_sample_intervals,
> > +				ctx->next_aggregation_sis))
> > +		left_window = 0;
> > +	else
> > +		left_window = ctx->next_aggregation_sis -
> > +			ctx->passed_sample_intervals;
> > +	left_window_bp = mult_frac(left_window, 10000, window_len);
> 
> [Severity: Medium]
> Can this cause an integer overflow on 32-bit architectures?
> 
> Looking at the mult_frac macro, the remainder is multiplied by the numerator:
> r * n_ / d_
> 
> If user-configurable parameters result in a large ratio, such as a 10
> microsecond sample interval and a 5 second aggregation interval, window_len
> becomes 500,000.
> 
> If left_window is near that maximum, for example 499,999, the intermediate
> multiplication inside the macro would be 499,999 * 10000. This evaluates to
> 4,999,990,000, which exceeds the 32-bit unsigned long maximum of
> 4,294,967,295.
> 
> Could this wrap around and result in an incorrectly small remainder,
> producing an inaccurate left_window_bp percentage? Passing 10000ULL might
> enforce 64-bit math to avoid this.

It happens only in weird setup, so we are ignoring this issue for now.  We can
revisit if a user claims this as an issue.


Thanks,
SJ

[...]