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

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

[Siva Reddy Kallam]

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?

The first warning is due to accessing rdev before validity check in
bng_re_net_ring_free.So, removed unnecessary rdev check in
bng_re_net_ring_free.
The smatch also flagged about unwinding bng_re_dev_init. So, added proper
unwinding ladder in bng_re_dev_init.

------
Changes from:
v1->v2
Addressed the following comments by Leon Romanovsky:
- provide proper commit message
- remove aux_dev check in bng_re_net_ring_free
- remove uncessary validity checks in driver paths
- remove uncessary NULL assignment to rdev->nqr in bng_re_dev_init.
--------

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 | 56 +++++++++-----------------
 1 file changed, 19 insertions(+), 37 deletions(-)

diff --git a/drivers/infiniband/hw/bng_re/bng_dev.c b/drivers/infiniband/hw/bng_re/bng_dev.c
index d8f8d7f7075f..fd0a4fe274ca 100644
--- a/drivers/infiniband/hw/bng_re/bng_dev.c
+++ b/drivers/infiniband/hw/bng_re/bng_dev.c
@@ -54,9 +54,6 @@ static void bng_re_destroy_chip_ctx(struct bng_re_dev *rdev)
 {
 	struct bng_re_chip_ctx *chip_ctx;
 
-	if (!rdev->chip_ctx)
-		return;
-
 	kfree(rdev->dev_attr);
 	rdev->dev_attr = NULL;
 
@@ -124,12 +121,6 @@ static int bng_re_net_ring_free(struct bng_re_dev *rdev,
 	struct bnge_fw_msg fw_msg = {};
 	int rc = -EINVAL;
 
-	if (!rdev)
-		return rc;
-
-	if (!aux_dev)
-		return rc;
-
 	bng_re_init_hwrm_hdr((void *)&req, HWRM_RING_FREE);
 	req.ring_type = type;
 	req.ring_id = cpu_to_le16(fw_ring_id);
@@ -150,10 +141,7 @@ static int bng_re_net_ring_alloc(struct bng_re_dev *rdev,
 	struct hwrm_ring_alloc_input req = {};
 	struct hwrm_ring_alloc_output resp;
 	struct bnge_fw_msg fw_msg = {};
-	int rc = -EINVAL;
-
-	if (!aux_dev)
-		return rc;
+	int rc;
 
 	bng_re_init_hwrm_hdr((void *)&req, HWRM_RING_ALLOC);
 	req.enables = 0;
@@ -184,10 +172,7 @@ static int bng_re_stats_ctx_free(struct bng_re_dev *rdev)
 	struct hwrm_stat_ctx_free_input req = {};
 	struct hwrm_stat_ctx_free_output resp = {};
 	struct bnge_fw_msg fw_msg = {};
-	int rc = -EINVAL;
-
-	if (!aux_dev)
-		return rc;
+	int rc;
 
 	bng_re_init_hwrm_hdr((void *)&req, HWRM_STAT_CTX_FREE);
 	req.stat_ctx_id = cpu_to_le32(rdev->stats_ctx.fw_id);
@@ -208,13 +193,10 @@ static int bng_re_stats_ctx_alloc(struct bng_re_dev *rdev)
 	struct hwrm_stat_ctx_alloc_output resp = {};
 	struct hwrm_stat_ctx_alloc_input req = {};
 	struct bnge_fw_msg fw_msg = {};
-	int rc = -EINVAL;
+	int rc;
 
 	stats->fw_id = BNGE_INVALID_STATS_CTX_ID;
 
-	if (!aux_dev)
-		return rc;
-
 	bng_re_init_hwrm_hdr((void *)&req, HWRM_STAT_CTX_ALLOC);
 	req.update_period_ms = cpu_to_le32(1000);
 	req.stats_dma_addr = cpu_to_le64(stats->dma_map);
@@ -303,7 +285,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 +294,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 +312,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 +388,15 @@ 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);
+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;
 }
 
@@ -486,8 +469,7 @@ static void bng_re_remove(struct auxiliary_device *adev)
 
 	rdev = dev_info->rdev;
 
-	if (rdev)
-		bng_re_remove_device(rdev, adev);
+	bng_re_remove_device(rdev, adev);
 	kfree(dev_info);
 }
 
-- 
2.25.1

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

[Dan Carpenter]

On Tue, Jan 20, 2026 at 09:02:04AM +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?

I would probably split this into two patches, one to remove the NULL
checks and one to fix the unwinding.  I would say that even though
removing the NULL checks doesn't fix a real life bug, it's probably
still worth having a Fixes tag because otherwise if this is backported
to an older kernel it will trigger the warning again and we'll have
to review it again.  Where if it has a fixes tag, the cleanup will be
backported as well.

> 
> The first warning is due to accessing rdev before validity check in
> bng_re_net_ring_free.So, removed unnecessary rdev check in
> bng_re_net_ring_free.
> The smatch also flagged about unwinding bng_re_dev_init. So, added proper
> unwinding ladder in bng_re_dev_init.
> 
> ------
> Changes from:
> v1->v2
> Addressed the following comments by Leon Romanovsky:
> - provide proper commit message
> - remove aux_dev check in bng_re_net_ring_free
> - remove uncessary validity checks in driver paths
> - remove uncessary NULL assignment to rdev->nqr in bng_re_dev_init.
> --------
> 

This isn't the right way to send a v2 patch.  I have written a blog
which explains more.

https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-patch/

> 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 | 56 +++++++++-----------------
>  1 file changed, 19 insertions(+), 37 deletions(-)
> 

regards,
dan carpenter

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

[Siva Reddy Kallam]

[-- Attachment #1: Type: text/plain, Size: 2390 bytes --]

On Tue, Jan 20, 2026 at 2:37 PM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> On Tue, Jan 20, 2026 at 09:02:04AM +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?
>
> I would probably split this into two patches, one to remove the NULL
> checks and one to fix the unwinding.  I would say that even though
> removing the NULL checks doesn't fix a real life bug, it's probably
> still worth having a Fixes tag because otherwise if this is backported
> to an older kernel it will trigger the warning again and we'll have
> to review it again.  Where if it has a fixes tag, the cleanup will be
> backported as well.
>
> >
> > The first warning is due to accessing rdev before validity check in
> > bng_re_net_ring_free.So, removed unnecessary rdev check in
> > bng_re_net_ring_free.
> > The smatch also flagged about unwinding bng_re_dev_init. So, added proper
> > unwinding ladder in bng_re_dev_init.
> >
> > ------
> > Changes from:
> > v1->v2
> > Addressed the following comments by Leon Romanovsky:
> > - provide proper commit message
> > - remove aux_dev check in bng_re_net_ring_free
> > - remove uncessary validity checks in driver paths
> > - remove uncessary NULL assignment to rdev->nqr in bng_re_dev_init.
> > --------
> >
>
> This isn't the right way to send a v2 patch.  I have written a blog
> which explains more.
>
> https://staticthinking.wordpress.com/2022/07/27/how-to-send-a-v2-patch/
Thank you for the details. I will modify and send it again.
>
> > 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 | 56 +++++++++-----------------
> >  1 file changed, 19 insertions(+), 37 deletions(-)
> >
>
> regards,
> dan carpenter
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5471 bytes --]