Re: [RFC PATCH 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 2 potential issue(s) to consider:
- [High] Calling `damon_nr_accesses_mvsum()` triggers a division by zero kernel panic when `aggr_interval` and `sample_interval` are configured to 0.
- [Medium] Integer overflow in `damon_nr_accesses_mvsum()` on 32-bit architectures when `window_len` is large.
--

commit 66c2d8d0e11cf5ef0032d41b0f9dcf8d2fd41a3b
Author: SeongJae Park <sj@kernel.org>

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

This commit updates the DAMON sysfs interface to use 
damon_nr_accesses_mvsum() for scheme-tried regions instead of reading 
nr_accesses_bp directly.

> 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 new call trigger a divide-by-zero kernel panic?

If a user configures both aggr_interval and sample_interval to 0 via the
DAMON sysfs interface, damon_set_attrs() accepts these values. During the
calculation of the moving sum in damon_nr_accesses_mvsum(), window_len
evaluates to 0:

mm/damon/core.c:damon_nr_accesses_mvsum() {
    sample_interval = ctx->attrs.sample_interval ? : 1;
    window_len = ctx->attrs.aggr_interval / sample_interval;
    ...
    left_window_bp = mult_frac(left_window, 10000, window_len);
    ...
}

Since window_len becomes 0/1 = 0, the subsequent call to mult_frac()
performs a division by window_len, which would crash the system.

[Severity: Medium]
Also, does this calculation cause an integer overflow on 32-bit architectures?

The mult_frac macro expands to (rem * 10000) / window_len. On 32-bit
systems where unsigned long is 32 bits, if the aggregation interval is large
and sample interval is small, window_len can exceed 429,496.

As rem approaches window_len, the intermediate multiplication (rem * 10000)
exceeds ULONG_MAX and wraps around, corrupting the moving sum access
statistics.

>  	sysfs_region->age = region->age;
>  	sysfs_region->probes = NULL;
>  	INIT_LIST_HEAD(&sysfs_region->list);

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