Re: [PATCH] gfs2: fix memory leaks in gfs2_fill_super error path

Hello Deepanshu,

thanks for this patch; see below.

On Sat, Jan 31, 2026 at 7:25 AM Deepanshu Kartikey
<[email protected]> wrote:
> Fix two memory leaks in the gfs2_fill_super() error handling path when
> transitioning a filesystem to read-write mode fails.
>
> First leak: kthread objects (thread_struct, task_struct, etc.)
> When gfs2_freeze_lock_shared() fails after init_threads() succeeds,
> the created kernel threads (logd and quotad) are never destroyed.
> This occurs because the fail_per_node label doesn't call
> gfs2_destroy_threads().
>
> Second leak: quota bitmap buffer (8192 bytes)
> When gfs2_make_fs_rw() fails after gfs2_quota_init() succeeds but
> before other operations complete, the allocated quota bitmap is never
> freed. The error path destroyed threads but didn't cleanup quota
> structures.
>
> The fix consolidates thread cleanup at the fail_per_node label for all
> error paths, which is safe because gfs2_destroy_threads() checks for
> NULL pointers before calling kthread_stop_put(). Quota cleanup is added
> specifically to the gfs2_make_fs_rw() error path where quota structures
> were initialized.
>
> Syzbot detected these leaks with the following signatures:
>
> Thread leak (PATH 3: gfs2_freeze_lock_shared failure):
>   unreferenced object 0xffff88801d7bca80 (size 4480):
>     copy_process+0x3a1/0x4670 kernel/fork.c:2422
>     kernel_clone+0xf3/0x6e0 kernel/fork.c:2779
>     kthread_create_on_node+0x100/0x150 kernel/kthread.c:478
>     init_threads+0xab/0x350 fs/gfs2/ops_fstype.c:611
>     gfs2_fill_super+0xe5c/0x1240 fs/gfs2/ops_fstype.c:1265
>
> Quota leak (PATH 4: gfs2_make_fs_rw failure):
>   unreferenced object 0xffff88812de7c000 (size 8192):
>     gfs2_quota_init+0xe5/0x820 fs/gfs2/quota.c:1409
>     gfs2_make_fs_rw+0x7a/0xe0 fs/gfs2/super.c:149
>     gfs2_fill_super+0xfbb/0x1240 fs/gfs2/ops_fstype.c:1275
>
> Reported-by: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=aac438d7a1c44071e04b
> Signed-off-by: Deepanshu Kartikey <[email protected]>
> ---
>  fs/gfs2/ops_fstype.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index e7a88b717991..fdc70189e4f1 100644
> --- a/fs/gfs2/ops_fstype.c
> +++ b/fs/gfs2/ops_fstype.c
> @@ -1276,7 +1276,7 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
>
>         if (error) {
>                 gfs2_freeze_unlock(sdp);
> -               gfs2_destroy_threads(sdp);
> +               gfs2_quota_cleanup(sdp);

This isn't pretty. Can it be replaced by the following?

--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -147,8 +147,10 @@ int gfs2_make_fs_rw(struct gfs2_sbd *sdp)
        }

        error = gfs2_quota_init(sdp);
-       if (!error && gfs2_withdrawn(sdp))
+       if (!error && gfs2_withdrawn(sdp)) {
+               gfs2_quota_cleanup(sdp);
                error = -EIO;
+       }
        if (!error)
                set_bit(SDF_JOURNAL_LIVE, &sdp->sd_flags);
        return error;

>                 fs_err(sdp, "can't make FS RW: %d\n", error);
>                 goto fail_per_node;
>         }
> @@ -1286,6 +1286,8 @@ static int gfs2_fill_super(struct super_block *sb, struct fs_context *fc)
>
>  fail_per_node:
>         init_per_node(sdp, UNDO);
> +       if (!sb_rdonly(sb))
> +               gfs2_destroy_threads(sdp);

gfs2_destroy_threads() can be called unconditionally here.

>  fail_inodes:
>         init_inodes(sdp, UNDO);
>  fail_sb:
> --
> 2.43.0
>

Thanks,
Andreas