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

[PATCH] 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>
---
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 ++++++--
 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;
 }
 
 

base-commit: 7a58ae62cdf3c006a53b805bbb12079ab2621a07
-- 
2.47.3

Re: [PATCH] mm/damon/core: handle zero intervals in damon_max_nr_accesses()

[sashiko-bot]

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 <sj@kernel.org>

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/20260619202459.145010-1-sj@kernel.org

> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index 6f7edb3590ef9..888570f55b416 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h

[ ... ]

> @@ -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?

>  }
>

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

Re: [PATCH] mm/damon/core: handle zero intervals in damon_max_nr_accesses()

[SeongJae Park]

On Sun, 21 Jun 2026 16:03:00 +0000 sashiko-bot@kernel.org wrote:

> 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 <sj@kernel.org>
> 
> 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/20260619202459.145010-1-sj@kernel.org
> 
> > diff --git a/include/linux/damon.h b/include/linux/damon.h
> > index 6f7edb3590ef9..888570f55b416 100644
> > --- a/include/linux/damon.h
> > +++ b/include/linux/damon.h
> 
> [ ... ]
> 
> > @@ -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?

Good finding!  Nonetheless, this is introduced by another commit: commit
2f5bef5a590b ("mm/damon/core: update monitoring results for new monitoring
attributes").

I will post this patch again as a seris having the fix of newly found bug,
together.


Thanks,
SJ

[...]

Re: [PATCH] mm/damon/core: handle zero intervals in damon_max_nr_accesses()

[SeongJae Park]

On Sun, 21 Jun 2026 08:48:06 -0700 SeongJae Park <sj@kernel.org> wrote:

> 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")

Sashiko found [1] another bug that was introduced by another commit.  I will
repost this patch with a fix for the another bug.

[1] https://lore.kernel.org/20260621175849.91990-1-sj@kernel.org


Thanks,
SJ

[...]