Skip to contentSkip to conversation

mm/damon/sysfs: fix resource leak and NULL pointer dereferences

PATCH Superseded Josh Law 2 participants 15 messages Mar 19 – Mar 20

Subsystems (3)
Stable branch Memory management DAMON

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Josh Law

author
#0 lore raw

cover mm/damon/sysfs: fix resource leak and NULL pointer dereferences

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(-)
Signature · 1 line
2.34.1
author
#1 lore raw

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

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 <[email protected]>
---
 mm/damon/sysfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
mm/damon/sysfs.c +3 -1
index 576d1ddd736b..b573b9d60784 100644
@@ -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;
Signature · 1 line
2.34.1
maintainer Reviewed-by on v1 1/4
#2 lore raw

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

Quoted 3 lines · On Thu, 19 Mar 2026 15:57:39 +0000 Josh Law <[email protected]> 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 <[email protected]> # 6.18.x
Quoted 2 lines
Signed-off-by Josh Law <[email protected]>

Other than the above,

Reviewed-by SeongJae Park <[email protected]>
Quoted 10 lines
---
 mm/damon/sysfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
mm/damon/sysfs.c +0 -0
index 576d1ddd736b..b573b9d60784 100644
@@ -1524,8 +1524,10 @@ static int damon_sysfs_commit_input(void *data)
 	if (IS_ERR(param_ctx))

mm/damon/sysfs.c static int damon_sysfs_commit_input(void *data) · line 1533

Quoted 10 lines
mm/damon/sysfs.c +3 -1
 		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/[email protected]

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

Thanks,
SJ

[...]

author
#3 lore raw

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

In reply to 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 <[email protected]>
---
 mm/damon/sysfs.c | 2 ++
 1 file changed, 2 insertions(+)
mm/damon/sysfs.c +2 -0
index b573b9d60784..36ad2e8956c9 100644
@@ -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:
Signature · 1 line
2.34.1
maintainer
#4 lore raw

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

Quoted 3 lines · On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <[email protected]> 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.

Quoted 20 lines
Signed-off-by Josh Law <[email protected]>
---
 mm/damon/sysfs.c | 2 ++
 1 file changed, 2 insertions(+)
mm/damon/sysfs.c +2 -1
index b573b9d60784..36ad2e8956c9 100644
@@ -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:
--
Quoted 1 line

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/[email protected]

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

mm/damon/sysfs.c +3 -0
@@ -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 <[email protected]> # 5.18.x

Thanks,
SJ

author
#5 lore raw

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

Quoted 106 lines · SeongJae Park
Quoted 3 lines · On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <[email protected]> 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.

Quoted 20 lines
Signed-off-by Josh Law <[email protected]>
---
 mm/damon/sysfs.c | 2 ++
 1 file changed, 2 insertions(+)
mm/damon/sysfs.c +2 -1
index b573b9d60784..36ad2e8956c9 100644
@@ -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:
--
Quoted 1 line

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/[email protected]

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

mm/damon/sysfs.c +3 -0
@@ -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.

Quoted 6 lines
Fixes 0ac32b8affb5 ("mm/damon/sysfs: support DAMOS stats")
Cc <[email protected]> # 5.18.x

Thanks,
SJ

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

V/R

Josh Law

maintainer
#6 lore raw

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

Quoted 113 lines · On Fri, 20 Mar 2026 07:06:48 +0000 Josh Law <[email protected]> wrote:
Quoted 110 lines · SeongJae Park
Quoted 3 lines · On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <[email protected]> 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.

Quoted 20 lines
Signed-off-by Josh Law <[email protected]>
---
 mm/damon/sysfs.c | 2 ++
 1 file changed, 2 insertions(+)
mm/damon/sysfs.c +2 -1
index b573b9d60784..36ad2e8956c9 100644
@@ -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:
--
Quoted 1 line

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/[email protected]

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

mm/damon/sysfs.c +3 -0
@@ -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 <[email protected]> # 5.18.x
Quoted 6 lines
Quoted 2 lines

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

[...]

author
#7 lore raw

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

