| 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
| ||
|
Message-ID: <3ad2e91e-2c7f-488b-a119-51d62a6e95b8@gmail.com>
Date: Fri, 21 Nov 2025 20:44:07 +0100
From: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@...il.com>
To: Viacheslav Dubeyko <Slava.Dubeyko@....com>,
"glaubitz@...sik.fu-berlin.de" <glaubitz@...sik.fu-berlin.de>,
"frank.li@...o.com" <frank.li@...o.com>,
"slava@...eyko.com" <slava@...eyko.com>,
"brauner@...nel.org" <brauner@...nel.org>,
"viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
"jack@...e.cz" <jack@...e.cz>
Cc: "khalid@...nel.org" <khalid@...nel.org>,
"linux-kernel-mentees@...ts.linuxfoundation.org"
<linux-kernel-mentees@...ts.linuxfoundation.org>,
"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"skhan@...uxfoundation.org" <skhan@...uxfoundation.org>,
"david.hunter.linux@...il.com" <david.hunter.linux@...il.com>,
"syzbot+ad45f827c88778ff7df6@...kaller.appspotmail.com"
<syzbot+ad45f827c88778ff7df6@...kaller.appspotmail.com>
Subject: Re: [PATCH v2] fs/hfs: fix s_fs_info leak on setup_bdev_super()
failure
On 11/19/25 8:58 PM, Viacheslav Dubeyko wrote:
> On Wed, 2025-11-19 at 08:38 +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")
>> Reported-by: syzbot+ad45f827c88778ff7df6@...kaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=ad45f827c88778ff7df6
>> Tested-by: syzbot+ad45f827c88778ff7df6@...kaller.appspotmail.com
>> Suggested-by: Al Viro <viro@...iv.linux.org.uk>
>> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@...il.com>
>> ---
>> ChangeLog:
>>
>> Changes from v1:
>>
>> -Changed the patch direction to focus on hfs changes specifically as
>> suggested by al viro
>>
>> Link:https://lore.kernel.org/all/20251114165255.101361-1-mehdi.benhadjkhelifa@gmail.com/
>>
>> Note:This patch might need some more testing as I only did run selftests
>> with no regression, check dmesg output for no regression, run reproducer
>> with no bug and test it with syzbot as well.
>
> Have you run xfstests for the patch? Unfortunately, we have multiple xfstests
> failures for HFS now. And you can check the list of known issues here [1]. The
> main point of such run of xfstests is to check that maybe some issue(s) could be
> fixed by the patch. And, more important that you don't introduce new issues. ;)
>
I have tried to run the xfstests with a kernel built with my patch and
also without my patch for TEST and SCRATCH devices and in both cases my
system crashes in running the generic/631 test.Still unsure of the
cause. For more context, I'm running the tests on the 6.18-rc5 version
of the kernel and the devices and the environment setup is as follows:
For device creation and mounting(also tried it with dd and had same
results):
fallocate -l 10G test.img
fallocate -l 10G scratch.img
sudo mkfs.hfs test.img
sudo losetup /dev/loop0 ./test.img
sudo losetup /dev/loop1 ./scratch.img
sudo mkdir -p /mnt/test /mnt/scratch
sudo mount /dev/loop0 /mnt/test
For environment setup(local.config):
export TEST_DEV=/dev/loop0
export TEST_DIR=/mnt/test
export SCRATCH_DEV=/dev/loop1
export SCRATCH_MNT=/mnt/scratch
Ran the tests using:sudo ./check -g auto
If more context is needed to know the point of failure or if I have made
a mistake during setup I'm happy to receive your comments since this is
my first time trying to run xfstests.
>>
>> fs/hfs/super.c | 16 ++++++++++++----
>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/hfs/super.c b/fs/hfs/super.c
>> index 47f50fa555a4..06e1c25e47dc 100644
>> --- a/fs/hfs/super.c
>> +++ b/fs/hfs/super.c
>> @@ -49,8 +49,6 @@ static void hfs_put_super(struct super_block *sb)
>> {
>> cancel_delayed_work_sync(&HFS_SB(sb)->mdb_work);
>> hfs_mdb_close(sb);
>> - /* release the MDB's resources */
>> - hfs_mdb_put(sb);
>> }
>>
>> static void flush_mdb(struct work_struct *work)
>> @@ -383,7 +381,6 @@ static int hfs_fill_super(struct super_block *sb, struct fs_context *fc)
>> bail_no_root:
>> pr_err("get root inode failed\n");
>> bail:
>> - hfs_mdb_put(sb);
>> return res;
>> }
>>
>> @@ -431,10 +428,21 @@ static int hfs_init_fs_context(struct fs_context *fc)
>> return 0;
>> }
>>
>> +static void hfs_kill_sb(struct super_block *sb)
>> +{
>> + generic_shutdown_super(sb);
>> + hfs_mdb_put(sb);
>> + if (sb->s_bdev) {
>> + sync_blockdev(sb->s_bdev);
>> + bdev_fput(sb->s_bdev_file);
>> + }
>> +
>> +}
>> +
>> static struct file_system_type hfs_fs_type = {
>> .owner = THIS_MODULE,
>> .name = "hfs",
>> - .kill_sb = kill_block_super,
>
> It looks like we have the same issue for the case of HFS+ [2]. Could you please
> double check that HFS+ should be fixed too?
>
I have checked the same error path and it seems that hfsplus_sb_info is
not freed in that path(I could provide the exact call stack which would
cause such a memory leak) although I didn't create or run any
reproducers for this particular filesystem type.
If you would like a patch for this issue, would something like what is
shown below be acceptable? :
+static void hfsplus_kill_super(struct super_block *sb)
+{
+ struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
+
+ kill_block_super(sb);
+ kfree(sbi);
+}
+
static struct file_system_type hfsplus_fs_type = {
.owner = THIS_MODULE,
.name = "hfsplus",
- .kill_sb = kill_block_super,
+ .kill_sb = hfsplus_kill_super,
.fs_flags = FS_REQUIRES_DEV,
.init_fs_context = hfsplus_init_fs_context,
};
If there is something to add, remove or adjust. Please let me know in
the case of you willing accepting such a patch of course.
> Thanks,
> Slava.
>
Best Regards,
Mehdi Ben Hadj Khelifa
>> + .kill_sb = hfs_kill_sb,
>> .fs_flags = FS_REQUIRES_DEV,
>> .init_fs_context = hfs_init_fs_context,
>> };
>
> [1] https://github.com/hfs-linux-kernel/hfs-linux-kernel/issues
> [2] https://elixir.bootlin.com/linux/v6.18-rc6/source/fs/hfsplus/super.c#L694
Powered by blists - more mailing lists