hfs: ensure sb->s_fs_info is always cleaned up

When hfs was converted to the new mount api a bug was introduced by
changing the allocation pattern of sb->s_fs_info. If setup_bdev_super()
fails after a new superblock has been allocated by sget_fc(), but before
hfs_fill_super() takes ownership of the filesystem-specific s_fs_info
data it was leaked.

Fix this by freeing sb->s_fs_info in hfs_kill_super().

Cc: stable@vger.kernel.org
Fixes: ffcd06b6d13b ("hfs: convert hfs to use the new mount api")
Reported-by: syzbot+ad45f827c88778ff7df6@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ad45f827c88778ff7df6
Tested-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
Link: https://lore.kernel.org/r/20251201222843.82310-2-mehdi.benhadjkhelifa@gmail.com
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>

diff --git a/fs/hfs/mdb.c b/fs/hfs/mdb.c
index 53f3fae602179772476e12a444a9b89e5404b1cb..f28cd24dee84254e2aff8abcceddf4cf5f91130b 100644 (file)
--- a/fs/hfs/mdb.c
+++ b/fs/hfs/mdb.c
@@ -92,7 +92,7 @@ int hfs_mdb_get(struct super_block *sb)
                /* See if this is an HFS filesystem */
                bh = sb_bread512(sb, part_start + HFS_MDB_BLK, mdb);
                if (!bh)
-                       goto out;
+                       return -EIO;
 
                if (mdb->drSigWord == cpu_to_be16(HFS_SUPER_MAGIC))
                        break;
@@ -102,13 +102,14 @@ int hfs_mdb_get(struct super_block *sb)
                 * (should do this only for cdrom/loop though)
                 */
                if (hfs_part_find(sb, &part_start, &part_size))
-                       goto out;
+                       return -EIO;
        }
 
        HFS_SB(sb)->alloc_blksz = size = be32_to_cpu(mdb->drAlBlkSiz);
        if (!size || (size & (HFS_SECTOR_SIZE - 1))) {
                pr_err("bad allocation block size %d\n", size);
-               goto out_bh;
+               brelse(bh);
+               return -EIO;
        }
 
        size = min(HFS_SB(sb)->alloc_blksz, (u32)PAGE_SIZE);
@@ -125,14 +126,16 @@ int hfs_mdb_get(struct super_block *sb)
        brelse(bh);
        if (!sb_set_blocksize(sb, size)) {
                pr_err("unable to set blocksize to %u\n", size);
-               goto out;
+               return -EIO;
        }
 
        bh = sb_bread512(sb, part_start + HFS_MDB_BLK, mdb);
        if (!bh)
-               goto out;
-       if (mdb->drSigWord != cpu_to_be16(HFS_SUPER_MAGIC))
-               goto out_bh;
+               return -EIO;
+       if (mdb->drSigWord != cpu_to_be16(HFS_SUPER_MAGIC)) {
+               brelse(bh);
+               return -EIO;
+       }
 
        HFS_SB(sb)->mdb_bh = bh;
        HFS_SB(sb)->mdb = mdb;
@@ -174,7 +177,7 @@ int hfs_mdb_get(struct super_block *sb)
 
        HFS_SB(sb)->bitmap = kzalloc(8192, GFP_KERNEL);
        if (!HFS_SB(sb)->bitmap)
-               goto out;
+               return -EIO;
 
        /* read in the bitmap */
        block = be16_to_cpu(mdb->drVBMSt) + part_start;
@@ -185,7 +188,7 @@ int hfs_mdb_get(struct super_block *sb)
                bh = sb_bread(sb, off >> sb->s_blocksize_bits);
                if (!bh) {
                        pr_err("unable to read volume bitmap\n");
-                       goto out;
+                       return -EIO;
                }
                off2 = off & (sb->s_blocksize - 1);
                len = min((int)sb->s_blocksize - off2, size);
@@ -199,12 +202,12 @@ int hfs_mdb_get(struct super_block *sb)
        HFS_SB(sb)->ext_tree = hfs_btree_open(sb, HFS_EXT_CNID, hfs_ext_keycmp);
        if (!HFS_SB(sb)->ext_tree) {
                pr_err("unable to open extent tree\n");
-               goto out;
+               return -EIO;
        }
        HFS_SB(sb)->cat_tree = hfs_btree_open(sb, HFS_CAT_CNID, hfs_cat_keycmp);
        if (!HFS_SB(sb)->cat_tree) {
                pr_err("unable to open catalog tree\n");
-               goto out;
+               return -EIO;
        }
 
        attrib = mdb->drAtrb;
@@ -229,12 +232,6 @@ int hfs_mdb_get(struct super_block *sb)
        }
 
        return 0;
-
-out_bh:
-       brelse(bh);
-out:
-       hfs_mdb_put(sb);
-       return -EIO;
 }
 
 /*
@@ -359,8 +356,6 @@ void hfs_mdb_close(struct super_block *sb)
  * Release the resources associated with the in-core MDB.  */
 void hfs_mdb_put(struct super_block *sb)
 {
-       if (!HFS_SB(sb))
-               return;
        /* free the B-trees */
        hfs_btree_close(HFS_SB(sb)->ext_tree);
        hfs_btree_close(HFS_SB(sb)->cat_tree);
@@ -373,6 +368,4 @@ void hfs_mdb_put(struct super_block *sb)
        unload_nls(HFS_SB(sb)->nls_disk);
 
        kfree(HFS_SB(sb)->bitmap);
-       kfree(HFS_SB(sb));
-       sb->s_fs_info = NULL;
 }

diff --git a/fs/hfs/super.c b/fs/hfs/super.c
index 47f50fa555a457fb6de83ba22d114f21847f2a3e..df289cbdd4e85b783ba3b2c5f35a13db733b4a5f 100644 (file)
--- a/fs/hfs/super.c
+++ b/fs/hfs/super.c
@@ -431,10 +431,18 @@ static int hfs_init_fs_context(struct fs_context *fc)
        return 0;
 }
 
+static void hfs_kill_super(struct super_block *sb)
+{
+       struct hfs_sb_info *hsb = HFS_SB(sb);
+
+       kill_block_super(sb);
+       kfree(hsb);
+}
+
 static struct file_system_type hfs_fs_type = {
        .owner          = THIS_MODULE,
        .name           = "hfs",
-       .kill_sb        = kill_block_super,
+       .kill_sb        = hfs_kill_super,
        .fs_flags       = FS_REQUIRES_DEV,
        .init_fs_context = hfs_init_fs_context,
 };