mm/damon/sysfs: fix memory leak and NULL dereference issues

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.

Quoted 3 lines

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.

Will it become helpful to use another label accordingly?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v7.0-rc5#n526
https://elixir.bootlin.com/linux/v7.0-rc4/source/mm/damon/sysfs.c#L1506-L1537

Regards,
Markus

Markus these patches are already merged

Quoted 1 line

Markus these patches are already merged

Are there still development interests for the application of a better goto chain?

Quoted 1 line · On Mon, 23 Mar 2026 09:25:52 +0100 Markus Elfring <[email protected]> wrote:

Quoted 1 line

Markus these patches are already merged

It's still in mm-hotfixes-unstable. We can still make changes if needed.

I understand what Markus is suggesting is adding another goto label to make the flow cleaner. Because this is a hotfix that aims to be also applied to stable kernels, I think the change is better to be as simple as possible. Adding another goto label could make it better, but I'm concerned if it will make porting difficult.

IMHO, it is better to do that as a followup cleanup, rather than make change into the hotfix. Let me know if this change is somewhat critical and I'm missing that.

Quoted 2 lines

Are there still development interests for the application of a better goto chain?

Sure, if it makes it better, why not? :)

Thanks,
SJ

[...]

Quoted 1 line · On Mon, 23 Mar 2026 09:25:52 +0100 Markus Elfring <[email protected]> wrote:

Quoted 1 line

Markus these patches are already merged

It's still in mm-hotfixes-unstable. We can still make changes if needed.

I understand what Markus is suggesting is adding another goto label to make the flow cleaner. Because this is a hotfix that aims to be also applied to stable kernels, I think the change is better to be as simple as possible. Adding another goto label could make it better, but I'm concerned if it will make porting difficult.

IMHO, it is better to do that as a followup cleanup, rather than make change into the hotfix. Let me know if this change is somewhat critical and I'm missing that.

Quoted 2 lines

Are there still development interests for the application of a better goto chain?

Sure, if it makes it better, why not? :)

Thanks,
SJ

[...]

Also, unconnected to our topic!

I've tried to backport Damon to 4.19 (for a personal android thing, and failed! Of course)

Can I have a bit of help if that's fine with you? The tree is based on GitHub a bit

On Mon, 23 Mar 2026 16:48:19 +0000 Josh Law <[email protected]> wrote:
[...]

Quoted 6 lines

Also, unconnected to our topic!

I've tried to backport Damon to 4.19 (for a personal android thing, and failed! Of course)

Can I have a bit of help if that's fine with you? The tree is based on GitHub a bit

Sure, I will be happy to help as much as I can without burning myself ;)

Seems [1] Alma Linux has backported DAMON on their 4.18 kernel. Maybe you can try their port first?

Also, what is the oldest kernel that you have to use? As newer it is, the backporting will be easier. When I was in AWS, I backported DAMON of v6.7 on the v5.10 based Amazon Linux kernel, and the source is available on GitHub. So if you can use 5.10 based kernel, using that could also be a good option.

[1] https://oracle.github.io/kconfigs/?config=UTS_RELEASE&config=DAMON

Thanks,
SJ

[...]

Quoted 22 lines · SeongJae Park

On Mon, 23 Mar 2026 16:48:19 +0000 Josh Law <[email protected]> wrote:
[...]

Quoted 6 lines

Also, unconnected to our topic!

I've tried to backport Damon to 4.19 (for a personal android thing, and failed! Of course)

Can I have a bit of help if that's fine with you? The tree is based on GitHub a bit

Sure, I will be happy to help as much as I can without burning myself ;)

Seems [1] Alma Linux has backported DAMON on their 4.18 kernel. Maybe you can try their port first?

Also, what is the oldest kernel that you have to use? As newer it is, the backporting will be easier. When I was in AWS, I backported DAMON of v6.7 on the v5.10 based Amazon Linux kernel, and the source is available on GitHub. So if you can use 5.10 based kernel, using that could also be a good option.

[1] https://oracle.github.io/kconfigs/?config=UTS_RELEASE&config=DAMON

Quoted 4 lines

Thanks,
SJ

[...]

well android likes using old kernels for some reasons, especially LTS, so 4.19..

Quoted 25 lines · On Tue, 24 Mar 2026 07:06:25 +0000 Josh Law <[email protected]> wrote:

Quoted 22 lines · SeongJae Park

On Mon, 23 Mar 2026 16:48:19 +0000 Josh Law <[email protected]> wrote:
[...]

