[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