mm/damon/sysfs: kobject_del() directories that users can create/remove

mm/damon/sysfs.c static void damon_sysfs_contexts_rm_dirs(struct damon_sysfs_contexts *contexts) · line 1649

[Severity: Medium]
This isn't a bug introduced by this patch, but does damos_sysfs_probes_rm_dirs() need a similar kobject_del() fix to avoid sysfs name conflicts?

mm/damon/sysfs-schemes.c:damos_sysfs_probes_rm_dirs() {
...

	for (i = 0; i < probes->nr; i++)
		kobject_put(&probes_arr[i]->kobj);
	probes->nr = 0;

...
}

[ ... ]

mm/damon/sysfs.c static int damon_sysfs_contexts_add_dirs(struct damon_sysfs_contexts *contexts, · line 1696

[Severity: Medium]
This is a pre-existing issue, but does damon_sysfs_schemes_add_dirs() need a similar kobject_del() fix in its error path?

mm/damon/sysfs-schemes.c:damon_sysfs_schemes_add_dirs() {
...

		err = kobject_init_and_add(&scheme->kobj, ...);
		if (err)
			goto out;
		err = damon_sysfs_scheme_add_dirs(scheme);
		if (err)
			goto out;
...
out:
	damon_sysfs_schemes_rm_dirs(schemes);
	kobject_put(&scheme->kobj);
	return err;
}

If damon_sysfs_scheme_add_dirs() fails after kobject_init_and_add() succeeds, the out label puts the reference without unlinking it.

Could this cause a directory creation failure on retry under delayed release?

[Severity: High]
This isn't a bug introduced by this patch, but while looking at error paths, does damon_sysfs_scheme_add_dirs() have an uninitialized pointer dereference and a resource leak?

mm/damon/sysfs-schemes.c:damon_sysfs_scheme_add_dirs() {
...

	err = damon_sysfs_scheme_set_stats(scheme);
	if (err)
		goto put_filters_watermarks_quotas_access_pattern_out;
	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;

...
}

If damon_sysfs_scheme_set_tried_regions() fails, the code jumps to put_tried_regions_out and dereferences scheme->tried_regions.

Since damon_sysfs_scheme_alloc() allocates memory without zeroing it, won't this dereference an uninitialized pointer?

Additionally, this jump entirely skips cleaning up the successfully initialized stats object.

Should there be a put_stats_out cleanup label to avoid leaking it?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

mm/damon/sysfs.c static int damon_sysfs_targets_add_dirs(struct damon_sysfs_targets *targets, · line 388

[Severity: Medium]
This is a pre-existing issue, but I noticed a similar pattern in mm/damon/sysfs-schemes.c that seems to be missing a kobject_del() call.

In mm/damon/sysfs-schemes.c:damon_sysfs_schemes_add_dirs():

	err = kobject_init_and_add(&scheme->kobj,
			&damon_sysfs_scheme_ktype, &schemes->kobj,
			"%d", i);
	if (err)
		goto out;
	err = damon_sysfs_scheme_add_dirs(scheme);
	if (err)
		goto out;

	schemes_arr[i] = scheme;
	schemes->nr++;
}

return 0;

out:

	damon_sysfs_schemes_rm_dirs(schemes);
	kobject_put(&scheme->kobj);
	return err;

If damon_sysfs_scheme_add_dirs() fails after a successful kobject_init_and_add(), we jump to out and call kobject_put() without first calling kobject_del().

With CONFIG_DEBUG_KOBJECT_RELEASE enabled, could the persisting parent schemes directory cause an -EEXIST conflict if userspace retries updating nr_schemes?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

mm/damon/sysfs-schemes.c void damon_sysfs_schemes_rm_dirs(struct damon_sysfs_schemes *schemes) · line 2692

[Severity: High]
This isn't a bug introduced by this patch, but it appears a similar issue might still exist in damon_sysfs_schemes_add_dirs() on the error path.

While subsequent patches in this series successfully fixed this exact pattern for other arrays, it seems damon_sysfs_schemes_add_dirs() might have been missed.

If kobject_init_and_add() succeeds for a new scheme, but child directory creation via damon_sysfs_scheme_add_dirs() fails, the code branches to the out label:

mm/damon/sysfs-schemes.c:damon_sysfs_schemes_add_dirs() {
    ...
		err = kobject_init_and_add(&scheme->kobj,
				&damon_sysfs_scheme_ktype, &schemes->kobj,
				"%d", i);
		if (err)
			goto out;
		err = damon_sysfs_scheme_add_dirs(scheme);
		if (err)
			goto out;
    ...
out:
	damon_sysfs_schemes_rm_dirs(schemes);
	kobject_put(&scheme->kobj);
	return err;
}

Can this lead to the scheme directory lingering in sysfs because kobject_del() is missing before kobject_put()?

If CONFIG_DEBUG_KOBJECT_RELEASE is enabled, kobject_cleanup() is delayed. Since the parent persistent schemes directory survives, could a subsequent user write to resize the array attempt to recreate a directory with the same numerical ID and result in an -EEXIST conflict?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3

mm/damon/sysfs-schemes.c static void damon_sysfs_scheme_regions_rm_dirs( · line 335

[Severity: High]
This is a pre-existing issue, but since damos_sysfs_region_rm_dirs() calls damos_sysfs_probes_rm_dirs(), does damos_sysfs_probes_rm_dirs() also need to explicitly call kobject_del() to avoid the same name conflict regressions?

Looking at damos_sysfs_probes_rm_dirs():

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

If kobject_del() isn't called here prior to kobject_put(), won't delayed releases under CONFIG_DEBUG_KOBJECT_RELEASE collide with immediate recreations, leading to -EEXIST failures when retrying probe creation?

[Severity: High]
This isn't a bug introduced by this patch, but does this code also miss kobject_del() in the error paths after successful kobject_init_and_add() calls?

In the error paths for directory creation, if kobject_init_and_add() succeeds but subsequent setup steps fail, should the error rollback explicitly call kobject_del() before kobject_put() to unlink the directory?

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;
}

mm/damon/sysfs-schemes.c:damon_sysfs_schemes_add_dirs() {
	...
		err = kobject_init_and_add(&scheme->kobj,
				&damon_sysfs_scheme_ktype, &schemes->kobj,
				"%d", i);
		if (err)
			goto out;
		err = damon_sysfs_scheme_add_dirs(scheme);
		if (err)
			goto out;
	...
out:
	damon_sysfs_schemes_rm_dirs(schemes);
	kobject_put(&scheme->kobj);
	return err;
}

If kobject_del() is omitted in these error paths, could the directory remain anchored in sysfs, causing -EEXIST name conflicts on retry because the array count was not incremented?

mm/damon/sysfs-schemes.c static void damon_sysfs_scheme_filters_rm_dirs( · line 920

[Severity: High]
This is a pre-existing issue, but while we are adding kobject_del() here to prevent name conflicts, are there other locations that need the same fix?

For instance, in damos_sysfs_probes_rm_dirs(), the code iterates over probes_arr and calls kobject_put() without a preceding kobject_del():

	for (i = 0; i < probes->nr; i++)
		kobject_put(&probes_arr[i]->kobj);

Additionally, in the error path of damon_sysfs_schemes_add_dirs(), if damon_sysfs_scheme_add_dirs() fails after a successful kobject_init_and_add(), the cleanup jumps to the out label:

out:

	damon_sysfs_schemes_rm_dirs(schemes);
	kobject_put(&scheme->kobj);
	return err;

Could these missing kobject_del() calls leave sysfs directories pending delayed release, allowing the same name conflict crashes to occur?

mm/damon/sysfs-schemes.c static void damos_sysfs_quota_goals_rm_dirs( · line 1474

[Severity: High]
This isn't a bug introduced by this patch, but it appears the same missing kobject_del() issue exists in damon_sysfs_schemes_add_dirs().

When damon_sysfs_schemes_add_dirs() fails after successfully calling kobject_init_and_add() for a new scheme, the error path drops the kobject reference using kobject_put(), but fails to call kobject_del():

mm/damon/sysfs-schemes.c:damon_sysfs_schemes_add_dirs() {
    ...
		err = kobject_init_and_add(&scheme->kobj,
				&damon_sysfs_scheme_ktype, &schemes->kobj,
				"%d", i);
		if (err)
			goto out;
		err = damon_sysfs_scheme_add_dirs(scheme);
		if (err)
			goto out;
    ...
out:
	damon_sysfs_schemes_rm_dirs(schemes);
	kobject_put(&scheme->kobj);
	return err;
}

Under CONFIG_DEBUG_KOBJECT_RELEASE, the release of the kobject is delayed. Because the parent kobject is persistent, the scheme directory remains linked in the sysfs tree until the delay expires.

If a user subsequently retries writing to nr_schemes, could the kernel attempt to create a new scheme kobject with the same name under the same parent, triggering a -EEXIST name collision?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6

mm/damon/sysfs-schemes.c static void damos_sysfs_dests_rm_dirs( · line 2157

[Severity: High]
This isn't a bug introduced by this patch, but does the out: error path in damon_sysfs_schemes_add_dirs() have a similar issue?

If kobject_init_and_add() succeeds for a new scheme but the subsequent damon_sysfs_scheme_add_dirs() call fails, it jumps to the out: label:

mm/damon/sysfs-schemes.c:damon_sysfs_schemes_add_dirs() {
...

		err = kobject_init_and_add(&scheme->kobj,
				&damon_sysfs_scheme_ktype, &schemes->kobj,
				"%d", i);
		if (err)
			goto out;
		err = damon_sysfs_scheme_add_dirs(scheme);
		if (err)
			goto out;
...
out:
	damon_sysfs_schemes_rm_dirs(schemes);
	kobject_put(&scheme->kobj);
	return err;
}

Since the failed scheme hasn't been added to schemes_arr yet, damon_sysfs_schemes_rm_dirs() only cleans up the previously added schemes. Then kobject_put() is called on the current scheme without a prior kobject_del().

With CONFIG_DEBUG_KOBJECT_RELEASE enabled, could this omission leave the newly created sysfs directory intact and cause a name collision (-EEXIST) if userspace retries creating the schemes shortly after?

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7

mm/damon/sysfs.c static void damon_sysfs_probes_rm_dirs( · line 1143

[Severity: Medium]
This is a pre-existing issue, but does damon_sysfs_schemes_add_dirs() in mm/damon/sysfs-schemes.c need a similar fix?

If kobject_init_and_add(&scheme->kobj) succeeds but the subsequent damon_sysfs_scheme_add_dirs(scheme) fails, the error path drops the reference without synchronously unregistering the kobject:

mm/damon/sysfs-schemes.c:damon_sysfs_schemes_add_dirs() {
        ...
out:
        damon_sysfs_schemes_rm_dirs(schemes);
        kobject_put(&scheme->kobj);
        return err;
}

Without an explicit kobject_del() here, the cleanup is deferred. If CONFIG_DEBUG_KOBJECT_RELEASE is enabled, could retrying the operation fail with -EEXIST due to a name collision?

mm/damon/sysfs.c static int damon_sysfs_probes_add_dirs( · line 1186

[Severity: High]
This is a pre-existing issue, but does damon_sysfs_schemes_add_dirs() in mm/damon/sysfs-schemes.c also need a similar fix?

If kobject_init_and_add() succeeds but the subsequent damon_sysfs_scheme_add_dirs() fails (e.g., due to -ENOMEM), the error path jumps to the out label:

out:

	damon_sysfs_schemes_rm_dirs(schemes);
	kobject_put(&scheme->kobj);
	return err;

At this point, damon_sysfs_schemes_rm_dirs() only cleans up previously fully initialized schemes. The current scheme is not yet part of the array, so kobject_del() is never called for it.

When CONFIG_DEBUG_KOBJECT_RELEASE is enabled, kobject_put() delays memory cleanup. Because kobject_del() wasn't called to unlink it from sysfs, the lingering kobject can cause a name collision on any subsequent attempt to write to nr_schemes, preventing further scheme creation until the delayed release occurs.

Should we add kobject_del(&scheme->kobj) in that error path as well?

mm/damon/sysfs-schemes.c void damos_sysfs_populate_region_dir(struct damon_sysfs_schemes *sysfs_schemes, · line 3148

[Severity: High]
This isn't a bug introduced by this patch, but does the exact same problem exist in damon_sysfs_schemes_add_dirs() when damon_sysfs_scheme_add_dirs() fails?

mm/damon/sysfs-schemes.c:damon_sysfs_schemes_add_dirs():

	for (i = 0; i < nr_schemes; i++) {
		...
		err = kobject_init_and_add(&scheme->kobj,
				&damon_sysfs_scheme_ktype, &schemes->kobj,
				"%d", i);
		if (err)
			goto out;
		err = damon_sysfs_scheme_add_dirs(scheme);
		if (err)
			goto out;

		schemes_arr[i] = scheme;
		schemes->nr++;
	}
	return 0;

out:

	damon_sysfs_schemes_rm_dirs(schemes);
	kobject_put(&scheme->kobj);
	return err;

If damon_sysfs_scheme_add_dirs() fails, the code jumps to the out: label and skips calling kobject_del(&scheme->kobj) for the scheme that successfully ran kobject_init_and_add().

Under CONFIG_DEBUG_KOBJECT_RELEASE, this delayed kobject_release() could lead to a directory name conflict (e.g. "0") if userspace retries writing to nr_schemes immediately.

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11