[PATCH v2 0/2] hfs, hfsplus: validate the partition map and wrapper before following them

[PATCH v2 0/2] hfs, hfsplus: validate the partition map and wrapper before following them

[Matthias Goergens]

hfs_mdb_get() and hfsplus_read_wrapper() reread the volume header in a
loop, following a partition map entry and, in hfsplus, an HFS wrapper's
embedded-volume descriptor.  An entry or descriptor with a zero offset
sends the loop back to the header it has just read, and a crafted image
hangs the mount.

v2 checks the offsets where they are parsed, in hfs_part_find() and
hfsplus_read_mdb(), as Slava suggested: a partition must start inside
the device and after a new-style partition map (TN1189), and a wrapper's
embedded volume must lie within its allocation blocks, which start after
its MDB (TN1150).

Every hop now moves past what it was read from, so the loop ends and the
work it does is linear in the size of the device.  For a new-style map,
a non-zero start alone would not be enough.  Take a map entry in every
block, all of type Apple_Free with a large pmMapBlkCnt, and one
Apple_HFS entry near the end with pmPyPartStart 2: each two-block hop
then rescans the map up to that entry.  With a check on the start alone,
an hfsplus mount of an 8 MiB image built like this was still busy after
ten minutes; with these patches it fails in about a second.  Chains of
small hops remain possible when each map has a single entry, and a
64 MiB image of two-block hops takes about three seconds to fail under
QEMU, the same as without these patches.  If that should be bounded as
well, v1's limit of one hop of each kind could go on top.  The
generator for these images, with timings for an unpatched kernel and
for these patches, is at

  https://github.com/matthiasgoergens/linux/tree/reproducer/2026-09-30-hfs-part-sanity-v2

Under QEMU, v1's reproducers now fail at once.  Plain, wrapped and
partitioned volumes made with newfs_hfs still mount, with maps written
by parted or, for the old-style format, by hand.  So do hybrid CD images
from genisoimage -hfs and xorriso -hfsplus.  Wrappers written by
newfs_hfs -w, for volumes up to 31 GB, pass the new check.

Changes in v2:
- Check the entries in hfs_part_find() and the wrapper in
  hfsplus_read_mdb() instead of limiting the number of hops (Slava).
- hfs: stop at the first matching old-style ("TS") map entry, as
  hfsplus already does, so that the start returned is one that was
  checked.

v1: https://lore.kernel.org/all/20260926084010.569552-1-matthias.goergens@gmail.com/

Matthias Goergens (2):
  hfs: validate partition map entries in hfs_part_find()
  hfsplus: validate the wrapper and partition map before following them

 fs/hfs/part_tbl.c          | 27 ++++++++++++++++++++++++++-
 fs/hfsplus/part_tbl.c      | 24 +++++++++++++++++++++++-
 fs/hfsplus/wrapper.c       | 16 +++++++++++++++-
 include/linux/hfs_common.h |  1 +
 4 files changed, 65 insertions(+), 3 deletions(-)


base-commit: 6812ce4e4379ffc99c52401ec28f0d7ffbc36206
-- 
2.55.0

[PATCH v2 1/2] hfs: validate partition map entries in hfs_part_find()

[Matthias Goergens]

hfs_mdb_get() loops around hfs_part_find(), rereading the MDB wherever
the partition map points, and hfs_part_find() takes the start of a
matching entry as it is.  A new-style entry with pmPyPartStart 0 leaves
part_start where it was, so the loop reads the same blocks forever and
the mount hangs.

Check each entry before following it.  The partition must be non-empty
and start inside the device.  For a new-style map it must also start
after the map, which begins at block 1 and whose size the first entry's
pmMapBlkCnt gives (TN1189); the old-style parser keeps its rule of a
non-zero start.  An entry that fails these checks is skipped, as the
old-style parser already skipped entries with a zero start or size.  The
old-style parser now also stops at the first match, as the new-style one
and hfsplus's copy do: it used to add up the starts of all matching
entries, so the start it returned was not one that had been checked.

