linux-kernel - Re: [PATCH v2] 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: <20251215015500.GM1712166@ZenIV>
Date: Mon, 15 Dec 2025 01:55:00 +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 v2] adfs: fix memory leak in sb->s_fs_info

On Mon, Dec 15, 2025 at 03:22:52AM +0300, Ahmet Eray Karadag wrote:

> @@ -403,15 +394,9 @@ static int adfs_fill_super(struct super_block *sb, struct fs_context *fc)
>  	if (!sb->s_root) {
>  		adfs_free_map(sb);
>  		adfs_error(sb, "get root inode failed\n");
> -		ret = -EIO;
> -		goto error;
> +		return -EIO;

Double-free again, this time of asb->s_map - adfs_free_map() is called
twice in that case, once here and once in adfs_kill_sb()...

> +static void adfs_kill_sb(struct super_block *sb)
> +{
> +	struct adfs_sb_info *asb = ADFS_SB(sb);
> +
> +	kill_block_super(sb);
> +
> +	adfs_free_map(sb);

... and calling adfs_map_relse(), which calls brelse() after the block
device has been closed.  IOW, unlike freeing the ->s_fs_info this can't
be done after kill_block_super().  And with this one we don't have that
kind of gap - it doesn't exist until adfs_fill_super(), so there's no
leak to be had anyway.  Yes, it would be possible to get rid of more
of cleanup on failure exit there, but it would require expanding
kill_block_super() and inserting adfs_free_map() call before the call
sync_blockdev() in there - more headache than you win in adfs_fill_super(),
IMO.

Leave adfs_free_map() in adfs_put_super() (or make _it_ ->put_super(),
for that matter).