Re: [RFC PATCH 4/9] mm/damon/sysfs-schemes: kobject_del() scheme region dirs

[SeongJae Park <sj@kernel.org>]

On Tue, 16 Jun 2026 15:25:35 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Incomplete Fix / Missing kobject_del() in sysfs directory array teardown and error paths.
> - [High] Memory leak of `stats` object in `damon_sysfs_scheme_add_dirs()` error path.
> --
> 
> commit 30488832bf0bcf915d5cbb6c2727b7cdedd8440e
> Author: SeongJae Park <sj@kernel.org>
> Subject: mm/damon/sysfs-schemes: kobject_del() scheme region dirs
> 
> This commit addresses directory creation failures due to name conflicts
> when CONFIG_DEBUG_KOBJECT_RELEASE is enabled. It fixes the issue by adding
> missing kobject_del() calls before kobject_put() for scheme region
> directories during removal.
> 
> Link: https://lore.kernel.org/20260517205828.6204-1-sj@kernel.org
> 
> > diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> > index 37cf6edb54f17..bf08e6e1f1635 100644
> > --- a/mm/damon/sysfs-schemes.c
> > +++ b/mm/damon/sysfs-schemes.c
> > @@ -334,6 +334,7 @@ static void damon_sysfs_scheme_regions_rm_dirs(
> >  	list_for_each_entry_safe(r, next, &regions->regions_list, list) {
> >  		damos_sysfs_region_rm_dirs(r);
> >  		list_del(&r->list);
> > +		kobject_del(&r->kobj);
> 
> [Severity: High]
> This is a pre-existing issue, but since we are adding kobject_del() here to
> fix the name collision bug, could there be similar omissions in other array
> teardown and error paths?
> 
> For instance, looking at damos_sysfs_probes_rm_dirs() which is called
> just above via damos_sysfs_region_rm_dirs():
> 
> mm/damon/sysfs-schemes.c:damos_sysfs_probes_rm_dirs() {
>     ...
>     for (i = 0; i < probes->nr; i++)
>         kobject_put(&probes_arr[i]->kobj);
>     ...
> }
> 
> Does this code also omit kobject_del() during probes array teardown,
> potentially leaving directories in sysfs during array resizing?

No.  probe directories are generated as sub directories of scheme region
directories, when scheme region is created.  They are also removed together
with the region directory, which is kboject_del()-ed by this patch.  Hence, the
not yet deleted probe directories are linked to the deleted region directory,
so no name conflicts can happen.

> 
> Similarly, in several initialization loop error paths, such as in
> damos_sysfs_populate_region_dir():
> 
> mm/damon/sysfs-schemes.c:damos_sysfs_populate_region_dir() {
>     ...
> out:
>     kobject_put(&region->kobj);
> }
> 
> If aborting region population, does this path fail to call kobject_del(),
> breaking subsequent retries by leaving the directory name occupied?
> 
> Other functions like damon_sysfs_schemes_add_dirs(),
> damon_sysfs_targets_add_dirs(), and damon_sysfs_contexts_add_dirs()
> appear to have the same out path pattern omitting kobject_del() for
> partially added objects. Would it make sense to update those paths as well?

Good catch.  I will fix this in the next revision.

> 
> >  		kobject_put(&r->kobj);
> 
> [Severity: High]
> This is a pre-existing issue, but I noticed a potential memory leak in
> an error path nearby.
> 
> In damon_sysfs_scheme_add_dirs(), the error cleanup sequence handles
> dropping the tried_regions and then ops_filters:
> 
> mm/damon/sysfs-schemes.c:damon_sysfs_scheme_add_dirs() {
>     ...
> put_tried_regions_out:
>     kobject_put(&scheme->tried_regions->kobj);
>     scheme->tried_regions = NULL;
> put_filters_watermarks_quotas_access_pattern_out:
>     kobject_put(&scheme->ops_filters->kobj);
>     ...
> }
> 
> Does this error path completely omit dropping the stats kobject?
> 
> The stats kobject is initialized prior to the failed call, so
> wouldn't skipping kobject_put(&scheme->stats->kobj) leak it permanently?

Good finding, but you already shared this with the reply to the first patch of
this series.  I promised to separately work on it.


Thanks,
SJ

[...]