* [RFC PATCH v1.2 0/2] mm/damon: handle zero {sample,aggr} intervals for DAMOS quota score
@ 2026-06-22 14:10 SeongJae Park
2026-06-22 14:10 ` [RFC PATCH v1.2 1/2] mm/damon/core: handle zero intervals in damon_max_nr_accesses() SeongJae Park
2026-06-22 14:10 ` [RFC PATCH v1.2 2/2] mm/damon/ops-common: prevent >DAMON_MAX_SUBSCORE freq_subscore SeongJae Park
0 siblings, 2 replies; 8+ messages in thread
From: SeongJae Park @ 2026-06-22 14:10 UTC (permalink / raw)
Cc: SeongJae Park, # 5 . 16 . x, Andrew Morton, damon, linux-kernel,
linux-mm
When the intervals are zero, divide-by-zero can happen during DAMOS
quota score calculation. Fixing it alone enables out-of-bound array
access. Fix those.
Changes from v1
- v1: https://lore.kernel.org/20260621154808.86431-1-sj@kernel.org
- Add out-of-bound array access bug fix as patch 2.
- Add the RFC tag again.
Changes from RFC v1.1
- RFC v1.1: https://lore.kernel.org/20260620171413.89555-1-sj@kernel.org
- Wordsmith commit message.
- Drop RFC tag.
Changes from RFC v1
- RFC v1: https://lore.kernel.org/20260619205144.150664-1-sj@kernel.org
- Handle zero aggr_interval case.
SeongJae Park (2):
mm/damon/core: handle zero intervals in damon_max_nr_accesses()
mm/damon/ops-common: prevent >DAMON_MAX_SUBSCORE freq_subscore
include/linux/damon.h | 8 ++++++--
mm/damon/ops-common.c | 3 +++
2 files changed, 9 insertions(+), 2 deletions(-)
base-commit: 42306d32afd75f28e3f13a1259c3b52191b4ff2c
--
2.47.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH v1.2 1/2] mm/damon/core: handle zero intervals in damon_max_nr_accesses()
2026-06-22 14:10 [RFC PATCH v1.2 0/2] mm/damon: handle zero {sample,aggr} intervals for DAMOS quota score SeongJae Park
@ 2026-06-22 14:10 ` SeongJae Park
2026-06-22 14:29 ` sashiko-bot
2026-06-22 14:10 ` [RFC PATCH v1.2 2/2] mm/damon/ops-common: prevent >DAMON_MAX_SUBSCORE freq_subscore SeongJae Park
1 sibling, 1 reply; 8+ messages in thread
From: SeongJae Park @ 2026-06-22 14:10 UTC (permalink / raw)
Cc: SeongJae Park, # 5 . 16 . x, Andrew Morton, damon, linux-kernel,
linux-mm
damon_max_nr_accesses() causes a divide-by-zero if the sampling interval
is set to zero by the user. If the aggregation interval is set to zero,
the function returns zero. It is wrong, since the real maximum
nr_acceses in the setup should be one. Worse yet, it can cause another
divide-by-zero from its caller, damon_hot_score(), since it uses
damon_max_nr_accesses() return value as a denominator.
Fix the problem by setting the denominator in the function as 1 when the
sampling interval is zero. Also ensure the return value is always 1 or
greater.
The issue was discovered [1] by Sashiko.
[1] https://lore.kernel.org/20260619202459.145010-1-sj@kernel.org
Fixes: 198f0f4c58b9 ("mm/damon/vaddr,paddr: support pageout prioritization")
Cc: <stable@vger.kernel.org> # 5.16.x
Signed-off-by: SeongJae Park <sj@kernel.org>
---
include/linux/damon.h | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/damon.h b/include/linux/damon.h
index 64d75c78f4df4..02ac34537df9a 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -1066,9 +1066,13 @@ static inline bool damon_target_has_pid(const struct damon_ctx *ctx)
static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrs)
{
- /* {aggr,sample}_interval are unsigned long, hence could overflow */
- return min(attrs->aggr_interval / attrs->sample_interval,
+ unsigned long sample_interval;
+ unsigned long max_nr_accesses;
+
+ sample_interval = attrs->sample_interval ? : 1;
+ max_nr_accesses = min(attrs->aggr_interval / sample_interval,
(unsigned long)UINT_MAX);
+ return max_nr_accesses ? : 1;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH v1.2 2/2] mm/damon/ops-common: prevent >DAMON_MAX_SUBSCORE freq_subscore
2026-06-22 14:10 [RFC PATCH v1.2 0/2] mm/damon: handle zero {sample,aggr} intervals for DAMOS quota score SeongJae Park
2026-06-22 14:10 ` [RFC PATCH v1.2 1/2] mm/damon/core: handle zero intervals in damon_max_nr_accesses() SeongJae Park
@ 2026-06-22 14:10 ` SeongJae Park
2026-06-22 14:24 ` sashiko-bot
1 sibling, 1 reply; 8+ messages in thread
From: SeongJae Park @ 2026-06-22 14:10 UTC (permalink / raw)
Cc: SeongJae Park, # 6 . 3 . x, Andrew Morton, damon, linux-kernel,
linux-mm
When a zero sampling interval and a zero aggregation interval are
online-committed, damon_max_nr_accesses() will return 1 right after the
update. damon_update_monitoring_results() skips updating nr_accesses of
regions for zero intervals, though. As a result, some regions could
have nr_acceses values that are larger than damon_max_nr_accesses() for
the remaining aggregation window. Note that the remaining aggregation
window will be quite short. It is just the remaining execution of the
kdamond_fn() main loop body, since the aggregation interval is zero.
If damon_hot_score() is called during the remaining aggregation window,
the function can calculate freq_subscore that is larger than
DAMON_MAX_SUBSCORE. Depending on the score weights and age/size scores,
damon_hot_score() can now return a score that is higher than
DAMOS_MAX_SCORE.
damos_adjust_quota(), which is an indirect caller of damon_hot_score()
uses the score as an index to regions_score_histogram array. The
array's size is set to only DAMOS_MAX_SCORE + 1. As a result, an
out-of-bound array access can happen.
The issue is expected to happen only rarely in the real world. After
all, zero aggregation interval is not supposed to be common. Also, the
online commit of zero intervals should be made on exactly when the DAMOS
scheme will be triggered. I was unable to trigger this on my own.
Nonetheless, it is possible in theory and the consequence is bad.
Fix the problem by applying an upper bound of the freq_subscore. This
is a short term fix. In the long term,
damon_update_monitoring_results() should be modified to update all
monitoring results even in case of zero aggregation interval. Add that
as a TODO.
The issue was discovered [1] by Sashiko.
[1] https://lore.kernel.org/20260621175849.91990-1-sj@kernel.org
Fixes: 2f5bef5a590b ("mm/damon/core: update monitoring results for new monitoring attributes")
Cc: <stable@vger.kernel.org> # 6.3.x
Signed-off-by: SeongJae Park <sj@kernel.org>
---
mm/damon/ops-common.c | 3 +++
1 file changed, 3 insertions(+)
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;
age_in_sec = (unsigned long)r->age * c->attrs.aggr_interval / 1000000;
if (age_in_sec)
--
2.47.3
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v1.2 2/2] mm/damon/ops-common: prevent >DAMON_MAX_SUBSCORE freq_subscore
2026-06-22 14:10 ` [RFC PATCH v1.2 2/2] mm/damon/ops-common: prevent >DAMON_MAX_SUBSCORE freq_subscore SeongJae Park
@ 2026-06-22 14:24 ` sashiko-bot
2026-06-22 14:42 ` SeongJae Park
0 siblings, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-06-22 14:24 UTC (permalink / raw)
To: SeongJae Park; +Cc: damon
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.
[Severity: Critical]
This is a pre-existing issue, but can the hotness weighted average calculation
overflow and bypass the DAMOS_MAX_SCORE bounds?
The user-configurable weights freq_weight (s->quota.weight_nr_accesses) and
age_weight (s->quota.weight_age) are set via sysfs without upper bounds,
allowing them to be arbitrarily large.
In damon_hot_score(), the hotness is computed in 32-bit unsigned arithmetic:
mm/damon/ops-common.c:damon_hot_score() {
...
hotness = (freq_weight * freq_subscore + age_weight * age_subscore);
if (freq_weight + age_weight)
hotness /= freq_weight + age_weight;
...
}
If the weights are chosen such that freq_weight + age_weight wraps around
UINT_MAX to a small value (e.g., 2), the numerator also wraps.
When the wrapped numerator is cast to the signed int hotness and divided by
the small unsigned sum, C's usual arithmetic conversions promote the negative
int hotness to a huge unsigned int before division.
The final scaled result entirely evades the intended DAMOS_MAX_SCORE bound,
returning an arbitrary out-of-bounds index that damos_adjust_quota() uses
for an unsafe write on c->regions_score_histogram.
>
> age_in_sec = (unsigned long)r->age * c->attrs.aggr_interval / 1000000;
> if (age_in_sec)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260622141027.29145-1-sj@kernel.org?part=2
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v1.2 1/2] mm/damon/core: handle zero intervals in damon_max_nr_accesses()
2026-06-22 14:10 ` [RFC PATCH v1.2 1/2] mm/damon/core: handle zero intervals in damon_max_nr_accesses() SeongJae Park
@ 2026-06-22 14:29 ` sashiko-bot
2026-06-22 14:36 ` SeongJae Park
0 siblings, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-06-22 14:29 UTC (permalink / raw)
To: SeongJae Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Integer underflow and state corruption in `nr_accesses_bp` calculation when `sample_interval == 0`.
- [High] 32-bit integer overflow in access rate scaling and hotness scoring silently breaks region statistics for long aggregation intervals.
--
commit 4e734fc35404d39d1209a202d1c72aa4e9ef543f
Author: SeongJae Park <sj@kernel.org>
mm/damon/core: handle zero intervals in damon_max_nr_accesses()
This commit addresses a divide-by-zero error in damon_max_nr_accesses()
by forcing a denominator of 1 when the sampling interval is zero. It also
ensures the function always returns at least 1.
Link: https://lore.kernel.org/20260619202459.145010-1-sj@kernel.org
> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index 6f7edb3590ef9..888570f55b416 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -1065,9 +1065,13 @@ static inline bool damon_target_has_pid(const struct damon_ctx *ctx)
>
> static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrs)
> {
> - /* {aggr,sample}_interval are unsigned long, hence could overflow */
> - return min(attrs->aggr_interval / attrs->sample_interval,
> + unsigned long sample_interval;
> + unsigned long max_nr_accesses;
> +
> + sample_interval = attrs->sample_interval ? : 1;
> + max_nr_accesses = min(attrs->aggr_interval / sample_interval,
> (unsigned long)UINT_MAX);
> + return max_nr_accesses ? : 1;
> }
[Severity: High]
This isn't a bug introduced by this patch, but while this commit fixes the
zero interval handling inside damon_max_nr_accesses(), there is a bypass in
mm/damon/core.c that still relies on sample_interval:
mm/damon/core.c:damon_update_region_access_rate()
if (attrs->sample_interval)
len_window = damon_max_nr_accesses(attrs);
When sample_interval is 0, this bypass circumvents the properly fixed
damon_max_nr_accesses() function, locking len_window at 1. During the
next aggregation window, damon_moving_sum() recalculates the moving
average. Because len_window is stuck at 1, the entire nomvsum is
subtracted from a smaller mvsum.
Will this cause nr_accesses_bp to massively underflow its unsigned int
type and wrap around, corrupting the region's access metrics?
[Severity: High]
This is also a pre-existing issue, but does damon_update_monitoring_result()
suffer from a 32-bit integer overflow for long aggregation intervals?
When a long aggregation interval and short sample interval are configured,
the accesses can be quite large:
mm/damon/core.c:damon_update_monitoring_result()
r->nr_accesses_bp = r->nr_accesses * 10000;
If nr_accesses is very high, does this 32-bit unsigned int arithmetic
silently wrap around, heavily corrupting nr_accesses_bp?
A similar overflow appears in damon_hot_score():
mm/damon/ops-common.c:damon_hot_score()
freq_subscore = r->nr_accesses * DAMON_MAX_SUBSCORE /
damon_max_nr_accesses(&c->attrs);
Does the numerator multiplication overflow the 32-bit bounds before
the division, resulting in an artificially skewed, low hotness score?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260622141027.29145-1-sj@kernel.org?part=1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v1.2 1/2] mm/damon/core: handle zero intervals in damon_max_nr_accesses()
2026-06-22 14:29 ` sashiko-bot
@ 2026-06-22 14:36 ` SeongJae Park
0 siblings, 0 replies; 8+ messages in thread
From: SeongJae Park @ 2026-06-22 14:36 UTC (permalink / raw)
To: sashiko-bot; +Cc: SeongJae Park, damon
On Mon, 22 Jun 2026 14:29:36 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Integer underflow and state corruption in `nr_accesses_bp` calculation when `sample_interval == 0`.
> - [High] 32-bit integer overflow in access rate scaling and hotness scoring silently breaks region statistics for long aggregation intervals.
> --
>
> commit 4e734fc35404d39d1209a202d1c72aa4e9ef543f
> Author: SeongJae Park <sj@kernel.org>
>
> mm/damon/core: handle zero intervals in damon_max_nr_accesses()
>
> This commit addresses a divide-by-zero error in damon_max_nr_accesses()
> by forcing a denominator of 1 when the sampling interval is zero. It also
> ensures the function always returns at least 1.
>
> Link: https://lore.kernel.org/20260619202459.145010-1-sj@kernel.org
>
> > diff --git a/include/linux/damon.h b/include/linux/damon.h
> > index 6f7edb3590ef9..888570f55b416 100644
> > --- a/include/linux/damon.h
> > +++ b/include/linux/damon.h
> > @@ -1065,9 +1065,13 @@ static inline bool damon_target_has_pid(const struct damon_ctx *ctx)
> >
> > static inline unsigned int damon_max_nr_accesses(const struct damon_attrs *attrs)
> > {
> > - /* {aggr,sample}_interval are unsigned long, hence could overflow */
> > - return min(attrs->aggr_interval / attrs->sample_interval,
> > + unsigned long sample_interval;
> > + unsigned long max_nr_accesses;
> > +
> > + sample_interval = attrs->sample_interval ? : 1;
> > + max_nr_accesses = min(attrs->aggr_interval / sample_interval,
> > (unsigned long)UINT_MAX);
> > + return max_nr_accesses ? : 1;
> > }
>
> [Severity: High]
> This isn't a bug introduced by this patch, but while this commit fixes the
> zero interval handling inside damon_max_nr_accesses(), there is a bypass in
> mm/damon/core.c that still relies on sample_interval:
>
> mm/damon/core.c:damon_update_region_access_rate()
> if (attrs->sample_interval)
> len_window = damon_max_nr_accesses(attrs);
>
> When sample_interval is 0, this bypass circumvents the properly fixed
> damon_max_nr_accesses() function, locking len_window at 1. During the
> next aggregation window, damon_moving_sum() recalculates the moving
> average. Because len_window is stuck at 1, the entire nomvsum is
> subtracted from a smaller mvsum.
>
> Will this cause nr_accesses_bp to massively underflow its unsigned int
> type and wrap around, corrupting the region's access metrics?
Good finding. Not a blocker of this patch, though. I will separately work on
this.
>
> [Severity: High]
> This is also a pre-existing issue, but does damon_update_monitoring_result()
> suffer from a 32-bit integer overflow for long aggregation intervals?
>
> When a long aggregation interval and short sample interval are configured,
> the accesses can be quite large:
>
> mm/damon/core.c:damon_update_monitoring_result()
> r->nr_accesses_bp = r->nr_accesses * 10000;
>
> If nr_accesses is very high, does this 32-bit unsigned int arithmetic
> silently wrap around, heavily corrupting nr_accesses_bp?
>
> A similar overflow appears in damon_hot_score():
>
> mm/damon/ops-common.c:damon_hot_score()
> freq_subscore = r->nr_accesses * DAMON_MAX_SUBSCORE /
> damon_max_nr_accesses(&c->attrs);
>
> Does the numerator multiplication overflow the 32-bit bounds before
> the division, resulting in an artificially skewed, low hotness score?
Known low priority issue. No blocker of this patch.
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v1.2 2/2] mm/damon/ops-common: prevent >DAMON_MAX_SUBSCORE freq_subscore
2026-06-22 14:24 ` sashiko-bot
@ 2026-06-22 14:42 ` SeongJae Park
2026-06-22 14:53 ` SeongJae Park
0 siblings, 1 reply; 8+ messages in thread
From: SeongJae Park @ 2026-06-22 14:42 UTC (permalink / raw)
To: sashiko-bot; +Cc: SeongJae Park, damon
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.
>
> [Severity: Critical]
> This is a pre-existing issue, but can the hotness weighted average calculation
> overflow and bypass the DAMOS_MAX_SCORE bounds?
>
> The user-configurable weights freq_weight (s->quota.weight_nr_accesses) and
> age_weight (s->quota.weight_age) are set via sysfs without upper bounds,
> allowing them to be arbitrarily large.
>
> In damon_hot_score(), the hotness is computed in 32-bit unsigned arithmetic:
>
> mm/damon/ops-common.c:damon_hot_score() {
> ...
> hotness = (freq_weight * freq_subscore + age_weight * age_subscore);
> if (freq_weight + age_weight)
> hotness /= freq_weight + age_weight;
> ...
> }
>
> If the weights are chosen such that freq_weight + age_weight wraps around
> UINT_MAX to a small value (e.g., 2), the numerator also wraps.
>
> When the wrapped numerator is cast to the signed int hotness and divided by
> the small unsigned sum, C's usual arithmetic conversions promote the negative
> int hotness to a huge unsigned int before division.
>
> The final scaled result entirely evades the intended DAMOS_MAX_SCORE bound,
> returning an arbitrary out-of-bounds index that damos_adjust_quota() uses
> for an unsafe write on c->regions_score_histogram.
Nice finding, but not necessarily part of this series. I will separately work
on this.
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v1.2 2/2] mm/damon/ops-common: prevent >DAMON_MAX_SUBSCORE freq_subscore
2026-06-22 14:42 ` SeongJae Park
@ 2026-06-22 14:53 ` SeongJae Park
0 siblings, 0 replies; 8+ messages in thread
From: SeongJae Park @ 2026-06-22 14:53 UTC (permalink / raw)
To: SeongJae Park; +Cc: sashiko-bot, damon
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.
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-06-22 14:53 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-22 14:10 [RFC PATCH v1.2 0/2] mm/damon: handle zero {sample,aggr} intervals for DAMOS quota score SeongJae Park
2026-06-22 14:10 ` [RFC PATCH v1.2 1/2] mm/damon/core: handle zero intervals in damon_max_nr_accesses() SeongJae Park
2026-06-22 14:29 ` sashiko-bot
2026-06-22 14:36 ` SeongJae Park
2026-06-22 14:10 ` [RFC PATCH v1.2 2/2] mm/damon/ops-common: prevent >DAMON_MAX_SUBSCORE freq_subscore SeongJae Park
2026-06-22 14:24 ` sashiko-bot
2026-06-22 14:42 ` SeongJae Park
2026-06-22 14:53 ` SeongJae Park
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.