[PATCH] mm/damon: reject zero sampling interval

[PATCH] mm/damon: reject zero sampling interval

[Karthikeyan KS]

damon_set_attrs() already rejects inverted intervals (sample_interval >
aggr_interval) and invalid region counts, but it accepts
sample_interval == 0.

A zero sampling interval is not a valid monitoring period.  Sysfs "on"
and "commit" both install attrs via damon_set_attrs(), so kdamond then
calls kdamond_usleep(0) and busy-spins.

Return -EINVAL if sample_interval is zero.

Signed-off-by: Karthikeyan KS <karthiproffesional@gmail.com>
---

Hello,

I recently started reading DAMON and just found this one.  This
change only tightens damon_set_attrs() validation, in the same place
as the existing sample > aggr and min_nr_regions checks.  It is not a
crash, UAF, or race.

sample_us=0 is accepted today, so kdamond_usleep(0) busy-spins (about
one core).  Sysfs "on" and "commit" both go through damon_set_attrs();
one check covers both.

Checked on v7.3-rc3 (238650ef6c7c):

  unpatched: echo on with sample_us=0 succeeds; kdamond uses ~97% of
             one core.  echo commit of 0 onto a running kdamond also
             succeeds and it starts spinning.

  patched:   both on and commit return -EINVAL.  Valid sample_us=5000
             still starts.  After a rejected commit the same pid stays
             on and does not spin.

aggr_us=0 with a positive sample_us is already -EINVAL from the
ordering check.

Thanks,

 mm/damon/core.c             |  2 ++
 mm/damon/tests/core-kunit.h | 13 +++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 644daf5a1656..376c3e2cbe00 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1060,6 +1060,8 @@ int damon_set_attrs(struct damon_ctx *ctx, struct damon_attrs *attrs)
 		return -EINVAL;
 	if (attrs->min_nr_regions > attrs->max_nr_regions)
 		return -EINVAL;
+	if (!attrs->sample_interval)
+		return -EINVAL;
 	if (attrs->sample_interval > attrs->aggr_interval)
 		return -EINVAL;
 
diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index 4a536d41cdb2..ec8ef71956c8 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -658,6 +658,19 @@ static void damon_test_set_attrs(struct kunit *test)
 	invalid_attrs.aggr_interval = 4999;
 	KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL);
 
+	invalid_attrs = valid_attrs;
+	invalid_attrs.sample_interval = 0;
+	KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL);
+
+	invalid_attrs = valid_attrs;
+	invalid_attrs.aggr_interval = 0;
+	KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL);
+
+	invalid_attrs = valid_attrs;
+	invalid_attrs.sample_interval = 0;
+	invalid_attrs.aggr_interval = 0;
+	KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL);
+
 	damon_destroy_ctx(c);
 }

-- 
2.34.1

Re: [PATCH] mm/damon: reject zero sampling interval

[Sang-Heon Jeon]

Hi,

On Fri, Sep 18, 2026 at 12:14 AM Karthikeyan KS
<karthiproffesional@gmail.com> wrote:
>
> damon_set_attrs() already rejects inverted intervals (sample_interval >
> aggr_interval) and invalid region counts, but it accepts
> sample_interval == 0.
>
> A zero sampling interval is not a valid monitoring period.  Sysfs "on"
> and "commit" both install attrs via damon_set_attrs(), so kdamond then
> calls kdamond_usleep(0) and busy-spins.
>
> Return -EINVAL if sample_interval is zero.

I think SJ already commented on a similar patch. please refer to [1]
If this happens a lot, maybe should we document or add a comment about this?

[1] https://lore.kernel.org/all/20260722094304.3132750-1-dayou5941@163.com/

