Skip to contentSkip to conversation

mm/damon/sysfs-schemes: delete tried region in regions_rmdirs()

RFC PATCH Superseded SeongJae Park 2 participants 3 messages May 16

Subsystems (3)
Stable branch Memory management DAMON

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

SeongJae Park

author maintainer
#0 lore raw

RFC PATCH 1/1 mm/damon/sysfs-schemes: delete tried region in regions_rmdirs()

DAMON sysfs maintains the DAMOS tried region directory objects via a linked list. When the user requests refresh of the directories, DAMON sysfs removes all the region directories first, and then generate updated regions directory on the empty space. The removal function (damon_sysfs_scheme_regions_rm_dirs()) only puts the kobj objects. Deletion of the container region object from the linked list is done inside the kobj release callback function.

If somehow the callback invocation is delayed, the list will contain regions list that gonna be freed. If the updated region directories creation is started in this situation, the list can be corrupted and use-after-free can happen.

Because the kobj objects are managed by only DAMON sysfs, the issue cannot happen in normal situation. But, such delays can be made on kernels that built with CONFIG_DEBUG_KOBJECT_RELEASE. On the kernel, the issue can indeed be reproduced like below.

    # damo start --damos_action stat
    # cd /sys/kernel/mm/damon/admin/kdamonds/0/
    # for i in {1..10}; do echo update_schemes_tried_regions > state; done
    # dmesg | grep underflow
    [   89.296152] refcount_t: underflow; use-after-free.

Fix the issue by removing the region object from the list when decrementing the reference count.

The issue was discovered [1] by Sashiko.

[1] https://lore.kernel.org/[email protected]

Fixes 9277d0367ba1 ("mm/damon/sysfs-schemes: implement scheme region directory")
Cc <[email protected]> # 6.2.x
Signed-off-by SeongJae Park <[email protected]>
---
 mm/damon/sysfs-schemes.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
mm/damon/sysfs-schemes.c +1 -2
index 5d966ac864193..52627ce3a1a62 100644
@@ -88,7 +88,6 @@ static void damon_sysfs_scheme_region_release(struct kobject *kobj)
 	struct damon_sysfs_scheme_region *region = container_of(kobj,
 			struct damon_sysfs_scheme_region, kobj);
 
-	list_del(&region->list);
 	kfree(region);
 }
 

@@ -164,7 +163,7 @@ static void damon_sysfs_scheme_regions_rm_dirs(
 	struct damon_sysfs_scheme_region *r, *next;
 
 	list_for_each_entry_safe(r, next, &regions->regions_list, list) {
-		/* release function deletes it from the list */
+		list_del(&r->list);
 		kobject_put(&r->kobj);
 		regions->nr_regions--;
 	}

base-commit: 950de73f0f8bb17fc322e8f9ec09a5fbb4a72ed8

Signature · 1 line
2.47.3
AI review
#1 lore raw
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Moving `list_del()` out of the kobject release callback causes a Use-After-Free when…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving list_del() out of the kobject release callback causes a Use-After-Free when kobject_init_and_add() fails in damos_sysfs_populate_region_dir(). --

commit 97136b36d31f71cb6972114db84fe86abd95246f
Author: SeongJae Park <[email protected]>
Subject: mm/damon/sysfs-schemes: delete tried region in regions_rmdirs()

DAMON sysfs maintains the DAMOS tried region directory objects via a linked list. When the user requests refresh of the directories, DAMON sysfs removes all the region directories first, and then generate updated regions directory on the empty space. Deletion of the container region object from the linked list is now done inside the removal function instead of the kobj release callback.

[1] https://lore.kernel.org/[email protected]

mm/damon/sysfs-schemes.c static void damon_sysfs_scheme_region_release(struct kobject *kobj) · line 92

Quoted 11 lines
mm/damon/sysfs-schemes.c +0 -1
index 622c3799db87b..aefeb5d22fb7d 100644
@@ -88,7 +88,6 @@ static void damon_sysfs_scheme_region_release(struct kobject *kobj)
 	struct damon_sysfs_scheme_region *region = container_of(kobj,
 			struct damon_sysfs_scheme_region, kobj);
 
-	list_del(&region->list);
 	kfree(region);
 }

Does removing list_del() here cause a use-after-free if kobject_init_and_add() fails in damos_sysfs_populate_region_dir()?

If kobject_init_and_add() fails, the error path drops the reference:

mm/damon/sysfs-schemes.c:damos_sysfs_populate_region_dir() {
    ...
	list_add_tail(&region->list, &sysfs_regions->regions_list);
	sysfs_regions->nr_regions++;
	if (kobject_init_and_add(&region->kobj,
				&damon_sysfs_scheme_region_ktype,
				&sysfs_regions->kobj, "%d",
				sysfs_regions->nr_regions++)) {
		kobject_put(&region->kobj);
	}
    ...
}

Because region is added to the list before kobject_init_and_add(), dropping the reference will free the region, but it remains in regions_list since the release callback no longer unlinks it.

Should we add an explicit list_del(&region->list) in the error path of damos_sysfs_populate_region_dir() to prevent list corruption?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
author maintainer
#2 lore raw
Quoted 47 lines · On Sat, 16 May 2026 21:39:58 +0000 [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving list_del() out of the kobject release callback causes a Use-After-Free when kobject_init_and_add() fails in damos_sysfs_populate_region_dir(). --

commit 97136b36d31f71cb6972114db84fe86abd95246f
Author: SeongJae Park <[email protected]>
Subject: mm/damon/sysfs-schemes: delete tried region in regions_rmdirs()

DAMON sysfs maintains the DAMOS tried region directory objects via a linked list. When the user requests refresh of the directories, DAMON sysfs removes all the region directories first, and then generate updated regions directory on the empty space. Deletion of the container region object from the linked list is now done inside the removal function instead of the kobj release callback.

[1] https://lore.kernel.org/[email protected]

mm/damon/sysfs-schemes.c static void damon_sysfs_scheme_region_release(struct kobject *kobj) · line 92

Quoted 11 lines
mm/damon/sysfs-schemes.c +0 -1
index 622c3799db87b..aefeb5d22fb7d 100644
@@ -88,7 +88,6 @@ static void damon_sysfs_scheme_region_release(struct kobject *kobj)
 	struct damon_sysfs_scheme_region *region = container_of(kobj,
 			struct damon_sysfs_scheme_region, kobj);
 
-	list_del(&region->list);
 	kfree(region);
 }

Does removing list_del() here cause a use-after-free if kobject_init_and_add() fails in damos_sysfs_populate_region_dir()?

If kobject_init_and_add() fails, the error path drops the reference:

mm/damon/sysfs-schemes.c:damos_sysfs_populate_region_dir() {
    ...
	list_add_tail(&region->list, &sysfs_regions->regions_list);
	sysfs_regions->nr_regions++;
	if (kobject_init_and_add(&region->kobj,
				&damon_sysfs_scheme_region_ktype,
				&sysfs_regions->kobj, "%d",
				sysfs_regions->nr_regions++)) {
		kobject_put(&region->kobj);
	}
    ...
}
Quoted 3 lines

Because region is added to the list before kobject_init_and_add(), dropping the reference will free the region, but it remains in regions_list since the release callback no longer unlinks it.

Good finding. I wrote this patch on a tree having the data attributes monitoring, which moved the list_add_tail() call to be done after kobject_init_and_add() success, so missed this.

Quoted 3 lines

Should we add an explicit list_del(&region->list) in the error path of damos_sysfs_populate_region_dir() to prevent list corruption?

I will move list_add_tail() call to be done after kobject_init_and_add() success.

Quoted 3 lines
Signature · 2 lines
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Thanks,
SJ