All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Ahmet Eray Karadag <eraykrdg1@gmail.com>
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, skhan@linuxfoundation.org,
	david.hunter.linux@gmail.com,
	syzbot+1c70732df5fd4f0e4fbb@syzkaller.appspotmail.com
Subject: Re: [PATCH] adfs: fix memory leak in sb->s_fs_info
Date: Sun, 14 Dec 2025 02:27:45 +0000	[thread overview]
Message-ID: <20251214022745.GK1712166@ZenIV> (raw)
In-Reply-To: <20251214020212.GJ1712166@ZenIV>

On Sun, Dec 14, 2025 at 02:02:12AM +0000, Al Viro wrote:

> IOW, there's our double-free.  For extra fun, it's not just kfree() + kfree(),
> it's kfree_rcu() + kfree().

[sorry, accidentally sent halfway through writing a reply; continued below]

So after successful mount, it gets freed (RCU-delayed) from ->kill_sb() called
at fs shutdown.

On adfs_fill_super() failure (hit #2) it is freed on failure exit - with non-delayed
kfree().

In case we never got to superblock allocation, the thing gets freed by adfs_free_fc()
(also non-delayed).

The gap is between a successful call of sget_fc() and call of adfs_fill_super()
(in get_tree_bdev(), which is where adfs_fill_super() is passed as a callback).
If setup_bdev_super() fails, we will
	* transfer it from fs_context to super_block, so the fs_context destruction
won't have anything to free
	* won't free it in never-called adfs_fill_super()
	* won't free it in ->kill_sb(), since ->s_root remains NULL and ->put_super()
is never called.

A leak is real, IOW.

Getting ->kill_sb() to do freeing unconditionally would cover the gap.  However,
to do that, we need to _move_ freeing (RCU-delayed) from adfs_put_super() to
adfs_kill_sb(), not just add kfree() in the latter.

What's more, that allows to simplify adfs_fill_super() failure exit: we can leave
freeing asb (and clearing ->s_fs_info, of course) to ->kill_sb() - the latter is
called on any superblock destruction, including that after failing fill_super()
callback.  Almost the first thing done by deactivate_locked_super() called in
that case is
                fs->kill_sb(s);

So if we go with "have it freed in ->kill_sb()" approach, the solution would be

1) adfs_kill_sb() calling kfree_rcu(asb, rcu) instead of kfree(asb)
2) call of kfree_rcu() removed from adfs_put_super()
3) all goto error; in adfs_fill_super() becoming return ret; (and error:
getting removed, that is)

  reply	other threads:[~2025-12-14  2:27 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-13 23:36 [PATCH] adfs: fix memory leak in sb->s_fs_info Ahmet Eray Karadag
2025-12-14  1:32 ` Al Viro
2025-12-14  2:02   ` Al Viro
2025-12-14  2:27     ` Al Viro [this message]
2025-12-14  2:58       ` Ahmet Eray Karadag
2025-12-15  0:22 ` [PATCH v2] " Ahmet Eray Karadag
2025-12-15  1:55   ` Al Viro
2025-12-15  3:14 ` [PATCH v3] " Ahmet Eray Karadag

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=20251214022745.GK1712166@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=akpm@linux-foundation.org \
    --cc=david.hunter.linux@gmail.com \
    --cc=eraykrdg1@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=syzbot+1c70732df5fd4f0e4fbb@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.