[RFC PATCH v1.3] mm/damon/ops-common: handle extreme intervals in damon_hot_score()

[RFC PATCH v1.3] mm/damon/ops-common: handle extreme intervals in damon_hot_score()

[SeongJae Park]

Fix three issues in damon_hot_score() that comes from wrong handling of
extreme (zero or too high) monitoring intervals user setup.

When the user sets sampling interval zero, damon_max_nr_accesses(),
which  is called from damon_hot_score(), causes a divide-by-zero.
Needless to say, it is a problem.

When the user sets the aggregation interval 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.

When the user sets the aggregation interval very high, damon_hot_score()
could return a value out of [0, DAMOS_MAX_SCORE] range.  Since the
return value is used as an index to the regions_score_histogram array,
which is DAMOS_MAX_SCORE+1 size, it causes out of bounds array access.

The issues can be relatively easily reproduced like below.  The sysfs
write permission is required, though.

    # ./damo start --damos_action lru_prio --damos_quota_space 100M \
            --damos_quota_interval 1s
    # cd /sys/kernel/mm/damon/admin/kdamonds/0
    # echo 0 > contexts/0/monitoring_attrs/intervals/sample_us
    # echo 0 > contexts/0/monitoring_attrs/intervals/aggr_us
    # echo commit > state
    # dmesg
    [...]
    [  131.329762] Oops: divide error: 0000 [#1] SMP NOPTI
    [...]
    [  131.336089] RIP: 0010:damon_hot_score+0x27/0xd0
    [...]

Fix the divide-by-zero intervals problems by explicitly handling the
zero intervals in damon_max_nr_accesses().  Fix the out-of-bound array
access by applying [0, DAMOS_MAX_SCORE] bounds before returning from
damon_hot_score().

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>
---
Changes from RFC v1.2
- RFC v1.2: https://lore.kernel.org/20260622141027.29145-1-sj@kernel.org
- Drop patch 2 and make patch 1 fixes all damon_hot_score() problems.
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.

 include/linux/damon.h | 8 ++++++--
 mm/damon/ops-common.c | 1 +
 2 files changed, 7 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;
 }
 
 
diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
index 5c93ef2bb8a97..d1842e2b00ef8 100644
--- a/mm/damon/ops-common.c
+++ b/mm/damon/ops-common.c
@@ -143,6 +143,7 @@ int damon_hot_score(struct damon_ctx *c, struct damon_region *r,
 	 * Transform it to fit in [0, DAMOS_MAX_SCORE]
 	 */
 	hotness = hotness * DAMOS_MAX_SCORE / DAMON_MAX_SUBSCORE;
+	hotness = max(min(hotness, DAMOS_MAX_SCORE), 0);
 
 	return hotness;
 }

base-commit: 0f0046f6171e76f3dba376cc13ace04d654bf372
-- 
2.47.3

Re: [RFC PATCH v1.3] mm/damon/ops-common: handle extreme intervals in damon_hot_score()

[sashiko-bot]

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

Pre-existing issues:
- [High] The `age_in_sec` calculation silently overflows on 32-bit architectures, breaking age-based subscores.
- [Medium] Multiplication of `r->nr_accesses` by `DAMON_MAX_SUBSCORE` silently overflows 32-bit arithmetic for heavily accessed regions, breaking hotness scoring.
- [Medium] Weighted average calculation in `damon_hot_score` can overflow if large weights are provided via sysfs.
--

commit 9cb0813af4dd40334d721106d6d0be2f72fa88ca
Author: SeongJae Park <sj@kernel.org>

mm/damon/ops-common: handle extreme intervals in damon_hot_score()

This patch fixes three issues in damon_hot_score() caused by extreme
monitoring intervals. It explicitly handles zero intervals to prevent
divide-by-zero errors and bounds the return value to prevent out-of-bounds
array access.

Link: https://lore.kernel.org/20260619202459.145010-1-sj@kernel.org

[ ... ]

> diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> index 5c93ef2bb8a97..d1842e2b00ef8 100644
> --- a/mm/damon/ops-common.c
> +++ b/mm/damon/ops-common.c
> @@ -143,6 +143,7 @@ int damon_hot_score(struct damon_ctx *c, struct damon_region *r,

[Severity: Medium]
This is a pre-existing issue, but while we are fixing extreme intervals in
damon_hot_score(), could the frequency subscore calculation silently overflow
on 32-bit architectures?

Looking at the calculation earlier in the function:

freq_subscore = r->nr_accesses * DAMON_MAX_SUBSCORE /
                damon_max_nr_accesses(&c->attrs);

If an administrator configures a large aggr_interval and a small
sample_interval, a frequently accessed region's nr_accesses could exceed
42,949,672. Since DAMON_MAX_SUBSCORE is 100, the multiplication would silently
wrap around modulo 2^32. This might give hot memory regions an artificially
low frequency score.

[Severity: High]
This is also a pre-existing issue, but does the age calculation earlier in
this function suffer from a similar 32-bit overflow?

age_in_sec = (unsigned long)r->age * c->attrs.aggr_interval / 1000000;

On 32-bit architectures where unsigned long is 32 bits, with a default
aggr_interval of 100,000 us (100ms), r->age increments every 100ms. After
about 71.6 minutes of continuous inactivity, r->age reaches 42,950. The
product reaches 4,295,000,000, overflowing ULONG_MAX (4,294,967,295).

The wrapped value divided by 1,000,000 yields 0, which artificially resets
age_in_sec to 0 and could cause old cold memory regions to be misclassified
as young.

[Severity: Medium]
This isn't a bug introduced by this patch, but could the weighted average
calculation also overflow if large weights are provided via sysfs?

hotness = (freq_weight * freq_subscore + age_weight * age_subscore);
if (freq_weight + age_weight)
        hotness /= freq_weight + age_weight;

The weights freq_weight and age_weight are read from sysfs and accept up to
UINT_MAX. This expression evaluates entirely in 32-bit unsigned math. If
large weights are provided, the calculation will wrap around the 32-bit
boundary. Because the intermediate hotness variable wraps and is later cast
to a signed int, this could result in wildly incorrect or even negative
prioritizations before clamping.

>  	 * Transform it to fit in [0, DAMOS_MAX_SCORE]
>  	 */
>  	hotness = hotness * DAMOS_MAX_SCORE / DAMON_MAX_SUBSCORE;
> +	hotness = max(min(hotness, DAMOS_MAX_SCORE), 0);
>  
>  	return hotness;
>  }

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