public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Shardul Bankar <shardulsb08@gmail.com>
Cc: brauner@kernel.org, jack@suse.cz, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, janak@mpiricsoftware.com,
	slava@dubeyko.com, Shardul Bankar <shardul.b@mpiricsoftware.com>,
	syzbot+99f6ed51479b86ac4c41@syzkaller.appspotmail.com
Subject: Re: [PATCH] fs/super: fix s_fs_info leak when setup_bdev_super() fails
Date: Sun, 1 Feb 2026 08:27:24 +0000	[thread overview]
Message-ID: <20260201082724.GC3183987@ZenIV> (raw)
In-Reply-To: <20260201073226.3445853-1-shardul.b@mpiricsoftware.com>

On Sun, Feb 01, 2026 at 01:02:26PM +0530, Shardul Bankar wrote:

> In get_tree_bdev_flags(), sget_dev() calls sget_fc(), which transfers
> ownership of fc->s_fs_info to the new superblock (s->s_fs_info) and
> clears fc->s_fs_info. If setup_bdev_super() then fails, the superblock
> is torn down via deactivate_locked_super(). However,
> generic_shutdown_super() only calls the filesystem's ->put_super()
> when sb->s_root is non-NULL. Since setup_bdev_super() fails before
> fill_super() runs, sb->s_root is never set, so ->put_super() is never
> called and the allocated s_fs_info is leaked.
> 
> Return ownership of s_fs_info to fc when setup_bdev_super() fails so
> put_fs_context() can free it via the filesystem's ->free() callback.
> Clear s->s_fs_info to avoid a stale reference. Do this only when
> setup_bdev_super() fails; when fill_super() fails, it already frees
> s_fs_info in its own error path.

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().

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.

  reply	other threads:[~2026-02-01  8:25 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-01  7:32 [PATCH] fs/super: fix s_fs_info leak when setup_bdev_super() fails Shardul Bankar
2026-02-01  8:27 ` Al Viro [this message]
2026-02-01 11:39   ` Shardul Bankar
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=20260201082724.GC3183987@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --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=shardul.b@mpiricsoftware.com \
    --cc=shardulsb08@gmail.com \
    --cc=slava@dubeyko.com \
    --cc=syzbot+99f6ed51479b86ac4c41@syzkaller.appspotmail.com \
    /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