[PATCH 6.1.y] mm/damon/sysfs-schemes: kobject_del() scheme dirs

[PATCH 6.1.y] mm/damon/sysfs-schemes: kobject_del() scheme dirs

[SJ Park]

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 scheme directories by adding kobject_del() calls.

Link: https://lore.kernel.org/20260628220121.97360-4-sj@kernel.org
Fixes: 7e84b1f8212a ("mm/damon/sysfs: support DAMON-based Operation Schemes")
Signed-off-by: SJ Park <sj@kernel.org>
Cc: <stable@vger.kernel.org> # 5.18.x
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit 05fb6ac98c49be870c5f9ccdfdf95f0107e564ae)
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/sysfs.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index bb2f4650741fa..7e4fb112d1154 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -954,6 +954,7 @@ static void damon_sysfs_schemes_rm_dirs(struct damon_sysfs_schemes *schemes)
 
 	for (i = 0; i < schemes->nr; i++) {
 		damon_sysfs_scheme_rm_dirs(schemes_arr[i]);
+		kobject_del(&schemes_arr[i]->kobj);
 		kobject_put(&schemes_arr[i]->kobj);
 	}
 	schemes->nr = 0;
@@ -991,13 +992,15 @@ static int damon_sysfs_schemes_add_dirs(struct damon_sysfs_schemes *schemes,
 			goto out;
 		err = damon_sysfs_scheme_add_dirs(scheme);
 		if (err)
-			goto out;
+			goto del_out;
 
 		schemes_arr[i] = scheme;
 		schemes->nr++;
 	}
 	return 0;
 
+del_out:
+	kobject_del(&scheme->kobj);
 out:
 	damon_sysfs_schemes_rm_dirs(schemes);
 	kobject_put(&scheme->kobj);
-- 
2.47.3

Re: [PATCH 6.1.y] mm/damon/sysfs-schemes: kobject_del() scheme dirs

[sashiko-bot]

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

Pre-existing issues:
- [High] Incomplete fix: Missing `kobject_del()` for dynamically sized sysfs array directories (kdamonds, contexts, targets, regions) causes name conflicts on recreation.
--

commit 313d3e99fc72ef8df36e0924dc61e045a278961d
Author: SJ Park <sj@kernel.org>

mm/damon/sysfs-schemes: kobject_del() scheme 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 scheme directories by adding kobject_del() calls.

Link: https://lore.kernel.org/20260628220121.97360-4-sj@kernel.org

> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index bb2f4650741fa..7e4fb112d1154 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c
> @@ -954,6 +954,7 @@ static void damon_sysfs_schemes_rm_dirs(struct damon_sysfs_schemes *schemes)
>  
>  	for (i = 0; i < schemes->nr; i++) {
>  		damon_sysfs_scheme_rm_dirs(schemes_arr[i]);
> +		kobject_del(&schemes_arr[i]->kobj);
>  		kobject_put(&schemes_arr[i]->kobj);
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this fix for
the schemes directories, it appears the exact same omission of kobject_del()
is still present for other dynamically sized sysfs arrays (kdamonds,
contexts, targets, and regions).

For instance, in damon_sysfs_kdamonds_rm_dirs():

static void damon_sysfs_kdamonds_rm_dirs(struct damon_sysfs_kdamonds *kdamonds)
{
	...
	for (i = 0; i < kdamonds->nr; i++) {
		damon_sysfs_kdamond_rm_dirs(kdamonds_arr[i]);
		kobject_put(&kdamonds_arr[i]->kobj);
	}
	...
}

When updating these directories, the old directories are dropped using only
kobject_put() and new ones are immediately added with kobject_init_and_add()
using the same names. If CONFIG_DEBUG_KOBJECT_RELEASE is enabled or a
reference is held, won't this deferred removal leave the old directory
active and cause the recreation to fail with -EEXIST?

Should kobject_del() also be added to damon_sysfs_kdamonds_rm_dirs(),
damon_sysfs_contexts_rm_dirs(), damon_sysfs_targets_rm_dirs(), and
damon_sysfs_regions_rm_dirs() to prevent these same name conflicts?

> @@ -991,13 +992,15 @@ static int damon_sysfs_schemes_add_dirs(struct damon_sysfs_schemes *schemes,
>  			goto out;
>  		err = damon_sysfs_scheme_add_dirs(scheme);
>  		if (err)
> -			goto out;
> +			goto del_out;
>  
>  		schemes_arr[i] = scheme;
>  		schemes->nr++;
>  	}
>  	return 0;
>  
> +del_out:
> +	kobject_del(&scheme->kobj);
>  out:
>  	damon_sysfs_schemes_rm_dirs(schemes);
>  	kobject_put(&scheme->kobj);

[Severity: High]
This is also a pre-existing issue, but similar to the rm_dirs() paths, the
error paths for adding other array elements appear to be missing
kobject_del().

For example, in damon_sysfs_kdamonds_add_dirs():

	for (i = 0; i < nr_kdamonds; i++) {
		...
		err = kobject_init_and_add(&kdamond->kobj,
				&damon_sysfs_kdamond_ktype, &kdamonds->kobj,
				"%d", i);
		if (err)
			goto out;

		err = damon_sysfs_kdamond_add_dirs(kdamond);
		if (err)
			goto out;
		...
out:
	damon_sysfs_kdamonds_rm_dirs(kdamonds);
	kobject_put(&kdamond->kobj);
	return err;

If adding the child directories via damon_sysfs_kdamond_add_dirs() fails,
the parent kdamond->kobj has already been added to sysfs. Doesn't branching
directly to 'out' leave a lingering sysfs node without calling kobject_del()?

Should a similar 'del_out' error path be added to
damon_sysfs_kdamonds_add_dirs(), damon_sysfs_contexts_add_dirs(), and
damon_sysfs_targets_add_dirs()?

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

Re: [PATCH 6.1.y] mm/damon/sysfs-schemes: kobject_del() scheme dirs

[Sasha Levin]

> 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 scheme directories by adding kobject_del() calls.

Queued for 6.1, thanks.

-- 
Thanks,
Sasha