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
next prev parent 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