Quoted 122 lines · SeongJae Park
Quoted 113 lines · On Fri, 20 Mar 2026 07:06:48 +0000 Josh Law <[email protected]> wrote:
Quoted 110 lines · SeongJae Park
Quoted 3 lines · On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <[email protected]> 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.

Quoted 20 lines
Signed-off-by Josh Law <[email protected]>
---
 mm/damon/sysfs.c | 2 ++
 1 file changed, 2 insertions(+)
mm/damon/sysfs.c +2 -1
index b573b9d60784..36ad2e8956c9 100644
@@ -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:
--
Quoted 1 line

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/[email protected]

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

mm/damon/sysfs.c +3 -0
@@ -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 <[email protected]> # 5.18.x
Quoted 6 lines
Quoted 2 lines

Thanks,
SJ

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

Quoted 8 lines

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

maintainer
#8 lore raw

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

Quoted 9 lines · On Fri, 20 Mar 2026 15:14:54 +0000 Josh Law <[email protected]> wrote:
Quoted 6 lines · SeongJae Park
Quoted 4 lines · On Fri, 20 Mar 2026 07:06:48 +0000 Josh Law <[email protected]> wrote:
Quoted 1 line · SeongJae Park

On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <[email protected]> wrote:

[...]

Quoted 36 lines
Quoted 36 lines
Quoted 25 lines
Quoted 25 lines

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?

mm/damon/sysfs.c +3 -0
@@ -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 <[email protected]> # 5.18.x
Quoted 6 lines
Quoted 2 lines

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?

Quoted 7 lines
Quoted 4 lines

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

[...]

author
#9 lore raw

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

Quoted 67 lines · SeongJae Park
Quoted 9 lines · On Fri, 20 Mar 2026 15:14:54 +0000 Josh Law <[email protected]> wrote:
Quoted 6 lines · SeongJae Park
Quoted 4 lines · On Fri, 20 Mar 2026 07:06:48 +0000 Josh Law <[email protected]> wrote:
Quoted 1 line · SeongJae Park

On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <[email protected]> wrote:

[...]

Quoted 36 lines
Quoted 36 lines
Quoted 25 lines
Quoted 25 lines

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?

mm/damon/sysfs.c +3 -0
@@ -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 <[email protected]> # 5.18.x
Quoted 6 lines
Quoted 2 lines

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?

Quoted 7 lines
Quoted 4 lines

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?

Quoted 4 lines

Thanks,
SJ

[...]

Absolutely!

Will do

author
#10 lore raw

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

In reply to 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 <[email protected]>
---
 mm/damon/sysfs.c | 2 ++
 1 file changed, 2 insertions(+)
mm/damon/sysfs.c +2 -0
index 36ad2e8956c9..ddcdc4e35b27 100644
@@ -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);
Signature · 1 line
2.34.1
maintainer
#11 lore raw

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

Quoted 5 lines · On Thu, 19 Mar 2026 15:57:41 +0000 Josh Law <[email protected]> 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

[...]

author
#12 lore raw

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

In reply to 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 <[email protected]>
---
 mm/damon/sysfs.c | 3 +++
 1 file changed, 3 insertions(+)
mm/damon/sysfs.c +3 -0
index ddcdc4e35b27..d982f2dc7a2b 100644
@@ -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;
 }
Signature · 1 line
2.34.1
maintainer Reviewed-by on v1 4/4
#13 lore raw

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

Quoted 5 lines · On Thu, 19 Mar 2026 15:57:42 +0000 Josh Law <[email protected]> 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 <[email protected]> # 6.17.x
Quoted 2 lines
Signed-off-by Josh Law <[email protected]>
Reviewed-by SeongJae Park <[email protected]>
Quoted 23 lines
---
 mm/damon/sysfs.c | 3 +++
 1 file changed, 3 insertions(+)
mm/damon/sysfs.c +3 -0
index ddcdc4e35b27..d982f2dc7a2b 100644
@@ -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;
 }
Signature · 1 line
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/[email protected]

Thanks,
SJ

author
#14 lore raw
In reply to Josh Law
Quoted 23 lines · 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

Quoted 3 lines

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