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

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

[Guangshuo Li]

On zoned filesystems, create_space_info() creates a space_info sub_group
before registering the parent space_info in sysfs.

If the sub_group registration succeeds but the parent
btrfs_sysfs_add_space_info_type() call fails, the parent space_info is
released through its kobject release path, but the already published
sub_group is left behind. The sub_group also keeps its logical parent
pointer, which then points at freed memory.

Remember the created sub_group before registering the parent and remove it
when parent sysfs registration fails. Cache the sub_group pointer before
calling btrfs_sysfs_add_space_info_type(), as that helper may already have
put and released the parent on failure.

Fixes: f92ee31e031c7 ("btrfs: introduce btrfs_space_info sub-group")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 fs/btrfs/space-info.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index f0436eea1544..03659dfcb830 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -296,6 +296,7 @@ static int create_space_info(struct btrfs_fs_info *info, u64 flags)
 {
 
 	struct btrfs_space_info *space_info;
+	struct btrfs_space_info *sub_group = NULL;
 	int ret = 0;
 
 	space_info = kzalloc_obj(*space_info, GFP_NOFS);
@@ -316,11 +317,15 @@ static int create_space_info(struct btrfs_fs_info *info, u64 flags)
 
 		if (ret)
 			goto out_free;
+		sub_group = space_info->sub_group[0];
 	}
 
 	ret = btrfs_sysfs_add_space_info_type(space_info);
-	if (ret)
+	if (ret) {
+		if (sub_group)
+			btrfs_sysfs_remove_space_info(sub_group);
 		return ret;
+	}
 
 	list_add(&space_info->list, &info->space_info);
 	if (flags & BTRFS_BLOCK_GROUP_DATA)
-- 
2.43.0

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

[Naohiro Aota]

On Thu Jun 4, 2026 at 9:10 PM JST, Guangshuo Li wrote:
> On zoned filesystems, create_space_info() creates a space_info sub_group
> before registering the parent space_info in sysfs.
>
> If the sub_group registration succeeds but the parent
> btrfs_sysfs_add_space_info_type() call fails, the parent space_info is
> released through its kobject release path, but the already published
> sub_group is left behind. The sub_group also keeps its logical parent
> pointer, which then points at freed memory.
>
> Remember the created sub_group before registering the parent and remove it
> when parent sysfs registration fails. Cache the sub_group pointer before
> calling btrfs_sysfs_add_space_info_type(), as that helper may already have
> put and released the parent on failure.

Thank you for the patch. The logic itself is correct, but I think there
is a better alternative. My suggestion is making
btrfs_sysfs_remove_space_info() responsible for removing sub-groups (if
any) first.

With that change, we can reverse the space-info creation order in
create_space_info(). If the parent space-info registration fails, no
sub-groups exist yet, so nothing extra needs to be cleaned up. If
sub-group creation fails, we can call btrfs_sysfs_remove_space_info() on
the parent to clean everything up.

This change also has a side benefit. Currently,
check_removing_space_info() is also removing sub-groups, which
contradicts its name. We can remove those
btrfs_sysfs_remove_space_info() calls for sub-groups from the function,
making it a pure checker.

>
> Fixes: f92ee31e031c7 ("btrfs: introduce btrfs_space_info sub-group")
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  fs/btrfs/space-info.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
> index f0436eea1544..03659dfcb830 100644
> --- a/fs/btrfs/space-info.c
> +++ b/fs/btrfs/space-info.c
> @@ -296,6 +296,7 @@ static int create_space_info(struct btrfs_fs_info *info, u64 flags)
>  {
>  
>  	struct btrfs_space_info *space_info;
> +	struct btrfs_space_info *sub_group = NULL;
>  	int ret = 0;
>  
>  	space_info = kzalloc_obj(*space_info, GFP_NOFS);
> @@ -316,11 +317,15 @@ static int create_space_info(struct btrfs_fs_info *info, u64 flags)
>  
>  		if (ret)
>  			goto out_free;
> +		sub_group = space_info->sub_group[0];
>  	}
>  
>  	ret = btrfs_sysfs_add_space_info_type(space_info);
> -	if (ret)
> +	if (ret) {
> +		if (sub_group)
> +			btrfs_sysfs_remove_space_info(sub_group);
>  		return ret;
> +	}
>  
>  	list_add(&space_info->list, &info->space_info);
>  	if (flags & BTRFS_BLOCK_GROUP_DATA)

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

