Skip to contentSkip to conversation

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

PATCH Applied SJ Park 1 participant 12 messages Jun 28

Subsystems (3)
lore
Share & tools
lore

SJ Park

author maintainer
#0 lore raw

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

DAMON sysfs interface allows users to create and remove arbitrary number of directories on sysfs, using a few files having 'nr_' prefix. For example, 'nr_kdamonds'. When the user writes a number 'N' to the files, directories having name starting from '0' to 'N - 1' are created in the same directory. The pre-existing number-named directories are removed before creating the new directories.

For the removal of the existing directories, DAMON sysfs interface use only kobject_put(). Because DAMON sysfs interface is the only kernel component that manages the directories, there is no problem in normal situations. However, if CONFIG_DEBUG_KOBJECT_RELEASE is enabled, the removal of dirs are delayed. Let's suppose a user writes a non-zero number to the 'nr_*' files while there are pre-existing number-named directories, on the config enabled kernel. DAMON sysfs interface decreases the reference counts of the existing directories and immediately creates new directories. Because the removal of the sysfs directories is delayed, it shows some pre-existing directories of the same names when it tries to create the new directories, and fails.

For example, the issue can be triggered like below:

    # grep DEBUG_KOBJECT_RELEASE /boot/config-$(uname -r)
    CONFIG_DEBUG_KOBJECT_RELEASE=y
    # ls
    nr_kdamonds
    # echo 1 > nr_kdamonds
    # echo 1 > nr_kdamonds
    bash: echo: write error: File exists
    # dmesg
    [...]
    [  300.880458] kobject: kobject_add_internal failed for 0 with -EEXIST, don't try to register things with the same name in the same directory.
    [...]

Some of the error handling paths of the directories also lack the kobject_del() call. If the user uses nr_* file right after the errors, similar issues can happen.

This doesn't cause catastrophic issues like kernel panics or memory corruptions. Users can work around by removing all directories first (write 0 to the nr_* files) and then create new directories after confirming the old directories are gone. But, this is definitely a bug that causes a bad user experience.

Fix the issues by calling kobject_del() before creating new directories.

Patches Sequence
================

There are a number of bugs of this class that are introduced by eleven different commits. The fixes are grouped and ordered for the introducing commits.

Changes from RFC v1.2
- RFC v1.2: https://lore.kernel.org/[email protected]
- Drop RFC tag.
- Rebase to latest mm-new.
Changes from RFC v1.1
- RFC v1.1: https://lore.kernel.org/[email protected]
- Add error path fix for damon_sysfs_schemes_add_dirs().
- Rebase to latest mm-new.
Changes from RFC v1
- RFC v1: https://lore.kernel.org/[email protected]
- Add error path fixes.

SJ Park (11):
mm/damon/sysfs: kobject_del() target (normal), context and kdamond
dirs
mm/damon/sysfs: kobject_del() region and target (error) dirs mm/damon/sysfs-schemes: kobject_del() scheme dirs mm/damon/sysfs-schemes: kobject_del() scheme region dirs mm/damon/sysfs-schemes: kobject_del() scheme filter dirs mm/damon/sysfs-schemes: kobject_del() scheme quota goal dirs mm/damon/sysfs-schemes: kobject_del() scheme action destination dirs mm/damon/sysfs: kobject_del() probe dirs
mm/damon/sysfs: kobject_del() probe filter dirs mm/damon/sysfs: kobject_del() probe dirs in probes_addd_dir error path mm/damon/sysfs-schemes: kobject_del() region for populate_region error

 mm/damon/sysfs-schemes.c | 22 +++++++++++++++++-----
 mm/damon/sysfs.c         | 25 ++++++++++++++++++++-----
 2 files changed, 37 insertions(+), 10 deletions(-)

base-commit: d1f5e472273baa030bf7a4b6063aa67053e4287d

Signature · 1 line
2.47.3
author maintainer
#1 lore raw

PATCH 1/11 mm/damon/sysfs: kobject_del() target (normal), 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 normal creation paths of target, context and kdamond directories, and error paths of context and kdamond directories by adding kobject_del() calls.