Quoted 6 lines

Also, unconnected to our topic!

I've tried to backport Damon to 4.19 (for a personal android thing, and failed! Of course)

Can I have a bit of help if that's fine with you? The tree is based on GitHub a bit

Sure, I will be happy to help as much as I can without burning myself ;)

Seems [1] Alma Linux has backported DAMON on their 4.18 kernel. Maybe you can try their port first?

Also, what is the oldest kernel that you have to use? As newer it is, the backporting will be easier. When I was in AWS, I backported DAMON of v6.7 on the v5.10 based Amazon Linux kernel, and the source is available on GitHub. So if you can use 5.10 based kernel, using that could also be a good option.

[1] https://oracle.github.io/kconfigs/?config=UTS_RELEASE&config=DAMON

Quoted 8 lines

Quoted 4 lines

Thanks,
SJ

[...]

well android likes using old kernels for some reasons, especially LTS, so 4.19..

Well, but the long term support of 4.19 has dead a few years ago. The oldest LTS kernel of today is 5.10 [1]. I understand some vendors might still use 4.19 kernel, though. Anyway, let me know if there is anything that I can help. I will try to help.

[1] https://www.kernel.org/category/releases.html

Thanks,
SJ

[...]

Subject: (Sashiko) Re: [PATCH v3 3/3] mm/damon/sysfs: check contexts->nr in repeat_call_fn

Forwarding Sashiko review for doing discussions via mails.

# review url: https://sashiko.dev/#/patchset/[email protected]
# start of sashiko.dev inline review
commit 349107c02ee8a122119ac77a51451bb3d9311a34
Author: Josh Law <[email protected]>

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

damon_sysfs_repeat_call_fn() calls damon_sysfs_upd_tuned_intervals(), damon_sysfs_upd_schemes_stats(), and damon_sysfs_upd_schemes_effective_quotas() without checking contexts->nr. If nr_contexts is set to 0 via sysfs while DAMON is running, these functions dereference contexts_arr[0] and cause a NULL pointer dereference. Add the missing check.

Link https://patch.msgid.link/[email protected] open here

mm/damon/sysfs.c static int damon_sysfs_repeat_call_fn(void *data) · line 1625

Quoted 11 lines

mm/damon/sysfs.c +2 -0

index ddc30586c0e6..6a44a2f3d8fc 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);

Does this code cause starvation or incorrect statistics when multiple kdamonds are active?

It appears that damon_sysfs_next_update_jiffies is declared as a static global variable and used by this function to throttle updates:

static unsigned long damon_sysfs_next_update_jiffies;

static int damon_sysfs_repeat_call_fn(void *data)

{
    ...
    if (time_before(jiffies, damon_sysfs_next_update_jiffies))
        return 0;
    damon_sysfs_next_update_jiffies = jiffies +
        msecs_to_jiffies(sysfs_kdamond->refresh_ms);
    ...
}

If multiple kdamonds run concurrently, wouldn't they all share and overwrite this single global timestamp?

If one kdamond sets the timestamp to a future value, another kdamond's thread might incorrectly evaluate time_before() to true and skip its update, potentially leading to starvation and incorrect sysfs statistics for all but the winning kdamond.

mm/damon/sysfs.c static int damon_sysfs_repeat_call_fn(void *data) · line 1631

Quoted 7 lines

mm/damon/sysfs.c +1 -0

 	damon_sysfs_upd_schemes_stats(sysfs_kdamond);
 	damon_sysfs_upd_schemes_effective_quotas(sysfs_kdamond);
+out:
 	mutex_unlock(&damon_sysfs_lock);
 	return 0;
 }

Could there also be a memory leak of repeat_call_control in damon_sysfs_turn_damon_on() if the target kdamond exits early?

In damon_sysfs_turn_damon_on(), repeat_call_control is allocated:

    repeat_call_control = kmalloc_obj(*repeat_call_control);

If the kdamond thread encounters an error or has no work, it can exit and set ctx->kdamond = NULL before damon_call() is fully executed.

In this race window, if damon_call() detects that the context is inactive it calls damon_call_handle_inactive_ctx(). This removes the control from the list and returns -EINVAL, but it does not free the memory.

Because damon_sysfs_turn_damon_on() ignores the return value of damon_call(), does this result in a permanent leak of the memory allocated for repeat_call_control?

# end of sashiko.dev inline review
# review url: https://sashiko.dev/#/patchset/[email protected]