Re: [RFC PATCH 8/9] mm/damon/sysfs: kobject_del() probe filter dirs

[SeongJae Park <sj@kernel.org>]

On Tue, 16 Jun 2026 15:27:51 +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] Missing `kobject_del()` in `damos_sysfs_probes_rm_dirs()` leading to sysfs name conflicts.
> - [High] Missing `kobject_del()` on error paths in `_add_dirs` array-initialization loops causing `-EEXIST` name conflicts on retries.
> --
> 
> commit d2e0c9ec8c59c96ff7123c668d926efc99e5b382
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/sysfs: kobject_del() probe filter dirs
> 
> This commit adds kobject_del() calls in damon_sysfs_filters_rm_dirs()
> before kobject_put(). This prevents directory creation failures caused
> by name conflicts when CONFIG_DEBUG_KOBJECT_RELEASE is enabled.
> 
> > diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> > index f72e1e37df9d8..04725ff3d9f3c 100644
> > --- a/mm/damon/sysfs.c
> > +++ b/mm/damon/sysfs.c
> > @@ -960,8 +960,10 @@ static void damon_sysfs_filters_rm_dirs(struct damon_sysfs_filters *filters)
> >  	struct damon_sysfs_filter **filters_arr = filters->filters_arr;
> >  	int i;
> >  
> > -	for (i = 0; i < filters->nr; i++)
> > +	for (i = 0; i < filters->nr; i++) {
> > +		kobject_del(&filters_arr[i]->kobj);
> >  		kobject_put(&filters_arr[i]->kobj);
> > +	}
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but should this fix also be
> applied to other similar cleanup loops?
> 
> Looking at mm/damon/sysfs-schemes.c:damos_sysfs_probes_rm_dirs():
> 
> 	for (i = 0; i < probes->nr; i++)
> 		kobject_put(&probes_arr[i]->kobj);
> 
> This iterates over probes_arr and calls kobject_put() without kobject_del(),
> mirroring the exact bug this patch addresses. When
> damos_sysfs_probes_add_dirs() fails and calls this cleanup function, does
> this delay directory removal and cause a -EEXIST collision if retried?

No.  I explained the reason to another same report that Sashiko reported as
another repply to this thread.

> 
> [Severity: High]
> This is a pre-existing issue, but do the error paths in the _add_dirs array
> initialization loops also need kobject_del() calls?
> 
> If a nested setup function fails after a successful kobject_init_and_add(),
> the error path drops the reference but misses a preceding kobject_del().
> 
> For example, in mm/damon/sysfs.c:damon_sysfs_targets_add_dirs():
> 
> out:
> 	damon_sysfs_targets_rm_dirs(targets);
> 	kobject_put(&target->kobj);
> 	return err;
> 
> If damon_sysfs_target_add_dirs() fails, the target is not yet in targets_arr.
> Does putting the kobject without a preceding kobject_del() leave the
> successfully added sysfs directory lingering, leading to -EEXIST name
> conflicts on retries?
> 
> Similar error paths missing kobject_del() appear in:
> - mm/damon/sysfs.c:damon_sysfs_contexts_add_dirs()
> - mm/damon/sysfs.c:damon_sysfs_kdamonds_add_dirs()
> - mm/damon/sysfs.c:damon_sysfs_schemes_add_dirs()
> - mm/damon/sysfs.c:damon_sysfs_probes_add_dirs()
> - mm/damon/sysfs-schemes.c:damos_sysfs_populate_region_dir()

Yes.  Sashiko reported same issues in other replies to this thread and I will
fix thse in the next revision.


Thanks,
SJ

[...]