Re: [PATCH v3 2/2] hfs: free cached B-tree nodes on hfs_btree_open() error path

[Viacheslav Dubeyko]

On Thu, 2026-10-01 at 23:38 +0200, Mahmut Emin Kurhan wrote:
> The classic HFS driver has the same B-tree node leak as hfsplus: on
> the
> hfs_btree_open() error path after hfs_bnode_find(tree,
> HFS_TREE_HEAD), an
> errored head node left in tree->node_hash is not freed because
> free_tree:
> does a bare kfree(tree) instead of walking the hash.
> 
> Apply the same fix: factor the node-hash freeing into
> hfs_bnode_hash_free()
> and call it from hfs_btree_close() and the hfs_btree_open() error
> path. The
> helper keeps the hash_lock serialization from commit dce0e0248205
> ("hfs/hfsplus:
> serialize B-tree close against folio release").
> 
> Found via coverage-guided fuzzing (syzkaller + kmemleak) by Noroxi.
> 
> Signed-off-by: Mahmut Emin Kurhan <guvenlik@xxxxxxxxxx>

Please, see my comments for HFS+ patch. This patch has the same issues.

Thanks,
Slava.

> ---
>  fs/hfs/btree.c | 40 +++++++++++++++++++++++-----------------
>  1 file changed, 23 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/hfs/btree.c b/fs/hfs/btree.c
> index 4f0ddc76e8..140cdb5ec6 100644
> --- a/fs/hfs/btree.c
> +++ b/fs/hfs/btree.c
> @@ -131,6 +131,27 @@ static int hfs_bmap_clear_bit(struct hfs_bnode
> *node, u32 node_bit_idx)
>  }
>  
>  /* Get a reference to a B*Tree and do some initial checks */
> +static void hfs_bnode_hash_free(struct hfs_btree *tree)
> +{
> +	struct hfs_bnode *node;
> +	int i;
> +
> +	for (i = 0; i < NODE_HASH_SIZE; i++) {
> +		spin_lock(&tree->hash_lock);
> +		while ((node = tree->node_hash[i])) {
> +			hfs_bnode_unhash(node);
> +			spin_unlock(&tree->hash_lock);
> +			if (atomic_read(&node->refcnt))
> +				pr_err("node %d:%d still has %d
> user(s)!\n",
> +				       node->tree->cnid, node->this,
> +				       atomic_read(&node->refcnt));
> +			hfs_bnode_free(node);
> +			spin_lock(&tree->hash_lock);
> +		}
> +		spin_unlock(&tree->hash_lock);
> +	}
> +}
> +
>  struct hfs_btree *hfs_btree_open(struct super_block *sb, u32 id,
> btree_keycmp keycmp)
>  {
>  	struct hfs_btree *tree;
> @@ -296,6 +317,7 @@ struct hfs_btree *hfs_btree_open(struct
> super_block *sb, u32 id, btree_keycmp ke
>  	tree->inode->i_mapping->a_ops = &hfs_aops;
>  	iput(tree->inode);
>  free_tree:
> +	hfs_bnode_hash_free(tree);
>  	kfree(tree);
>  	return NULL;
>  }
> @@ -303,26 +325,10 @@ struct hfs_btree *hfs_btree_open(struct
> super_block *sb, u32 id, btree_keycmp ke
>  /* Release resources used by a btree */
>  void hfs_btree_close(struct hfs_btree *tree)
>  {
> -	struct hfs_bnode *node;
> -	int i;
> -
>  	if (!tree)
>  		return;
>  
> -	for (i = 0; i < NODE_HASH_SIZE; i++) {
> -		spin_lock(&tree->hash_lock);
> -		while ((node = tree->node_hash[i])) {
> -			hfs_bnode_unhash(node);
> -			spin_unlock(&tree->hash_lock);
> -			if (atomic_read(&node->refcnt))
> -				pr_err("node %d:%d still has %d
> user(s)!\n",
> -				       node->tree->cnid, node->this,
> -				       atomic_read(&node->refcnt));
> -			hfs_bnode_free(node);
> -			spin_lock(&tree->hash_lock);
> -		}
> -		spin_unlock(&tree->hash_lock);
> -	}
> +	hfs_bnode_hash_free(tree);
>  	iput(tree->inode);
>  	kfree(tree);
>  }