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

[sashiko-bot@kernel.org]

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?

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