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

Viacheslav Dubeyko
2025-11-21T23:01:45Z

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?

Thanks,
Slava.