linux-kernel - Re: [PATCH] xfs: removed useless condition in function xfs_attr_node

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: <YsXCNSIeP9y9S2fa@magnolia>
Date:   Wed, 6 Jul 2022 10:11:17 -0700
From:   "Darrick J. Wong" <djwong@...nel.org>
To:     Andrey Strachuk <strochuk@...ras.ru>
Cc:     Dave Chinner <dchinner@...hat.com>,
        Allison Henderson <allison.henderson@...cle.com>,
        Chandan Babu R <chandan.babu@...cle.com>,
        linux-xfs@...r.kernel.org, linux-kernel@...r.kernel.org,
        ldv-project@...uxtesting.org
Subject: Re: [PATCH] xfs: removed useless condition in function
 xfs_attr_node_get

On Wed, Jul 06, 2022 at 04:36:27PM +0300, Andrey Strachuk wrote:
> At line 1561, variable "state" is being compared
> with NULL every loop iteration.
> 
> -------------------------------------------------------------------
> 1561	for (i = 0; state != NULL && i < state->path.active; i++) {
> 1562		xfs_trans_brelse(args->trans, state->path.blk[i].bp);
> 1563		state->path.blk[i].bp = NULL;
> 1564	}
> -------------------------------------------------------------------
> 
> However, it cannot be NULL.
> 
> ----------------------------------------
> 1546	state = xfs_da_state_alloc(args);
> ----------------------------------------
> 
> xfs_da_state_alloc calls kmem_cache_zalloc. kmem_cache_zalloc is
> called with __GFP_NOFAIL flag and, therefore, it cannot return NULL.
> 
> --------------------------------------------------------------------------
> 	struct xfs_da_state *
> 	xfs_da_state_alloc(
> 	struct xfs_da_args	*args)
> 	{
> 		struct xfs_da_state	*state;
> 
> 		state = kmem_cache_zalloc(xfs_da_state_cache, GFP_NOFS | __GFP_NOFAIL);
> 		state->args = args;
> 		state->mp = args->dp->i_mount;
> 		return state;
> 	}
> --------------------------------------------------------------------------
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Andrey Strachuk <strochuk@...ras.ru>
> 
> Fixes: 4d0cdd2bb8f0 ("xfs: clean up xfs_attr_node_hasname")

Nit: usually the tags go at the end with no empty lines between them.

The change looks correct to me, so
Reviewed-by: Darrick J. Wong <djwong@...nel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_attr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 224649a76cbb..6b8857e53add 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -1558,7 +1558,7 @@ xfs_attr_node_get(
>  	 * If not in a transaction, we have to release all the buffers.
>  	 */
>  out_release:
> -	for (i = 0; state != NULL && i < state->path.active; i++) {
> +	for (i = 0; i < state->path.active; i++) {
>  		xfs_trans_brelse(args->trans, state->path.blk[i].bp);
>  		state->path.blk[i].bp = NULL;
>  	}
> -- 
> 2.25.1
>