Message-ID: <6e1bb0361c7e675f971fc322b061fef6e9c7413e.camel@ibm.com>
Date: Tue, 25 Nov 2025 22:36:46 +0000
From: Viacheslav Dubeyko <Slava.Dubeyko@....com>
To: "jack@...e.cz" <jack@...e.cz>,
        "glaubitz@...sik.fu-berlin.de"
	<glaubitz@...sik.fu-berlin.de>,
        "slava@...eyko.com" <slava@...eyko.com>,
        "frank.li@...o.com" <frank.li@...o.com>,
        "mehdi.benhadjkhelifa@...il.com"
	<mehdi.benhadjkhelifa@...il.com>,
        "brauner@...nel.org" <brauner@...nel.org>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>
CC: "syzbot+ad45f827c88778ff7df6@...kaller.appspotmail.com"
	<syzbot+ad45f827c88778ff7df6@...kaller.appspotmail.com>,
        "linux-kernel-mentees@...ts.linuxfoundation.org"
	<linux-kernel-mentees@...ts.linuxfoundation.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "david.hunter.linux@...il.com" <david.hunter.linux@...il.com>,
        "skhan@...uxfoundation.org" <skhan@...uxfoundation.org>,
        "khalid@...nel.org"
	<khalid@...nel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2] fs/hfs: fix s_fs_info leak on setup_bdev_super()
 failure

On Tue, 2025-11-25 at 23:14 +0100, Mehdi Ben Hadj Khelifa wrote:
> On 11/22/25 12:01 AM, Viacheslav Dubeyko wrote:
> > On Sat, 2025-11-22 at 00:36 +0100, Mehdi Ben Hadj Khelifa wrote:
> > > On 11/21/25 11:28 PM, Viacheslav Dubeyko wrote:
> > > > On Sat, 2025-11-22 at 00:16 +0100, Mehdi Ben Hadj Khelifa wrote:
> > > > > On 11/21/25 11:04 PM, Viacheslav Dubeyko wrote:
> > > > > > On Fri, 2025-11-21 at 23:48 +0100, Mehdi Ben Hadj Khelifa wrote:
> > > > > > > On 11/21/25 10:15 PM, Viacheslav Dubeyko wrote:
> > > > > > > > On Fri, 2025-11-21 at 20:44 +0100, Mehdi Ben Hadj Khelifa wrote:
> > > > > > > > > On 11/19/25 8:58 PM, Viacheslav Dubeyko wrote:
> > > > > > > > > > On Wed, 2025-11-19 at 08:38 +0100, Mehdi Ben Hadj Khelifa wrote:
> > > > > > > > > > > 
> > 
> > <skipped>
> > 
> > > > > > > > 
> > > > > > > IIUC, hfs_mdb_put() isn't called in the case of hfs_kill_super() in
> > > > > > > christian's patch because fill_super() (for the each specific
> > > > > > > filesystem) is responsible for cleaning up the superblock in case of
> > > > > > > failure and you can reference christian's patch[1] which he explained
> > > > > > > the reasoning for here[2].And in the error path the we are trying to
> > > > > > > fix, fill_super() isn't even called yet. So such pointers shouldn't be
> > > > > > > pointing to anything allocated yet hence only freeing the pointer to the
> > > > > > > sb_info here is sufficient I think.
> > > > 
> > > > I was confused that your code with hfs_mdb_put() is still in this email. So,
> > > > yes, hfs_fill_super()/hfsplus_fill_super() try to free the memory in the case of
> > > > failure. It means that if something wasn't been freed, then it will be issue in
> > > > these methods. Then, I don't see what should else need to be added here. Some
> > > > file systems do sb->s_fs_info = NULL. But absence of this statement is not
> > > > critical, from my point of view.
> > > > 
> > > Thanks for the input. I will be sending the same mentionned patch after
> > > doing testing for it and also after finishing my testing for the hfs
> > > patch too.
> > > > 
> > 
> > I am guessing... Should we consider to introduce some xfstest, self-test, or
> > unit-test to detect this issue in all Linux's file systems family?
> > 
> Yes, It isn't that hard either IIUC you just need to fail the 
> bdev_file_open_by_dev() function somehow to trigger this error path..
> > Thanks,
> > Slava.
> 
> So I wanted to update you on my testing for the hfs patch and the 
> hfsplus patch. For the testing I used both my desktop pc and my laptop 
> pc running the same configuraitons and the same linux distribution to 
> have more accurate testing. There are three variants that I used for 
> testing : A stable kernel, 6.18-rc7 kernel with no patch, 6.18-rc7 
> kernel with hfs or hfsplus patch.
> 
> Firstly, I couldn't run the hfs tests due to mkfs.hfs being unavailable 
> in my search for it. they all point to mkfs.hfsplus and you pointed out 
> that mkfs.hfsplus can create hfs filesystems with the -h flag but in my 
> case it doesn't. I pointed out last time that I found a tool to create 
> HFS filesystems which it does (it's called hformat) but the xfstests 
> require the availability of mkfs.hfs and fsck.hfs for them to run. More 
> help on this is needed for me to run hfs tests. I also tested ext4 as 
> you have suggested as a base to compare to. Here is my summary of testing:
> 
> For Stable kernel 6.17.8:
> 
> On desktop:
> ext4 tests ran successfully.
> hfsplus tests crash the pc around generic 631 test.
> 
> On Laptop:
> ext4 and hfsplus tests ran successfully.
> 
> For 6.18-rc7 kernel:
> 
> On desktop:
> ext4 tests ran successfully same results as the stable kernel.
> hfsplus crashes on testing startup.For launching any test.
> 
> On Laptop:
> ext4 tests ran successfully same results as the stable kernel.
> hfsplus crashes on testing startup.For launcing any test.
> 
> 
> For the patched 6.18-rc7 kernel.
> 
> Same results for both desktop and laptop pcs as in the 6.18-rc7 kernel.
> 
> 
> Should be noted that I have tried many different setups regarding the 
> devices and their creation for the 6.18-rc7 kernel and none of them 
> worked.Still I can't deduce what is causing the issue.If they work for 
> you, my only assumption is that some dependency of xfstests is not met 
> on my part even though I made sure that I do cover them all especially 
> with repeatedly failed testing...
> 
> What could be the issue here on my end if you have any idea?

Which particular crash do you have? Could you please share the call trace?

Let me build the latest kernel and run xfstests again on my side.

> 
> Also should I send you the hfsplus patch in one of my earlier replies[1] 
> for you to test too and maybe add it to hfsplus?
> 
> 

Let's clarify at first what's going on with xfstests on your side.

Thanks,
Slava.

Powered by blists - more mailing lists