Re: [PATCH v2] hfsplus: fix s_fs_info leak on mount setup failure

[Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>]

On Sun, 2026-02-01 at 18:42 +0530, Shardul Bankar wrote:
> Syzkaller reported a memory leak in hfsplus where s_fs_info (sbi) is
> allocated in hfsplus_init_fs_context() but never freed if the mount
> setup fails during setup_bdev_super().
> 
> In get_tree_bdev_flags(), if setup_bdev_super() fails, the superblock
> is torn down via deactivate_locked_super(). Since this failure occurs
> before fill_super() is called, the superblock's operations (sb->s_op)
> are not yet set. Consequently, the standard ->put_super() callback
> cannot be invoked, and the allocated s_fs_info remains leaked.
> 
> Fix this by implementing a custom ->kill_sb() handler,
> hfsplus_kill_sb(), which explicitly frees s_fs_info using RCU
> synchronization. This ensures cleanup happens regardless of whether
> fill_super() succeeded or ->put_super() was called.
> 
> To support this new lifecycle:
> 1. In hfsplus_put_super(), remove the call_rcu() call. The actual freeing
>    of s_fs_info is deferred to hfsplus_kill_sb().
> 2. In hfsplus_fill_super(), remove the explicit cleanup of sbi (both kfree
>    and unload_nls) in the error path. The VFS will call ->kill_sb() on
>    failure, so retaining these would result in double-frees or refcount
>    underflows.
> 3. Implement hfsplus_kill_sb() to invoke kill_block_super() and then free
>    s_fs_info via RCU.
> 
> Reported-by: syzbot+99f6ed51479b86ac4c41@syzkaller.appspotmail.com
> Closes: https://urldefense.proofpoint.com/v2/url?u=https-3A__syzkaller.appspot.com_bug-3Fextid-3D99f6ed51479b86ac4c41&d=DwIDAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=cdfmHveCNB06e-RCTCO9KaiHPPrhWiFs1cPzJzLjy18vWg3XFh8UctRvQeTTr3CH&s=d5VA34InahfL4dkSPDXzJOsXnmg7NO_SsZWTVUsd8wU&e= 
> Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
> ---
> v1:
>  - tried to fix the leak in fs/super.c (VFS layer).
>  - Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_20260201082724.GC3183987-40ZenIV_&d=DwIDAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=cdfmHveCNB06e-RCTCO9KaiHPPrhWiFs1cPzJzLjy18vWg3XFh8UctRvQeTTr3CH&s=q6W22LIrAIHCp7LSxAZI00f4z8FBpgFEYVnbQklH9KU&e= 
> v2:
>  - abandons the VFS changes in favor of a driver-specific fix in
>    hfsplus, implementing a custom ->kill_sb() to handle the cleanup
>    lifecycle, as suggested by Al Viro.
> 
>  fs/hfsplus/super.c | 30 ++++++++++++++++++------------
>  1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
> index aaffa9e060a0..cc80cd545a3e 100644
> --- a/fs/hfsplus/super.c
> +++ b/fs/hfsplus/super.c
> @@ -311,14 +311,6 @@ void hfsplus_mark_mdb_dirty(struct super_block *sb)
>  	spin_unlock(&sbi->work_lock);
>  }
>  
> -static void delayed_free(struct rcu_head *p)
> -{
> -	struct hfsplus_sb_info *sbi = container_of(p, struct hfsplus_sb_info, rcu);
> -
> -	unload_nls(sbi->nls);
> -	kfree(sbi);
> -}
> -
>  static void hfsplus_put_super(struct super_block *sb)
>  {
>  	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
> @@ -344,7 +336,6 @@ static void hfsplus_put_super(struct super_block *sb)
>  	hfs_btree_close(sbi->ext_tree);
>  	kfree(sbi->s_vhdr_buf);
>  	kfree(sbi->s_backup_vhdr_buf);
> -	call_rcu(&sbi->rcu, delayed_free);
>  
>  	hfs_dbg("finished\n");
>  }
> @@ -648,9 +639,7 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
>  	kfree(sbi->s_vhdr_buf);
>  	kfree(sbi->s_backup_vhdr_buf);
>  out_unload_nls:
> -	unload_nls(sbi->nls);
>  	unload_nls(nls);
> -	kfree(sbi);
>  	return err;
>  }
>  
> @@ -709,10 +698,27 @@ static int hfsplus_init_fs_context(struct fs_context *fc)
>  	return 0;
>  }
>  
> +static void delayed_free(struct rcu_head *p)
> +{
> +	struct hfsplus_sb_info *sbi = container_of(p, struct hfsplus_sb_info, rcu);
> +
> +	unload_nls(sbi->nls);
> +	kfree(sbi);
> +}
> +
> +static void hfsplus_kill_sb(struct super_block *sb)
> +{
> +	struct hfsplus_sb_info *sbi = sb->s_fs_info;
> +
> +	kill_block_super(sb);
> +	if (sbi)
> +		call_rcu(&sbi->rcu, delayed_free);
> +}
> +
>  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,
>  };

The patch [1] fixes the issue and it in HFS/HFS+ tree already.

Thanks,
Slava.

[1]
https://lore.kernel.org/linux-fsdevel/20251201222843.82310-3-mehdi.benhadjkhelifa@gmail.com/