Every partition-table hop now moves part_start past the map entries it
has read, so the loop in hfs_mdb_get() ends and reads each block of a
map at most once.  Rejecting only a zero start would end the loop too,
but a crafted new-style map with an entry in every block could then make
each of many small hops rescan most of the device.

The end of the partition is not checked against the device.  hfs already
mounts such a volume without its alternate MDB, and the block layer
likewise keeps a partition that runs past the end of the disk, trimmed
to fit.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com>
---
v2: check the entries in hfs_part_find() instead of limiting the hops
in hfs_mdb_get(); stop at the first matching old-style entry.

A partition entry pointing at its own map hangs the mount without this
patch and fails at once with it:

  img=hfs-partmap-loop.img
  put() { printf "$2" | dd of=$img bs=1 seek=$1 conv=notrunc status=none; }
  truncate --size=64K $img
  put $((512+0x00)) '\x50\x4d'          # pmSig 'PM'
  put $((512+0x04)) '\x00\x00\x00\x01'  # pmMapBlkCnt 1
  put $((512+0x08)) '\x00\x00\x00\x00'  # pmPyPartStart 0 (self)
  put $((512+0x0c)) '\x00\x00\x00\x64'  # pmPartBlkCnt 100
  put $((512+0x30)) 'Apple_HFS'         # pmPartType
  mount -o ro,loop -t hfs $img /mnt

 fs/hfs/part_tbl.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/fs/hfs/part_tbl.c b/fs/hfs/part_tbl.c
index 36add537d153e..2c15afe098127 100644
--- a/fs/hfs/part_tbl.c
+++ b/fs/hfs/part_tbl.c
@@ -9,6 +9,8 @@
  * a patch contributed by Holger Schemel (aeglos@valinor.owl.de).
  */
 
+#include <linux/blkdev.h>
+
 #include "hfs_fs.h"
 
 /*
@@ -49,6 +51,22 @@ struct old_pmap {
 	}	pdEntry[42];
 } __packed;
 
+/*
+ * Check a partition map entry before following it.  The partition must
+ * be non-empty, start inside the device and start at or after @first:
+ * after the driver descriptor map in block 0 for an old-style map, and
+ * after the whole of a new-style map, whose size the first entry's
+ * pmMapBlkCnt gives (TN1189).  Every hop then moves forward, past the
+ * map entries just read, so hfs_mdb_get() cannot loop and reads each
+ * block of a map at most once.
+ */
+static bool hfs_part_valid(struct super_block *sb, sector_t base,
+			   u64 first, u32 start, u32 size)
+{
+	return start >= first && size &&
+	       base + start < bdev_nr_sectors(sb->s_bdev);
+}
+
 /*
  * hfs_part_find()
  *
@@ -77,12 +95,15 @@ int hfs_part_find(struct super_block *sb,
 		p = pm->pdEntry;
 		size = 42;
 		for (i = 0; i < size; p++, i++) {
-			if (p->pdStart && p->pdSize &&
+			if (hfs_part_valid(sb, *part_start, HFS_DD_BLK + 1,
+					   be32_to_cpu(p->pdStart),
+					   be32_to_cpu(p->pdSize)) &&
 			    p->pdFSID == cpu_to_be32(0x54465331)/*"TFS1"*/ &&
 			    (HFS_SB(sb)->part < 0 || HFS_SB(sb)->part == i)) {
 				*part_start += be32_to_cpu(p->pdStart);
 				*part_size = be32_to_cpu(p->pdSize);
 				res = 0;
+				break;
 			}
 		}
 		break;
