Re: [RFC PATCH] mm/damon: handle zero sampling interval in damon_max_nr_accesses()

[SeongJae Park <sj@kernel.org>]

On Fri, 19 Jun 2026 21:03:04 +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] The patch replaces a divide-by-zero crash in `damon_max_nr_accesses()` with a divide-by-zero crash in caller functions such as `damon_hot_score()`. When `aggr_interval` and `sample_interval` are 0, `damon_max_nr_accesses()` returns 0, which is then used as a divisor by its callers.
> --
> 
> commit ca6b91d50d3b7234e014066f6e835c8aab906f49
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon: handle zero sampling interval in damon_max_nr_accesses()
> 
> damon_max_nr_accesses() causes divide-by-zero if sampling interval is
> zero.  Fix it by handling the case.
> 
> 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..83b43d8ffef38 100644
> > --- a/include/linux/damon.h
> > +++ b/include/linux/damon.h
> > @@ -1065,8 +1065,10 @@ 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)
> >  {
> > +	unsigned long sample_interval = attrs->sample_interval ? : 1;
> > +
> >  	/* {aggr,sample}_interval are unsigned long, hence could overflow */
> > -	return min(attrs->aggr_interval / attrs->sample_interval,
> > +	return min(attrs->aggr_interval / sample_interval,
> >  			(unsigned long)UINT_MAX);
> >  }
> 
> [Severity: High]
> This is a pre-existing issue, but does this patch shift the divide-by-zero
> crash to the callers of damon_max_nr_accesses()?
> 
> If a user configures DAMON via sysfs with both sample_interval and 
> aggr_interval set to 0, damon_set_attrs() allows it because the validation 
> check evaluates to false (0 > 0):
> 
> mm/damon/core.c:damon_set_attrs() {
> 	if (attrs->sample_interval > attrs->aggr_interval)
> 		return -EINVAL;
> 	...
> }
> 
> With this patch applied, damon_max_nr_accesses() prevents the initial
> division by zero by defaulting sample_interval to 1. However, it then
> evaluates min(0 / 1, UINT_MAX) and returns 0.
> 
> Later, when a DAMOS scheme with a quota is evaluated, damon_hot_score() uses
> the return value as a divisor:
> 
> mm/damon/ops-common.c:damon_hot_score() {
> 	...
> 	freq_subscore = r->nr_accesses * DAMON_MAX_SUBSCORE /
> 			damon_max_nr_accesses(&c->attrs);
> 	...
> }
> 
> Since damon_max_nr_accesses() now successfully returns 0, wouldn't this
> trigger a divide-by-zero exception in damon_hot_score() instead?
> 
> Could we explicitly reject an aggr_interval of 0 in damon_set_attrs(), or 
> ensure damon_max_nr_accesses() always returns at least 1?

Ah, good catch.  I will fix this in the next revision, like below:

'''
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -1252,11 +1252,11 @@ 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)
 {
+       unsigned long aggr_interval = attrs->aggr ? : 1;
        unsigned long sample_interval = attrs->sample_interval ? : 1;

        /* {aggr,sample}_interval are unsigned long, hence could overflow */
-       return min(attrs->aggr_interval / sample_interval,
-                       (unsigned long)UINT_MAX);
+       return min(ggr_interval / sample_interval, (unsigned long)UINT_MAX);
 }
'''


Thanks,
SJ

[...]