Re: [RFC PATCH v1.1 05/13] mm/damon/sysfs-schemes: use damon_nr_accesses_mvsum() for damo regions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] If a user configures `sample_interval=0` and `aggr_interval=0`, the time difference calculation inside `damon_nr_accesses_mvsum()` underflows, resulting in a huge garbage value for `nr_accesses`. This corrupts sysfs outputs and erratically influences DAMOS operations.
--

commit 2a77ab9c787a549b18af4699f5ffa8d3e4d60ca2
Author: SeongJae Park <sj@kernel.org>

mm/damon/sysfs-schemes: use damon_nr_accesses_mvsum() for damo regions

This commit modifies the DAMON sysfs interface to calculate region
accesses using damon_nr_accesses_mvsum() rather than a simple division.

> diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> index 329cfd0bbe9f3..e648b0f809332 100644
> --- a/mm/damon/sysfs-schemes.c
> +++ b/mm/damon/sysfs-schemes.c
> @@ -159,7 +159,7 @@ struct damon_sysfs_scheme_region {
>  };
>  
>  static struct damon_sysfs_scheme_region *damon_sysfs_scheme_region_alloc(
> -		struct damon_region *region)
> +		struct damon_region *region, struct damon_ctx *ctx)
>  {
>  	struct damon_sysfs_scheme_region *sysfs_region = kmalloc_obj(*sysfs_region);
>  
> @@ -167,7 +167,7 @@ static struct damon_sysfs_scheme_region *damon_sysfs_scheme_region_alloc(
>  		return NULL;
>  	sysfs_region->kobj = (struct kobject){};
>  	sysfs_region->ar = region->ar;
> -	sysfs_region->nr_accesses = region->nr_accesses_bp / 10000;
> +	sysfs_region->nr_accesses = damon_nr_accesses_mvsum(region, ctx);

[Severity: High]
Could this lead to an unsigned integer underflow if a user configures
both sample_us and aggr_us to 0 via sysfs?

If a user configures sample_interval=0 and aggr_interval=0, the
intervals bypass the validation in damon_set_attrs(). Inside kdamond_fn(),
ctx->next_aggregation_sis is updated by adding 0 and never advances,
but ctx->passed_sample_intervals continues to increment on every loop
iteration.

When damon_nr_accesses_mvsum() is called, it calculates the difference:

mm/damon/core.c:damon_nr_accesses_mvsum() {
        ...
        left_window = ctx->next_aggregation_sis - ctx->passed_sample_intervals;
        ...
}

Because ctx->passed_sample_intervals quickly exceeds
ctx->next_aggregation_sis, does this subtraction result in an unsigned
integer underflow?

If left_window becomes a massive value, it appears it would propagate
through mult_frac() and damon_mvsum() to yield a garbage value for
nr_accesses. Since __damos_valid_target() evaluates scheme criteria
using this value, could this cause DAMON to act on wildly inaccurate
access scores and disrupt system stability?

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