Messages in this thread Patch in this message | | | Date | Sun, 20 Nov 2022 11:29:32 +0800 | From | Qu Wenruo <> | Subject | Re: [PATCH] btrfs: fix a resource leak in btrfs_init_sysfs() |
| |
On 2022/11/20 11:11, Qu Wenruo wrote: > > > On 2022/11/19 14:53, Qu Wenruo wrote: >> >> >> On 2022/11/19 14:43, Zhen Lei wrote: >>> When btrfs_debug_feature_attr_group fails to be created, >>> btrfs_feature_attr_group is not removed. >>> >>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> >> >> Reviewed-by: Qu Wenruo <wqu@suse.com> > > Wait for a minute, should we call sysfs_unmerge_group() first before > calling sysfs_remove_group()? > > As the sysfs_remove_group() will only remove the > btrfs_feature_attr_group, and kset_unregister() will only free > btrfs_kset, without removing the added btrfs_static_feature_attr_group. > > I haven't yet find a function that will remove all children attrs in > just one go, or did I miss something?
Oh, indeed I missed something.
The following call chain would properly handle every child of a kobj:
kset_unregister() |- kobject_del() |- __kobject_del() |- sysfs_remove_dir() |- kernfs_remove() |- __kernfs_remove() |- do { kernfs_leftmost_descendant(); } while (pos != kn)
The final do {} while () loop will unlink each child kernel_node and free it.
This means, as long as we call kset_unregister(), we should be able to free every child of it.
So with or without this patch, the error handling should be fine.
Although personally speaking, I'd prefer a more explicit cleanup (with extra sysfs_unmerge_group() call) as going through the above call chain is not that straightforward.
Thanks, Qu
> > Thanks, > Qu >> >> Thanks, >> Qu >>> --- >>> fs/btrfs/sysfs.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c >>> index 699b54b3acaae0b..947125f2ceaaf96 100644 >>> --- a/fs/btrfs/sysfs.c >>> +++ b/fs/btrfs/sysfs.c >>> @@ -2322,7 +2322,7 @@ int __init btrfs_init_sysfs(void) >>> #ifdef CONFIG_BTRFS_DEBUG >>> ret = sysfs_create_group(&btrfs_kset->kobj, >>> &btrfs_debug_feature_attr_group); >>> if (ret) >>> - goto out2; >>> + goto out_remove_group; >>> #endif >>> return 0;
| |