linux-kernel - Re: [PATCH] hfsplus: fix missing hfs_bnode_get() in hfs_bnode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a817a3a65e5a0fe33dbdf1322f4909c3ff1edfcc.camel@mpiricsoftware.com>
Date: Tue, 16 Dec 2025 11:31:44 +0530
From: Shardul Bankar <shardul.b@...ricsoftware.com>
To: Viacheslav Dubeyko <Slava.Dubeyko@....com>, "zippel@...ux-m68k.org"
	 <zippel@...ux-m68k.org>, "glaubitz@...sik.fu-berlin.de"
	 <glaubitz@...sik.fu-berlin.de>, "linux-fsdevel@...r.kernel.org"
	 <linux-fsdevel@...r.kernel.org>, "slava@...eyko.com" <slava@...eyko.com>, 
	"frank.li@...o.com"
	 <frank.li@...o.com>
Cc: "akpm@...l.org" <akpm@...l.org>, "janak@...ricsoftware.com"
	 <janak@...ricsoftware.com>, "linux-kernel@...r.kernel.org"
	 <linux-kernel@...r.kernel.org>
Subject: Re:  [PATCH] hfsplus: fix missing hfs_bnode_get() in
 hfs_bnode_create()

On Mon, 2025-12-15 at 19:29 +0000, Viacheslav Dubeyko wrote:
> Frankly speaking, I don't see the fix here. You are trying to hide
> the issue but
> not fix it. This is situation of the wrong call because we sharing
> error message
> and call WARN_ON() here. And the critical question here: why do we
> call
> hfs_bnode_create() for already created node? Is it issue of tree-
> >node_hash? Or
> something wrong with logic that calls the hfs_bnode_create()? You
> don't suggest
> answer to this question(s). I've tried to debug likewise issue two
> months ago
> and I don't know the answer yet. So, you need to dive deeper in the
> issue or,
> please, convince that I am wrong here. Currently, your commit message
> doesn't
> convince me at all.
> 

Hi Slava,

Thank you for the review. You are absolutely correct- the panic is a
symptom of a deeper logic error where the allocator attempts to re-
allocate an existing node.

I have investigated the root cause using the Syzkaller reproducer and
analyzed the crash logs. I found two distinct issues that need to be
addressed, which I plan to submit as a v2 patch series.

1. The Root Cause: Corrupted Allocation Bitmap (Allocator Logic Error):
The Syzkaller-generated image has a corrupted allocation bitmap where
Node 0 (the Header Node) is marked as "Free" (0).

    Mechanism: hfs_bmap_alloc trusts the on-disk bitmap, sees bit 0 is
clear, and attempts to allocate Node 0.

    Conflict: It calls hfs_bnode_create(tree, 0). Since Node 0 is the
header, it is already in the hash table. hfs_bnode_create correctly
detects this and warns:
[41767.838946] hfsplus: new node 0 already hashed?
[41767.839097] WARNING: fs/hfsplus/bnode.c:631 at
hfsplus_bnode_create.cold+0x41/0x49

Proposed Fix (Patch 1): Modify hfs_bmap_alloc to explicitly guard
against allocating Node 0. Node 0 is the B-Tree header and is
structurally reserved; it should never be allocated as a record node,
regardless of what the bitmap claims.

2. The Crash: Unsafe Error Handling (Refcount Violation) Even though
the allocator shouldn't request Node 0, hfs_bnode_create currently
handles the "node exists" case unsafely.

    Mechanism: When it finds the existing node (the header), it prints
the warning but returns the pointer without incrementing the reference
count.

    Result: The caller receives a node pointer, uses it, and eventually
calls hfs_bnode_put. Since the refcount wasn't incremented, this leads
to a refcount underflow/panic.

    Evidence: The panic occurs later in the execution flow (e.g.,
inside hfs_bnode_split or hfsplus_create_cat), proving the system is
running with a "ticking time bomb" node pointer.
[41767.840709] kernel BUG at fs/hfsplus/bnode.c:676!
[41767.840751] RIP: 0010:hfsplus_bnode_put+0x4a0/0x5c0
[41767.840826] Call Trace:
[41767.840833]  hfs_btree_inc_height.isra.0+0x64e/0x8b0
[41767.840878]  hfsplus_brec_insert+0x97b/0xcf0

Proposed Fix (Patch 2): I still believe we should add
hfs_bnode_get(node) in hfs_bnode_create (the original patch). Even if
the allocator is fixed, hfs_bnode_create should be robust. If it
returns a valid pointer, it must guarantee that pointer has a valid
reference reference to prevent UAF/panics, consistent with the fix in
__hfs_bnode_create (commit 152af1142878).

Plan for v2:

    Patch 1: hfsplus: prevent allocation of header node (node 0) (Fixes
the logic error)

    Patch 2: hfsplus: fix missing hfs_bnode_get() in hfs_bnode_create()
(Fixes the crash safety)

Does this approach and analysis address your concerns?

Thanks, Shardul