Re: [PATCH v3] hfsplus: validate btree bitmap during mount and handle corruption gracefully

[Viacheslav Dubeyko]

On 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@xxxxxxxxxxxxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxxxxx>
> ---
> 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