Re: [PATCH 03/18] mm/damon/core: always update ->last_nr_accesses for intervals change

[SJ Park <sj@kernel.org>]

On Tue, 30 Jun 2026 04:21:43 +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] A 32-bit unsigned integer overflow in `mult_frac()` silently corrupts region access basis point conversions when the aggregation interval is significantly larger than the sample interval.
> --
> 
> commit f786cbe1a79f2e1958843b60ee1875084aebedbc
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: always update ->last_nr_accesses for intervals change
> 
> Each iteration of kdamond_fn() caches the next aggregation time. This patch
> ensures last_nr_accesses is always updated to prevent returning wrong values
> when damon_nr_accesses_mvsum() is called during an intervals change.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -871,6 +871,8 @@ static void damon_update_monitoring_result(struct damon_region *r,
> >  		struct damon_attrs *old_attrs, struct damon_attrs *new_attrs,
> >  		bool aggregating)
> >  {
> > +	r->last_nr_accesses = damon_nr_accesses_for_new_attrs(
> > +			r->last_nr_accesses, old_attrs, new_attrs);
> 
> [Severity: High]
> This is a pre-existing issue, but does the underlying access basis point
> conversion silently overflow on 32-bit values?
> 
> When this calls damon_nr_accesses_for_new_attrs(), it relies on
> damon_nr_accesses_to_accesses_bp() to calculate the scaling:
> 
> mm/damon/core.c:damon_nr_accesses_to_accesses_bp() {
>     return mult_frac(nr_accesses, 10000, damon_max_nr_accesses(attrs));
> }
> 
> Because mult_frac() evaluates the multiplication using the type of
> nr_accesses (a 32-bit unsigned int), multiplying by 10000 will silently wrap
> around if nr_accesses exceeds ~429,496. This could happen if a large ratio
> between the aggregation interval and sample interval is configured via sysfs,
> causing hot memory regions to be incorrectly evaluated as cold.
> 
> A similar issue appears to exist in damon_nr_accesses_mvsum() where
> left_window (a 32-bit unsigned long on 32-bit architectures) is multiplied
> by 10000 inside mult_frac():
> 
> mm/damon/core.c:damon_nr_accesses_mvsum() {
>     ...
>     left_window_bp = mult_frac(left_window, 10000, window_len);
>     ...
> }
> 
> Could these operands be safely cast to a larger type to prevent the overflow
> before passing them to mult_frac()?

Known low priority issue.  No blocker for this patch.


Thanks,
SJ

[...]