linux-kernel - Re: [PATCH] HFS: btree: fix missing error check after 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] [day] [month] [year] [list]

Message-ID: <3a143f53da945f7bad35aaff7bb40b1b6255d5ba.camel@dubeyko.com>
Date: Thu, 11 Dec 2025 15:13:01 -0800
From: Viacheslav Dubeyko <slava@...eyko.com>
To: Haotian Zhang <vulab@...as.ac.cn>, glaubitz@...sik.fu-berlin.de, 
	frank.li@...o.com
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] HFS: btree: fix missing error check after
 hfs_bnode_find()

On Tue, 2025-12-09 at 10:14 +0800, Haotian Zhang wrote:
> In hfs_brec_insert() and hfs_brec_update_parent(), hfs_bnode_find()
> may return ERR_PTR() on failure, but the result was used without
> checking, risking NULL pointer dereference or invalid pointer usage.
> 
> Add IS_ERR() checks after these calls and return PTR_ERR()
> on error.
> 
> Signed-off-by: Haotian Zhang <vulab@...as.ac.cn>
> ---
>  fs/hfs/brec.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/hfs/brec.c b/fs/hfs/brec.c
> index e49a141c87e5..afa1840a4847 100644
> --- a/fs/hfs/brec.c
> +++ b/fs/hfs/brec.c
> @@ -149,6 +149,8 @@ int hfs_brec_insert(struct hfs_find_data *fd,
> void *entry, int entry_len)
>  			new_node->parent = tree->root;
>  		}
>  		fd->bnode = hfs_bnode_find(tree, new_node->parent);
> +		if (IS_ERR(fd->bnode))
> +			return PTR_ERR(fd->bnode);
>  
>  		/* create index data entry */
>  		cnid = cpu_to_be32(new_node->this);
> @@ -449,6 +451,8 @@ static int hfs_brec_update_parent(struct
> hfs_find_data *fd)
>  			new_node->parent = tree->root;
>  		}
>  		fd->bnode = hfs_bnode_find(tree, new_node->parent);
> +		if (IS_ERR(fd->bnode))
> +			return PTR_ERR(fd->bnode);
>  		/* create index key and entry */
>  		hfs_bnode_read_key(new_node, fd->search_key, 14);
>  		cnid = cpu_to_be32(new_node->this);

Frankly speaking, I am not sure that we need to add this check.
Because, we are trying to find the parent node that already has been
found in above logic of the method. So, we should have the parent node
available. Potentially, logic could work in wrong way, but we should
already have a reported bug already.

Even if this check makes sense, then we cannot simply return the error
code here. If you check the following logic, then you can see that we
call hfs_bnode_put() for the new node. So, if this check doesn't do
this in the case of error, then we create the leak here.

Have you ever reproduced the issue that you are trying to fix?

Thanks,
Slava.