[Bug report] hfsplus: extents-tree search_key buffer is 24 bytes but hfs_bnode_read_key() bounds key reads against 518 - slab OOB write on fsync

[Bug report] hfsplus: extents-tree search_key buffer is 24 bytes but hfs_bnode_read_key() bounds key reads against 518 - slab OOB write on fsync

[Guikun Yang]

Hi,

Appending to a file on a crafted HFS+ image ends in a
slab-out-of-bounds write into a 24-byte allocation. The extents tree
sizes its search key buffer from max_key_len, but
hfs_bnode_read_key() bounds the copy against the generic union size
(518), so any on-disk key length between 10 and 516 passes the bound
check and overflows the extents-tree buffer - and the reject path
itself memsets 518 bytes into the same 24-byte allocation.

  BUG: KASAN: slab-out-of-bounds in hfsplus_bnode_read+0x30/0x5f0
  Write of size 502 at addr ffff888005fb4100 by task init/1
   hfsplus_bnode_read -> hfs_bnode_read_key
   hfs_brec_insert -> __hfsplus_ext_write_extent
   hfsplus_ext_write_extent -> hfsplus_write_inode
   hfsplus_file_fsync -> do_fsync
  Allocated by task 1:
   hfsplus_find_init (kzalloc 24) -> hfsplus_ext_write_extent
  The buggy address is located 0 bytes inside of
   allocated 24-byte region, cache kmalloc-32

## The size mismatch

hfs_find_init() sizes the search key per tree (fs/hfsplus/bfind.c:22):

  ptr = kzalloc(tree->max_key_len * 2 + 4, GFP_KERNEL);

The extents tree's max_key_len is validated at mount to
HFSPLUS_EXT_KEYLEN - 2 = 10 (fs/hfsplus/btree.c:318), so its search
key is 24 bytes. The catalog tree's is 516*2+4 = 1036.

hfs_bnode_read_key() bounds the read with the generic key union
(fs/hfsplus/bnode.c):

  if (node->type == HFS_NODE_LEAF || ...)
          key_len = hfs_bnode_read_u16(node, off) + 2;   /* raw disk u16 */
  else
          key_len = tree->max_key_len + 2;

  if (key_len > sizeof(hfsplus_btree_key) || key_len < 1) {
          memset(key, 0, sizeof(hfsplus_btree_key));     /* 518 */
          pr_err("hfsplus: Invalid key length: %u\n", key_len);
          return;
  }
  hfs_bnode_read(node, key, off, key_len);

sizeof(hfsplus_btree_key) is the union of cat (518), attr (268) and
ext (12) keys - 518. That bound is correct only where the buffer is
the catalog one. For the extents tree it means:

- key_len 10..516 passes the check and memcpy writes key_len bytes
  into a 24-byte allocation (502 with a crafted key length of 500)
- key_len 517..65535, or any odd value, takes the reject path, which
  memsets 518 bytes into the same 24-byte allocation

Both paths are out-of-bounds writes; the second one fires even for a
"rejected" key.

## Why the disk value reaches this point

The bnode loader validates key_size only against the record entry
size (fs/hfsplus/bnode.c:601):

  key_size = hfs_bnode_read_u16(node, off) + 2;
  if (key_size >= entry_size || key_size & 1)
          goto node_error;

A record with key length 500 in a 3814-byte slot passes: 502 < 3814
and even. The binary-search path that does check against
max_key_len (hfs_brec_keylen(), fs/hfsplus/brec.c:71) only runs for
the records it probes; placing the crafted record's cnid above the
search key keeps the search converging past it without ever probing
it. When an insert then forces a node split, hfs_bnode_split() moves
records verbatim into the new node, and brec.c:169 calls

  hfs_bnode_read_key(new_node, fd->search_key, 14);

on the new node's first record - the crafted one - with no
max_key_len bound in between.

## Reproducer

