Re: [RFC PATCH 2/9] mm/damon/sysfs: kobject_del() region dirs

[SeongJae Park <sj@kernel.org>]

On Tue, 16 Jun 2026 15:29:11 +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 explicit kobject_del() on error paths in _add_dirs loops (Incomplete Fix).
> - [High] Memory leak of stats and scheme structures on the error path of damon_sysfs_scheme_add_dirs().
> --
> 
> commit ba4dfdbea398b3306e96fd3662e872e97251fbe9
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/sysfs: kobject_del() region dirs
> 
> This commit fixes directory creation failures caused by name conflicts
> when CONFIG_DEBUG_KOBJECT_RELEASE is enabled. It achieves this by adding
> missing kobject_del() calls to the region directories removal function.
> 
> > diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> > index d93f7919c3ca1..f72e1e37df9d8 100644
> > --- a/mm/damon/sysfs.c
> > +++ b/mm/damon/sysfs.c
> > @@ -107,8 +107,10 @@ static void damon_sysfs_regions_rm_dirs(struct damon_sysfs_regions *regions)
> >  	struct damon_sysfs_region **regions_arr = regions->regions_arr;
> >  	int i;
> >  
> > -	for (i = 0; i < regions->nr; i++)
> > +	for (i = 0; i < regions->nr; i++) {
> > +		kobject_del(&regions_arr[i]->kobj);
> >  		kobject_put(&regions_arr[i]->kobj);
> > +	}
> >  	regions->nr = 0;
> >  	kfree(regions_arr);
> >  	regions->regions_arr = NULL;
> 
> [Severity: High]
> This is a pre-existing issue, but the fix for this specific bug class seems
> incomplete as it misses initialization error paths. If a nested initialization
> fails after kobject_init_and_add() succeeds, the code jumps to an error path
> and calls kobject_put() without first calling 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;
> 
> This same pattern occurs in damon_sysfs_contexts_add_dirs(),
> damon_sysfs_kdamonds_add_dirs(), damon_sysfs_probes_add_dirs(), and
> damon_sysfs_schemes_add_dirs().
> 
> Does this cause the directory removal to be delayed asynchronously, meaning
> if userspace immediately retries the operation, it fails with an -EEXIST
> conflict?

Good findings, I will fix these in the next revisioin of this series.

> 
> [Severity: High]
> This is also a pre-existing issue, but there appears to be a memory leak
> in the error path of damon_sysfs_scheme_add_dirs() in
> mm/damon/sysfs-schemes.c:
> 
>         err = damon_sysfs_scheme_set_tried_regions(scheme);
>         if (err)
>                 goto put_tried_regions_out;
>         return 0;
> 
> 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);
> 
> If damon_sysfs_scheme_set_tried_regions() fails, the code jumps to
> put_tried_regions_out and bypasses releasing the successfully created
> scheme->stats kobject. Since damon_sysfs_scheme_release() only frees the
> scheme struct and does not explicitly clean up lingering children, both the
> stats memory and the parent scheme memory are kept referenced.
> 
> Could this permanently leak both the stats and its parent scheme when
> triggered from userspace via sysfs configuration?

This may deserve another fix.  I will separtely work on this.


Thanks,
SJ

[...]