[PATCH 0/4] mm/damon/sysfs: fix resource leak and NULL pointer dereferences

[PATCH 0/4] mm/damon/sysfs: fix resource leak and NULL pointer dereferences

[Josh Law]

This series fixes a memory leak and three NULL pointer dereferences in
the DAMON sysfs interface, all in mm/damon/sysfs.c.

Patch 1 fixes a damon_ctx leak in damon_sysfs_commit_input() when
damon_sysfs_new_test_ctx() fails after param_ctx was already built.

Patches 2-4 fix missing contexts->nr checks before dereferencing
contexts_arr[0]. A user can trigger these by setting nr_contexts to 0
via sysfs and then issuing commands that assume a context exists:

  - Patch 2: CLEAR_SCHEMES_TRIED_REGIONS handler in damon_sysfs_handle_cmd()
  - Patch 3: damon_sysfs_update_schemes_tried_regions(), reached via
    UPDATE_SCHEMES_TRIED_BYTES and UPDATE_SCHEMES_TRIED_REGIONS
  - Patch 4: damon_sysfs_repeat_call_fn(), reachable when nr_contexts is
    set to 0 while DAMON is running

Josh Law (4):
  mm/damon/sysfs: fix param_ctx leak on damon_sysfs_new_test_ctx()
    failure
  mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions
  mm/damon/sysfs: check contexts->nr in update_schemes_tried_regions
  mm/damon/sysfs: check contexts->nr in repeat_call_fn

 mm/damon/sysfs.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

-- 
2.34.1

[PATCH 1/4] mm/damon/sysfs: fix param_ctx leak on damon_sysfs_new_test_ctx() failure

[Josh Law]

When damon_sysfs_new_test_ctx() fails in damon_sysfs_commit_input(),
param_ctx is leaked because the early return skips the cleanup at the
out label. Destroy param_ctx before returning.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 mm/damon/sysfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index 576d1ddd736b..b573b9d60784 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -1524,8 +1524,10 @@ static int damon_sysfs_commit_input(void *data)
 	if (IS_ERR(param_ctx))
 		return PTR_ERR(param_ctx);
 	test_ctx = damon_sysfs_new_test_ctx(kdamond->damon_ctx);
-	if (!test_ctx)
+	if (!test_ctx) {
+		damon_destroy_ctx(param_ctx);
 		return -ENOMEM;
+	}
 	err = damon_commit_ctx(test_ctx, param_ctx);
 	if (err)
 		goto out;
-- 
2.34.1

Re: [PATCH 1/4] mm/damon/sysfs: fix param_ctx leak on damon_sysfs_new_test_ctx() failure

[SeongJae Park]

On Thu, 19 Mar 2026 15:57:39 +0000 Josh Law <objecting@objecting.org> wrote:

> When damon_sysfs_new_test_ctx() fails in damon_sysfs_commit_input(),
> param_ctx is leaked because the early return skips the cleanup at the
> out label. Destroy param_ctx before returning.

Nice catch, thank you!

The problematic failure can happen only when the arguably too small to fail
allocations fail.  So, the user impact may be not big.  But, still the
consequence is bad.  I think it is better to add Fixes: and Cc stable@, as
below.

Fixes: f0c5118ebb0e ("mm/damon/sysfs: catch commit test ctx alloc failure")
Cc: <stable@vger.kernel.org> # 6.18.x

> 
> Signed-off-by: Josh Law <objecting@objecting.org>

Other than the above,

Reviewed-by: SeongJae Park <sj@kernel.org>

> ---
>  mm/damon/sysfs.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index 576d1ddd736b..b573b9d60784 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c
> @@ -1524,8 +1524,10 @@ static int damon_sysfs_commit_input(void *data)
>  	if (IS_ERR(param_ctx))
>  		return PTR_ERR(param_ctx);
>  	test_ctx = damon_sysfs_new_test_ctx(kdamond->damon_ctx);
> -	if (!test_ctx)
> +	if (!test_ctx) {
> +		damon_destroy_ctx(param_ctx);
>  		return -ENOMEM;
> +	}
>  	err = damon_commit_ctx(test_ctx, param_ctx);
>  	if (err)
>  		goto out;

Sashiko added below comment.  But that's orthogonal to this patch, so never a
blocker of this patch.

: If damon_commit_ctx() fails midway through damon_commit_targets(), could
: struct pid references be leaked?
: 
: When transitioning a DAMON context from DAMON_OPS_PADDR to DAMON_OPS_VADDR,
: param_ctx is built with VADDR ops, while test_ctx inherits PADDR ops from
: the running context.
: 
: Inside damon_commit_targets(), it iterates over targets and calls get_pid()
: for each target since param_ctx has VADDR ops, adding them to test_ctx.
: 
: If a subsequent target fails to allocate memory (like -ENOMEM in
: damon_commit_target_regions()), damon_commit_ctx() returns early and skips
: the dst->ops = src->ops assignment.
: 
: This leaves test_ctx->ops as PADDR, which lacks a cleanup_target callback.
: 
: When the error path jumps to the out label and calls
: damon_destroy_ctx(test_ctx), put_pid() is skipped for the partially
: committed targets because the context still has PADDR ops, permanently
: leaking the struct pid references.
: 
: Is there a way to ensure test_ctx is cleaned up with the correct ops
: if damon_commit_ctx() fails?

# review url: https://sashiko.dev/#/patchset/20260319155742.186627-2-objecting@objecting.org

Sounds like correct.  But defninitely orthogonal to this patch, so no blocker
for this patch.  I will work on this later.


Thanks,
SJ

[...]

