[PATCH v2] adfs: fix memory leak in sb->s_fs_info

Ahmet Eray Karadag posted 1 patch 3 days, 3 hours ago
There is a newer version of this series
fs/adfs/super.c | 32 ++++++++++++++------------------
1 file changed, 14 insertions(+), 18 deletions(-)
[PATCH v2] adfs: fix memory leak in sb->s_fs_info
Posted by Ahmet Eray Karadag 3 days, 3 hours ago
Syzbot reported a memory leak in adfs during the mount process. The issue
arises because the ownership of the allocated (struct adfs_sb_info) is
transferred from the filesystem context to the superblock via sget_fc().
This function sets fc->s_fs_info to NULL after the transfer.

The ADFS filesystem previously used the default kill_block_super for
superblock destruction. This helper performs generic cleanup but does not
free the private sb->s_fs_info data. Since fc->s_fs_info is set to
NULL during the transfer, the standard context cleanup (adfs_free_fc)
also skips freeing this memory. As a result, if the superblock is
destroyed, the allocated struct adfs_sb_info is leaked.

Fix this by implementing a custom .kill_sb callback (adfs_kill_sb)
that explicitly frees sb->s_fs_info before invoking the generic
kill_block_super.

Reported-by: syzbot+1c70732df5fd4f0e4fbb@syzkaller.appspotmail.com
Fixes: https://syzkaller.appspot.com/bug?extid=1c70732df5fd4f0e4fbb
Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.com>
---
v2:
 - Remove adfs_put_super
 - Remove error label in adfs_fill_super
 - Use kfree_rcu instead kfree
 - Free map in adfs_kill_sb
 - Tested with ADFS test images
---
 fs/adfs/super.c | 32 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 18 deletions(-)

diff --git a/fs/adfs/super.c b/fs/adfs/super.c
index fdccdbbfc213..96855f1086cd 100644
--- a/fs/adfs/super.c
+++ b/fs/adfs/super.c
@@ -90,14 +90,6 @@ static int adfs_checkdiscrecord(struct adfs_discrecord *dr)
 	return 0;
 }
 
-static void adfs_put_super(struct super_block *sb)
-{
-	struct adfs_sb_info *asb = ADFS_SB(sb);
-
-	adfs_free_map(sb);
-	kfree_rcu(asb, rcu);
-}
-
 static int adfs_show_options(struct seq_file *seq, struct dentry *root)
 {
 	struct adfs_sb_info *asb = ADFS_SB(root->d_sb);
@@ -246,7 +238,6 @@ static const struct super_operations adfs_sops = {
 	.free_inode	= adfs_free_inode,
 	.drop_inode	= adfs_drop_inode,
 	.write_inode	= adfs_write_inode,
-	.put_super	= adfs_put_super,
 	.statfs		= adfs_statfs,
 	.show_options	= adfs_show_options,
 };
@@ -362,7 +353,7 @@ static int adfs_fill_super(struct super_block *sb, struct fs_context *fc)
 		ret = -EINVAL;
 	}
 	if (ret)
-		goto error;
+		return ret;
 
 	/* set up enough so that we can read an inode */
 	sb->s_op = &adfs_sops;
@@ -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;
 	}
 	return 0;
-
-error:
-	sb->s_fs_info = NULL;
-	kfree(asb);
-	return ret;
 }
 
 static int adfs_get_tree(struct fs_context *fc)
@@ -462,10 +447,21 @@ static int adfs_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
+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);
+
+	kfree_rcu(asb, rcu);
+}
+
 static struct file_system_type adfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "adfs",
-	.kill_sb	= kill_block_super,
+	.kill_sb	= adfs_kill_sb,
 	.fs_flags	= FS_REQUIRES_DEV,
 	.init_fs_context = adfs_init_fs_context,
 	.parameters	= adfs_param_spec,
-- 
2.43.0
Re: [PATCH v2] adfs: fix memory leak in sb->s_fs_info
Posted by Al Viro 3 days, 2 hours ago
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).