Re: [RFC PATCH v1.2 2/2] mm/damon/ops-common: prevent >DAMON_MAX_SUBSCORE freq_subscore

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] The patch attempts to fix an out-of-bounds array access by capping `freq_subscore`, but fails to account for integer overflow, leaving the vulnerability fully exploitable.
- [Critical] The `hotness` weighted average calculation is vulnerable to integer overflows via unchecked user-provided weights, leading to an arbitrary out-of-bounds array write.
--

commit 0014f07b693432d299630c4586e19c0cad5fbec9
Author: SeongJae Park <sj@kernel.org>

mm/damon/ops-common: prevent >DAMON_MAX_SUBSCORE freq_subscore

When a zero sampling interval and a zero aggregation interval are
online-committed, some regions could have nr_acceses values that are
larger than damon_max_nr_accesses() for the remaining aggregation window.
This commit attempts to fix out-of-bounds array accesses by applying an
upper bound of DAMON_MAX_SUBSCORE to the freq_subscore.

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

> diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> index 5c93ef2bb8a97..8d516851a69e4 100644
> --- a/mm/damon/ops-common.c
> +++ b/mm/damon/ops-common.c
> @@ -115,6 +115,9 @@ int damon_hot_score(struct damon_ctx *c, struct damon_region *r,
>  
>  	freq_subscore = r->nr_accesses * DAMON_MAX_SUBSCORE /
>  		damon_max_nr_accesses(&c->attrs);
> +	/* TODO: update monitoring results always to avoid this. */
> +	if (freq_subscore > DAMON_MAX_SUBSCORE)
> +		freq_subscore = DAMON_MAX_SUBSCORE;

[Severity: Critical]
This is a pre-existing issue, but does the new bounds check fully prevent the
out-of-bounds access if freq_subscore overflows and becomes negative?

Since r->nr_accesses is an unsigned int and DAMON_MAX_SUBSCORE is 100,
r->nr_accesses * DAMON_MAX_SUBSCORE is evaluated as an unsigned int.

If a user previously configured a large aggregation interval, r->nr_accesses
can grow up to ~22,000,000. If intervals are then online-committed to 0,
damon_update_monitoring_results() skips scaling, leaving nr_accesses at
its large value, while damon_max_nr_accesses() returns 1.

When r->nr_accesses * 100 exceeds 2^31 - 1, the division result assigned
to the signed int freq_subscore overflows and becomes a large negative value.

The newly added check if (freq_subscore > DAMON_MAX_SUBSCORE) uses signed
comparison, evaluates to false for negative values, and entirely bypasses
the clamp.

This negative value propagates to the hotness score. Finally,
damos_adjust_quota() implicitly casts this negative score to an unsigned int
array index, resulting in a huge offset causing an arbitrary out-of-bounds
memory write on c->regions_score_histogram.

[Severity: Critical]
This is a pre-existing issue, but can the hotness weighted average calculation
overflow and bypass the DAMOS_MAX_SCORE bounds?

The user-configurable weights freq_weight (s->quota.weight_nr_accesses) and
age_weight (s->quota.weight_age) are set via sysfs without upper bounds,
allowing them to be arbitrarily large.

In damon_hot_score(), the hotness is computed in 32-bit unsigned arithmetic:

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

If the weights are chosen such that freq_weight + age_weight wraps around
UINT_MAX to a small value (e.g., 2), the numerator also wraps.

When the wrapped numerator is cast to the signed int hotness and divided by
the small unsigned sum, C's usual arithmetic conversions promote the negative
int hotness to a huge unsigned int before division.

The final scaled result entirely evades the intended DAMOS_MAX_SCORE bound,
returning an arbitrary out-of-bounds index that damos_adjust_quota() uses
for an unsafe write on c->regions_score_histogram.

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

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