[PATCH 2/4] mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions

[Josh Law]

The CLEAR_SCHEMES_TRIED_REGIONS command accesses contexts_arr[0]
without verifying nr_contexts >= 1, causing a NULL pointer dereference
when no context is configured. Add the missing check.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 mm/damon/sysfs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index b573b9d60784..36ad2e8956c9 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -1769,6 +1769,8 @@ static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
 	case DAMON_SYSFS_CMD_UPDATE_SCHEMES_TRIED_REGIONS:
 		return damon_sysfs_update_schemes_tried_regions(kdamond, false);
 	case DAMON_SYSFS_CMD_CLEAR_SCHEMES_TRIED_REGIONS:
+		if (kdamond->contexts->nr != 1)
+			return -EINVAL;
 		return damon_sysfs_schemes_clear_regions(
 			kdamond->contexts->contexts_arr[0]->schemes);
 	case DAMON_SYSFS_CMD_UPDATE_SCHEMES_EFFECTIVE_QUOTAS:
-- 
2.34.1

Re: [PATCH 2/4] mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions

[SeongJae Park]

On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <objecting@objecting.org> wrote:

> The CLEAR_SCHEMES_TRIED_REGIONS command accesses contexts_arr[0]
> without verifying nr_contexts >= 1, causing a NULL pointer dereference
> when no context is configured. Add the missing check.

Nice catch, thank you!

Privileged users can trigger this using DAMON sysfs interface.  E.g.,

    # cd /sys/kernel/mm/damon/admin/kdamonds/
    # echo 1 > nr_kdamonds
    # echo clear_schemes_tried_regions > state
    killed
    # dmesg
    [...]
    [63541.377604] BUG: kernel NULL pointer dereference, address: 0000000000000000
    [...]

Privileged users can do anything even worse than this, but they might also do
this by a mistake.

So this deserves Fixes: and Cc stable.

>
> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
>  mm/damon/sysfs.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index b573b9d60784..36ad2e8956c9 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c
> @@ -1769,6 +1769,8 @@ static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
>       case DAMON_SYSFS_CMD_UPDATE_SCHEMES_TRIED_REGIONS:
>               return damon_sysfs_update_schemes_tried_regions(kdamond, false);
>       case DAMON_SYSFS_CMD_CLEAR_SCHEMES_TRIED_REGIONS:
> +             if (kdamond->contexts->nr != 1)
> +                     return -EINVAL;
>               return damon_sysfs_schemes_clear_regions(
>                       kdamond->contexts->contexts_arr[0]->schemes);
>       case DAMON_SYSFS_CMD_UPDATE_SCHEMES_EFFECTIVE_QUOTAS:
> --
> 2.34.1

So this patch looks good as an individual fix for the individual bug, but...

Sashiko commented.

# review url: https://sashiko.dev/#/patchset/20260319155742.186627-3-objecting@objecting.org

: Does this missing check also affect other manual commands?
:
: If a user writes UPDATE_SCHEMES_STATS, UPDATE_SCHEMES_EFFECTIVE_QUOTAS,
: or UPDATE_TUNED_INTERVALS to the state file after setting nr_contexts
: to 0, damon_sysfs_handle_cmd() queues the corresponding callback via
: damon_sysfs_damon_call().
:
: When the kdamond thread executes the callback, it appears functions like
: damon_sysfs_upd_schemes_stats() access contexts_arr[0] without verifying
: contexts->nr:
:
: static int damon_sysfs_upd_schemes_stats(void *data)
: {
:         struct damon_sysfs_kdamond *kdamond = data;
:         struct damon_ctx *ctx = kdamond->damon_ctx;
:
:         damon_sysfs_schemes_update_stats(
:                         kdamond->contexts->contexts_arr[0]->schemes, ctx);
:         return 0;
: }
:
: Could this result in a similar NULL pointer dereference if these commands
: are triggered while no context is configured?

Sashiko is correct.  Privileged users can trigger the issues like below.

# damo start
# cd /sys/kernel/mm/damon/admin/kdamonds/0
# echo 0 > contexts/nr_contexts
# echo update_schemes_stats > state
# echo update_schemes_effective_quotas > state
# echo update_tuned_intervals > state

Not necessarily blocker of this patch, but seems all the issues are in a same
category.  The third patch of this series is also fixing one of the category
bugs.  How about fixing all at once by checking kdamond->contexts->nr at the
beginning of damon_sysfs_handle_cmd(), like below?

