mm/damon/core: handle zero intervals in damon_max_nr_accesses()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The patch resolves a divide-by-zero crash but inadvertently introduces a critical out-of-bounds heap array read and write when updating the region score histogram. --

commit ba1da1a76126be43fbfd5ff0c59d54a52b8cddf8
Author: SeongJae Park <[email protected]>

mm/damon/core: handle zero intervals in damon_max_nr_accesses()

This patch modifies damon_max_nr_accesses() to avoid a divide-by-zero when the user sets the sampling or aggregation interval to zero. It ensures the denominator and return value are at least 1.

Link https://lore.kernel.org/[email protected] open here

Quoted 4 lines

include/linux/damon.h +0 -0

index 6f7edb3590ef9..888570f55b416 100644

[ ... ]

include/linux/damon.h static inline bool damon_target_has_pid(const struct damon_ctx *ctx) · line 1074

Quoted 13 lines

include/linux/damon.h +6 -2

@@ -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: Critical]
If sample_interval and aggr_interval are set to 0 via sysfs, damon_max_nr_accesses() now safely returns 1 instead of panicking.

However, damon_set_attrs() validation succeeds because attrs->sample_interval > attrs->aggr_interval evaluates to false (0 > 0). Because damon_update_monitoring_results() then skips resetting r->nr_accesses, it can retain a previous large value.

In mm/damon/ops-common.c:damon_hot_score(), the frequency subscore is calculated using the return value of damon_max_nr_accesses():

freq_subscore = r->nr_accesses * DAMON_MAX_SUBSCORE / max_nr_accesses;

Since max_nr_accesses is 1, a large r->nr_accesses will result in a hotness score that exceeds DAMOS_MAX_SCORE and is never explicitly clamped.

Later in mm/damon/core.c:damos_adjust_quota(), this score is used directly as an array index:

c->regions_score_histogram[score] += damon_sz_region(r);

Does this allow an out-of-bounds heap write when the histogram array is indexed by the unbounded score?

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]

Fixes 198f0f4c58b9 ("mm/damon/vaddr,paddr: support pageout prioritization")