Re: [RFC PATCH v1.3] mm/damon/ops-common: handle extreme intervals in damon_hot_score()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The `age_in_sec` calculation silently overflows on 32-bit architectures, breaking age-based subscores.
- [Medium] Multiplication of `r->nr_accesses` by `DAMON_MAX_SUBSCORE` silently overflows 32-bit arithmetic for heavily accessed regions, breaking hotness scoring.
- [Medium] Weighted average calculation in `damon_hot_score` can overflow if large weights are provided via sysfs.
--

commit 9cb0813af4dd40334d721106d6d0be2f72fa88ca
Author: SeongJae Park <sj@kernel.org>

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

This patch fixes three issues in damon_hot_score() caused by extreme
monitoring intervals. It explicitly handles zero intervals to prevent
divide-by-zero errors and bounds the return value to prevent out-of-bounds
array access.

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: Medium]
This is a pre-existing issue, but while we are fixing extreme intervals in
damon_hot_score(), could the frequency subscore calculation silently overflow
on 32-bit architectures?

Looking at the calculation earlier in the function:

freq_subscore = r->nr_accesses * DAMON_MAX_SUBSCORE /
                damon_max_nr_accesses(&c->attrs);

If an administrator configures a large aggr_interval and a small
sample_interval, a frequently accessed region's nr_accesses could exceed
42,949,672. Since DAMON_MAX_SUBSCORE is 100, the multiplication would silently
wrap around modulo 2^32. This might give hot memory regions an artificially
low frequency score.

[Severity: High]
This is also a pre-existing issue, but does the age calculation earlier in
this function suffer from a similar 32-bit overflow?

age_in_sec = (unsigned long)r->age * c->attrs.aggr_interval / 1000000;

On 32-bit architectures where unsigned long is 32 bits, with a default
aggr_interval of 100,000 us (100ms), r->age increments every 100ms. After
about 71.6 minutes of continuous inactivity, r->age reaches 42,950. The
product reaches 4,295,000,000, overflowing ULONG_MAX (4,294,967,295).

The wrapped value divided by 1,000,000 yields 0, which artificially resets
age_in_sec to 0 and could cause old cold memory regions to be misclassified
as young.

[Severity: Medium]
This isn't a bug introduced by this patch, but could the weighted average
calculation also overflow if large weights are provided via sysfs?

hotness = (freq_weight * freq_subscore + age_weight * age_subscore);
if (freq_weight + age_weight)
        hotness /= freq_weight + age_weight;

The weights freq_weight and age_weight are read from sysfs and accept up to
UINT_MAX. This expression evaluates entirely in 32-bit unsigned math. If
large weights are provided, the calculation will wrap around the 32-bit
boundary. Because the intermediate hotness variable wraps and is later cast
to a signed int, this could result in wildly incorrect or even negative
prioritizations before clamping.

>  	 * Transform it to fit in [0, DAMOS_MAX_SCORE]
>  	 */
>  	hotness = hotness * DAMOS_MAX_SCORE / DAMON_MAX_SUBSCORE;
> +	hotness = max(min(hotness, DAMOS_MAX_SCORE), 0);
>  
>  	return hotness;
>  }

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