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

[SeongJae Park <sj@kernel.org>]

On Mon, 22 Jun 2026 07:42:43 -0700 SeongJae Park <sj@kernel.org> wrote:

> On Mon, 22 Jun 2026 14:24:27 +0000 sashiko-bot@kernel.org wrote:
> 
> > 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.
> 
> Nice finding.  I will fix this in the next revision.

Hmm, no.  This is not related to zero interval.  I will fix this out of this
series.


Thanks,
SJ

[...]