[RFC PATCH v1.2 0/2] mm/damon: handle zero {sample,aggr} intervals for DAMOS quota score

[RFC PATCH v1.2 0/2] mm/damon: handle zero {sample,aggr} intervals for DAMOS quota score

[SeongJae Park]

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

[RFC PATCH v1.2 1/2] mm/damon/core: handle zero intervals in damon_max_nr_accesses()

[SeongJae Park]

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

[RFC PATCH v1.2 2/2] mm/damon/ops-common: prevent >DAMON_MAX_SUBSCORE freq_subscore

[SeongJae Park]

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