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

[sashiko-bot@kernel.org]

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?

> +
> +	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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619193415.73833-1-sj@kernel.org?part=9