[PATCH v2] hfs: validate allocation block parameters in hfs_mdb_get()

[PATCH v2] hfs: validate allocation block parameters in hfs_mdb_get()

[Hui Peng]

In hfs_mdb_get(), drNmAlBlks (fs_ablocks), drAlBlkSiz (alloc_blksz), and
drFreeBks (free_ablocks) are read from the on-disk Master Directory Block
without checking their mutual consistency against the partition size, and
failure to allocate HFS_SB(sb)->bitmap returns -ENOMEM without releasing
HFS_SB(sb)->mdb_bh, HFS_SB(sb)->mdb, HFS_SB(sb)->alt_mdb_bh, or
HFS_SB(sb)->alt_mdb.

Validate that fs_ablocks is non-zero, free_ablocks does not exceed
fs_ablocks, and drAlBlSt + fs_ablocks * (alloc_blksz >>
HFS_SECTOR_SIZE_BITS) fits within part_size, and clean up via
hfs_mdb_put(sb) on error.

Tested in QEMU against Linux 7.3.0-rc3 by mounting a crafted HFS image
with drNmAlBlks = 10 and drFreeBks = 50: on the unfixed kernel
hfs_mdb_get() accepts the inconsistent MDB parameters, causing filesystem
corruption ("hfs: (loop2): extents (cnid 0x3) bitmap corrupted"); whereas
with the fix applied mount fails immediately with "hfs: inconsistent
allocation block parameters in MDB" (-EINVAL).

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
Changes in v2:
- Validate consistency between drNmAlBlks, drAlBlkSiz, drFreeBks, and
  part_size in hfs_mdb_get(), as requested by Viacheslav Dubeyko.

 fs/hfs/mdb.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/fs/hfs/mdb.c b/fs/hfs/mdb.c
index 277de712f9d4..665753a2cba9 100644
--- a/fs/hfs/mdb.c
+++ b/fs/hfs/mdb.c
@@ -214,6 +214,14 @@ int hfs_mdb_get(struct super_block *sb)
 
 	/* These parameters are read from and written to the MDB */
 	HFS_SB(sb)->free_ablocks = be16_to_cpu(mdb->drFreeBks);
+	if (!HFS_SB(sb)->fs_ablocks ||
+	    HFS_SB(sb)->free_ablocks > HFS_SB(sb)->fs_ablocks ||
+	    (sector_t)be16_to_cpu(mdb->drAlBlSt) +
+	    (sector_t)HFS_SB(sb)->fs_ablocks *
+	    (HFS_SB(sb)->alloc_blksz >> HFS_SECTOR_SIZE_BITS) > part_size) {
+		pr_err("inconsistent allocation block parameters in MDB\n");
+		goto out_err;
+	}
 	atomic64_set(&HFS_SB(sb)->next_id, be32_to_cpu(mdb->drNxtCNID));
 	HFS_SB(sb)->root_files = be16_to_cpu(mdb->drNmFls);
 	HFS_SB(sb)->root_dirs = be16_to_cpu(mdb->drNmRtDirs);
@@ -305,6 +313,10 @@ int hfs_mdb_get(struct super_block *sb)
 	}
 
 	return 0;
+
+out_err:
+	hfs_mdb_put(sb);
+	return -EINVAL;
 }
 
 /*
-- 
2.55.0.1082.g2b9226bbc0-goog