From: Shardul Bankar <shardul.b@mpiricsoftware.com>
To: Al Viro <viro@zeniv.linux.org.uk>
Cc: brauner@kernel.org, jack@suse.cz, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org, janak@mpiricsoftware.com,
slava@dubeyko.com,
syzbot+99f6ed51479b86ac4c41@syzkaller.appspotmail.com,
shardulsb08@gmail.com
Subject: Re: [PATCH] fs/super: fix s_fs_info leak when setup_bdev_super() fails
Date: Sun, 01 Feb 2026 17:09:44 +0530 [thread overview]
Message-ID: <5edc62f11796c116fb499fe2934c487f087a2cdd.camel@mpiricsoftware.com> (raw)
In-Reply-To: <20260201082724.GC3183987@ZenIV>
On Sun, 2026-02-01 at 08:27 +0000, Al Viro wrote:
> On Sun, Feb 01, 2026 at 01:02:26PM +0530, Shardul Bankar wrote:
>
>
> First of all, _what_ ->put_super()? Forget ->s_root, ->s_op is
> not going to be set there, so there's nowhere to get ->put_super()
> from. Relevant thing here is ->kill_sb().
>
Right, good catch- at that point ->s_op isn’t set yet, so ->put_super()
isn’t even a thing to rely on. Thanks for pointing that out.
The leak here is actually originating from hfsplus
(hfsplus_init_fs_context()).
> Freeing ->s_fs_info is better done there anyway - makes for simpler
> handling of foo_fill_super() failure exits, exactly because you don't
> need to free the damn thing there - just let your ->kill_sb() deal
> with
> it.
>
> The thing is, there are ->kill_sb() instances that do just that and
> I'm not at all sure they won't be broken by this patch.
>
> Note that right now it's either "deactivate_locked_super() done, -
> >free()
> is told to bugger off" or "->free() is called,
> deactivate_locked_super()
> and ->kill_sb() isn't"; you are introducing a new situation here.
I see your concern about introducing a new ownership state for
s_fs_info (handing it back to fc while still going through
deactivate_locked_super() / ->kill_sb()), which could break filesystems
that already free s_fs_info from ->kill_sb().
I'll drop this patch and raise a new one by fixing the leak in the
filesystem side instead (HFS+), making sure its ->kill_sb() path
reliably frees s_fs_info even when we fail before fill_super() runs,
and I’ll add the HFS+ maintainer(s) on that patch.
Thanks,
Shardul
next prev parent reply other threads:[~2026-02-01 11:40 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-01 7:32 Shardul Bankar
2026-02-01 8:27 ` Al Viro
2026-02-01 11:39 ` Shardul Bankar [this message]
2026-02-01 11:50 ` [syzbot ci] " syzbot ci
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5edc62f11796c116fb499fe2934c487f087a2cdd.camel@mpiricsoftware.com \
--to=shardul.b@mpiricsoftware.com \
--cc=brauner@kernel.org \
--cc=jack@suse.cz \
--cc=janak@mpiricsoftware.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=shardulsb08@gmail.com \
--cc=slava@dubeyko.com \
--cc=syzbot+99f6ed51479b86ac4c41@syzkaller.appspotmail.com \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®