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 <slava@dubeyko.com>]

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.