Re: [RFC PATCH 1/9] mm/damon/sysfs: kobject_del() target, context and kdamond dirs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix: Missing kobject_del() in sysfs directory creation error paths leads to persistent -EEXIST collisions.
--

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

mm/damon/sysfs: kobject_del() target, context and kdamond dirs

On CONFIG_DEBUG_KOBJECT_RELEASE enabled kernel, lack of kobject_del()
could cause directories creation failures due to the name conflicts.
Fix those issues for target, context and kdamond directories by adding
kobject_del() calls.

Fixes: c951cd3b8901 ("mm/damon: implement a minimal stub for sysfs-based DAMON interface")
Cc: <stable@vger.kernel.org> # 5.18.x

> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index 2e95e3bac774d..d93f7919c3ca1 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c
> @@ -333,6 +333,7 @@ static void damon_sysfs_targets_rm_dirs(struct damon_sysfs_targets *targets)
>  
>  	for (i = 0; i < targets->nr; i++) {
>  		damon_sysfs_target_rm_dirs(targets_arr[i]);
> +		kobject_del(&targets_arr[i]->kobj);

[Severity: High]
This isn't a regression introduced by this patch, but while you are fixing
the -EEXIST name collisions in the rm_dirs() paths, are the same
kobject_del() calls also needed in the error paths of the corresponding
add_dirs() functions?

For example, in damon_sysfs_targets_add_dirs():

    mm/damon/sysfs.c:damon_sysfs_targets_add_dirs() {
        ...
            err = kobject_init_and_add(&target->kobj, ...);
            if (err)
                goto out;

            err = damon_sysfs_target_add_dirs(target);
            if (err)
                goto out;
        ...
    out:
        damon_sysfs_targets_rm_dirs(targets);
        kobject_put(&target->kobj);
        return err;
    }

If damon_sysfs_target_add_dirs() fails, kobject_put() is called without
kobject_del(). Does this result in a lingering directory and a persistent
-EEXIST if the user retries?

Similarly, in mm/damon/sysfs-schemes.c:damos_sysfs_populate_region_dir():

    mm/damon/sysfs-schemes.c:damos_sysfs_populate_region_dir() {
        ...
        if (kobject_init_and_add(&region->kobj,
                    &damon_sysfs_scheme_region_ktype,
                    &sysfs_regions->kobj, "%d",
                    sysfs_regions->nr_regions))
            goto out;
        if (damos_sysfs_region_add_dirs(region, ctx, r))
            goto out;
        ...
        sysfs_regions->nr_regions++;
        return;

    out:
        kobject_put(&region->kobj);
    }

If this fails, it returns without incrementing sysfs_regions->nr_regions.
Could the next region processed in the loop then attempt to initialize with
the exact same name string, immediately hitting -EEXIST and breaking the
entire region population loop?

>  		kobject_put(&targets_arr[i]->kobj);
>  	}
>  	targets->nr = 0;

[ ... ]

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