Note that this fix for target directories is not complete since it has a similar issue in the damon_sysfs_targets_add_dirs() error path. Because the normal path issue and the error path issue are introduced by different commits, this commit is fixing only the normal path issue. A commit for the error path will be added next.

Fixes c951cd3b8901 ("mm/damon: implement a minimal stub for sysfs-based DAMON interface")
Cc <[email protected]> # 5.18.x
---
 mm/damon/sysfs.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
mm/damon/sysfs.c +9 -2
index a9e187158067d..38f3b02481f0a 100644
@@ -331,6 +331,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);
 		kobject_put(&targets_arr[i]->kobj);
 	}
 	targets->nr = 0;
@@ -1640,6 +1641,7 @@ static void damon_sysfs_contexts_rm_dirs(struct damon_sysfs_contexts *contexts)
 
 	for (i = 0; i < contexts->nr; i++) {
 		damon_sysfs_context_rm_dirs(contexts_arr[i]);
+		kobject_del(&contexts_arr[i]->kobj);
 		kobject_put(&contexts_arr[i]->kobj);
 	}
 	contexts->nr = 0;
@@ -1678,13 +1680,15 @@ static int damon_sysfs_contexts_add_dirs(struct damon_sysfs_contexts *contexts,
 
 		err = damon_sysfs_context_add_dirs(context);
 		if (err)
-			goto out;
+			goto del_out;
 
 		contexts_arr[i] = context;
 		contexts->nr++;
 	}
 	return 0;
 
+del_out:
+	kobject_del(&context->kobj);
 out:
 	damon_sysfs_contexts_rm_dirs(contexts);
 	kobject_put(&context->kobj);
@@ -2499,6 +2503,7 @@ 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_del(&kdamonds_arr[i]->kobj);
 		kobject_put(&kdamonds_arr[i]->kobj);
 	}
 	kdamonds->nr = 0;
@@ -2553,13 +2558,15 @@ static int damon_sysfs_kdamonds_add_dirs(struct damon_sysfs_kdamonds *kdamonds,
 
 		err = damon_sysfs_kdamond_add_dirs(kdamond);
 		if (err)
-			goto out;
+			goto del_out;
 
 		kdamonds_arr[i] = kdamond;
 		kdamonds->nr++;
 	}
 	return 0;
 
+del_out:
+	kobject_del(&kdamond->kobj);
 out:
 	damon_sysfs_kdamonds_rm_dirs(kdamonds);
 	kobject_put(&kdamond->kobj);
Signature · 1 line
2.47.3
author maintainer
#2 lore raw

PATCH 2/11 mm/damon/sysfs: kobject_del() region and target (error) dirs

In reply to 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 the normal creation path of region directories and the error path of target directories, by adding kobject_del() calls.

Fixes 2031b14ea757 ("mm/damon/sysfs: support the physical address space monitoring")
Cc <[email protected]> # 5.18.x
---
 mm/damon/sysfs.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
mm/damon/sysfs.c +6 -2
index 38f3b02481f0a..204aed6a3e5da 100644
@@ -105,8 +105,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;
@@ -370,13 +372,15 @@ static int damon_sysfs_targets_add_dirs(struct damon_sysfs_targets *targets,
 
 		err = damon_sysfs_target_add_dirs(target);
 		if (err)
-			goto out;
+			goto del_out;
 
 		targets_arr[i] = target;
 		targets->nr++;
 	}
 	return 0;
 
+del_out:
+	kobject_del(&target->kobj);
 out:
 	damon_sysfs_targets_rm_dirs(targets);
 	kobject_put(&target->kobj);
Signature · 1 line
2.47.3
author maintainer
#3 lore raw

PATCH 3/11 mm/damon/sysfs-schemes: kobject_del() scheme dirs

In reply to 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.

Fixes 7e84b1f8212a ("mm/damon/sysfs: support DAMON-based Operation Schemes")
Cc <[email protected]> # 5.18.x
---
 mm/damon/sysfs-schemes.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
