DAMON development mailing list
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "SJ Park" <sj@kernel.org>
Cc: damon@lists.linux.dev
Subject: Re: [PATCH 10/19] mm/damon/core: validate params for probe hits weighted sum overflow
Date: Fri, 10 Jul 2026 14:07:43 +0000	[thread overview]
Message-ID: <20260710140743.A469F1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260710134651.18084-11-sj@kernel.org>

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

Pre-existing issues:
- [High] Pre-existing integer overflow in `damon_merge_two_regions()` during weighted average calculations.
- [High] Pre-existing integer overflows in `damon_sample_bp_to_count()` and `damon_sample_count_to_bp()`.
--

--- Patch [10]: [PATCH 10/19] mm/damon/core: validate params for probe hits weighted sum overflow ---
commit 74455514643a15cd1b70c8a9afa3237ba4226d34
Author: SJ Park <sj@kernel.org>
Date:   Fri Jul 10 13:46:39 2026 +0000

    mm/damon/core: validate params for probe hits weighted sum overflow

This commit adds parameter validation to prevent integer overflows in probe
hits weighted sum calculations. It ensures that user-configured probe weights
do not cause the weighted sum of probe hits to exceed the maximum possible
value during monitoring.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 4f1425e56950b..a0e6a5129fbf3 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -1335,6 +1335,9 @@ static void damos_set_filters_default_reject(struct damos *s)
>  static bool damon_valid_probe_params(struct damon_ctx *ctx)
>  {
>  	unsigned long sample_interval;
> +	unsigned char max_probe_hits;
> +	struct damon_probe *probe;
> +	unsigned int wsum, wsum_to_add;
>  
>  	if (!damon_has_probe_weights(ctx))
>  		return true;
> @@ -1342,6 +1345,18 @@ static bool damon_valid_probe_params(struct damon_ctx *ctx)
>  	sample_interval = ctx->attrs.sample_interval ? : 1;
>  	if (ctx->attrs.aggr_interval / sample_interval > U8_MAX)
>  		return false;
> +
> +	/* invalid if probe hits weighted sum can overflow */
> +	max_probe_hits = damon_nr_samples_per_aggr(&ctx->attrs);
> +	wsum = 0;
> +	damon_for_each_probe(probe, ctx) {
> +		if (probe->weight > UINT_MAX / max_probe_hits)
> +			return false;
> +		wsum_to_add = probe->weight * max_probe_hits;
> +		if (UINT_MAX - wsum < wsum_to_add)
> +			return false;
> +		wsum += wsum_to_add;
> +	}
>  	return true;
>  }
>  

[Severity: High]
This is a pre-existing issue, but does damon_merge_two_regions() suffer from an
unsigned long overflow during average calculations?

mm/damon/core.c:damon_merge_two_regions() {
	...
	l->nr_accesses = (l->nr_accesses * sz_l + r->nr_accesses * sz_r) /
			(sz_l + sz_r);
	l->age = (l->age * sz_l + r->age * sz_r) / (sz_l + sz_r);
	...
}

During the calculation of the merged region metrics, if a region size reaches
1GB and nr_accesses is just 4, their product is 4,294,967,296. On 32-bit
architectures where unsigned long is 32 bits, this will instantly overflow.

Additionally, on 64-bit architectures, the age variable could grow indefinitely
and overflow if a region becomes very large over a long uptime. Can this lead
to corrupted statistics and misfiring of memory management actions?


[Severity: High]
This is a pre-existing issue, but I noticed a potential integer overflow in
damon_sample_bp_to_count() and damon_sample_count_to_bp():

mm/damon/core.c:damon_sample_bp_to_count() {
	return bp * damon_nr_samples_per_aggr(attrs) / 10000;
}

mm/damon/core.c:damon_sample_count_to_bp() {
	return mult_frac(count, 10000, damon_nr_samples_per_aggr(attrs));
}

Can these calculations overflow? The functions perform 32-bit multiplication of
bp (or the remainder in mult_frac) and the max samples using unsigned int
arithmetic.