The trigger is write(2) + fsync on a file whose data fork has 8
extents followed by a non-contiguous allocation, forcing
hfsplus_ext_write_extent() to insert into the extents tree and split
the crafted leaf:

  # positive: ext leaf rec2 key_len = 500
  python3 mk_hfsplus_ext.py base_file.img run.img
  python3 trigger_write.py run.img pos.log     # KASAN OOB write 502

  # negative: same image, only that field differs
  python3 mk_hfsplus_ext.py base_file.img run.img neg   # key_len = 10
  python3 trigger_write.py run.img neg.log      # clean, write returns 4096

The negative control differs from the positive image in exactly one
field - the crafted key length - and parses normally, printing
"vda1 <lv...>"-equivalent extents output with no KASAN report.
Positive 4/4, negative 3/3. Each run regenerates from the base image
(the mount consumes it by writing the real extents tree back).

## Relation to the existing fixes

bb5e07cb92 ("hfs/hfsplus: fix slab-out-of-bounds in
hfs_bnode_read_key", 2024) added the 518 bound and the memset reject
path after syzbot hit the same function through fs/hfs and the
catalog tree, where the buffer is 1036 bytes and the bound holds.
The extents tree's buffer (24 bytes) was never covered by that fix -
the bound is larger than the buffer it protects, so the check is
inert there and the reject path is itself an overflow.

## Suggested fix

Bound against the actual buffer, not the union:

  if (key_len > tree->max_key_len + 2 || key_len < 1) {
          memset(key, 0, tree->max_key_len + 2);
          pr_err("hfsplus: Invalid key length: %u\n", key_len);
          return;
  }

This covers both the memcpy and the reject-path memset for every
tree with one change. The extents tree additionally can never see
a key_len above 12 on a well-formed volume (HFSPLUS_EXT_KEYLEN),
so max_key_len + 2 is the natural bound.

The reproducer (image builder for positive and negative cases,
serial logs) is available on request.

Tested on 7.3.0; fs/hfsplus/bnode.c is unchanged in current
mainline as of 2026-11-12.

Thanks,
Guikun Yang

Re: [Bug report] hfsplus: extents-tree search_key buffer is 24 bytes but hfs_bnode_read_key() bounds key reads against 518 - slab OOB write on fsync

[Viacheslav Dubeyko]

On Thu, 2026-11-12 at 22:00 +0800, Guikun Yang wrote:
> Hi,
> 
> Appending to a file on a crafted HFS+ image ends in a
> slab-out-of-bounds write into a 24-byte allocation. The extents tree
> sizes its search key buffer from max_key_len, but
> hfs_bnode_read_key() bounds the copy against the generic union size
> (518), so any on-disk key length between 10 and 516 passes the bound
> check and overflows the extents-tree buffer - and the reject path
> itself memsets 518 bytes into the same 24-byte allocation.
> 
>   BUG: KASAN: slab-out-of-bounds in hfsplus_bnode_read+0x30/0x5f0
>   Write of size 502 at addr ffff888005fb4100 by task init/1
>    hfsplus_bnode_read -> hfs_bnode_read_key
>    hfs_brec_insert -> __hfsplus_ext_write_extent
>    hfsplus_ext_write_extent -> hfsplus_write_inode
>    hfsplus_file_fsync -> do_fsync
>   Allocated by task 1:
>    hfsplus_find_init (kzalloc 24) -> hfsplus_ext_write_extent
>   The buggy address is located 0 bytes inside of
>    allocated 24-byte region, cache kmalloc-32
> 
> ## The size mismatch
> 
> hfs_find_init() sizes the search key per tree
> (fs/hfsplus/bfind.c:22):
> 
>   ptr = kzalloc(tree->max_key_len * 2 + 4, GFP_KERNEL);
> 
> The extents tree's max_key_len is validated at mount to
> HFSPLUS_EXT_KEYLEN - 2 = 10 (fs/hfsplus/btree.c:318), so its search
> key is 24 bytes. The catalog tree's is 516*2+4 = 1036.
> 
> hfs_bnode_read_key() bounds the read with the generic key union
> (fs/hfsplus/bnode.c):
> 
>   if (node->type == HFS_NODE_LEAF || ...)
>           key_len = hfs_bnode_read_u16(node, off) + 2;   /* raw disk
> u16 */
>   else
>           key_len = tree->max_key_len + 2;
> 
>   if (key_len > sizeof(hfsplus_btree_key) || key_len < 1) {
>           memset(key, 0, sizeof(hfsplus_btree_key));     /* 518 */
>           pr_err("hfsplus: Invalid key length: %u\n", key_len);
>           return;
>   }
>   hfs_bnode_read(node, key, off, key_len);
> 
> sizeof(hfsplus_btree_key) is the union of cat (518), attr (268) and
> ext (12) keys - 518. That bound is correct only where the buffer is
> the catalog one. For the extents tree it means:
> 
> - key_len 10..516 passes the check and memcpy writes key_len bytes
>   into a 24-byte allocation (502 with a crafted key length of 500)
> - key_len 517..65535, or any odd value, takes the reject path, which
>   memsets 518 bytes into the same 24-byte allocation
> 
> Both paths are out-of-bounds writes; the second one fires even for a
> "rejected" key.
> 
> ## Why the disk value reaches this point
> 
> The bnode loader validates key_size only against the record entry
> size (fs/hfsplus/bnode.c:601):
> 
>   key_size = hfs_bnode_read_u16(node, off) + 2;
>   if (key_size >= entry_size || key_size & 1)
>           goto node_error;
> 
> A record with key length 500 in a 3814-byte slot passes: 502 < 3814
> and even. The binary-search path that does check against
> max_key_len (hfs_brec_keylen(), fs/hfsplus/brec.c:71) only runs for
> the records it probes; placing the crafted record's cnid above the
> search key keeps the search converging past it without ever probing
> it. When an insert then forces a node split, hfs_bnode_split() moves
> records verbatim into the new node, and brec.c:169 calls
> 
>   hfs_bnode_read_key(new_node, fd->search_key, 14);
> 
> on the new node's first record - the crafted one - with no
> max_key_len bound in between.
> 
> ## Reproducer
> 
> The trigger is write(2) + fsync on a file whose data fork has 8
> extents followed by a non-contiguous allocation, forcing
> hfsplus_ext_write_extent() to insert into the extents tree and split
> the crafted leaf:
> 
>   # positive: ext leaf rec2 key_len = 500
>   python3 mk_hfsplus_ext.py base_file.img run.img
>   python3 trigger_write.py run.img pos.log     # KASAN OOB write 502
> 
>   # negative: same image, only that field differs
>   python3 mk_hfsplus_ext.py base_file.img run.img neg   # key_len =
> 10
>   python3 trigger_write.py run.img neg.log      # clean, write
> returns 4096
> 
> The negative control differs from the positive image in exactly one
> field - the crafted key length - and parses normally, printing
> "vda1 <lv...>"-equivalent extents output with no KASAN report.
> Positive 4/4, negative 3/3. Each run regenerates from the base image
> (the mount consumes it by writing the real extents tree back).
> 
> ## Relation to the existing fixes
> 
> bb5e07cb92 ("hfs/hfsplus: fix slab-out-of-bounds in
> hfs_bnode_read_key", 2024) added the 518 bound and the memset reject
> path after syzbot hit the same function through fs/hfs and the
> catalog tree, where the buffer is 1036 bytes and the bound holds.
> The extents tree's buffer (24 bytes) was never covered by that fix -
> the bound is larger than the buffer it protects, so the check is
> inert there and the reject path is itself an overflow.
> 
> ## Suggested fix
> 
> Bound against the actual buffer, not the union:
> 
>   if (key_len > tree->max_key_len + 2 || key_len < 1) {
>           memset(key, 0, tree->max_key_len + 2);
>           pr_err("hfsplus: Invalid key length: %u\n", key_len);
>           return;
>   }
> 
> This covers both the memcpy and the reject-path memset for every
> tree with one change. The extents tree additionally can never see
> a key_len above 12 on a well-formed volume (HFSPLUS_EXT_KEYLEN),
> so max_key_len + 2 is the natural bound.
> 
> The reproducer (image builder for positive and negative cases,
> serial logs) is available on request.
> 
> Tested on 7.3.0; fs/hfsplus/bnode.c is unchanged in current
> mainline as of 2026-11-12.
> 


Please, send the formal patch for review and testing.

Thanks,
Slava.

[PATCH] hfsplus: bound hfs_bnode_read_key() against the per-tree key buffer size

[Guikun Yang]

hfs_find_init() sizes the per-tree key buffers from max_key_len:

	ptr = kzalloc(tree->max_key_len * 2 + 4, GFP_KERNEL);

which is 24 bytes for the extents tree (max_key_len 10) and 1036 for
the catalog tree. hfs_bnode_read_key() however bounds both the copy
and the reject-path memset against the generic key union:

	if (key_len > sizeof(hfsplus_btree_key) || key_len < 1) {
		memset(key, 0, sizeof(hfsplus_btree_key));
		pr_err("hfsplus: Invalid key length: %u\n", key_len);
		return;
	}

	hfs_bnode_read(node, key, off, key_len);

sizeof(hfsplus_btree_key) is 518 (the catalog key size; the union of
cat/ext/attr keys). For the extents tree the bound is larger than the
24-byte buffer it protects, so the check is inert there: an on-disk
key length between 10 and 516 passes it and the memcpy overflows the
allocation, and a "rejected" key length takes the memset path, which
writes 518 bytes into the same 24-byte allocation. Both paths were
observed as KASAN slab-out-of-bounds writes on the writeback path of
a crafted image:

  BUG: KASAN: slab-out-of-bounds in hfsplus_bnode_read+0x30/0x5f0
  Write of size 502 at addr ... by task init/1
   hfsplus_bnode_read -> hfs_bnode_read_key
   hfs_brec_insert -> __hfsplus_ext_write_extent
   hfsplus_ext_write_extent -> hfsplus_write_inode -> hfsplus_file_fsync
  allocated 24-byte region (kmalloc-32) by hfsplus_find_init

Bound both the copy and the memset against the actual buffer size,
tree->max_key_len + 2, which is the natural limit for every tree
(the extents tree can never see a key above HFSPLUS_EXT_KEYLEN on a
well-formed volume; the catalog and attribute trees already allocate
buffers that cover their full key sizes).

Tested with the reproducer from the bug report: the extents-tree
image that previously wrote 502 bytes into the 24-byte allocation now
fails the bound and logs "Invalid key length"; the negative control
(NUL-terminated short key) still parses and writes normally.

Reported-by: Guikun Yang <yangguikun19@qq.com>
Signed-off-by: Guikun Yang <yangguikun19@qq.com>
---
diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
index 3a1a1fa6f..0774b945c 100644
--- a/fs/hfsplus/bnode.c
+++ b/fs/hfsplus/bnode.c
@@ -84,8 +84,8 @@ void hfs_bnode_read_key(struct hfs_bnode *node, void *key, u32 off)
 	else
 		key_len = tree->max_key_len + 2;
 
-	if (key_len > sizeof(hfsplus_btree_key) || key_len < 1) {
-		memset(key, 0, sizeof(hfsplus_btree_key));
+	if (key_len > tree->max_key_len + 2 || key_len < 1) {
+		memset(key, 0, tree->max_key_len + 2);
 		pr_err("hfsplus: Invalid key length: %u\n", key_len);
 		return;
 	}