Messages in this thread |  | | Date | Thu, 27 Nov 2025 12:45:42 -0500 | | Subject | Re: [PATCH v2] fs/hfs: fix s_fs_info leak on setup_bdev_super() failure | | From | David Hunter <> |
| |
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
|  |