All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v1.3] mm/damon/ops-common: handle extreme intervals in damon_hot_score()
@ 2026-06-23  1:16 SeongJae Park
  2026-06-23  1:34 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: SeongJae Park @ 2026-06-23  1:16 UTC (permalink / raw)
  Cc: SeongJae Park, # 5 . 16 . x, Andrew Morton, damon, linux-kernel,
	linux-mm

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH v1.3] mm/damon/ops-common: handle extreme intervals in damon_hot_score()
  2026-06-23  1:16 [RFC PATCH v1.3] mm/damon/ops-common: handle extreme intervals in damon_hot_score() SeongJae Park
@ 2026-06-23  1:34 ` sashiko-bot
  2026-06-23 13:42   ` SeongJae Park
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-06-23  1:34 UTC (permalink / raw)
  To: SeongJae Park; +Cc: damon

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH v1.3] mm/damon/ops-common: handle extreme intervals in damon_hot_score()
  2026-06-23  1:34 ` sashiko-bot
@ 2026-06-23 13:42   ` SeongJae Park
  0 siblings, 0 replies; 3+ messages in thread
From: SeongJae Park @ 2026-06-23 13:42 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SeongJae Park, damon

On Tue, 23 Jun 2026 01:34:24 +0000 sashiko-bot@kernel.org wrote:

> 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.

All of these require unnormal user setup, and causes only monitoring results
accuracy degradation rather than some criticial issues like panic or memory
corruption.  No blocker.


Thanks,
SJ

[...]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-06-23 13:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-23  1:16 [RFC PATCH v1.3] mm/damon/ops-common: handle extreme intervals in damon_hot_score() SeongJae Park
2026-06-23  1:34 ` sashiko-bot
2026-06-23 13:42   ` 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.