If max_samples (aggr_interval / sample_interval) exceeds 429,496, which is
possible with a high aggregation interval compared to the sampling interval
(e.g., 5s aggregation and 10us sample intervals), the result could exceed
UINT_MAX (4,294,967,295) and silently overflow. Could this corrupt monitoring
region access counts during dynamic DAMON attribute updates?

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

  reply	other threads:[~2026-07-10 14:07 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-10 13:46 [PATCH 00/19] mm/damon: introduce data attributes only monitoring SJ Park
2026-07-10 13:46 ` [PATCH 01/19] mm/damon/core: introduce damon_probe->weight SJ Park
2026-07-10 13:46 ` [PATCH 02/19] mm/damon/core: ask apply_probes() ops callback to set sampling address SJ Park
2026-07-10 13:56   ` sashiko-bot
2026-07-10 14:32     ` SJ Park
2026-07-10 13:46 ` [PATCH 03/19] mm/damon/paddr: set samples in apply_probes() if requested SJ Park
2026-07-10 14:01   ` sashiko-bot
2026-07-10 14:33     ` SJ Park
2026-07-10 13:46 ` [PATCH 04/19] mm/damon/core: ask apply_probe() to return max probe hits weighted sum SJ Park
2026-07-10 14:01   ` sashiko-bot
2026-07-10 14:35     ` SJ Park
2026-07-10 13:46 ` [PATCH 05/19] mm/damon/core: implement damon_probe_hits_wsum() SJ Park
2026-07-10 13:46 ` [PATCH 06/19] mm/damon/paddr: respect return_max_wsum SJ Park
2026-07-10 13:46 ` [PATCH 07/19] mm/damon/core: use abs_diff() instead of abs() SJ Park
2026-07-10 14:10   ` sashiko-bot
2026-07-10 14:43     ` SJ Park
2026-07-10 13:46 ` [PATCH 08/19] mm/damon/core: extend merge function to work with probe hits SJ Park
2026-07-10 14:05   ` sashiko-bot
2026-07-10 14:49     ` SJ Park
2026-07-10 13:46 ` [PATCH 09/19] mm/damon/core: disallow probe_hits overflow on attrs only monitoring SJ Park
2026-07-10 13:46 ` [PATCH 10/19] mm/damon/core: validate params for probe hits weighted sum overflow SJ Park
2026-07-10 14:07   ` sashiko-bot [this message]
2026-07-10 14:51     ` SJ Park
2026-07-10 13:46 ` [PATCH 11/19] mm/damon/core: disable access monitoring when probe weights are set SJ Park
2026-07-10 13:46 ` [PATCH 12/19] mm/damon/core: set samples in apply_probes() if " SJ Park
2026-07-10 14:02   ` sashiko-bot
2026-07-10 14:52     ` SJ Park
2026-07-10 13:46 ` [PATCH 13/19] mm/damon/core: s/max_nr_accesses/max_merge_score/ in kdamond_fn() SJ Park
2026-07-10 13:46 ` [PATCH 14/19] mm/damon/core: get merge threshold from probe hits when weights are set SJ Park
2026-07-10 14:05   ` sashiko-bot
2026-07-10 14:54     ` SJ Park
2026-07-10 13:46 ` [PATCH 15/19] mm/damon/core: implement damon_has_probe_weight() SJ Park
2026-07-10 14:11   ` sashiko-bot
2026-07-10 14:55     ` SJ Park
2026-07-10 13:46 ` [PATCH 16/19] mm/damon/sysfs: implement probe/weight file SJ Park
2026-07-10 13:46 ` [PATCH 17/19] Docs/mm/damon/design: document attrs-only monitoring SJ Park
2026-07-10 13:46 ` [PATCH 18/19] Docs/admin-guide/mm/damon/usage: document weight sysfs file SJ Park
2026-07-10 13:46 ` [PATCH 19/19] Docs/ABI/damon: document probe weight file SJ Park
2026-07-10 14:59 ` [PATCH 00/19] mm/damon: introduce data attributes only monitoring SJ Park

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260710140743.A469F1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=damon@lists.linux.dev \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=sj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox