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

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

[SeongJae Park]

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.

Also update damos_sysfs_populate_region_dir() to add the region object
to the list only after the kobject_init_and_add() is success, so that
fail of kobject_init_and_add() is not leaving the deallocated object on
the list.

The issue was discovered [1] by Sashiko.

[1] https://lore.kernel.org/20260513011920.119183-1-sj@kernel.org

Fixes: 9277d0367ba1 ("mm/damon/sysfs-schemes: implement scheme region directory")
Cc: <stable@vger.kernel.org> # 6.2.x
Signed-off-by: SeongJae Park <sj@kernel.org>
---
Changes from RFC v2
- RFC v2: https://lore.kernel.org/20260517172624.888-1-sj@kernel.org
- Rebase to mm-stable (7.1-rc3) for Sashiko review.
Changes from RFC
- RFC: https://lore.kernel.org/20260516211436.1883-1-sj@kernel.org
- Add region to the list after kobject_init_and_add() success.

 mm/damon/sysfs-schemes.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index 245d63808411a..a2ebc752d9332 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -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--;
 	}
@@ -2926,15 +2925,16 @@ void damos_sysfs_populate_region_dir(struct damon_sysfs_schemes *sysfs_schemes,
 	region = damon_sysfs_scheme_region_alloc(r);
 	if (!region)
 		return;
-	region->sz_filter_passed = sz_filter_passed;
-	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);
+		return;
 	}
+	region->sz_filter_passed = sz_filter_passed;
+	list_add_tail(&region->list, &sysfs_regions->regions_list);
+	sysfs_regions->nr_regions++;
 }
 
 int damon_sysfs_schemes_clear_regions(

base-commit: 5d6919055dec134de3c40167a490f33c74c12581
-- 
2.47.3

Re: [RFC PATCH v2.1] mm/damon/sysfs-schemes: delete tried region in regions_rmdirs()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Moving the `sz_filter_passed` assignment to after `kobject_init_and_add()` exposes uninitialized kernel heap memory to user space.
- [High] Relying solely on `kobject_put()` to remove directories from sysfs leaves directories visible if references are held.
--

commit eda0f7269f695739057cee7bd02d81742d566163
Author: SeongJae Park <sj@kernel.org>

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

This commit updates DAMON sysfs to remove region objects from the linked list
immediately before decrementing the kobject reference count to prevent
use-after-free races. It also modifies region population to only add the
region to the list after a successful kobject initialization.

Link: https://lore.kernel.org/20260513011920.119183-1-sj@kernel.org

> diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> --- a/mm/damon/sysfs-schemes.c
> +++ b/mm/damon/sysfs-schemes.c
> @@ -163,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);

This is not a new regression, but does relying solely on kobject_put() to
remove directories from sysfs leave them visible if references are still
held?

If an elevated reference count exists, such as a process having an open file
descriptor to a sysfs attribute, the kobject is not immediately released and
kobject_del() is delayed.

Could this leave the old region directory accessible in sysfs indefinitely,
causing stale directories to accumulate alongside new ones? Should
kobject_del(&r->kobj) be called explicitly before kobject_put() to cleanly
and synchronously remove the directory?

>  		regions->nr_regions--;
>  	}

[ ... ]

> @@ -2925,15 +2925,16 @@ void damos_sysfs_populate_region_dir(struct damon_sysfs_schemes *sysfs_schemes,
>  	region = damon_sysfs_scheme_region_alloc(r);
>  	if (!region)
>  		return;
> -	region->sz_filter_passed = sz_filter_passed;
> -	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);
> +		return;
>  	}
> +	region->sz_filter_passed = sz_filter_passed;

Could moving the sz_filter_passed assignment after kobject_init_and_add()
expose uninitialized memory to user space?

If the region object is allocated without zeroing the memory, calling
kobject_init_and_add() makes the region's kobject immediately visible in
sysfs.

If a concurrent read to the sz_filter_passed sysfs attribute occurs during
this window, could it read the field without locking and leak uninitialized
kernel heap memory to user space? Should the assignment occur before the
kobject is registered?

> +	list_add_tail(&region->list, &sysfs_regions->regions_list);
> +	sysfs_regions->nr_regions++;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517175915.3352-1-sj@kernel.org?part=1

Re: [RFC PATCH v2.1] mm/damon/sysfs-schemes: delete tried region in regions_rmdirs()

[SeongJae Park]

On Sun, 17 May 2026 18:27:49 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Moving the `sz_filter_passed` assignment to after `kobject_init_and_add()` exposes uninitialized kernel heap memory to user space.
> - [High] Relying solely on `kobject_put()` to remove directories from sysfs leaves directories visible if references are held.
> --
> 
> commit eda0f7269f695739057cee7bd02d81742d566163
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/sysfs-schemes: delete tried region in regions_rmdirs()
> 
> This commit updates DAMON sysfs to remove region objects from the linked list
> immediately before decrementing the kobject reference count to prevent
> use-after-free races. It also modifies region population to only add the
> region to the list after a successful kobject initialization.
> 
> Link: https://lore.kernel.org/20260513011920.119183-1-sj@kernel.org
> 
> > diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> > --- a/mm/damon/sysfs-schemes.c
> > +++ b/mm/damon/sysfs-schemes.c
> > @@ -163,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);
> 
> This is not a new regression, but does relying solely on kobject_put() to
> remove directories from sysfs leave them visible if references are still
> held?
> 
> If an elevated reference count exists, such as a process having an open file
> descriptor to a sysfs attribute, the kobject is not immediately released and
> kobject_del() is delayed.
> 
> Could this leave the old region directory accessible in sysfs indefinitely,
> causing stale directories to accumulate alongside new ones? Should
> kobject_del(&r->kobj) be called explicitly before kobject_put() to cleanly
> and synchronously remove the directory?

Makes sense, I will work on this.  Not a blocker of this patch, as Sashiko also
mentioned, though.

> 
> >  		regions->nr_regions--;
> >  	}
> 
> [ ... ]
> 
> > @@ -2925,15 +2925,16 @@ void damos_sysfs_populate_region_dir(struct damon_sysfs_schemes *sysfs_schemes,
> >  	region = damon_sysfs_scheme_region_alloc(r);
> >  	if (!region)
> >  		return;
> > -	region->sz_filter_passed = sz_filter_passed;
> > -	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);
> > +		return;
> >  	}
> > +	region->sz_filter_passed = sz_filter_passed;
> 
> Could moving the sz_filter_passed assignment after kobject_init_and_add()
> expose uninitialized memory to user space?
> 
> If the region object is allocated without zeroing the memory, calling
> kobject_init_and_add() makes the region's kobject immediately visible in
> sysfs.
> 
> If a concurrent read to the sz_filter_passed sysfs attribute occurs during
> this window, could it read the field without locking and leak uninitialized
> kernel heap memory to user space? Should the assignment occur before the
> kobject is registered?

Good finding.  I will set sz_filter_passed before kobject_init_and_add() in the
next version.

> 
> > +	list_add_tail(&region->list, &sysfs_regions->regions_list);
> > +	sysfs_regions->nr_regions++;
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260517175915.3352-1-sj@kernel.org?part=1


Thanks,
SJ