linux-kernel - Re: [PATCH] adfs: fix memory leak in sb->s_fs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251214022745.GK1712166@ZenIV>
Date: Sun, 14 Dec 2025 02:27:45 +0000
From: Al Viro <viro@...iv.linux.org.uk>
To: Ahmet Eray Karadag <eraykrdg1@...il.com>
Cc: akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, skhan@...uxfoundation.org,
	david.hunter.linux@...il.com,
	syzbot+1c70732df5fd4f0e4fbb@...kaller.appspotmail.com
Subject: Re: [PATCH] adfs: fix memory leak in sb->s_fs_info

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)