Re: [PATCH v2] fs/hfs: fix s_fs_info leak on setup_bdev_super() failure

[David Hunter <david.hunter.linux@gmail.com>]

On 11/25/25 17:14, 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?
> 
> 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?
> 
> Best Regards,
> Mehdi Ben Hadj Khelifa
> 
> [1]:https://lore.kernel.org/all/3ad2e91e-2c7f-488b-
> a119-51d62a6e95b8@gmail.com/
> 
> 
> 
> 
> 
> 

Hey everyone,

I am helping Shuah with the Linux Kernel Mentorship Program. I wanted to
report that many mentees have also had problems with xfstests recently.

I am tracking what happens here, so I can help future developers, but I
just wanted to let everyone know that others are also having issues with
xfstests.

Also, Mehdi, by any chance have you used any of the configuration
targets like "localmodconfig". I am wondering if something in your
configuration file is missing.

Thanks,
David Hunter