@@ -95,6 +116,10 @@ int hfs_part_find(struct super_block *sb,
 		size = be32_to_cpu(pm->pmMapBlkCnt);
 		for (i = 0; i < size;) {
 			if (!memcmp(pm->pmPartType,"Apple_HFS", 9) &&
+			    hfs_part_valid(sb, *part_start,
+					   HFS_PMAP_BLK + (u64)size,
+					   be32_to_cpu(pm->pmPyPartStart),
+					   be32_to_cpu(pm->pmPartBlkCnt)) &&
 			    (HFS_SB(sb)->part < 0 || HFS_SB(sb)->part == i)) {
 				*part_start += be32_to_cpu(pm->pmPyPartStart);
 				*part_size = be32_to_cpu(pm->pmPartBlkCnt);
-- 
2.55.0

[PATCH v2 2/2] hfsplus: validate the wrapper and partition map before following them

[Matthias Goergens]

hfsplus_read_wrapper() rereads the volume header through a bare "goto
reread", following an HFS wrapper's embedded-volume descriptor or, if
the header matches neither signature, the partition map found by
hfs_part_find().  hfsplus_read_mdb() does not check the offset it
returns, and hfs_part_find() does not check the start of a new-style map
entry, so a descriptor or entry with a zero offset leaves part_start
where it was and the mount loops forever.

Check both where they are parsed.  TN1150 places the embedded volume in
the wrapper's allocation blocks, and the boot blocks, MDB and volume
bitmap of an HFS volume are not part of any allocation block.  So
hfsplus_read_mdb() now rejects a wrapper whose allocation blocks start
at or before its MDB (drAlBlSt <= 2), whose embedded volume is empty, or
whose embedded volume ends past the wrapper's last allocation block
(drNmAlBlks).  hfs_part_find() gets the same check as in hfs: a
partition must be non-empty, start inside the device and, for a
new-style map, start after the map, which begins at block 1 and whose
size the first entry's pmMapBlkCnt gives (TN1189).

A wrapper hop now moves part_start forward by at least three sectors,
and a partition-table hop moves it past the map entries it has read.  So
the loop ends, at the latest when a read goes past the end of the
device, and the maps read on the way do not overlap.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com>
---
v2: check the wrapper in hfsplus_read_mdb() and the entries in
hfs_part_find() instead of limiting the hops in hfsplus_read_wrapper().

A wrapper whose embedded-volume descriptor points back at itself hangs
the mount without this patch and fails at once with it:

  img=hfsplus-wrapper-loop.img
  put() { printf "$2" | dd of=$img bs=1 seek=$1 conv=notrunc status=none; }
  truncate --size=64K $img
  put $((1024 + 0x00)) '\x42\x44'          # drSigWord 'BD'
  put $((1024 + 0x0a)) '\x82\x00'          # drAtrb: SLOCK | SPARED
  put $((1024 + 0x14)) '\x00\x00\x02\x00'  # drAlBlkSiz 512
  put $((1024 + 0x7c)) '\x48\x2b'          # drEmbedSigWord 'H+'
  put $((1024 + 0x7e)) '\x00\x00\x00\x64'  # drEmbedExtent: start 0, count 100
  mount -o ro,loop -t hfsplus $img /mnt

The partition map from the hfs patch hangs an hfsplus mount the same
way, with "-t hfsplus".

 fs/hfsplus/part_tbl.c      | 24 +++++++++++++++++++++++-
 fs/hfsplus/wrapper.c       | 16 +++++++++++++++-
 include/linux/hfs_common.h |  1 +
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/fs/hfsplus/part_tbl.c b/fs/hfsplus/part_tbl.c
index 9ec21664eda6b..ceba3139f1f56 100644
--- a/fs/hfsplus/part_tbl.c
+++ b/fs/hfsplus/part_tbl.c
@@ -67,6 +67,22 @@ struct old_pmap {
 	}	pdEntry[42];
 } __packed;
 
+/*
+ * Check a partition map entry before following it.  The partition must
+ * be non-empty, start inside the device and start at or after @first:
+ * after the driver descriptor map in block 0 for an old-style map, and
+ * after the whole of a new-style map, whose size the first entry's
+ * pmMapBlkCnt gives (TN1189).  Every hop then moves forward, past the
+ * map entries just read, so hfsplus_read_wrapper() cannot loop and
+ * reads each block of a map at most once.
+ */
+static bool hfs_part_valid(struct super_block *sb, sector_t base,
+			   u64 first, u32 start, u32 size)
+{
+	return start >= first && size &&
+	       base + start < bdev_nr_sectors(sb->s_bdev);
+}
+
 static int hfs_parse_old_pmap(struct super_block *sb, struct old_pmap *pm,
 		sector_t *part_start, sector_t *part_size)
 {
@@ -76,7 +92,9 @@ static int hfs_parse_old_pmap(struct super_block *sb, struct old_pmap *pm,
 	for (i = 0; i < 42; i++) {
 		struct old_pmap_entry *p = &pm->pdEntry[i];
 
-		if (p->pdStart && p->pdSize &&
+		if (hfs_part_valid(sb, *part_start, HFS_DD_BLK + 1,
+				   be32_to_cpu(p->pdStart),
+				   be32_to_cpu(p->pdSize)) &&
 		    p->pdFSID == cpu_to_be32(0x54465331)/*"TFS1"*/ &&
 		    (sbi->part < 0 || sbi->part == i)) {
 			*part_start += be32_to_cpu(p->pdStart);
@@ -99,6 +117,10 @@ static int hfs_parse_new_pmap(struct super_block *sb, void *buf,
 
 	do {
 		if (!memcmp(pm->pmPartType, "Apple_HFS", 9) &&
+		    hfs_part_valid(sb, *part_start,
+				   HFS_PMAP_BLK + (u64)size,
+				   be32_to_cpu(pm->pmPyPartStart),
+				   be32_to_cpu(pm->pmPartBlkCnt)) &&
 		    (sbi->part < 0 || sbi->part == i)) {
 			*part_start += be32_to_cpu(pm->pmPyPartStart);
 			*part_size = be32_to_cpu(pm->pmPartBlkCnt);
diff --git a/fs/hfsplus/wrapper.c b/fs/hfsplus/wrapper.c
index 30cf4fe78b3d2..35cd2563dfdfe 100644
--- a/fs/hfsplus/wrapper.c
+++ b/fs/hfsplus/wrapper.c
@@ -66,7 +66,7 @@ int hfsplus_submit_bio(struct super_block *sb, sector_t sector,
 static int hfsplus_read_mdb(void *bufptr, struct hfsplus_wd *wd)
 {
 	u32 extent;
-	u16 attrib;
+	u16 attrib, nmalblks;
 	__be16 sig;
 
 	sig = *(__be16 *)(bufptr + HFSP_WRAPOFF_EMBEDSIG);
@@ -87,11 +87,25 @@ static int hfsplus_read_mdb(void *bufptr, struct hfsplus_wd *wd)
 		return 0;
 	wd->ablk_start =
 		be16_to_cpu(*(__be16 *)(bufptr + HFSP_WRAPOFF_ABLKSTART));
+	nmalblks = be16_to_cpu(*(__be16 *)(bufptr + HFSP_WRAPOFF_NMALBLKS));
 
 	extent = get_unaligned_be32(bufptr + HFSP_WRAPOFF_EMBEDEXT);
 	wd->embed_start = (extent >> 16) & 0xFFFF;
 	wd->embed_count = extent & 0xFFFF;
 
+	/*
+	 * The boot blocks, MDB and volume bitmap of an HFS volume are not
+	 * part of any allocation block, and the embedded volume occupies
+	 * allocation blocks of the wrapper (TN1150).  So the embedded
+	 * volume starts after the wrapper's MDB and ends inside the
+	 * wrapper.
+	 */
+	if (wd->ablk_start <= HFS_MDB_BLK)
+		return 0;
+	if (!wd->embed_count ||
+	    wd->embed_start + wd->embed_count > nmalblks)
+		return 0;
+
 	return 1;
 }
 
diff --git a/include/linux/hfs_common.h b/include/linux/hfs_common.h
index d6a615e74b26e..123475efe5b37 100644
--- a/include/linux/hfs_common.h
+++ b/include/linux/hfs_common.h
@@ -44,6 +44,7 @@
 
 #define HFSP_WRAPOFF_SIG		0x00
 #define HFSP_WRAPOFF_ATTRIB		0x0A
+#define HFSP_WRAPOFF_NMALBLKS		0x12
 #define HFSP_WRAPOFF_ABLKSIZE		0x14
 #define HFSP_WRAPOFF_ABLKSTART		0x1C
 #define HFSP_WRAPOFF_EMBEDSIG		0x7C
-- 
2.55.0

Re: [PATCH v2 1/2] hfs: validate partition map entries in hfs_part_find()

[Viacheslav Dubeyko]

On Fri, 2026-10-02 at 00:21 +0800, Matthias Goergens wrote:
> hfs_mdb_get() loops around hfs_part_find(), rereading the MDB
> wherever
> the partition map points, and hfs_part_find() takes the start of a
> matching entry as it is.  A new-style entry with pmPyPartStart 0
> leaves
> part_start where it was, so the loop reads the same blocks forever
> and
> the mount hangs.
> 
> Check each entry before following it.  The partition must be non-
> empty
> and start inside the device.  For a new-style map it must also start
> after the map, which begins at block 1 and whose size the first
> entry's
> pmMapBlkCnt gives (TN1189); the old-style parser keeps its rule of a
> non-zero start.  An entry that fails these checks is skipped, as the
> old-style parser already skipped entries with a zero start or size. 
> The
> old-style parser now also stops at the first match, as the new-style
> one
> and hfsplus's copy do: it used to add up the starts of all matching
> entries, so the start it returned was not one that had been checked.
> 
> Every partition-table hop now moves part_start past the map entries
> it
> has read, so the loop in hfs_mdb_get() ends and reads each block of a
> map at most once.  Rejecting only a zero start would end the loop
> too,
> but a crafted new-style map with an entry in every block could then
> make
> each of many small hops rescan most of the device.
> 
> The end of the partition is not checked against the device.  hfs
> already
> mounts such a volume without its alternate MDB, and the block layer
> likewise keeps a partition that runs past the end of the disk,
> trimmed
> to fit.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com>
> ---
> v2: check the entries in hfs_part_find() instead of limiting the hops
> in hfs_mdb_get(); stop at the first matching old-style entry.
> 
> A partition entry pointing at its own map hangs the mount without
> this
> patch and fails at once with it:
> 
>   img=hfs-partmap-loop.img
>   put() { printf "$2" | dd of=$img bs=1 seek=$1 conv=notrunc
> status=none; }
>   truncate --size=64K $img
>   put $((512+0x00)) '\x50\x4d'          # pmSig 'PM'
>   put $((512+0x04)) '\x00\x00\x00\x01'  # pmMapBlkCnt 1
>   put $((512+0x08)) '\x00\x00\x00\x00'  # pmPyPartStart 0 (self)
>   put $((512+0x0c)) '\x00\x00\x00\x64'  # pmPartBlkCnt 100
>   put $((512+0x30)) 'Apple_HFS'         # pmPartType
>   mount -o ro,loop -t hfs $img /mnt
> 
>  fs/hfs/part_tbl.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/hfs/part_tbl.c b/fs/hfs/part_tbl.c
> index 36add537d153e..2c15afe098127 100644
> --- a/fs/hfs/part_tbl.c
> +++ b/fs/hfs/part_tbl.c
> @@ -9,6 +9,8 @@
>   * a patch contributed by Holger Schemel (aeglos@valinor.owl.de).
>   */
>  
> +#include <linux/blkdev.h>
> +
>  #include "hfs_fs.h"
>  
>  /*
> @@ -49,6 +51,22 @@ struct old_pmap {
>  	}	pdEntry[42];
>  } __packed;
>  
> +/*
> + * Check a partition map entry before following it.  The partition
> must
> + * be non-empty, start inside the device and start at or after
> @first:
> + * after the driver descriptor map in block 0 for an old-style map,
> and
> + * after the whole of a new-style map, whose size the first entry's
> + * pmMapBlkCnt gives (TN1189).  Every hop then moves forward, past
> the
> + * map entries just read, so hfs_mdb_get() cannot loop and reads
> each
> + * block of a map at most once.
> + */

Frankly speaking, comment is long and it only complicates everything. I
don't follow what hop means. Could we make the comment short, clear,
and more informative?

> +static bool hfs_part_valid(struct super_block *sb, sector_t base,
> +			   u64 first, u32 start, u32 size)

The set of argument is very confusing. As a result, it's really hard to
follow what we are checking and it is correct check or not. I think it
will be more clear to provide struct old_pmap pointer as argument. Why
not use part_start instead of base? What the first argument means?

> +{
> +	return start >= first && size &&
> +	       base + start < bdev_nr_sectors(sb->s_bdev);

We have part_size. Is it not the same as bdev_nr_sectors(sb->s_bdev)?

> +}
> +
>  /*
>   * hfs_part_find()
>   *
> @@ -77,12 +95,15 @@ int hfs_part_find(struct super_block *sb,
>  		p = pm->pdEntry;
>  		size = 42;
>  		for (i = 0; i < size; p++, i++) {
> -			if (p->pdStart && p->pdSize &&
> +			if (hfs_part_valid(sb, *part_start,
> HFS_DD_BLK + 1,

Do you mean HFS_PMAP_BLK here by HFS_DD_BLK + 1?

> +					   be32_to_cpu(p->pdStart),
> +					   be32_to_cpu(p->pdSize))
> &&
>  			    p->pdFSID ==
> cpu_to_be32(0x54465331)/*"TFS1"*/ &&
>  			    (HFS_SB(sb)->part < 0 || HFS_SB(sb)-
> >part == i)) {

This check becomes too long and complicated now. I think we need to
introduce some good checking function.

>  				*part_start += be32_to_cpu(p-
> >pdStart);
>  				*part_size = be32_to_cpu(p->pdSize);
>  				res = 0;
> +				break;
>  			}
>  		}
>  		break;
> @@ -95,6 +116,10 @@ int hfs_part_find(struct super_block *sb,
>  		size = be32_to_cpu(pm->pmMapBlkCnt);
>  		for (i = 0; i < size;) {
>  			if (!memcmp(pm->pmPartType,"Apple_HFS", 9)
> &&
> +			    hfs_part_valid(sb, *part_start,
> +					   HFS_PMAP_BLK + (u64)size,

Why exactly HFS_PMAP_BLK + (u64)size?

> +					   be32_to_cpu(pm-
> >pmPyPartStart),
> +					   be32_to_cpu(pm-
> >pmPartBlkCnt)) &&
>  			    (HFS_SB(sb)->part < 0 || HFS_SB(sb)-
> >part == i)) {

Ditto.

Thanks,
Slava.

>  				*part_start += be32_to_cpu(pm-
> >pmPyPartStart);
>  				*part_size = be32_to_cpu(pm-
> >pmPartBlkCnt);

Re: [PATCH v2 2/2] hfsplus: validate the wrapper and partition map before following them

[Viacheslav Dubeyko]

On Fri, 2026-10-02 at 00:21 +0800, Matthias Goergens wrote:
> hfsplus_read_wrapper() rereads the volume header through a bare "goto
> reread", following an HFS wrapper's embedded-volume descriptor or, if
> the header matches neither signature, the partition map found by
> hfs_part_find().  hfsplus_read_mdb() does not check the offset it
> returns, and hfs_part_find() does not check the start of a new-style
> map
> entry, so a descriptor or entry with a zero offset leaves part_start
> where it was and the mount loops forever.
> 
> Check both where they are parsed.  TN1150 places the embedded volume
> in
> the wrapper's allocation blocks, and the boot blocks, MDB and volume
> bitmap of an HFS volume are not part of any allocation block.  So
> hfsplus_read_mdb() now rejects a wrapper whose allocation blocks
> start
> at or before its MDB (drAlBlSt <= 2), whose embedded volume is empty,
> or
> whose embedded volume ends past the wrapper's last allocation block
> (drNmAlBlks).  hfs_part_find() gets the same check as in hfs: a
> partition must be non-empty, start inside the device and, for a
> new-style map, start after the map, which begins at block 1 and whose
> size the first entry's pmMapBlkCnt gives (TN1189).
> 
> A wrapper hop now moves part_start forward by at least three sectors,
> and a partition-table hop moves it past the map entries it has read. 
> So
> the loop ends, at the latest when a read goes past the end of the
> device, and the maps read on the way do not overlap.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com>
> ---
> v2: check the wrapper in hfsplus_read_mdb() and the entries in
> hfs_part_find() instead of limiting the hops in
> hfsplus_read_wrapper().
> 
> A wrapper whose embedded-volume descriptor points back at itself
> hangs
> the mount without this patch and fails at once with it:
> 
>   img=hfsplus-wrapper-loop.img
>   put() { printf "$2" | dd of=$img bs=1 seek=$1 conv=notrunc
> status=none; }
>   truncate --size=64K $img
>   put $((1024 + 0x00)) '\x42\x44'          # drSigWord 'BD'
>   put $((1024 + 0x0a)) '\x82\x00'          # drAtrb: SLOCK | SPARED
>   put $((1024 + 0x14)) '\x00\x00\x02\x00'  # drAlBlkSiz 512
>   put $((1024 + 0x7c)) '\x48\x2b'          # drEmbedSigWord 'H+'
>   put $((1024 + 0x7e)) '\x00\x00\x00\x64'  # drEmbedExtent: start 0,
> count 100
>   mount -o ro,loop -t hfsplus $img /mnt
> 
> The partition map from the hfs patch hangs an hfsplus mount the same
> way, with "-t hfsplus".
> 
>  fs/hfsplus/part_tbl.c      | 24 +++++++++++++++++++++++-
>  fs/hfsplus/wrapper.c       | 16 +++++++++++++++-
>  include/linux/hfs_common.h |  1 +
>  3 files changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/hfsplus/part_tbl.c b/fs/hfsplus/part_tbl.c
> index 9ec21664eda6b..ceba3139f1f56 100644
> --- a/fs/hfsplus/part_tbl.c
> +++ b/fs/hfsplus/part_tbl.c
> @@ -67,6 +67,22 @@ struct old_pmap {
>  	}	pdEntry[42];
>  } __packed;
>  
> +/*
> + * Check a partition map entry before following it.  The partition
> must
> + * be non-empty, start inside the device and start at or after
> @first:
> + * after the driver descriptor map in block 0 for an old-style map,
> and
> + * after the whole of a new-style map, whose size the first entry's
> + * pmMapBlkCnt gives (TN1189).  Every hop then moves forward, past
> the
> + * map entries just read, so hfsplus_read_wrapper() cannot loop and
> + * reads each block of a map at most once.
> + */
> +static bool hfs_part_valid(struct super_block *sb, sector_t base,
> +			   u64 first, u32 start, u32 size)
> +{
> +	return start >= first && size &&
> +	       base + start < bdev_nr_sectors(sb->s_bdev);
> +}

Please, see my comments for HFS patch. I have pretty the same comments
here.

> +
>  static int hfs_parse_old_pmap(struct super_block *sb, struct
> old_pmap *pm,
>  		sector_t *part_start, sector_t *part_size)
>  {
> @@ -76,7 +92,9 @@ static int hfs_parse_old_pmap(struct super_block
> *sb, struct old_pmap *pm,
>  	for (i = 0; i < 42; i++) {
>  		struct old_pmap_entry *p = &pm->pdEntry[i];
>  
> -		if (p->pdStart && p->pdSize &&
> +		if (hfs_part_valid(sb, *part_start, HFS_DD_BLK + 1,
> +				   be32_to_cpu(p->pdStart),
> +				   be32_to_cpu(p->pdSize)) &&
>  		    p->pdFSID == cpu_to_be32(0x54465331)/*"TFS1"*/
> &&
>  		    (sbi->part < 0 || sbi->part == i)) {
>  			*part_start += be32_to_cpu(p->pdStart);
> @@ -99,6 +117,10 @@ static int hfs_parse_new_pmap(struct super_block
> *sb, void *buf,
>  
>  	do {
>  		if (!memcmp(pm->pmPartType, "Apple_HFS", 9) &&
> +		    hfs_part_valid(sb, *part_start,
> +				   HFS_PMAP_BLK + (u64)size,
> +				   be32_to_cpu(pm->pmPyPartStart),
> +				   be32_to_cpu(pm->pmPartBlkCnt)) &&
>  		    (sbi->part < 0 || sbi->part == i)) {
>  			*part_start += be32_to_cpu(pm-
> >pmPyPartStart);
>  			*part_size = be32_to_cpu(pm->pmPartBlkCnt);
> diff --git a/fs/hfsplus/wrapper.c b/fs/hfsplus/wrapper.c
> index 30cf4fe78b3d2..35cd2563dfdfe 100644
> --- a/fs/hfsplus/wrapper.c
> +++ b/fs/hfsplus/wrapper.c
> @@ -66,7 +66,7 @@ int hfsplus_submit_bio(struct super_block *sb,
> sector_t sector,
>  static int hfsplus_read_mdb(void *bufptr, struct hfsplus_wd *wd)
>  {
>  	u32 extent;
> -	u16 attrib;
> +	u16 attrib, nmalblks;
>  	__be16 sig;
>  
>  	sig = *(__be16 *)(bufptr + HFSP_WRAPOFF_EMBEDSIG);
> @@ -87,11 +87,25 @@ static int hfsplus_read_mdb(void *bufptr, struct
> hfsplus_wd *wd)
>  		return 0;
>  	wd->ablk_start =
>  		be16_to_cpu(*(__be16 *)(bufptr +
> HFSP_WRAPOFF_ABLKSTART));
> +	nmalblks = be16_to_cpu(*(__be16 *)(bufptr +
> HFSP_WRAPOFF_NMALBLKS));
>  
>  	extent = get_unaligned_be32(bufptr + HFSP_WRAPOFF_EMBEDEXT);
>  	wd->embed_start = (extent >> 16) & 0xFFFF;
>  	wd->embed_count = extent & 0xFFFF;
>  
> +	/*
> +	 * The boot blocks, MDB and volume bitmap of an HFS volume
> are not
> +	 * part of any allocation block, and the embedded volume
> occupies
> +	 * allocation blocks of the wrapper (TN1150).  So the
> embedded
> +	 * volume starts after the wrapper's MDB and ends inside the
> +	 * wrapper.
> +	 */
> +	if (wd->ablk_start <= HFS_MDB_BLK)
> +		return 0;
> +	if (!wd->embed_count ||
> +	    wd->embed_start + wd->embed_count > nmalblks)
> +		return 0;
> +
>  	return 1;
>  }
>  
> diff --git a/include/linux/hfs_common.h b/include/linux/hfs_common.h
> index d6a615e74b26e..123475efe5b37 100644
> --- a/include/linux/hfs_common.h
> +++ b/include/linux/hfs_common.h
> @@ -44,6 +44,7 @@
>  
>  #define HFSP_WRAPOFF_SIG		0x00
>  #define HFSP_WRAPOFF_ATTRIB		0x0A
> +#define HFSP_WRAPOFF_NMALBLKS		0x12

Does this value is based on specification? Where does it come from?

Thanks,
Slava.

>  #define HFSP_WRAPOFF_ABLKSIZE		0x14
>  #define HFSP_WRAPOFF_ABLKSTART		0x1C
>  #define HFSP_WRAPOFF_EMBEDSIG		0x7C