fs/hfsplus/btree.c | 27 +++++++++++++++++++++++++++ include/linux/hfs_common.h | 2 ++ 2 files changed, 29 insertions(+)
Add bitmap validation during HFS+ btree open to detect corruption where
node 0 (header node) is not marked allocated.
Syzkaller reported issues with corrupted HFS+ images where the btree
allocation bitmap indicates that the header node is free. Node 0 must
always be allocated as it contains the btree header record and the
allocation bitmap itself. Violating this invariant can lead to kernel
panics or undefined behavior when the filesystem attempts to allocate
blocks or manipulate the btree.
The validation checks the node allocation bitmap in the btree header
node (record #2) and verifies that bit 7 (MSB) of the first byte is
set.
Implementation details:
- Perform validation inside hfs_btree_open() to allow identifying the
specific tree (Extents, Catalog, or Attributes) involved.
- Use hfs_bnode_find() and hfs_brec_lenoff() to safely access the
bitmap record using existing infrastructure, ensuring correct handling
of multi-page nodes and endianness.
- If corruption is detected, print a warning identifying the specific
btree and force the filesystem to mount read-only (SB_RDONLY).
This prevents kernel panics from corrupted syzkaller-generated images
while enabling data recovery by allowing the mount to proceed in
read-only mode rather than failing completely.
Reported-by: syzbot+1c8ff72d0cd8a50dfeaa@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=1c8ff72d0cd8a50dfeaa
Link: https://lore.kernel.org/all/b78c1e380a17186b73bc8641b139eca56a8de964.camel@ibm.com/
Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
---
v3:
- Moved validation logic inline into hfs_btree_open() to allow
reporting the specific corrupted tree ID.
- Replaced custom offset calculations with existing hfs_bnode_find()
and hfs_brec_lenoff() infrastructure to handle node sizes and
page boundaries correctly.
- Removed temporary 'btree_bitmap_corrupted' superblock flag; setup
SB_RDONLY directly upon detection.
- Moved logging to hfs_btree_open() to include the specific tree ID in
the warning message
- Used explicit bitwise check (&) instead of test_bit() to ensure
portability. test_bit() bit-numbering is architecture-dependent
(e.g., bit 0 vs bit 7 can swap meanings on BE vs LE), whereas
masking 0x80 consistently targets the MSB required by the HFS+
on-disk format.
v2:
- Fix compiler warning about comparing u16 bitmap_off with PAGE_SIZE which
can exceed u16 maximum on some architectures
- Cast bitmap_off to unsigned int for the PAGE_SIZE comparison to avoid
tautological constant-out-of-range comparison warning.
- Link: https://lore.kernel.org/oe-kbuild-all/202601251011.kJUhBF3P-lkp@intel.com/
fs/hfsplus/btree.c | 27 +++++++++++++++++++++++++++
include/linux/hfs_common.h | 2 ++
2 files changed, 29 insertions(+)
diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
index 229f25dc7c49..ae81608ba3cf 100644
--- a/fs/hfsplus/btree.c
+++ b/fs/hfsplus/btree.c
@@ -135,9 +135,12 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
struct hfs_btree *tree;
struct hfs_btree_header_rec *head;
struct address_space *mapping;
+ struct hfs_bnode *node;
+ u16 len, bitmap_off;
struct inode *inode;
struct page *page;
unsigned int size;
+ u8 bitmap_byte;
tree = kzalloc(sizeof(*tree), GFP_KERNEL);
if (!tree)
@@ -242,6 +245,30 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
kunmap_local(head);
put_page(page);
+
+ /*
+ * Validate bitmap: node 0 (header node) must be marked allocated.
+ */
+
+ node = hfs_bnode_find(tree, 0);
+ if (IS_ERR(node))
+ goto free_inode;
+
+ len = hfs_brec_lenoff(node,
+ HFSPLUS_BTREE_HDR_MAP_REC, &bitmap_off);
+
+ if (len != 0 && bitmap_off >= sizeof(struct hfs_bnode_desc)) {
+ hfs_bnode_read(node, &bitmap_byte, bitmap_off, 1);
+ if (!(bitmap_byte & HFSPLUS_BTREE_NODE0_BIT)) {
+ pr_warn("(%s): Btree 0x%x bitmap corruption detected, forcing read-only.\n",
+ sb->s_id, id);
+ pr_warn("Run fsck.hfsplus to repair.\n");
+ sb->s_flags |= SB_RDONLY;
+ }
+ }
+
+ hfs_bnode_put(node);
+
return tree;
fail_page:
diff --git a/include/linux/hfs_common.h b/include/linux/hfs_common.h
index dadb5e0aa8a3..8d21d476cb57 100644
--- a/include/linux/hfs_common.h
+++ b/include/linux/hfs_common.h
@@ -510,7 +510,9 @@ struct hfs_btree_header_rec {
#define HFSPLUS_NODE_MXSZ 32768
#define HFSPLUS_ATTR_TREE_NODE_SIZE 8192
#define HFSPLUS_BTREE_HDR_NODE_RECS_COUNT 3
+#define HFSPLUS_BTREE_HDR_MAP_REC 2 /* Map (bitmap) record in header node */
#define HFSPLUS_BTREE_HDR_USER_BYTES 128
+#define HFSPLUS_BTREE_NODE0_BIT 0x80
/* btree key type */
#define HFSPLUS_KEY_CASEFOLDING 0xCF /* case-insensitive */
--
2.34.1On Sat, 2026-01-31 at 19:34 +0530, Shardul Bankar wrote:
> Add bitmap validation during HFS+ btree open to detect corruption where
> node 0 (header node) is not marked allocated.
>
> Syzkaller reported issues with corrupted HFS+ images where the btree
> allocation bitmap indicates that the header node is free. Node 0 must
> always be allocated as it contains the btree header record and the
> allocation bitmap itself. Violating this invariant can lead to kernel
> panics or undefined behavior when the filesystem attempts to allocate
> blocks or manipulate the btree.
>
> The validation checks the node allocation bitmap in the btree header
> node (record #2) and verifies that bit 7 (MSB) of the first byte is
> set.
>
> Implementation details:
> - Perform validation inside hfs_btree_open() to allow identifying the
> specific tree (Extents, Catalog, or Attributes) involved.
> - Use hfs_bnode_find() and hfs_brec_lenoff() to safely access the
> bitmap record using existing infrastructure, ensuring correct handling
> of multi-page nodes and endianness.
> - If corruption is detected, print a warning identifying the specific
> btree and force the filesystem to mount read-only (SB_RDONLY).
>
> This prevents kernel panics from corrupted syzkaller-generated images
> while enabling data recovery by allowing the mount to proceed in
> read-only mode rather than failing completely.
>
> Reported-by: syzbot+1c8ff72d0cd8a50dfeaa@syzkaller.appspotmail.com
> Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__syzkaller.appspot.com_bug-3Fextid-3D1c8ff72d0cd8a50dfeaa&d=DwIDAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=ZiJOY272ajYn0UgEhQq0U3C9KoL8bXVIz-hEdqVFFyq4-BUdF-Y9LyG7SY6KXPxy&s=-ky4imqw2w8Seb2r8c8J_0WPl8IixUJ_l5gd_Qa5jaY&e=
> Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_b78c1e380a17186b73bc8641b139eca56a8de964.camel-40ibm.com_&d=DwIDAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=ZiJOY272ajYn0UgEhQq0U3C9KoL8bXVIz-hEdqVFFyq4-BUdF-Y9LyG7SY6KXPxy&s=i7Wn6enf_IfV0HJbT1gEQzRANQeS2mYs7PxZCfeFDpo&e=
> Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
> ---
> v3:
> - Moved validation logic inline into hfs_btree_open() to allow
> reporting the specific corrupted tree ID.
> - Replaced custom offset calculations with existing hfs_bnode_find()
> and hfs_brec_lenoff() infrastructure to handle node sizes and
> page boundaries correctly.
> - Removed temporary 'btree_bitmap_corrupted' superblock flag; setup
> SB_RDONLY directly upon detection.
> - Moved logging to hfs_btree_open() to include the specific tree ID in
> the warning message
> - Used explicit bitwise check (&) instead of test_bit() to ensure
> portability. test_bit() bit-numbering is architecture-dependent
> (e.g., bit 0 vs bit 7 can swap meanings on BE vs LE), whereas
> masking 0x80 consistently targets the MSB required by the HFS+
> on-disk format.
> v2:
> - Fix compiler warning about comparing u16 bitmap_off with PAGE_SIZE which
> can exceed u16 maximum on some architectures
> - Cast bitmap_off to unsigned int for the PAGE_SIZE comparison to avoid
> tautological constant-out-of-range comparison warning.
> - Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_oe-2Dkbuild-2Dall_202601251011.kJUhBF3P-2Dlkp-40intel.com_&d=DwIDAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=ZiJOY272ajYn0UgEhQq0U3C9KoL8bXVIz-hEdqVFFyq4-BUdF-Y9LyG7SY6KXPxy&s=xyhPoIYqdHtYWxSD6xwNESfdIbOCKcu-xjE10KCMsAk&e=
>
> fs/hfsplus/btree.c | 27 +++++++++++++++++++++++++++
> include/linux/hfs_common.h | 2 ++
> 2 files changed, 29 insertions(+)
>
> diff --git a/fs/hfsplus/btree.c b/fs/hfsplus/btree.c
> index 229f25dc7c49..ae81608ba3cf 100644
> --- a/fs/hfsplus/btree.c
> +++ b/fs/hfsplus/btree.c
> @@ -135,9 +135,12 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
> struct hfs_btree *tree;
> struct hfs_btree_header_rec *head;
> struct address_space *mapping;
> + struct hfs_bnode *node;
> + u16 len, bitmap_off;
> struct inode *inode;
> struct page *page;
> unsigned int size;
> + u8 bitmap_byte;
>
> tree = kzalloc(sizeof(*tree), GFP_KERNEL);
> if (!tree)
> @@ -242,6 +245,30 @@ struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id)
>
> kunmap_local(head);
> put_page(page);
> +
> + /*
> + * Validate bitmap: node 0 (header node) must be marked allocated.
> + */
> +
> + node = hfs_bnode_find(tree, 0);
If you introduce named constant for herder node, then you don't need add this
comment. And I don't like hardcoded value, anyway. :)
> + if (IS_ERR(node))
> + goto free_inode;
> +
> + len = hfs_brec_lenoff(node,
> + HFSPLUS_BTREE_HDR_MAP_REC, &bitmap_off);
> +
> + if (len != 0 && bitmap_off >= sizeof(struct hfs_bnode_desc)) {
If we read incorrect len and/or bitmap_off, then it sounds like corruption too.
We need to process this issue somehow but you ignore this, currently. ;)
> + hfs_bnode_read(node, &bitmap_byte, bitmap_off, 1);
I assume that 1 is the size of byte, then sizeof(u8) or sizeof(bitmap_byte)
could look much better than hardcoded value.
> + if (!(bitmap_byte & HFSPLUS_BTREE_NODE0_BIT)) {
Why don't use the test_bit() [1] here? I believe that code will be more simple
in such case.
> + pr_warn("(%s): Btree 0x%x bitmap corruption detected, forcing read-only.\n",
I prefer to mention what do we mean by 0x%x. Currently, it looks complicated to
follow.
> + sb->s_id, id);
> + pr_warn("Run fsck.hfsplus to repair.\n");
> + sb->s_flags |= SB_RDONLY;
> + }
> + }
> +
> + hfs_bnode_put(node);
> +
> return tree;
>
> fail_page:
> diff --git a/include/linux/hfs_common.h b/include/linux/hfs_common.h
> index dadb5e0aa8a3..8d21d476cb57 100644
> --- a/include/linux/hfs_common.h
> +++ b/include/linux/hfs_common.h
> @@ -510,7 +510,9 @@ struct hfs_btree_header_rec {
> #define HFSPLUS_NODE_MXSZ 32768
> #define HFSPLUS_ATTR_TREE_NODE_SIZE 8192
> #define HFSPLUS_BTREE_HDR_NODE_RECS_COUNT 3
> +#define HFSPLUS_BTREE_HDR_MAP_REC 2 /* Map (bitmap) record in header node */
Maybe, HFSPLUS_BTREE_HDR_MAP_REC_INDEX?
> #define HFSPLUS_BTREE_HDR_USER_BYTES 128
> +#define HFSPLUS_BTREE_NODE0_BIT 0x80
Maybe, (1 << something) instead of 0x80? I am OK with constant too.
>
> /* btree key type */
> #define HFSPLUS_KEY_CASEFOLDING 0xCF /* case-insensitive */
Thanks,
Slava.
[1] https://elixir.bootlin.com/linux/v6.19-rc5/source/include/linux/bitops.h#L60
On Mon, 2026-02-02 at 20:52 +0000, Viacheslav Dubeyko wrote:
> On Sat, 2026-01-31 at 19:34 +0530, Shardul Bankar wrote:
> >
> > +
> > + /*
> > + * Validate bitmap: node 0 (header node) must be marked
> > allocated.
> > + */
> > +
> > + node = hfs_bnode_find(tree, 0);
>
> If you introduce named constant for herder node, then you don't need
> add this
> comment. And I don't like hardcoded value, anyway. :)
Hi Slava, thank you for the review.
Ack'ed. I will use HFSPLUS_TREE_HEAD (0) in v4.
> > + len = hfs_brec_lenoff(node,
> > + HFSPLUS_BTREE_HDR_MAP_REC, &bitmap_off);
> > +
> > + if (len != 0 && bitmap_off >= sizeof(struct
> > hfs_bnode_desc)) {
>
> If we read incorrect len and/or bitmap_off, then it sounds like
> corruption too.
> We need to process this issue somehow but you ignore this, currently.
> ;)
>
I agree that invalid offsets constitute corruption. However, properly
validating the structure of the record table and offsets is a larger
scope change. I prefer to keep this patch focused specifically on the
"unallocated node 0" vulnerability reported by syzbot. I am happy to
submit a follow-up patch to harden hfs_brec_lenoff usage. As per your
suggestion, ignoring this currently. ;)
> > + hfs_bnode_read(node, &bitmap_byte, bitmap_off, 1);
>
> I assume that 1 is the size of byte, then sizeof(u8) or
> sizeof(bitmap_byte)
> could look much better than hardcoded value.
Ack'ed. Changing to sizeof(bitmap_byte).
>
> > + if (!(bitmap_byte & HFSPLUS_BTREE_NODE0_BIT)) {
>
> Why don't use the test_bit() [1] here? I believe that code will be
> more simple
> in such case.
I reviewed test_bit(), but I believe the explicit mask is safer and
more correct here for three reasons:
1. Endianness:
The value we’re checking is an on-disk bitmap byte (MSB of the first
byte in the header map record). test_bit() is designed for CPU-native
memory bitmaps. HFS+ bitmaps use Big-Endian bit ordering (Node 0 is the
MSB/0x80). On Little-Endian architectures (like x86), test_bit(0, ...)
checks the LSB (0x01). Using it here could introduce bit-numbering
ambiguity.
For example, reading into an unsigned long:
unsigned long word = 0;
hfs_bnode_read(node, &word, bitmap_off, sizeof(word));
if (!test_bit(N, &word))
...
...but now N is not obviously “MSB of first on-disk byte”; it depends
on CPU endianness/bit numbering conventions, so it becomes easy to get
wrong.
2. Consistency with Existing HFS+ Bitmap Code:
The existing allocator code already handles on-disk bitmap bytes via
explicit masking (hfs_bmap_alloc uses 0x80, 0x40, ...), so for
consistency with existing on-disk bitmap handling and to avoid the
above ambiguity, I kept the explicit mask check here as well:
if (!(bitmap_byte & HFSPLUS_BTREE_NODE0_BIT)) (with
HFSPLUS_BTREE_NODE0_BIT = BIT(7) (or (1 <<7)))
3. Buffer safety:
Reading exactly 1 byte (u8) guarantees we never read more data than
strictly required, avoiding potential boundary issues.
Am I missing something here or does this make sense?
If there's a strong preference for bitops helpers, I could investigate
the big-endian bit helpers (*_be), but for this single-byte invariant
check, the explicit mask seems clearest and most consistent with
existing code.
>
> > + pr_warn("(%s): Btree 0x%x bitmap corruption
> > detected, forcing read-only.\n",
>
> I prefer to mention what do we mean by 0x%x. Currently, it looks
> complicated to
> follow.
Ack'ed. I am adding a lookup to print the human-readable tree name
(Catalog, Extents, Attributes) alongside the ID.
>
> > #define HFSPLUS_ATTR_TREE_NODE_SIZE 8192
> > #define HFSPLUS_BTREE_HDR_NODE_RECS_COUNT 3
> > +#define HFSPLUS_BTREE_HDR_MAP_REC 2 /* Map
> > (bitmap) record in header node */
>
> Maybe, HFSPLUS_BTREE_HDR_MAP_REC_INDEX?
Ack'ed.
>
> > #define HFSPLUS_BTREE_HDR_USER_BYTES 128
> > +#define HFSPLUS_BTREE_NODE0_BIT 0x80
>
> Maybe, (1 << something) instead of 0x80? I am OK with constant too.
Ack'ed, will use (1 << 7). Can also use BIT(7) if you prefer.
> Thanks,
Shardul
© 2016 - 2026 Red Hat, Inc.