mm/damon: handle zero {sample,aggr} intervals for DAMOS quota score
Subsystems (3)
SeongJae Park
maintainercover mm/damon: handle zero {sample,aggr} intervals for DAMOS quota score
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/[email protected]
- 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/[email protected]
- Wordsmith commit message.
- Drop RFC tag.
Changes from RFC v1
- RFC v1: https://lore.kernel.org/[email protected]
- 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
Signature · 1 line
2.47.3
SeongJae Park
maintainerRFC PATCH 1/2 mm/damon/core: handle zero intervals in damon_max_nr_accesses()
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/[email protected]
--- include/linux/damon.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
include/linux/damon.h +6 -2
@@ -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 ; }
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [RFC PATCH v1.2 1/2] mm/damon/core: handle zero intervals in damon_max_nr_accesses()
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
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 <[email protected]>
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.
Quoted 8 lines
include/linux/damon.h +0
-0@@ -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) {
include/linux/damon.h static inline bool damon_target_has_pid(const struct damon_ctx *ctx) · line 1075
Quoted 10 lines
include/linux/damon.h +6
-2-/* {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?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
SeongJae Park
maintainerRe: [RFC PATCH v1.2 1/2] mm/damon/core: handle zero intervals in damon_max_nr_accesses()
Quoted 46 lines · On Mon, 22 Jun 2026 14:29:36 +0000 [email protected] 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 innr_accesses_bpcalculation whensample_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 <[email protected]>
mm/damon/core: handle zero intervals indamon_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.Quoted 8 lines
include/linux/damon.h +0
-0@@ -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) {include/linux/damon.h static inline bool damon_target_has_pid(const struct damon_ctx *ctx) · line 1075
Quoted 10 lines
include/linux/damon.h +6
-2-/* {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 insidedamon_max_nr_accesses(), there is a bypass inmm/damon/core.cthat still relies onsample_interval:mm/damon/core.c:damon_update_region_access_rate()
if (attrs->sample_interval) len_window = damon_max_nr_accesses(attrs);
Quoted 8 lines
When
sample_intervalis 0, this bypass circumvents the properly fixeddamon_max_nr_accesses()function, lockinglen_windowat 1. During the next aggregation window,damon_moving_sum()recalculates the moving average. Becauselen_windowis stuck at 1, the entire nomvsum is subtracted from a smaller mvsum.Will this cause
nr_accesses_bpto 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.
Quoted 14 lines
[Severity: High]
This is also a pre-existing issue, but doesdamon_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_accessesis very high, does this 32-bit unsigned int arithmetic silently wrap around, heavily corruptingnr_accesses_bp?
Quoted 8 lines
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
[...]
SeongJae Park
maintainerRFC PATCH 2/2 mm/damon/ops-common: prevent >DAMON_MAX_SUBSCORE freq_subscore
In reply to SeongJae ParkWhen 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/[email protected]
--- mm/damon/ops-common.c | 3 +++ 1 file changed, 3 insertions(+)
mm/damon/ops-common.c +3 -0
@@ -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)
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [RFC PATCH v1.2 2/2] mm/damon/ops-common: prevent >DAMON_MAX_SUBSCORE freq_subscore
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
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 <[email protected]>
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.
mm/damon/ops-common.c int damon_hot_score(struct damon_ctx *c, struct damon_region *r, · line 120
Quoted 11 lines
mm/damon/ops-common.c +3
-0@@ -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.
Quoted 3 lines
mm/damon/ops-common.c +0
-0age_in_sec = (unsigned long )r->age * c->attrs.aggr_interval /1000000 ;if (age_in_sec)
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
SeongJae Park
maintainerRe: [RFC PATCH v1.2 2/2] mm/damon/ops-common: prevent >DAMON_MAX_SUBSCORE freq_subscore
Quoted 47 lines · On Mon, 22 Jun 2026 14:24:27 +0000 [email protected] 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 cappingfreq_subscore, but fails to account for integer overflow, leaving the vulnerability fully exploitable.
- [Critical] Thehotnessweighted 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 <[email protected]>
mm/damon/ops-common: prevent >DAMON_MAX_SUBSCOREfreq_subscoreWhen a zero sampling interval and a zero aggregation interval are online-committed, some regions could have
nr_accesesvalues that are larger thandamon_max_nr_accesses()for the remaining aggregation window. This commit attempts to fix out-of-bounds array accesses by applying an upper bound ofDAMON_MAX_SUBSCOREto thefreq_subscore.mm/damon/ops-common.c int damon_hot_score(struct damon_ctx *c, struct damon_region *r, · line 120
Quoted 11 lines
mm/damon/ops-common.c +3
-0@@ -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 iffreq_subscoreoverflows and becomes negative?Since
r->nr_accessesis an unsigned int andDAMON_MAX_SUBSCOREis 100,r->nr_accesses*DAMON_MAX_SUBSCOREis evaluated as an unsigned int.If a user previously configured a large aggregation interval,
r->nr_accessescan grow up to ~22,000,000. If intervals are then online-committed to 0,damon_update_monitoring_results()skips scaling, leavingnr_accessesat its large value, whiledamon_max_nr_accesses()returns 1.When
r->nr_accesses* 100 exceeds 2^31 - 1, the division result assigned to the signed intfreq_subscoreoverflows and becomes a large negative value.
Quoted 8 lines
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 onc->regions_score_histogram.
Nice finding. I will fix this in the next revision.
Quoted 22 lines
[Severity: Critical]
This is a pre-existing issue, but can the hotness weighted average calculation overflow and bypass theDAMOS_MAX_SCOREbounds?The user-configurable weights
freq_weight(s->quota.weight_nr_accesses) andage_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_weightwraps aroundUINT_MAXto a small value (e.g., 2), the numerator also wraps.
Quoted 7 lines
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_SCOREbound, returning an arbitrary out-of-bounds index thatdamos_adjust_quota()uses for an unsafe write onc->regions_score_histogram.
Nice finding, but not necessarily part of this series. I will separately work on this.
Thanks,
SJ
[...]
SeongJae Park
maintainerRe: [RFC PATCH v1.2 2/2] mm/damon/ops-common: prevent >DAMON_MAX_SUBSCORE freq_subscore
Quoted 53 lines · On Mon, 22 Jun 2026 07:42:43 -0700 SeongJae Park <[email protected]> wrote:
Quoted 51 lines · On Mon, 22 Jun 2026 14:24:27 +0000 [email protected] 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 cappingfreq_subscore, but fails to account for integer overflow, leaving the vulnerability fully exploitable.
- [Critical] Thehotnessweighted 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 <[email protected]>
mm/damon/ops-common: prevent >DAMON_MAX_SUBSCOREfreq_subscoreWhen a zero sampling interval and a zero aggregation interval are online-committed, some regions could have
nr_accesesvalues that are larger thandamon_max_nr_accesses()for the remaining aggregation window. This commit attempts to fix out-of-bounds array accesses by applying an upper bound ofDAMON_MAX_SUBSCOREto thefreq_subscore.mm/damon/ops-common.c int damon_hot_score(struct damon_ctx *c, struct damon_region *r, · line 120
Quoted 11 lines
mm/damon/ops-common.c +3
-0@@ -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 iffreq_subscoreoverflows and becomes negative?Since
r->nr_accessesis an unsigned int andDAMON_MAX_SUBSCOREis 100,r->nr_accesses*DAMON_MAX_SUBSCOREis evaluated as an unsigned int.If a user previously configured a large aggregation interval,
r->nr_accessescan grow up to ~22,000,000. If intervals are then online-committed to 0,damon_update_monitoring_results()skips scaling, leavingnr_accessesat its large value, whiledamon_max_nr_accesses()returns 1.When
r->nr_accesses* 100 exceeds 2^31 - 1, the division result assigned to the signed intfreq_subscoreoverflows 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.
Quoted 6 lines
Quoted 4 lines
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 onc->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
[...]
SeongJae Park
maintainerRe: [RFC PATCH v1.2 2/2] mm/damon/ops-common: prevent >DAMON_MAX_SUBSCORE freq_subscore
Quoted 60 lines · On Mon, 22 Jun 2026 07:53:27 -0700 SeongJae Park <[email protected]> wrote:
Quoted 58 lines · On Mon, 22 Jun 2026 07:42:43 -0700 SeongJae Park <[email protected]> wrote:
Quoted 55 lines · On Mon, 22 Jun 2026 14:24:27 +0000 [email protected] 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 cappingfreq_subscore, but fails to account for integer overflow, leaving the vulnerability fully exploitable.
- [Critical] Thehotnessweighted 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 <[email protected]>
mm/damon/ops-common: prevent >DAMON_MAX_SUBSCOREfreq_subscoreWhen a zero sampling interval and a zero aggregation interval are online-committed, some regions could have
nr_accesesvalues that are larger thandamon_max_nr_accesses()for the remaining aggregation window. This commit attempts to fix out-of-bounds array accesses by applying an upper bound ofDAMON_MAX_SUBSCOREto thefreq_subscore.mm/damon/ops-common.c int damon_hot_score(struct damon_ctx *c, struct damon_region *r, · line 120
Quoted 11 lines
mm/damon/ops-common.c +3
-0@@ -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 iffreq_subscoreoverflows and becomes negative?Since
r->nr_accessesis an unsigned int andDAMON_MAX_SUBSCOREis 100,r->nr_accesses*DAMON_MAX_SUBSCOREis evaluated as an unsigned int.If a user previously configured a large aggregation interval,
r->nr_accessescan grow up to ~22,000,000. If intervals are then online-committed to 0,damon_update_monitoring_results()skips scaling, leavingnr_accessesat its large value, whiledamon_max_nr_accesses()returns 1.When
r->nr_accesses* 100 exceeds 2^31 - 1, the division result assigned to the signed intfreq_subscoreoverflows 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 onc->regions_score_histogram.
Quoted 4 lines
Quoted 1 line
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.
Changed my mind again. I will fix this together, in the patch 1.
I initially thought this is better to be fixed separately, mainly because those are different class of bugs to my perspective. In more detail, this patch (patch 2) is for the lack of monitoring results update, which is introduced by commit 2f5bef5a590b ("mm/damon/core: update monitoring results for new monitoring attributes"). Meanwhile, the bug SAshiko found is for wrong monitoring intervals handling, which is introduced by commit 198f0f4c58b9 ("mm/damon/vaddr,paddr: support pageout prioritization").
But, this bug shares a same class with a bug that the patch 1 is fixing. Wrong handling of intervals in damon_hot_score(), which was introduced by the same commit.
I will update the first patch to fix two bugs together, and drop patch 2, as the issue will no more be reproducible.
Thanks,
SJ
[...]