Re: [PATCH] RDMA/bng_re: Unwind bng_re_dev_init properly and remove unnecessary rdev check

[Leon Romanovsky <leon@kernel.org>]

On Wed, Jan 07, 2026 at 09:16:07AM +0000, Siva Reddy Kallam wrote:
> Fix below smatch warnings:
> drivers/infiniband/hw/bng_re/bng_dev.c:113
> bng_re_net_ring_free() warn: variable dereferenced before check 'rdev'
> (see line 107)
> drivers/infiniband/hw/bng_re/bng_dev.c:270
> bng_re_dev_init() warn: missing unwind goto?

Please provide commit message.

> 
> Fixes: 4f830cd8d7fe ("RDMA/bng_re: Add infrastructure for enabling Firmware channel")
> Reported-by: Simon Horman <horms@kernel.org>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <error27@gmail.com>
> Closes: https://lore.kernel.org/r/202601010413.sWadrQel-lkp@intel.com/
> Signed-off-by: Siva Reddy Kallam <siva.kallam@broadcom.com>
> Reviewed-by: Usman Ansari <usman.ansari@broadcom.com>
> ---
>  drivers/infiniband/hw/bng_re/bng_dev.c | 33 +++++++++++++-------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/bng_re/bng_dev.c b/drivers/infiniband/hw/bng_re/bng_dev.c
> index d8f8d7f7075f..e2dd2c8eb6d2 100644
> --- a/drivers/infiniband/hw/bng_re/bng_dev.c
> +++ b/drivers/infiniband/hw/bng_re/bng_dev.c
> @@ -124,9 +124,6 @@ static int bng_re_net_ring_free(struct bng_re_dev *rdev,
>  	struct bnge_fw_msg fw_msg = {};
>  	int rc = -EINVAL;
>  
> -	if (!rdev)

You have other places with impossible "if (rdev)" check in this path which you should
delete as well.

> -		return rc;
> -
>  	if (!aux_dev)

You should remove this check too.

>  		return rc;
>  
> @@ -303,7 +300,7 @@ static int bng_re_dev_init(struct bng_re_dev *rdev)
>  	if (rc) {
>  		ibdev_err(&rdev->ibdev,
>  				"Failed to register with netedev: %#x\n", rc);
> -		return -EINVAL;
> +		goto reg_netdev_fail;
>  	}
>  
>  	set_bit(BNG_RE_FLAG_NETDEV_REGISTERED, &rdev->flags);
> @@ -312,19 +309,16 @@ static int bng_re_dev_init(struct bng_re_dev *rdev)
>  		ibdev_err(&rdev->ibdev,
>  			  "RoCE requires minimum 2 MSI-X vectors, but only %d reserved\n",
>  			  rdev->aux_dev->auxr_info->msix_requested);
> -		bnge_unregister_dev(rdev->aux_dev);
> -		clear_bit(BNG_RE_FLAG_NETDEV_REGISTERED, &rdev->flags);
> -		return -EINVAL;
> +		rc = -EINVAL;
> +		goto msix_ctx_fail;
>  	}
>  	ibdev_dbg(&rdev->ibdev, "Got %d MSI-X vectors\n",
>  		  rdev->aux_dev->auxr_info->msix_requested);
>  
>  	rc = bng_re_setup_chip_ctx(rdev);
>  	if (rc) {
> -		bnge_unregister_dev(rdev->aux_dev);
> -		clear_bit(BNG_RE_FLAG_NETDEV_REGISTERED, &rdev->flags);
>  		ibdev_err(&rdev->ibdev, "Failed to get chip context\n");
> -		return -EINVAL;
> +		goto msix_ctx_fail;
>  	}
>  
>  	bng_re_query_hwrm_version(rdev);
> @@ -333,16 +327,14 @@ static int bng_re_dev_init(struct bng_re_dev *rdev)
>  	if (rc) {
>  		ibdev_err(&rdev->ibdev,
>  			  "Failed to allocate RCFW Channel: %#x\n", rc);
> -		goto fail;
> +		goto alloc_fw_chl_fail;
>  	}
>  
>  	/* Allocate nq record memory */
>  	rdev->nqr = kzalloc(sizeof(*rdev->nqr), GFP_KERNEL);
>  	if (!rdev->nqr) {
> -		bng_re_destroy_chip_ctx(rdev);
> -		bnge_unregister_dev(rdev->aux_dev);
> -		clear_bit(BNG_RE_FLAG_NETDEV_REGISTERED, &rdev->flags);
> -		return -ENOMEM;
> +		rc = -ENOMEM;
> +		goto nq_alloc_fail;
>  	}
>  
>  	rdev->nqr->num_msix = rdev->aux_dev->auxr_info->msix_requested;
> @@ -411,9 +403,16 @@ static int bng_re_dev_init(struct bng_re_dev *rdev)
>  free_ring:
>  	bng_re_net_ring_free(rdev, rdev->rcfw.creq.ring_id, type);
>  free_rcfw:
> +	kfree(rdev->nqr);
> +	rdev->nqr = NULL;

Why do you need to set NULL here?

> +nq_alloc_fail:
>  	bng_re_free_rcfw_channel(&rdev->rcfw);
> -fail:
> -	bng_re_dev_uninit(rdev);
> +alloc_fw_chl_fail:
> +	bng_re_destroy_chip_ctx(rdev);
> +msix_ctx_fail:
> +	bnge_unregister_dev(rdev->aux_dev);
> +	clear_bit(BNG_RE_FLAG_NETDEV_REGISTERED, &rdev->flags);
> +reg_netdev_fail:
>  	return rc;
>  }
>  
> -- 
> 2.25.1
> 
>