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@xxxxxxx> 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@xxxxxxxxx>
> > ---
> >  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