Re: [PATCH v2] mm/damon/ops-common: handle extreme intervals in damon_hot_score()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Multiplication in `freq_subscore` calculation overflows 32-bit `unsigned int` for extreme intervals, reversing region hotness prioritization.
- [High] User-supplied sysfs weights can overflow, resulting in a bypassed division and Signed Integer Overflow (UB).
--

commit 8323dd74c4905401bc8701431eb5fd1a6b12a452
Author: SeongJae Park <sj@kernel.org>

mm/damon/ops-common: handle extreme intervals in damon_hot_score()

Fix three issues in damon_hot_score() that comes from wrong handling of
extreme (zero or too high) monitoring intervals user setup.

Link: https://lore.kernel.org/20260619202459.145010-1-sj@kernel.org

> diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> index 5c93ef2bb8a97..d1842e2b00ef8 100644
> --- a/mm/damon/ops-common.c
> +++ b/mm/damon/ops-common.c
> @@ -143,6 +143,7 @@ int damon_hot_score(struct damon_ctx *c, struct damon_region *r,

[Severity: High]
This isn't a bug introduced by this patch, but could the freq_subscore
calculation overflow earlier in this function?

If a user configures a normal sample_interval (e.g., 100us) but a long
aggr_interval (e.g., 2 hours), r->nr_accesses can legitimately reach
72,000,000.

mm/damon/ops-common.c:damon_hot_score() {
    ...
    freq_subscore = r->nr_accesses * DAMON_MAX_SUBSCORE /
                    damon_max_nr_accesses(&c->attrs);
    ...
}

The multiplication of r->nr_accesses * DAMON_MAX_SUBSCORE (100) evaluates
to 7,200,000,000, which wraps around modulo 2^32, causing extremely hot
memory regions to be incorrectly scored as completely cold.

[Severity: High]
This is also a pre-existing issue, but can user-supplied sysfs weights
overflow and bypass the averaging division before this transformation?

mm/damon/ops-common.c:damon_hot_score() {
    ...
    if (freq_weight + age_weight)
        hotness /= freq_weight + age_weight;
    ...
}

If a user supplies weights for weight_nr_accesses and weight_age whose
sum is exactly UINT_MAX + 1 (e.g., 4,000,000,000 and 294,967,296), the
unsigned addition overflows to 0.

This causes the branch to evaluate as false, bypassing the averaging
division. The unaveraged weighted sum then evaluates to a large positive
integer.

>  	 * Transform it to fit in [0, DAMOS_MAX_SCORE]
>  	 */
>  	hotness = hotness * DAMOS_MAX_SCORE / DAMON_MAX_SUBSCORE;

Multiplying it by DAMOS_MAX_SCORE (99) here then overflows the signed int
result, resulting in undefined behavior.

> +	hotness = max(min(hotness, DAMOS_MAX_SCORE), 0);
>  
>  	return hotness;
>  }

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