> Signed-off-by: Karthikeyan KS <karthiproffesional@gmail.com>
> ---
>
> Hello,
>
> I recently started reading DAMON and just found this one.  This
> change only tightens damon_set_attrs() validation, in the same place
> as the existing sample > aggr and min_nr_regions checks.  It is not a
> crash, UAF, or race.
>
> sample_us=0 is accepted today, so kdamond_usleep(0) busy-spins (about
> one core).  Sysfs "on" and "commit" both go through damon_set_attrs();
> one check covers both.
>
> Checked on v7.3-rc3 (238650ef6c7c):
>
>   unpatched: echo on with sample_us=0 succeeds; kdamond uses ~97% of
>              one core.  echo commit of 0 onto a running kdamond also
>              succeeds and it starts spinning.
>
>   patched:   both on and commit return -EINVAL.  Valid sample_us=5000
>              still starts.  After a rejected commit the same pid stays
>              on and does not spin.
>
> aggr_us=0 with a positive sample_us is already -EINVAL from the
> ordering check.
>
> Thanks,
>
>  mm/damon/core.c             |  2 ++
>  mm/damon/tests/core-kunit.h | 13 +++++++++++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 644daf5a1656..376c3e2cbe00 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -1060,6 +1060,8 @@ int damon_set_attrs(struct damon_ctx *ctx, struct damon_attrs *attrs)
>                 return -EINVAL;
>         if (attrs->min_nr_regions > attrs->max_nr_regions)
>                 return -EINVAL;
> +       if (!attrs->sample_interval)
> +               return -EINVAL;
>         if (attrs->sample_interval > attrs->aggr_interval)
>                 return -EINVAL;
>
> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 4a536d41cdb2..ec8ef71956c8 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
> @@ -658,6 +658,19 @@ static void damon_test_set_attrs(struct kunit *test)
>         invalid_attrs.aggr_interval = 4999;
>         KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL);
>
> +       invalid_attrs = valid_attrs;
> +       invalid_attrs.sample_interval = 0;
> +       KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL);
> +
> +       invalid_attrs = valid_attrs;
> +       invalid_attrs.aggr_interval = 0;
> +       KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL);
> +
> +       invalid_attrs = valid_attrs;
> +       invalid_attrs.sample_interval = 0;
> +       invalid_attrs.aggr_interval = 0;
> +       KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL);
> +
>         damon_destroy_ctx(c);
>  }
>
> --
> 2.34.1
>

Best regards,
Sang-Heon Jeon

Re: [PATCH] mm/damon: reject zero sampling interval

[Karthikeyan KS]

Hi Sang-Heon, Hi SJ,

> I think SJ already commented on a similar patch. please refer to [1]
>
> [1] https://lore.kernel.org/all/20260722094304.3132750-1-dayou5941@163.com/

Thanks, missed that thread. Same objection applies to mine.

> Can't we keep supporting zero sample interval and fix the warning
> or the real bug instead?

I measured a few floors for kdamond_usleep():

  floor    kdamond CPU (3s)   vs sample_us=5000
  1us      84%                28x
  100us    42%                14x
  1000us   12%                ~3x
  5000us   ~2%                1x (matches control)

5000us is damon_new_ctx()'s own default, and at that floor the spin
is just gone.

Would you rather we just document sample_us=0 as accepted/expected
behavior, or clamp it to that default inside damon_set_attrs()?

Thanks,
Karthikeyan KS

Re: [PATCH] mm/damon: reject zero sampling interval

[SeongJae Park]

Hi Karthikeyan and Sang-Heon,

On Thu, 17 Sep 2026 18:00:13 +0000 Karthikeyan KS <karthiproffesional@gmail.com> wrote:

> Hi Sang-Heon, Hi SJ,
> 
> > I think SJ already commented on a similar patch. please refer to [1]
> >
> > [1] https://lore.kernel.org/all/20260722094304.3132750-1-dayou5941@163.com/

Thank you for pointing this out, Sang-Heon!

> 
> Thanks, missed that thread. Same objection applies to mine.
> 
> > Can't we keep supporting zero sample interval and fix the warning
> > or the real bug instead?
> 
> I measured a few floors for kdamond_usleep():
> 
>   floor    kdamond CPU (3s)   vs sample_us=5000
>   1us      84%                28x
>   100us    42%                14x
>   1000us   12%                ~3x
>   5000us   ~2%                1x (matches control)
> 
> 5000us is damon_new_ctx()'s own default, and at that floor the spin
> is just gone.
> 
> Would you rather we just document sample_us=0 as accepted/expected
> behavior, or clamp it to that default inside damon_set_attrs()?

I'd prefer adding a comment saying zero sampling interval is accepted.  I think
damon_attr kernel-doc comment is a good place for adding that.


Thanks,
SJ

[...]