--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -2404,6 +2404,9 @@ static int damon_sysfs_update_schemes_tried_regions(
 static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
                struct damon_sysfs_kdamond *kdamond)
 {
+       if (cmd != DAMON_SYSFS_CMD_OFF && kdamond->contexts->nr != 1)
+               return -EINVAL;
+
        switch (cmd) {
        case DAMON_SYSFS_CMD_ON:
                return damon_sysfs_turn_damon_on(kdamond);

If we pick this, Fixes: would be deserve to the oldest buggy commit that
introduced the first bug of this category.  It is indeed quite old.

Fixes: 0ac32b8affb5 ("mm/damon/sysfs: support DAMOS stats")
Cc: <stable@vger.kernel.org> # 5.18.x


Thanks,
SJ

Re: [PATCH 2/4] mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions

[Josh Law]

On 20 March 2026 02:13:17 GMT, SeongJae Park <sj@kernel.org> wrote:
>On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <objecting@objecting.org> wrote:
>
>> The CLEAR_SCHEMES_TRIED_REGIONS command accesses contexts_arr[0]
>> without verifying nr_contexts >= 1, causing a NULL pointer dereference
>> when no context is configured. Add the missing check.
>
>Nice catch, thank you!
>
>Privileged users can trigger this using DAMON sysfs interface.  E.g.,
>
>    # cd /sys/kernel/mm/damon/admin/kdamonds/
>    # echo 1 > nr_kdamonds
>    # echo clear_schemes_tried_regions > state
>    killed
>    # dmesg
>    [...]
>    [63541.377604] BUG: kernel NULL pointer dereference, address: 0000000000000000
>    [...]
>
>Privileged users can do anything even worse than this, but they might also do
>this by a mistake.
>
>So this deserves Fixes: and Cc stable.
>
>>
>> Signed-off-by: Josh Law <objecting@objecting.org>
>> ---
>>  mm/damon/sysfs.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
>> index b573b9d60784..36ad2e8956c9 100644
>> --- a/mm/damon/sysfs.c
>> +++ b/mm/damon/sysfs.c
>> @@ -1769,6 +1769,8 @@ static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
>>       case DAMON_SYSFS_CMD_UPDATE_SCHEMES_TRIED_REGIONS:
>>               return damon_sysfs_update_schemes_tried_regions(kdamond, false);
>>       case DAMON_SYSFS_CMD_CLEAR_SCHEMES_TRIED_REGIONS:
>> +             if (kdamond->contexts->nr != 1)
>> +                     return -EINVAL;
>>               return damon_sysfs_schemes_clear_regions(
>>                       kdamond->contexts->contexts_arr[0]->schemes);
>>       case DAMON_SYSFS_CMD_UPDATE_SCHEMES_EFFECTIVE_QUOTAS:
>> --
>> 2.34.1
>
>So this patch looks good as an individual fix for the individual bug, but...
>
>Sashiko commented.
>
># review url: https://sashiko.dev/#/patchset/20260319155742.186627-3-objecting@objecting.org
>
>: Does this missing check also affect other manual commands?
>:
>: If a user writes UPDATE_SCHEMES_STATS, UPDATE_SCHEMES_EFFECTIVE_QUOTAS,
>: or UPDATE_TUNED_INTERVALS to the state file after setting nr_contexts
>: to 0, damon_sysfs_handle_cmd() queues the corresponding callback via
>: damon_sysfs_damon_call().
>:
>: When the kdamond thread executes the callback, it appears functions like
>: damon_sysfs_upd_schemes_stats() access contexts_arr[0] without verifying
>: contexts->nr:
>:
>: static int damon_sysfs_upd_schemes_stats(void *data)
>: {
>:         struct damon_sysfs_kdamond *kdamond = data;
>:         struct damon_ctx *ctx = kdamond->damon_ctx;
>:
>:         damon_sysfs_schemes_update_stats(
>:                         kdamond->contexts->contexts_arr[0]->schemes, ctx);
>:         return 0;
>: }
>:
>: Could this result in a similar NULL pointer dereference if these commands
>: are triggered while no context is configured?
>
>Sashiko is correct.  Privileged users can trigger the issues like below.
>
># damo start
># cd /sys/kernel/mm/damon/admin/kdamonds/0
># echo 0 > contexts/nr_contexts
># echo update_schemes_stats > state
># echo update_schemes_effective_quotas > state
># echo update_tuned_intervals > state
>
>Not necessarily blocker of this patch, but seems all the issues are in a same
>category.  The third patch of this series is also fixing one of the category
>bugs.  How about fixing all at once by checking kdamond->contexts->nr at the
>beginning of damon_sysfs_handle_cmd(), like below?
>
>--- a/mm/damon/sysfs.c
>+++ b/mm/damon/sysfs.c
>@@ -2404,6 +2404,9 @@ static int damon_sysfs_update_schemes_tried_regions(
> static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
>                struct damon_sysfs_kdamond *kdamond)
> {
>+       if (cmd != DAMON_SYSFS_CMD_OFF && kdamond->contexts->nr != 1)
>+               return -EINVAL;
>+
>        switch (cmd) {
>        case DAMON_SYSFS_CMD_ON:
>                return damon_sysfs_turn_damon_on(kdamond);
>
>If we pick this, Fixes: would be deserve to the oldest buggy commit that
>introduced the first bug of this category.  It is indeed quite old.
>
>Fixes: 0ac32b8affb5 ("mm/damon/sysfs: support DAMOS stats")
>Cc: <stable@vger.kernel.org> # 5.18.x
>
>
>Thanks,
>SJ



Hello, did you give Reviewed by you? Or not..

V/R


Josh Law

Re: [PATCH 2/4] mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions

[SeongJae Park]

On Fri, 20 Mar 2026 07:06:48 +0000 Josh Law <objecting@objecting.org> wrote:

> 
> 
> On 20 March 2026 02:13:17 GMT, SeongJae Park <sj@kernel.org> wrote:
> >On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <objecting@objecting.org> wrote:
> >
> >> The CLEAR_SCHEMES_TRIED_REGIONS command accesses contexts_arr[0]
> >> without verifying nr_contexts >= 1, causing a NULL pointer dereference
> >> when no context is configured. Add the missing check.
> >
> >Nice catch, thank you!
> >
> >Privileged users can trigger this using DAMON sysfs interface.  E.g.,
> >
> >    # cd /sys/kernel/mm/damon/admin/kdamonds/
> >    # echo 1 > nr_kdamonds
> >    # echo clear_schemes_tried_regions > state
> >    killed
> >    # dmesg
> >    [...]
> >    [63541.377604] BUG: kernel NULL pointer dereference, address: 0000000000000000
> >    [...]
> >
> >Privileged users can do anything even worse than this, but they might also do
> >this by a mistake.
> >
> >So this deserves Fixes: and Cc stable.
> >
> >>
> >> Signed-off-by: Josh Law <objecting@objecting.org>
> >> ---
> >>  mm/damon/sysfs.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> >> index b573b9d60784..36ad2e8956c9 100644
> >> --- a/mm/damon/sysfs.c
> >> +++ b/mm/damon/sysfs.c
> >> @@ -1769,6 +1769,8 @@ static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
> >>       case DAMON_SYSFS_CMD_UPDATE_SCHEMES_TRIED_REGIONS:
> >>               return damon_sysfs_update_schemes_tried_regions(kdamond, false);
> >>       case DAMON_SYSFS_CMD_CLEAR_SCHEMES_TRIED_REGIONS:
> >> +             if (kdamond->contexts->nr != 1)
> >> +                     return -EINVAL;
> >>               return damon_sysfs_schemes_clear_regions(
> >>                       kdamond->contexts->contexts_arr[0]->schemes);
> >>       case DAMON_SYSFS_CMD_UPDATE_SCHEMES_EFFECTIVE_QUOTAS:
> >> --
> >> 2.34.1
> >
> >So this patch looks good as an individual fix for the individual bug, but...
> >
> >Sashiko commented.
> >
> ># review url: https://sashiko.dev/#/patchset/20260319155742.186627-3-objecting@objecting.org
> >
> >: Does this missing check also affect other manual commands?
> >:
> >: If a user writes UPDATE_SCHEMES_STATS, UPDATE_SCHEMES_EFFECTIVE_QUOTAS,
> >: or UPDATE_TUNED_INTERVALS to the state file after setting nr_contexts
> >: to 0, damon_sysfs_handle_cmd() queues the corresponding callback via
> >: damon_sysfs_damon_call().
> >:
> >: When the kdamond thread executes the callback, it appears functions like
> >: damon_sysfs_upd_schemes_stats() access contexts_arr[0] without verifying
> >: contexts->nr:
> >:
> >: static int damon_sysfs_upd_schemes_stats(void *data)
> >: {
> >:         struct damon_sysfs_kdamond *kdamond = data;
> >:         struct damon_ctx *ctx = kdamond->damon_ctx;
> >:
> >:         damon_sysfs_schemes_update_stats(
> >:                         kdamond->contexts->contexts_arr[0]->schemes, ctx);
> >:         return 0;
> >: }
> >:
> >: Could this result in a similar NULL pointer dereference if these commands
> >: are triggered while no context is configured?
> >
> >Sashiko is correct.  Privileged users can trigger the issues like below.
> >
> ># damo start
> ># cd /sys/kernel/mm/damon/admin/kdamonds/0
> ># echo 0 > contexts/nr_contexts
> ># echo update_schemes_stats > state
> ># echo update_schemes_effective_quotas > state
> ># echo update_tuned_intervals > state
> >
> >Not necessarily blocker of this patch, but seems all the issues are in a same
> >category.  The third patch of this series is also fixing one of the category
> >bugs.  How about fixing all at once by checking kdamond->contexts->nr at the
> >beginning of damon_sysfs_handle_cmd(), like below?
> >
> >--- a/mm/damon/sysfs.c
> >+++ b/mm/damon/sysfs.c
> >@@ -2404,6 +2404,9 @@ static int damon_sysfs_update_schemes_tried_regions(
> > static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
> >                struct damon_sysfs_kdamond *kdamond)
> > {
> >+       if (cmd != DAMON_SYSFS_CMD_OFF && kdamond->contexts->nr != 1)
> >+               return -EINVAL;
> >+
> >        switch (cmd) {
> >        case DAMON_SYSFS_CMD_ON:
> >                return damon_sysfs_turn_damon_on(kdamond);
> >
> >If we pick this, Fixes: would be deserve to the oldest buggy commit that
> >introduced the first bug of this category.  It is indeed quite old.
> >
> >Fixes: 0ac32b8affb5 ("mm/damon/sysfs: support DAMOS stats")
> >Cc: <stable@vger.kernel.org> # 5.18.x
> >
> >
> >Thanks,
> >SJ
> 
> 
> 
> Hello, did you give Reviewed by you? Or not..

Are you meaning Reviewed-by: tag?  If so, no, not yet.  I want to get your
answer to above question first.  Could you please answer?


Thanks,
SJ

[...]

Re: [PATCH 2/4] mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions

[Josh Law]

On 20 March 2026 14:47:40 GMT, SeongJae Park <sj@kernel.org> wrote:
>On Fri, 20 Mar 2026 07:06:48 +0000 Josh Law <objecting@objecting.org> wrote:
>
>> 
>> 
>> On 20 March 2026 02:13:17 GMT, SeongJae Park <sj@kernel.org> wrote:
>> >On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <objecting@objecting.org> wrote:
>> >
>> >> The CLEAR_SCHEMES_TRIED_REGIONS command accesses contexts_arr[0]
>> >> without verifying nr_contexts >= 1, causing a NULL pointer dereference
>> >> when no context is configured. Add the missing check.
>> >
>> >Nice catch, thank you!
>> >
>> >Privileged users can trigger this using DAMON sysfs interface.  E.g.,
>> >
>> >    # cd /sys/kernel/mm/damon/admin/kdamonds/
>> >    # echo 1 > nr_kdamonds
>> >    # echo clear_schemes_tried_regions > state
>> >    killed
>> >    # dmesg
>> >    [...]
>> >    [63541.377604] BUG: kernel NULL pointer dereference, address: 0000000000000000
>> >    [...]
>> >
>> >Privileged users can do anything even worse than this, but they might also do
>> >this by a mistake.
>> >
>> >So this deserves Fixes: and Cc stable.
>> >
>> >>
>> >> Signed-off-by: Josh Law <objecting@objecting.org>
>> >> ---
>> >>  mm/damon/sysfs.c | 2 ++
>> >>  1 file changed, 2 insertions(+)
>> >>
>> >> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
>> >> index b573b9d60784..36ad2e8956c9 100644
>> >> --- a/mm/damon/sysfs.c
>> >> +++ b/mm/damon/sysfs.c
>> >> @@ -1769,6 +1769,8 @@ static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
>> >>       case DAMON_SYSFS_CMD_UPDATE_SCHEMES_TRIED_REGIONS:
>> >>               return damon_sysfs_update_schemes_tried_regions(kdamond, false);
>> >>       case DAMON_SYSFS_CMD_CLEAR_SCHEMES_TRIED_REGIONS:
>> >> +             if (kdamond->contexts->nr != 1)
>> >> +                     return -EINVAL;
>> >>               return damon_sysfs_schemes_clear_regions(
>> >>                       kdamond->contexts->contexts_arr[0]->schemes);
>> >>       case DAMON_SYSFS_CMD_UPDATE_SCHEMES_EFFECTIVE_QUOTAS:
>> >> --
>> >> 2.34.1
>> >
>> >So this patch looks good as an individual fix for the individual bug, but...
>> >
>> >Sashiko commented.
>> >
>> ># review url: https://sashiko.dev/#/patchset/20260319155742.186627-3-objecting@objecting.org
>> >
>> >: Does this missing check also affect other manual commands?
>> >:
>> >: If a user writes UPDATE_SCHEMES_STATS, UPDATE_SCHEMES_EFFECTIVE_QUOTAS,
>> >: or UPDATE_TUNED_INTERVALS to the state file after setting nr_contexts
>> >: to 0, damon_sysfs_handle_cmd() queues the corresponding callback via
>> >: damon_sysfs_damon_call().
>> >:
>> >: When the kdamond thread executes the callback, it appears functions like
>> >: damon_sysfs_upd_schemes_stats() access contexts_arr[0] without verifying
>> >: contexts->nr:
>> >:
>> >: static int damon_sysfs_upd_schemes_stats(void *data)
>> >: {
>> >:         struct damon_sysfs_kdamond *kdamond = data;
>> >:         struct damon_ctx *ctx = kdamond->damon_ctx;
>> >:
>> >:         damon_sysfs_schemes_update_stats(
>> >:                         kdamond->contexts->contexts_arr[0]->schemes, ctx);
>> >:         return 0;
>> >: }
>> >:
>> >: Could this result in a similar NULL pointer dereference if these commands
>> >: are triggered while no context is configured?
>> >
>> >Sashiko is correct.  Privileged users can trigger the issues like below.
>> >
>> ># damo start
>> ># cd /sys/kernel/mm/damon/admin/kdamonds/0
>> ># echo 0 > contexts/nr_contexts
>> ># echo update_schemes_stats > state
>> ># echo update_schemes_effective_quotas > state
>> ># echo update_tuned_intervals > state
>> >
>> >Not necessarily blocker of this patch, but seems all the issues are in a same
>> >category.  The third patch of this series is also fixing one of the category
>> >bugs.  How about fixing all at once by checking kdamond->contexts->nr at the
>> >beginning of damon_sysfs_handle_cmd(), like below?
>> >
>> >--- a/mm/damon/sysfs.c
>> >+++ b/mm/damon/sysfs.c
>> >@@ -2404,6 +2404,9 @@ static int damon_sysfs_update_schemes_tried_regions(
>> > static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
>> >                struct damon_sysfs_kdamond *kdamond)
>> > {
>> >+       if (cmd != DAMON_SYSFS_CMD_OFF && kdamond->contexts->nr != 1)
>> >+               return -EINVAL;
>> >+
>> >        switch (cmd) {
>> >        case DAMON_SYSFS_CMD_ON:
>> >                return damon_sysfs_turn_damon_on(kdamond);
>> >
>> >If we pick this, Fixes: would be deserve to the oldest buggy commit that
>> >introduced the first bug of this category.  It is indeed quite old.
>> >
>> >Fixes: 0ac32b8affb5 ("mm/damon/sysfs: support DAMOS stats")
>> >Cc: <stable@vger.kernel.org> # 5.18.x
>> >
>> >
>> >Thanks,
>> >SJ
>> 
>> 
>> 
>> Hello, did you give Reviewed by you? Or not..
>
>Are you meaning Reviewed-by: tag?  If so, no, not yet.  I want to get your
>answer to above question first.  Could you please answer?
>
>
>Thanks,
>SJ
>
>[...]


Well, two is in the same catagory. But seperate fixes may be best.  Because patch 3 dont call that function, so it may be screwy, i mean, if you want me to. Ill guard it. But its a bit on the hacky side


V/R


Josh Law

Re: [PATCH 2/4] mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions

[SeongJae Park]

On Fri, 20 Mar 2026 15:14:54 +0000 Josh Law <objecting@objecting.org> wrote:

> 
> 
> On 20 March 2026 14:47:40 GMT, SeongJae Park <sj@kernel.org> wrote:
> >On Fri, 20 Mar 2026 07:06:48 +0000 Josh Law <objecting@objecting.org> wrote:
> >
> >> 
> >> 
> >> On 20 March 2026 02:13:17 GMT, SeongJae Park <sj@kernel.org> wrote:
> >> >On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <objecting@objecting.org> wrote:
[...]
> >> >Not necessarily blocker of this patch, but seems all the issues are in a same
> >> >category.  The third patch of this series is also fixing one of the category
> >> >bugs.  How about fixing all at once by checking kdamond->contexts->nr at the
> >> >beginning of damon_sysfs_handle_cmd(), like below?
> >> >
> >> >--- a/mm/damon/sysfs.c
> >> >+++ b/mm/damon/sysfs.c
> >> >@@ -2404,6 +2404,9 @@ static int damon_sysfs_update_schemes_tried_regions(
> >> > static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
> >> >                struct damon_sysfs_kdamond *kdamond)
> >> > {
> >> >+       if (cmd != DAMON_SYSFS_CMD_OFF && kdamond->contexts->nr != 1)
> >> >+               return -EINVAL;
> >> >+
> >> >        switch (cmd) {
> >> >        case DAMON_SYSFS_CMD_ON:
> >> >                return damon_sysfs_turn_damon_on(kdamond);
> >> >
> >> >If we pick this, Fixes: would be deserve to the oldest buggy commit that
> >> >introduced the first bug of this category.  It is indeed quite old.
> >> >
> >> >Fixes: 0ac32b8affb5 ("mm/damon/sysfs: support DAMOS stats")
> >> >Cc: <stable@vger.kernel.org> # 5.18.x
> >> >
> >> >
> >> >Thanks,
> >> >SJ
> >> 
> >> 
> >> 
> >> Hello, did you give Reviewed by you? Or not..
> >
> >Are you meaning Reviewed-by: tag?  If so, no, not yet.  I want to get your
> >answer to above question first.  Could you please answer?
> >
> >
> >Thanks,
> >SJ
> >
> >[...]
> 
> 
> Well, two is in the same catagory. But seperate fixes may be best.  Because patch 3 dont call that function, so it may be screwy, i mean, if you want me to. Ill guard it. But its a bit on the hacky side

I agree there could be more cleaner way.  But these fixes need to go to stable,
so I'd prefer a change that also easier to backport.

So, yes, I want to.  Thank you for kindly accepting my suggestion.

Could you please re-post this series for the first and the fourth patches as
they are, after adding my Reviewed-by:, Fixes: and Cc: stable tags, and a patch
checking kdamond->contexts->nr at the beginning of damon_sysfs_handle_cmd() as
I suggested?


Thanks,
SJ

[...]

Re: [PATCH 2/4] mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions

[Josh Law]

On 20 March 2026 15:51:14 GMT, SeongJae Park <sj@kernel.org> wrote:
>On Fri, 20 Mar 2026 15:14:54 +0000 Josh Law <objecting@objecting.org> wrote:
>
>> 
>> 
>> On 20 March 2026 14:47:40 GMT, SeongJae Park <sj@kernel.org> wrote:
>> >On Fri, 20 Mar 2026 07:06:48 +0000 Josh Law <objecting@objecting.org> wrote:
>> >
>> >> 
>> >> 
>> >> On 20 March 2026 02:13:17 GMT, SeongJae Park <sj@kernel.org> wrote:
>> >> >On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <objecting@objecting.org> wrote:
>[...]
>> >> >Not necessarily blocker of this patch, but seems all the issues are in a same
>> >> >category.  The third patch of this series is also fixing one of the category
>> >> >bugs.  How about fixing all at once by checking kdamond->contexts->nr at the
>> >> >beginning of damon_sysfs_handle_cmd(), like below?
>> >> >
>> >> >--- a/mm/damon/sysfs.c
>> >> >+++ b/mm/damon/sysfs.c
>> >> >@@ -2404,6 +2404,9 @@ static int damon_sysfs_update_schemes_tried_regions(
>> >> > static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
>> >> >                struct damon_sysfs_kdamond *kdamond)
>> >> > {
>> >> >+       if (cmd != DAMON_SYSFS_CMD_OFF && kdamond->contexts->nr != 1)
>> >> >+               return -EINVAL;
>> >> >+
>> >> >        switch (cmd) {
>> >> >        case DAMON_SYSFS_CMD_ON:
>> >> >                return damon_sysfs_turn_damon_on(kdamond);
>> >> >
>> >> >If we pick this, Fixes: would be deserve to the oldest buggy commit that
>> >> >introduced the first bug of this category.  It is indeed quite old.
>> >> >
>> >> >Fixes: 0ac32b8affb5 ("mm/damon/sysfs: support DAMOS stats")
>> >> >Cc: <stable@vger.kernel.org> # 5.18.x
>> >> >
>> >> >
>> >> >Thanks,
>> >> >SJ
>> >> 
>> >> 
>> >> 
>> >> Hello, did you give Reviewed by you? Or not..
>> >
>> >Are you meaning Reviewed-by: tag?  If so, no, not yet.  I want to get your
>> >answer to above question first.  Could you please answer?
>> >
>> >
>> >Thanks,
>> >SJ
>> >
>> >[...]
>> 
>> 
>> Well, two is in the same catagory. But seperate fixes may be best.  Because patch 3 dont call that function, so it may be screwy, i mean, if you want me to. Ill guard it. But its a bit on the hacky side
>
>I agree there could be more cleaner way.  But these fixes need to go to stable,
>so I'd prefer a change that also easier to backport.
>
>So, yes, I want to.  Thank you for kindly accepting my suggestion.
>
>Could you please re-post this series for the first and the fourth patches as
>they are, after adding my Reviewed-by:, Fixes: and Cc: stable tags, and a patch
>checking kdamond->contexts->nr at the beginning of damon_sysfs_handle_cmd() as
>I suggested?
>
>
>Thanks,
>SJ
>
>[...]


Absolutely!

Will do

[PATCH 3/4] mm/damon/sysfs: check contexts->nr in update_schemes_tried_regions

[Josh Law]

damon_sysfs_update_schemes_tried_regions() and its callback
damon_sysfs_schemes_tried_regions_upd_one() access contexts_arr[0]
without verifying nr_contexts >= 1. This can NULL deref if damon_ctx is
non-NULL (preserved after stop) but nr_contexts has been set to 0. Add
the missing check.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 mm/damon/sysfs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index 36ad2e8956c9..ddcdc4e35b27 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -1731,6 +1731,8 @@ static int damon_sysfs_update_schemes_tried_regions(
 
 	if (!ctx)
 		return -EINVAL;
+	if (sysfs_kdamond->contexts->nr != 1)
+		return -EINVAL;
 
 	damon_sysfs_schemes_clear_regions(
 			sysfs_kdamond->contexts->contexts_arr[0]->schemes);
-- 
2.34.1

Re: [PATCH 3/4] mm/damon/sysfs: check contexts->nr in update_schemes_tried_regions

[SeongJae Park]

On Thu, 19 Mar 2026 15:57:41 +0000 Josh Law <objecting@objecting.org> wrote:

> damon_sysfs_update_schemes_tried_regions() and its callback
> damon_sysfs_schemes_tried_regions_upd_one() access contexts_arr[0]
> without verifying nr_contexts >= 1. This can NULL deref if damon_ctx is
> non-NULL (preserved after stop) but nr_contexts has been set to 0. Add
> the missing check.

Nice catch.  This can be triggered by privileged users.

    # cd /sys/kernel/mm/damon/admin/kdamonds/
    # echo 1 > nr_kdamonds
    # echo 1 > contexts/nr_contexts
    # echo on > state
    # echo off > state
    # echo 0 > contexts/nr_contexts
    # echo update_schemes_tried_regions > state
    # dmesg
    [...]
    [  222.362338] BUG: kernel NULL pointer dereference, address: 0000000000000000
    [...]

Weird sequence of commands, but even privileged users can make mistakes.  So I
think this deserves Fixes: and Cc: stable.

But, this is just another instance of a class of bugs that I mentioned on the
reply to the second patch of this series.  I'd suggest fixing all bugs of the
class with single fix, as I also mentioned on the second patch thread.  Let's
discuss on the thread.


Thanks,
SJ

[...]

[PATCH 4/4] mm/damon/sysfs: check contexts->nr in repeat_call_fn

[Josh Law]

damon_sysfs_repeat_call_fn() accesses contexts_arr[0] in
upd_tuned_intervals, upd_schemes_stats, and upd_schemes_effective_quotas
without checking nr_contexts. A user can set nr_contexts to 0 via sysfs
while DAMON is running, causing a NULL pointer dereference in the
repeat callback. Add a guard under the lock.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 mm/damon/sysfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index ddcdc4e35b27..d982f2dc7a2b 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -1620,9 +1620,12 @@ static int damon_sysfs_repeat_call_fn(void *data)
 
 	if (!mutex_trylock(&damon_sysfs_lock))
 		return 0;
+	if (sysfs_kdamond->contexts->nr != 1)
+		goto out;
 	damon_sysfs_upd_tuned_intervals(sysfs_kdamond);
 	damon_sysfs_upd_schemes_stats(sysfs_kdamond);
 	damon_sysfs_upd_schemes_effective_quotas(sysfs_kdamond);
+out:
 	mutex_unlock(&damon_sysfs_lock);
 	return 0;
 }
-- 
2.34.1

Re: [PATCH 4/4] mm/damon/sysfs: check contexts->nr in repeat_call_fn

[SeongJae Park]

On Thu, 19 Mar 2026 15:57:42 +0000 Josh Law <objecting@objecting.org> wrote:

> damon_sysfs_repeat_call_fn() accesses contexts_arr[0] in
> upd_tuned_intervals, upd_schemes_stats, and upd_schemes_effective_quotas
> without checking nr_contexts. A user can set nr_contexts to 0 via sysfs
> while DAMON is running, causing a NULL pointer dereference in the
> repeat callback. Add a guard under the lock.

Good catch!

Priveleged users can trigger this.

    # damo start --refresh_stat 1s
    # echo 0 > /sys/kernel/mm/damon/admin/kdamonds/0/contexts/nr_contexts
    # dmesg
    [...]
    [  277.616182] BUG: kernel NULL pointer dereference, address: 0000000000000000
    [...]

So, I think this deserves Fixes: and Cc: stable.

Fixes: d809a7c64ba8 ("mm/damon/sysfs: implement refresh_ms file internal work")
Cc: <stable@vger.kernel.org> # 6.17.x

> 
> Signed-off-by: Josh Law <objecting@objecting.org>

Reviewed-by: SeongJae Park <sj@kernel.org>
> ---
>  mm/damon/sysfs.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index ddcdc4e35b27..d982f2dc7a2b 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c
> @@ -1620,9 +1620,12 @@ static int damon_sysfs_repeat_call_fn(void *data)
>  
>  	if (!mutex_trylock(&damon_sysfs_lock))
>  		return 0;
> +	if (sysfs_kdamond->contexts->nr != 1)
> +		goto out;
>  	damon_sysfs_upd_tuned_intervals(sysfs_kdamond);
>  	damon_sysfs_upd_schemes_stats(sysfs_kdamond);
>  	damon_sysfs_upd_schemes_effective_quotas(sysfs_kdamond);
> +out:
>  	mutex_unlock(&damon_sysfs_lock);
>  	return 0;
>  }
> -- 
> 2.34.1

Sashiko comment.  TL; DR: Good points, but orthogonal to this patch.  I will
work for those.

: Does this check fully prevent the NULL pointer dereference? A user can still
: manually trigger these callbacks by writing commands like
: update_tuned_intervals or update_schemes_stats to the state sysfs file.
: 
: Those commands execute through damon_sysfs_handle_cmd() and
: damon_sysfs_damon_call(), which add the callback to the running DAMON thread
: without checking if contexts->nr != 1. When the kdamond thread executes the
: manual command, it seems it would unconditionally dereference
: contexts_arr[0], which could be NULL.
: 
: Would it be safer to place the contexts->nr != 1 check directly inside the
: callback functions (damon_sysfs_upd_tuned_intervals, etc.) to cover all
: entry points?

Good point, but this is orthogonal to this patch.

And the second and the third patches of this series is actually making fixes
for parts of the problems Sashiko mentioned.  I also have an opinion to fix
similar issues in one place.  Let's discuss that on the second patch.

: 
: >  	damon_sysfs_upd_tuned_intervals(sysfs_kdamond);
: >  	damon_sysfs_upd_schemes_stats(sysfs_kdamond);
: >  	damon_sysfs_upd_schemes_effective_quotas(sysfs_kdamond);
: > +out:
: >  	mutex_unlock(&damon_sysfs_lock);
: >  	return 0;
: >  }
: 
: Looking at the context just above this diff in damon_sysfs_repeat_call_fn(),
: is it intentional that damon_sysfs_next_update_jiffies is a static global
: variable?
: 
: static int damon_sysfs_repeat_call_fn(void *data)
: {
:         struct damon_sysfs_kdamond *sysfs_kdamond = data;
: 
:         if (!sysfs_kdamond->refresh_ms)
:                 return 0;
:         if (time_before(jiffies, damon_sysfs_next_update_jiffies))
:                 return 0;
:         damon_sysfs_next_update_jiffies = jiffies +
:                 msecs_to_jiffies(sysfs_kdamond->refresh_ms);
: 
: Since users can create multiple independent kdamond instances via sysfs,
: each running its own thread with a unique refresh_ms interval, multiple
: threads running this function could concurrently read and overwrite the exact
: same timer.
: 
: This seems like it would cause independent kdamond instances to clobber each
: other's refresh cycles, resulting in delayed or skipped interval updates.
: Could this be moved to a per-kdamond field?

Makes sense, but orthogonal to this patch.  I will work on this.

: 
: Also, looking at damon_sysfs_turn_damon_on() which sets up this repeat
: call function:
: 
: static int damon_sysfs_turn_damon_on(struct damon_sysfs_kdamond *kdamond)
: {
:         ...
:         repeat_call_control = kmalloc_obj(*repeat_call_control);
:         ...
:         err = damon_start(&ctx, 1, false);
:         ...
:         damon_call(ctx, repeat_call_control);
:         return err;
: }
: 
: Could this leak the repeat_call_control allocation?
: 
: If the kdamond thread exits prematurely right after damon_start(), the
: context becomes inactive. When damon_call() executes, it checks
: damon_is_running(). Since it is false, damon_call_handle_inactive_ctx()
: unlinks the control block and returns -EINVAL without freeing the control
: block or evaluating dealloc_on_cancel.
: 
: Because damon_sysfs_turn_damon_on() ignores the return value of damon_call(),
: returning the success code from damon_start() instead, it appears the memory
: allocated for repeat_call_control is lost.

Makes sense, but orthogonal to this patch.  I will work on this.

# review url: https://sashiko.dev/#/patchset/20260319155742.186627-5-objecting@objecting.org


Thanks,
SJ

Re: [PATCH 0/4] mm/damon/sysfs: fix resource leak and NULL pointer dereferences

[Josh Law]

On 19 March 2026 15:57:38 GMT, Josh Law <objecting@objecting.org> wrote:
>This series fixes a memory leak and three NULL pointer dereferences in
>the DAMON sysfs interface, all in mm/damon/sysfs.c.
>
>Patch 1 fixes a damon_ctx leak in damon_sysfs_commit_input() when
>damon_sysfs_new_test_ctx() fails after param_ctx was already built.
>
>Patches 2-4 fix missing contexts->nr checks before dereferencing
>contexts_arr[0]. A user can trigger these by setting nr_contexts to 0
>via sysfs and then issuing commands that assume a context exists:
>
>  - Patch 2: CLEAR_SCHEMES_TRIED_REGIONS handler in damon_sysfs_handle_cmd()
>  - Patch 3: damon_sysfs_update_schemes_tried_regions(), reached via
>    UPDATE_SCHEMES_TRIED_BYTES and UPDATE_SCHEMES_TRIED_REGIONS
>  - Patch 4: damon_sysfs_repeat_call_fn(), reachable when nr_contexts is
>    set to 0 while DAMON is running
>
>Josh Law (4):
>  mm/damon/sysfs: fix param_ctx leak on damon_sysfs_new_test_ctx()
>    failure
>  mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions
>  mm/damon/sysfs: check contexts->nr in update_schemes_tried_regions
>  mm/damon/sysfs: check contexts->nr in repeat_call_fn
>
> mm/damon/sysfs.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>


[0] <https://sashiko.dev/#/patchset/20260319155742.186627-1-objecting%40objecting.org>



I don't think this is right. Probably best you don't listen to that..

V/R


Josh Law