Subject: [PATCH] hfsplus: Refuse to mount volumes larger than 2TB

From: Ben Hutchings <[email-masked]>
Date: Sun, 11 Oct 2009 03:11:27 +0100

As found in <http://bugs.debian.org/550010>, hfsplus is using type u32
rather than sector_t for some sector number calculations.

In particular, hfsplus_get_block() does:

        u32 ablock, dblock, mask;
...
        map_bh(bh_result, sb, (dblock << HFSPLUS_SB(sb).fs_shift) + HFSPLUS_SB(sb).blockoffset + (iblock & mask));

I am not confident that I can find and fix all cases where a sector
number may be truncated.  For now, avoid data loss by refusing to mount
HFS+ volumes with more than 2^32 sectors (2TB).

Signed-off-by: Ben Hutchings <[email-masked]>
Cc: [email-masked]
---
--- a/fs/hfsplus/wrapper.c
+++ b/fs/hfsplus/wrapper.c
@@ -99,6 +99,10 @@
 
 	if (hfsplus_get_last_session(sb, &part_start, &part_size))
 		return -EINVAL;
+	if (part_start + part_size > 0x100000000) {
+		pr_err("hfs: volumes larger than 2TB are not supported yet\n");
+		return -EINVAL;
+	}
 	while (1) {
 		bh = sb_bread512(sb, part_start + HFSPLUS_VOLHEAD_SECTOR, vhdr);
 		if (!bh)

-- 
Ben Hutchings
Quantity is no substitute for quality, but it's the only one we've got.

---

From: Andrew Morton <[email-masked]>
Date: Sun, 11 Oct 2009 00:51:53 -0700

> +++ b/fs/hfsplus/wrapper.c

part_start and part_size are sector_t.  This code will do weird overflow
things when sector_t is 32-bit.  Also 32-bit compilers will get upset at the
excessively large hex constant.

This should fix both issues:

--- a/fs/hfsplus/wrapper.c~hfsplus-refuse-to-mount-volumes-larger-than-2tb-fix
+++ a/fs/hfsplus/wrapper.c
@@ -99,7 +99,7 @@ int hfsplus_read_wrapper(struct super_bl
 
 	if (hfsplus_get_last_session(sb, &part_start, &part_size))
 		return -EINVAL;
-	if (part_start + part_size > 0x100000000) {
+	if ((u64)part_start + part_size > 0x100000000ULL) {
 		pr_err("hfs: volumes larger than 2TB are not supported yet\n");
 		return -EINVAL;
 	}
_

---

From: Ben Hutchings <[email-masked]>
Date: Sun, 11 Oct 2009 18:01:30 +0100

On Sun, 2009-10-11 at 00:51 -0700, Andrew Morton wrote:
[...]
> things when sector_t is 32-bit.

Sorry, I forgot CONFIG_LBD is still optional.

> Also 32-bit compilers will get upset at the excessively large hex constant.
[...]

Good point.

Ben.

-- 
Ben Hutchings
DNRC Motto:  I can please only one person per day.
Today is not your day.  Tomorrow isn't looking good either.