Re: [PATCH] hfsplus: fix missing hfs_bnode_get() in hfs_bnode_create()

[Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>]

On Sun, 2025-12-14 at 05:02 +0530, Shardul Bankar wrote:
> When hfs_bnode_create() finds an existing node in the hash table, it
> returns the node without incrementing its reference count. This causes
> the reference count to become inconsistent, leading to a kernel panic
> when hfs_bnode_put() is later called with refcnt=0:
> 
>     BUG_ON(!atomic_read(&node->refcnt))
> 
> This occurs because hfs_bmap_alloc() calls hfs_bnode_create() expecting
> to receive a node with a proper reference count, but if the node is
> already in the hash table, it is returned without the required refcnt
> increment.
> 
> Fix this by calling hfs_bnode_get() when returning an existing node,
> ensuring the reference count is properly incremented. This follows the
> same pattern as the fix in __hfs_bnode_create() (commit 152af1142878
>  ("hfsplus: fix missing hfs_bnode_get() in __hfs_bnode_create")).
> 
> Note: While finding an existing node in hfs_bnode_create() is unexpected
> (indicated by the pr_crit warning), we still need proper refcnt management
> to prevent crashes. The warning will still fire to alert about the
> underlying issue (e.g., bitmap corruption or logic error causing an
> existing node to be requested for allocation).
> 
> Link: https://syzkaller.appspot.com/bug?extid=1c8ff72d0cd8a50dfeaa  
> Fixes: 634725a92938 ("[PATCH] hfs: cleanup HFS+ prints")
> Signed-off-by: Shardul Bankar <shardul.b@mpiricsoftware.com>
> ---
>  fs/hfsplus/bnode.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
> index 191661af9677..e098e05add43 100644
> --- a/fs/hfsplus/bnode.c
> +++ b/fs/hfsplus/bnode.c
> @@ -629,6 +629,7 @@ struct hfs_bnode *hfs_bnode_create(struct hfs_btree *tree, u32 num)
>  	if (node) {
>  		pr_crit("new node %u already hashed?\n", num);
>  		WARN_ON(1);
> +		hfs_bnode_get(node);

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.

Thanks,
Slava.

>  		return node;
>  	}
>  	node = __hfs_bnode_create(tree, num);