All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.