[PATCH v2] btrfs: fix zoned sub_group leak on space_info sysfs failure

[Guangshuo Li]

On zoned filesystems, create_space_info() creates space_info sub_groups
for the primary DATA and METADATA space infos.

Before this change, the sub_group was created before registering the
parent space_info in sysfs. If the sub_group registration succeeded but
the parent btrfs_sysfs_add_space_info_type() call failed, the parent
space_info was released through its kobject release path while the
already published sub_group was left behind. The sub_group also kept its
logical parent pointer, which then pointed at freed memory.

Fix this by making btrfs_sysfs_remove_space_info() responsible for
removing any sub_groups before removing the space_info itself. Then
register the parent space_info before creating zoned sub_groups. With
that order, a parent registration failure has no sub_groups to unwind;
if sub_group creation fails, btrfs_sysfs_remove_space_info() can tear
down the parent and any sub_groups created so far.

This also keeps check_removing_space_info() as a pure checker by moving
sub_group removal into the common sysfs removal helper.

Fixes: f92ee31e031c7 ("btrfs: introduce btrfs_space_info sub-group")
Suggested-by: Naohiro Aota <naohiro.aota@xxxxxxx>
Signed-off-by: Guangshuo Li <lgs201920130244@xxxxxxxxx>
---
v2:
  - Make btrfs_sysfs_remove_space_info() remove sub_groups before removing
    the space_info itself, as suggested by Naohiro.
  - Register the parent space_info before creating zoned sub_groups in
    create_space_info().
  - Use btrfs_sysfs_remove_space_info() to unwind the parent when sub_group
    creation fails.
  - Remove sub_group removal from check_removing_space_info(), keeping it as
    a pure checker.
  - Use ARRAY_SIZE(space_info->sub_group) instead of
    BTRFS_SPACE_INFO_SUB_GROUP_MAX.

 fs/btrfs/block-group.c |  5 +----
 fs/btrfs/space-info.c  | 14 +++++++-------
 fs/btrfs/sysfs.c       | 16 ++++++++++++++++
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index b611c64119db..ad8e0a27101a 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -4615,11 +4615,8 @@ static void check_removing_space_info(struct btrfs_space_info *space_info)
 	if (space_info->subgroup_id == BTRFS_SUB_GROUP_PRIMARY) {
 		/* This is a top space_info, proceed with its children first. */
 		for (int i = 0; i < BTRFS_SPACE_INFO_SUB_GROUP_MAX; i++) {
-			if (space_info->sub_group[i]) {
+			if (space_info->sub_group[i])
 				check_removing_space_info(space_info->sub_group[i]);
-				btrfs_sysfs_remove_space_info(space_info->sub_group[i]);
-				space_info->sub_group[i] = NULL;
-			}
 		}
 	}
 
diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index f0436eea1544..42179d89e52b 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -304,6 +304,10 @@ static int create_space_info(struct btrfs_fs_info *info, u64 flags)
 
 	init_space_info(info, space_info, flags);
 
+	ret = btrfs_sysfs_add_space_info_type(space_info);
+	if (ret)
+		return ret;
+
 	if (btrfs_is_zoned(info)) {
 		if (flags & BTRFS_BLOCK_GROUP_DATA)
 			ret = create_space_info_sub_group(space_info, flags,
@@ -315,21 +319,17 @@ static int create_space_info(struct btrfs_fs_info *info, u64 flags)
 							  0);
 
 		if (ret)
-			goto out_free;
+			goto out_remove;
 	}
 
-	ret = btrfs_sysfs_add_space_info_type(space_info);
-	if (ret)
-		return ret;
-
 	list_add(&space_info->list, &info->space_info);
 	if (flags & BTRFS_BLOCK_GROUP_DATA)
 		info->data_sinfo = space_info;
 
 	return ret;
 
-out_free:
-	kfree(space_info);
+out_remove:
+	btrfs_sysfs_remove_space_info(space_info);
 	return ret;
 }
 
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 0d14570c8bc2..ce1109e47cc7 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -1885,6 +1885,22 @@ void btrfs_sysfs_remove_space_info(struct btrfs_space_info *space_info)
 {
 	int i;
 
+	for (i = 0; i < ARRAY_SIZE(space_info->sub_group); i++) {
+		struct btrfs_space_info *sub_group;
+
+		sub_group = space_info->sub_group[i];
+		if (!sub_group)
+			continue;
+
+		/*
+		 * Remove sub-groups before removing their parent space_info.
+		 * Clear the pointer first so recursive removal cannot leave a
+		 * stale child pointer behind.
+		 */
+		space_info->sub_group[i] = NULL;
+		btrfs_sysfs_remove_space_info(sub_group);
+	}
+
 	for (i = 0; i < BTRFS_NR_RAID_TYPES; i++) {
 		struct kobject *kobj;
 
-- 
2.43.0