Re: [PATCH v2] hfsplus: free failed b-tree nodes in hfs_bnode_put()

[Viacheslav Dubeyko <slava@dubeyko.com>]

On Tue, 2026-11-03 at 11:25 +0900, Song jeonghak wrote:
> hfs_bnode_find() flags a node it fails to load with HFS_BNODE_ERROR
> and
> then drops its reference with hfs_bnode_put(). When the last
> reference
> goes away, hfs_bnode_put() only releases nodes flagged
> HFS_BNODE_DELETED.
> Every other node stays hashed in tree->node_hash as a cache entry and
> is
> freed later by hfs_btree_close().
> 
> That is fine while the tree is open, but hfs_btree_open() looks up
> the
> header node itself. If that lookup fails, hfs_btree_open() unwinds
> with
> iput() and kfree(tree) and never calls hfs_btree_close(). The failed
> node
> is still hashed, nothing references it anymore, and it is leaked.
> syzbot
> triggers this by mounting a corrupted image:
> 
>   hfsplus: failed to load extents file
>   BUG: memory leak
>   unreferenced object 0xffff8881040930c0 (size 96):
>     __hfs_bnode_create+0x57/0x310 fs/hfsplus/bnode.c:476
>     hfsplus_bnode_find+0x138/0x660 fs/hfsplus/bnode.c:554
>     hfsplus_btree_open+0x30b/0x5d0 fs/hfsplus/btree.c:384
>     hfsplus_fill_super+0x272/0x880 fs/hfsplus/super.c:546
> 
> Free a node that is flagged HFS_BNODE_ERROR when its last reference
> is
> dropped instead of leaving it hashed. This follows the existing
> HFS_BNODE_DELETED handling: unhash the node under hash_lock, drop the
> lock, then free it. Unlike a deleted node, a node that failed to load
> is
> still part of the on-disk tree; only its in-memory copy is dropped.
> Its
> bit in the allocation map therefore stays set and its on-disk
> contents
> are left alone, so neither hfs_bmap_free() nor hfs_bnode_clear() is
> called for it. A later hfs_bnode_find() of the same node number now
> reads the node from disk again instead of getting the cached failure.
> 
> Tested with the syzbot C reproducer in QEMU (x86_64, kmemleak) on
> next-20261023: the leak was reported in 3 of 3 runs before the change
> and
> in 0 of 3 runs after it, two of which ran with slub_debug=FZPU. On a
> valid image, creating 3000 files, deleting half of them, and
> remounting
> gives identical results before and after, and fsck.hfsplus reports
> the
> volume as OK. checkpatch.pl --strict and sparse report nothing for
> bnode.c. This was not tested with KASAN or with concurrent access to
> the
> same node.
> 
> Claude (Anthropic) was used to help with the root cause analysis, the
> reproduction setup and drafting this changelog. The code change
> itself
> was written by the submitter.
> 
> Fixes: 8ad2c6a36ac4 ("hfsplus: validate b-tree node 0 bitmap at mount
> time")
> Reported-by: syzbot+98547b0428b6a6a3467c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=98547b0428b6a6a3467c
> Signed-off-by: Song jeonghak <thdwjdgkr123@gmail.com>
> ---
> Changes in v2:
> - Corrected the changelog. v1 said a failed node "has no b-tree map
> bit
>   to release and no contents to zero out". That is wrong: the node is
>   still part of the on-disk tree, so its map bit is set and its
> contents
>   are on disk. They are left alone on purpose. No code change.
> - Kept HFS_BNODE_ERROR as its own branch rather than folding it into
>   the HFS_BNODE_DELETED one as Markus Elfring suggested, so the fix
> does
>   not touch the existing HFS_BNODE_DELETED path. I can merge them if
>   that is preferred.
> - fs/hfsplus is unchanged between next-20261023 and next-20261102 and
>   the patch applies as is, so the test results above still hold.
> 
> v1:
> https://lore.kernel.org/all/20261026152139.458677-1-thdwjdgkr123@gmail.com/
> 
> Viacheslav, v1 did not show up in your mail client. It was sent with
> git send-email as plain text and is on lore at the link above. This
> version is sent the same way.
> 
> This takes the direction Viacheslav suggested while reviewing Edward
> Adam Davis's earlier attempt, "[PATCH] hfsplus: Supports freeing
> newly
> created tree head" (April 2026): look at HFS_BNODE_ERROR in
> hfs_bnode_put() rather than special-casing the header node. That
> thread
> did not get a follow-up version.
> Link:
> https://lore.kernel.org/all/tencent_C4F16CD096B0EAEAEE5ACDB07A266E382C0A@qq.com/
> 
> fs/hfs has the same structure: hfs_bnode_put() has no HFS_BNODE_ERROR
> handling, and hfs_btree_open() looks up the header node and then
> frees
> the tree without hfs_btree_close() on failure. I have not reproduced
> a
> leak there, so this patch leaves fs/hfs alone.
> 
> sparse reports a few warnings elsewhere in fs/hfsplus (ioctl.c and
> attributes.c) that are not touched by this patch.
> 
>  fs/hfsplus/bnode.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fs/hfsplus/bnode.c b/fs/hfsplus/bnode.c
> index 3a1a1fa6f2e2..3d159db229e5 100644
> --- a/fs/hfsplus/bnode.c
> +++ b/fs/hfsplus/bnode.c
> @@ -702,6 +702,14 @@ void hfs_bnode_put(struct hfs_bnode *node)
>  			hfs_bnode_free(node);
>  			return;
>  		}
> +
> +		if (test_bit(HFS_BNODE_ERROR, &node->flags)) {
> +			hfs_bnode_unhash(node);
> +			spin_unlock(&tree->hash_lock);
> +			hfs_bnode_free(node);
> +			return;
> +		}
> +
>  		spin_unlock(&tree->hash_lock);
>  	}
>  }

I've accepted this patchset [1] recently with some modifications on my
side. I assume that it has fixed the issue already. Am I wrong?

Thanks,
Slava.

[1]
https://lore.kernel.org/linux-fsdevel/17c099490060e40254ef25dacdad713d5356c2b7.camel@dubeyko.com/