Re: [RFC PATCH 09/13] mm/damon/tests/core-kunit: remove nr_accesses_bp setup and tests

[SeongJae Park <sj@kernel.org>]

On Fri, 19 Jun 2026 19:52:31 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] Divide-by-zero in `damon_nr_accesses_mvsum()` when `aggr_interval` is 0.
> 
> Pre-existing issues:
> - [High] Divide-by-zero in `damon_max_nr_accesses()` when `sample_interval` is 0.
> --
> 
> commit 6519e9a3b09dd0793b79fa00b085d57ba68bc353
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/tests/core-kunit: remove nr_accesses_bp setup and tests
>     
> DAMON core unit tests set up nr_accesses_bp for representing realistic
> damon_region, and also test the field.  nr_acceses_bp is no longer being
> used for a real use case.  Remove the setup and tests.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -210,6 +210,61 @@ static struct damon_probe *damon_nth_probe(int n, struct damon_ctx *ctx)
> [ ... ]
> > +unsigned int damon_nr_accesses_mvsum(struct damon_region *r,
> > +		struct damon_ctx *ctx)
> > +{
> > +	unsigned long sample_interval, window_len, left_window, left_window_bp;
> > +
> > +	sample_interval = ctx->attrs.sample_interval ? : 1;
> > +	window_len = ctx->attrs.aggr_interval / sample_interval;
> > +	left_window = ctx->next_aggregation_sis - ctx->passed_sample_intervals;
> > +	left_window_bp = mult_frac(left_window, 10000, window_len);
>                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> [Severity: High]
> If `ctx->attrs.aggr_interval` is 0, `window_len` evaluates to 0.
> 
> Since `damon_set_attrs()` accepts configurations where both `sample_us` and
> `aggr_us` are 0, does this lead to a divide-by-zero here?
> 
> When the kdamond thread evaluates user-defined schemes, it appears to
> unconditionally call __damos_valid_target() -> damon_nr_accesses_mvsum().
> 
> Could this regression be triggered via the DAMON sysfs interface to cause
> a crash?

Yes, I will fix this in the next revision.

> 
> > +
> > +	return damon_mvsum(r->nr_accesses, r->last_nr_accesses,
> > +			left_window_bp);
> > +}
> 
> include/linux/damon.h:damon_max_nr_accesses() {
>     ...
>     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)UINT_MAX);
>     }
>     ...
> }
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but there is a pre-existing
> divide-by-zero risk if `sample_interval` is 0.
> 
> Does this calculation cause an immediate divide-by-zero exception?
> 
> Userspace can configure `sample_us` to 0 via sysfs, which avoids validation
> in `damon_set_attrs()` if `aggr_us` is also 0.
> 
> Since `damon_get_scheme_score()` unconditionally invokes
> `damon_max_nr_accesses()` when evaluating user-defined schemes, could this
> allow a user to intentionally crash the kernel?

Nice finding.  I will separately fix this.


Thanks,
SJ

[...]