Re: [PATCH] mm/damon/core: handle zero intervals in damon_max_nr_accesses()

[SeongJae Park <sj@kernel.org>]

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

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] The patch resolves a divide-by-zero crash but inadvertently introduces a critical out-of-bounds heap array read and write when updating the region score histogram.
> --
> 
> commit ba1da1a76126be43fbfd5ff0c59d54a52b8cddf8
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: handle zero intervals in damon_max_nr_accesses()
> 
> This patch modifies damon_max_nr_accesses() to avoid a divide-by-zero
> when the user sets the sampling or aggregation interval to zero. It
> ensures the denominator and return value are at least 1.
> 
> Link: https://lore.kernel.org/20260619202459.145010-1-sj@kernel.org
> 
> > diff --git a/include/linux/damon.h b/include/linux/damon.h
> > index 6f7edb3590ef9..888570f55b416 100644
> > --- a/include/linux/damon.h
> > +++ b/include/linux/damon.h
> 
> [ ... ]
> 
> > @@ -1065,9 +1065,13 @@ static inline bool damon_target_has_pid(const struct damon_ctx *ctx)
> >  
> >  static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrs)
> >  {
> > -	/* {aggr,sample}_interval are unsigned long, hence could overflow */
> > -	return min(attrs->aggr_interval / attrs->sample_interval,
> > +	unsigned long sample_interval;
> > +	unsigned long max_nr_accesses;
> > +
> > +	sample_interval = attrs->sample_interval ? : 1;
> > +	max_nr_accesses = min(attrs->aggr_interval / sample_interval,
> >  			(unsigned long)UINT_MAX);
> > +	return max_nr_accesses ? : 1;
> 
> [Severity: Critical]
> If sample_interval and aggr_interval are set to 0 via sysfs,
> damon_max_nr_accesses() now safely returns 1 instead of panicking.
> 
> However, damon_set_attrs() validation succeeds because
> attrs->sample_interval > attrs->aggr_interval evaluates to false (0 > 0).
> Because damon_update_monitoring_results() then skips resetting
> r->nr_accesses, it can retain a previous large value.
> 
> In mm/damon/ops-common.c:damon_hot_score(), the frequency subscore is
> calculated using the return value of damon_max_nr_accesses():
> 
> freq_subscore = r->nr_accesses * DAMON_MAX_SUBSCORE / max_nr_accesses;
> 
> Since max_nr_accesses is 1, a large r->nr_accesses will result in
> a hotness score that exceeds DAMOS_MAX_SCORE and is never explicitly
> clamped.
> 
> Later in mm/damon/core.c:damos_adjust_quota(), this score is used
> directly as an array index:
> 
> c->regions_score_histogram[score] += damon_sz_region(r);
> 
> Does this allow an out-of-bounds heap write when the histogram array
> is indexed by the unbounded score?

Good finding!  Nonetheless, this is introduced by another commit: commit
2f5bef5a590b ("mm/damon/core: update monitoring results for new monitoring
attributes").

I will post this patch again as a seris having the fix of newly found bug,
together.


Thanks,
SJ

[...]