linux-kernel - Re: [PATCH v2] fs/hfs: fix s_fs_info leak on setup_bdev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251119-delfin-bioladen-6bf291941d4f@brauner>
Date: Wed, 19 Nov 2025 15:14:39 +0100
From: Christian Brauner <brauner@...nel.org>
To: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@...il.com>
Cc: slava@...eyko.com, glaubitz@...sik.fu-berlin.de, frank.li@...o.com, 
	jack@...e.cz, viro@...iv.linux.org.uk, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, skhan@...uxfoundation.org, david.hunter.linux@...il.com, 
	khalid@...nel.org, linux-kernel-mentees@...ts.linuxfoundation.org, 
	syzbot+ad45f827c88778ff7df6@...kaller.appspotmail.com
Subject: Re: [PATCH v2] fs/hfs: fix s_fs_info leak on setup_bdev_super()
 failure

On Wed, Nov 19, 2025 at 08:38:20AM +0100, Mehdi Ben Hadj Khelifa wrote:
> The regression introduced by commit aca740cecbe5 ("fs: open block device
> after superblock creation") allows setup_bdev_super() to fail after a new
> superblock has been allocated by sget_fc(), but before hfs_fill_super()
> takes ownership of the filesystem-specific s_fs_info data.
> 
> In that case, hfs_put_super() and the failure paths of hfs_fill_super()
> are never reached, leaving the HFS mdb structures attached to s->s_fs_info
> unreleased.The default kill_block_super() teardown also does not free 
> HFS-specific resources, resulting in a memory leak on early mount failure.
> 
> Fix this by moving all HFS-specific teardown (hfs_mdb_put()) from
> hfs_put_super() and the hfs_fill_super() failure path into a dedicated
> hfs_kill_sb() implementation. This ensures that both normal unmount and
> early teardown paths (including setup_bdev_super() failure) correctly
> release HFS metadata.
> 
> This also preserves the intended layering: generic_shutdown_super()
> handles VFS-side cleanup, while HFS filesystem state is fully destroyed
> afterwards.
> 
> Fixes: aca740cecbe5 ("fs: open block device after superblock creation")

I don't think that's correct.

The bug was introduced when hfs was converted to the new mount api as
this was the point where sb->s_fs_info allocation was moved from
fill_super() to init_fs_context() in ffcd06b6d13b ("hfs: convert hfs to
use the new mount api") which was way after that commit.

I also think this isn't the best way to do it. There's no need to
open-code kill_block_super() at all.

That whole hfs_mdb_get() calling hfs_mdb_put() is completely backwards
and the cleanup labels make no sense - predated anything you did ofc. It
should not call hfs_mdb_put(). It's only caller is fill_super() which
already cleans everything up. So really hfs_kill_super() should just
free the allocation and it should be moved out of hfs_mdb_put().

And that solution is already something I mentioned in my earlier review.
Let me test a patch.