Re: [RFC PATCH v1.2 03/19] mm/damon/paddr: set samples in apply_probes() if requested

[SJ Park <sj@kernel.org>]

On Thu, 09 Jul 2026 14:21:02 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Integer overflow in `r->probe_hits[]` counter when `aggr_samples` exceeds 255.
> --
> 
> --- Patch [3]: [RFC PATCH v1.2 03/19] mm/damon/paddr: set samples in apply_probes() if requested ---
> commit 81ba2fab5bb28d260739f7b8c81129066d602897
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/paddr: set samples in apply_probes() if requested
> 
> This commit updates the apply_probe() callback implementation in DAMON_PADDR
> so that it respects the set_samples parameter.
> 
> > diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> > index b13bf7c6eade3..ff4fcca949660 100644
> > --- a/mm/damon/paddr.c
> > +++ b/mm/damon/paddr.c
> > @@ -178,6 +178,10 @@ static void damon_pa_apply_probes(struct damon_ctx *ctx, bool set_samples)
> >  			phys_addr_t pa;
> >  			struct folio *folio;
> >  
> > +			if (set_samples)
> > +				r->sampling_addr = damon_rand(ctx, r->ar.start,
> > +						r->ar.end);
> > +
> >  			pa = damon_pa_phys_addr(r->sampling_addr,
> >  					ctx->addr_unit);
> >  			folio = damon_get_folio(PHYS_PFN(pa));
> 
> [Severity: High]
> This is a pre-existing issue, but does this code overflow probe_hits[]?
> 
> Looking at include/linux/damon.h, probe_hits is defined as an unsigned char
> array:
> 
> struct damon_region {
> 	...
> 	unsigned char probe_hits[DAMON_MAX_PROBES];
> 	...
> };
> 
> Immediately following the code in this diff, damon_pa_apply_probes()
> unconditionally increments the counter:
> 
> 			folio = damon_get_folio(PHYS_PFN(pa));
> 			damon_for_each_probe(p, ctx) {
> 				if (damon_pa_filter_pass(pa, folio, p))
> 					r->probe_hits[i]++;
> 				i++;
> 			}
> 
> If a user configures DAMON via sysfs with an aggr_interval and sample_interval
> ratio that yields more than 255 samples per aggregation interval (for example,
> aggr_interval=100000us and sample_interval=100us), this counter will silently
> wrap around.
> 
> Could this cause heavily accessed regions to wrap to a small number and appear
> cold, potentially leading DAMOS to make incorrect memory eviction or migration
> decisions?

Known low priority issue.

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


Thanks,
SJ