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

[sashiko-bot@kernel.org]

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?

[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?

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