Re: [PATCH v1] hfsplus: fix memory leak on mount failure

Viacheslav Dubeyko
2025-12-05T20:02:47Z

On Sat, 2025-12-06 at 00:09 +0000, Swaraj Gaikwad wrote:
> syzbot reported a memory leak in the hfsplus mount path when the
> mount
> fails, which occurs because the fs_context API moves ownership of
> fc->s_fs_info to sb->s_fs_info early in sget_fc().
>
> When filesystems are mounted using the new API, the VFS (specifically
> sget_fc) transfers the ownership of the context's s_fs_info (the
> 'sbi'
> struct) to the superblock (sb->s_fs_info) and clears the context
> pointer.
>
> If the mount fails after this transfer the VFS calls
> deactivate_locked_super, which invokes the filesystem's kill_sb
> callback. Previously, hfsplus used the generic kill_block_super,
> which
> does not free sb->s_fs_info, resulting in the 'sbi' structure and its
> loaded NLS tables being leaked.
>
> Fix this by implementing a filesystem-specific ->kill_sb() that frees
> sb->s_fs_info and its NLS resources before calling
> kill_block_super().
> Also remove the early kfree(sbi) from hfsplus_fill_super()’s error
> path,
> because the superblock unconditionally owns s_fs_info when using the
> fs_context API.
>
> Testing:
> This fix was verified by building the kernel with the .config
> provided
> by the syzkaller reporter and running the reproducer. The reproducer
> now runs successfully without triggering any memory leaks or kernel
> errors.
>
> #syz test:
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> e69c7c175115
>
> Reported-by: syzbot+99f6ed51479b86ac4c41@syzkaller.appspotmail.com
> Signed-off-by: Swaraj Gaikwad <swarajgaikwad1925@gmail.com>
> ---
>  fs/hfsplus/super.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
> index 16bc4abc67e0..fa7420d08da1 100644
> --- a/fs/hfsplus/super.c
> +++ b/fs/hfsplus/super.c
> @@ -629,7 +629,6 @@ static int hfsplus_fill_super(struct super_block
> *sb, struct fs_context *fc)
>  out_unload_nls:
>   unload_nls(sbi->nls);
>   unload_nls(nls);
> - kfree(sbi);
>   return err;
>  }
>
> @@ -688,10 +687,23 @@ static int hfsplus_init_fs_context(struct
> fs_context *fc)
>   return 0;
>  }
>
> +static void hfsplus_kill_sb(struct super_block *sb)
> +{
> +    struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
> +
> +    if (sbi) {
> +        unload_nls(sbi->nls);
> +        kfree(sbi);
> +        sb->s_fs_info = NULL;
> +    }
> +
> +    kill_block_super(sb);
> +}
> +
>  static struct file_system_type hfsplus_fs_type = {
>   .owner = THIS_MODULE,
>   .name = "hfsplus",
> - .kill_sb = kill_block_super,
> + .kill_sb = hfsplus_kill_sb,
>   .fs_flags = FS_REQUIRES_DEV,
>   .init_fs_context = hfsplus_init_fs_context,
>  };
>
> base-commit: 6bda50f4333fa61c07f04f790fdd4e2c9f4ca610
> --
> 2.52.0

Sorry, but this patch [1] already fixes the issue.

Thanks,
Slava.

[1]