mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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

  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®