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:53:27 -0700 SeongJae Park <sj@kernel.org> wrote:

> 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.

Changed my mind again.  I will fix this together, in the patch 1.

I initially thought this is better to be fixed separately, mainly because those
are different class of bugs to my perspective.  In more detail, this patch
(patch 2) is for the lack of monitoring results update, which is introduced by
commit 2f5bef5a590b ("mm/damon/core: update monitoring results for new
monitoring attributes").  Meanwhile, the bug SAshiko found is for wrong
monitoring intervals handling, which is introduced by commit 198f0f4c58b9
("mm/damon/vaddr,paddr: support pageout prioritization").

But, this bug shares a same class with a bug that the patch 1 is fixing.  Wrong
handling of intervals in damon_hot_score(), which was introduced by the same
commit.

I will update the first patch to fix two bugs together, and drop patch 2, as
the issue will no more be reproducible.


Thanks,
SJ

[...]