[Guangshuo Li]

Hi Naohiro,

Thank you for your review and suggestion.

On Fri, 5 Jun 2026 at 10:22, Naohiro Aota <Naohiro.Aota@wdc.com> wrote:
>
> On Thu Jun 4, 2026 at 9:10 PM JST, Guangshuo Li wrote:
> > On zoned filesystems, create_space_info() creates a space_info sub_group
> > before registering the parent space_info in sysfs.
> >
> > If the sub_group registration succeeds but the parent
> > btrfs_sysfs_add_space_info_type() call fails, the parent space_info is
> > released through its kobject release path, but the already published
> > sub_group is left behind. The sub_group also keeps its logical parent
> > pointer, which then points at freed memory.
> >
> > Remember the created sub_group before registering the parent and remove it
> > when parent sysfs registration fails. Cache the sub_group pointer before
> > calling btrfs_sysfs_add_space_info_type(), as that helper may already have
> > put and released the parent on failure.
>
> Thank you for the patch. The logic itself is correct, but I think there
> is a better alternative. My suggestion is making
> btrfs_sysfs_remove_space_info() responsible for removing sub-groups (if
> any) first.
>
> With that change, we can reverse the space-info creation order in
> create_space_info(). If the parent space-info registration fails, no
> sub-groups exist yet, so nothing extra needs to be cleaned up. If
> sub-group creation fails, we can call btrfs_sysfs_remove_space_info() on
> the parent to clean everything up.
>
> This change also has a side benefit. Currently,
> check_removing_space_info() is also removing sub-groups, which
> contradicts its name. We can remove those
> btrfs_sysfs_remove_space_info() calls for sub-groups from the function,
> making it a pure checker.
>
> >
> > Fixes: f92ee31e031c7 ("btrfs: introduce btrfs_space_info sub-group")
> > Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> > ---
> >  fs/btrfs/space-info.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
> > index f0436eea1544..03659dfcb830 100644
> > --- a/fs/btrfs/space-info.c
> > +++ b/fs/btrfs/space-info.c
> > @@ -296,6 +296,7 @@ static int create_space_info(struct btrfs_fs_info *info, u64 flags)
> >  {
> >
> >       struct btrfs_space_info *space_info;
> > +     struct btrfs_space_info *sub_group = NULL;
> >       int ret = 0;
> >
> >       space_info = kzalloc_obj(*space_info, GFP_NOFS);
> > @@ -316,11 +317,15 @@ static int create_space_info(struct btrfs_fs_info *info, u64 flags)
> >
> >               if (ret)
> >                       goto out_free;
> > +             sub_group = space_info->sub_group[0];
> >       }
> >
> >       ret = btrfs_sysfs_add_space_info_type(space_info);
> > -     if (ret)
> > +     if (ret) {
> > +             if (sub_group)
> > +                     btrfs_sysfs_remove_space_info(sub_group);
> >               return ret;
> > +     }
> >
> >       list_add(&space_info->list, &info->space_info);
> >       if (flags & BTRFS_BLOCK_GROUP_DATA)
>

I agree that making btrfs_sysfs_remove_space_info() responsible for
removing sub-groups first is a cleaner approach. It also makes sense to
register the parent space_info before creating the zoned sub-groups, so
the error paths can be handled by the common removal helper.

I will update the patch accordingly and send a v2.

Thanks,
Guangshuo