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://syzkaller.appspot.com/bug?extid=99f6ed51479b86ac4c41 Signed-off-by: Shardul Bankar --- v1: - tried to fix the leak in fs/super.c (VFS layer). - Link: https://lore.kernel.org/all/20260201082724.GC3183987@ZenIV/ 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, }; -- 2.34.1