mm/damon/sysfs-schemes.c +4 -1
index 3cbeccd436e40..db496d2e493a4 100644
@@ -2681,6 +2681,7 @@ 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;
@@ -2722,13 +2723,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);
Signature · 1 line
2.47.3
author maintainer
#4 lore raw

PATCH 4/11 mm/damon/sysfs-schemes: kobject_del() scheme region dirs

In reply to 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 region directories by adding kobject_del() calls.

This issue was discovered [1] by Sashiko, though its analysis was partially incorrect.

[1] https://lore.kernel.org/[email protected]

Fixes 9277d0367ba1 ("mm/damon/sysfs-schemes: implement scheme region directory")
Cc <[email protected]> # 6.2.x
---
 mm/damon/sysfs-schemes.c | 1 +
 1 file changed, 1 insertion(+)
mm/damon/sysfs-schemes.c +1 -0
index db496d2e493a4..9eb28fe77b5b0 100644
@@ -332,6 +332,7 @@ static void damon_sysfs_scheme_regions_rm_dirs(
 	list_for_each_entry_safe(r, next, &regions->regions_list, list) {
 		damos_sysfs_region_rm_dirs(r);
 		list_del(&r->list);
+		kobject_del(&r->kobj);
 		kobject_put(&r->kobj);
 		regions->nr_regions--;
 	}
Signature · 1 line
2.47.3
author maintainer
#5 lore raw

PATCH 5/11 mm/damon/sysfs-schemes: kobject_del() scheme filter dirs

In reply to 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 filter directories by adding kobject_del() calls.

Fixes 472e2b70eda6 ("mm/damon/sysfs-schemes: connect filter directory and filters directory")
Cc <[email protected]> # 6.3.x
---
 mm/damon/sysfs-schemes.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
mm/damon/sysfs-schemes.c +3 -1
index 9eb28fe77b5b0..e955fb916a7e2 100644
@@ -912,8 +912,10 @@ static void damon_sysfs_scheme_filters_rm_dirs(
 	struct damon_sysfs_scheme_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;
Signature · 1 line
2.47.3
author maintainer
#6 lore raw

PATCH 6/11 mm/damon/sysfs-schemes: kobject_del() scheme quota goal dirs

In reply to 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 quota goal directories by adding kobject_del() calls.

Fixes 7f262da0a30d ("mm/damon/sysfs-schemes: implement files for scheme quota goals setup")
Cc <[email protected]> # 6.8.x
---
 mm/damon/sysfs-schemes.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
mm/damon/sysfs-schemes.c +3 -1
index e955fb916a7e2..58051185713cf 100644
@@ -1463,8 +1463,10 @@ static void damos_sysfs_quota_goals_rm_dirs(
 	struct damos_sysfs_quota_goal **goals_arr = goals->goals_arr;
 	int i;
 
-	for (i = 0; i < goals->nr; i++)
+	for (i = 0; i < goals->nr; i++) {
+		kobject_del(&goals_arr[i]->kobj);
 		kobject_put(&goals_arr[i]->kobj);
+	}
 	goals->nr = 0;
 	kfree(goals_arr);
 	goals->goals_arr = NULL;
Signature · 1 line
2.47.3
author maintainer
#7 lore raw

PATCH 7/11 mm/damon/sysfs-schemes: kobject_del() scheme action destination dirs

In reply to 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 action destination directories by adding kobject_del() calls.

Fixes 2cd0bf85a203 ("mm/damon/sysfs-schemes: implement DAMOS action destinations directory")
Cc <[email protected]> # 6.17.x
---
 mm/damon/sysfs-schemes.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
mm/damon/sysfs-schemes.c +3 -1
index 58051185713cf..e2c8716be6c9c 100644
@@ -2143,8 +2143,10 @@ static void damos_sysfs_dests_rm_dirs(
 	struct damos_sysfs_dest **dests_arr = dests->dests_arr;
 	int i;
 
-	for (i = 0; i < dests->nr; i++)
+	for (i = 0; i < dests->nr; i++) {
+		kobject_del(&dests_arr[i]->kobj);
 		kobject_put(&dests_arr[i]->kobj);
+	}
 	dests->nr = 0;
 	kfree(dests_arr);
 	dests->dests_arr = NULL;
Signature · 1 line
2.47.3
author maintainer
#8 lore raw

PATCH 8/11 mm/damon/sysfs: kobject_del() probe dirs

In reply to 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 data attribute probe directories by adding kobject_del() calls.

Fixes bf3ea3d30880 ("mm/damon/sysfs: implement probe dir")
---
 mm/damon/sysfs.c | 1 +
 1 file changed, 1 insertion(+)
mm/damon/sysfs.c +1 -0
index 204aed6a3e5da..9f92ebdb28574 100644
@@ -1137,6 +1137,7 @@ static void damon_sysfs_probes_rm_dirs(
 
 	for (i = 0; i < probes->nr; i++) {
 		damon_sysfs_probe_rm_dirs(probes_arr[i]);
+		kobject_del(&probes_arr[i]->kobj);
 		kobject_put(&probes_arr[i]->kobj);
 	}
 	probes->nr = 0;
Signature · 1 line
2.47.3
author maintainer
#9 lore raw

PATCH 9/11 mm/damon/sysfs: kobject_del() probe filter dirs

In reply to 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 data attribute probe filter directories by adding kobject_del() calls.

Fixes 82e66aef7714 ("mm/damon/sysfs: implement filter dir")
---
 mm/damon/sysfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
mm/damon/sysfs.c +3 -1
index 9f92ebdb28574..0ccdc71275d52 100644
@@ -960,8 +960,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;
Signature · 1 line
2.47.3
author maintainer
#10 lore raw

PATCH 10/11 mm/damon/sysfs: kobject_del() probe dirs in probes_addd_dir error path

In reply to SJ Park

On CONFIG_DEBUG_KOBJECT_RELEASE enabled kernel, lack of kobject_del() could cause directories creation failures due to the name conflicts. Fix the issue for data attribute probe filter directories in the error handling path of damon_sysfs_probes_add_dirs() by adding a kobject_del() call.

Fixes af7cb41af9a9 ("mm/damon/sysfs: implement filters directory")
---
 mm/damon/sysfs.c | 1 +
 1 file changed, 1 insertion(+)
mm/damon/sysfs.c +1 -0
index 0ccdc71275d52..e3526a263e205 100644
@@ -1181,6 +1181,7 @@ static int damon_sysfs_probes_add_dirs(
 
 		err = damon_sysfs_probe_add_dirs(probe);
 		if (err) {
+			kobject_del(&probe->kobj);
 			kobject_put(&probe->kobj);
 			damon_sysfs_probes_rm_dirs(probes);
 			return err;
Signature · 1 line
2.47.3
author maintainer
#11 lore raw

PATCH 11/11 mm/damon/sysfs-schemes: kobject_del() region for populate_region error

In reply to SJ Park

On CONFIG_DEBUG_KOBJECT_RELEASE enabled kernel, lack of kobject_del() could cause directories creation failures due to the name conflicts. Fix the issue for tried region directories in the error handling path of damon_sysfs_populate_region_dir() by adding a kobject_del() call.

Fixes b574a82d10de ("mm/damon/sysfs-schemes: implement tried_regions/<r>/probes/")
---
 mm/damon/sysfs-schemes.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
mm/damon/sysfs-schemes.c +3 -1
index e2c8716be6c9c..41f93a1823bf1 100644
@@ -3132,12 +3132,14 @@ void damos_sysfs_populate_region_dir(struct damon_sysfs_schemes *sysfs_schemes,
 				sysfs_regions->nr_regions))
 		goto out;
 	if (damos_sysfs_region_add_dirs(region, ctx, r))
-		goto out;
+		goto del_out;
 
 	list_add_tail(&region->list, &sysfs_regions->regions_list);
 	sysfs_regions->nr_regions++;
 	return;
 
+del_out:
+	kobject_del(&region->kobj);
 out:
 	kobject_put(&region->kobj);
 }
Signature · 1 line
2.47.3