Re: [RFC PATCH v1.2 09/11] mm/damon/sysfs: kobject_del() probe filter dirs

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Medium] Missing `kobject_del()` call in `damos_sysfs_region_add_dirs()` error path causes delayed sysfs directory removal and subsequent `-EEXIST` name conflicts.
- [Medium] Missing `kobject_del()` in `damos_sysfs_probes_rm_dirs()` leaves child probe directories lingering in sysfs on error paths.
--

commit 6a62b56c2acdfd514de2a6e4cd36c3839e0c6db5
Author: SeongJae Park <sj@kernel.org>
Subject: mm/damon/sysfs: kobject_del() probe filter dirs

This commit fixes directory creation failures due to name conflicts on kernels
with CONFIG_DEBUG_KOBJECT_RELEASE. It does so by adding missing kobject_del()
calls before kobject_put() in the data attribute probe filter directories'
teardown path.

> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index 5ca45654cdd39..36a0e82601697 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c
> @@ -962,8 +962,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);
> +	}
>  	filters->nr = 0;
>  	kfree(filters_arr);
>  	filters->filters_arr = NULL;

[Severity: Medium]
This is a pre-existing issue, but since we are fixing missing kobject_del()
calls here, should we also update damos_sysfs_probes_rm_dirs() in
mm/damon/sysfs-schemes.c?

When tearing down probe directories, the loop iterates over child probes and
calls kobject_put(), but omitting kobject_del() might leave the successfully
added child directories in sysfs pending delayed release:

mm/damon/sysfs-schemes.c:damos_sysfs_probes_rm_dirs() {
    ...
	for (i = 0; i < probes->nr; i++)
		kobject_put(&probes_arr[i]->kobj);
    ...
}

Would this also result in -EEXIST name collisions on subsequent retry attempts?

[Severity: Medium]
This isn't a bug introduced by this patch, but does the error path in
damos_sysfs_region_add_dirs() need similar handling?

If kobject_init_and_add() succeeds but the subsequent call to
damos_sysfs_probes_add_dirs() fails, we jump to the fail path:

mm/damon/sysfs-schemes.c:damos_sysfs_region_add_dirs() {
    ...
	err = kobject_init_and_add(&probes->kobj, &damos_sysfs_probes_ktype,
			&region->kobj, "probes");
	if (err)
		goto fail;

	err = damos_sysfs_probes_add_dirs(probes, ctx, dregion);
	if (err)
		goto fail;
    ...
fail:
	kobject_put(&probes->kobj);
	return err;
}

Could the lack of kobject_del(&probes->kobj) here delay the removal of the
sysfs directory and cause an unexpected -EEXIST conflict if userspace
attempts to recreate the region?

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