[PATCH net v2] bnxt_en: Handle partially initialized auxiliary devices

[PATCH net v2] bnxt_en: Handle partially initialized auxiliary devices

[Ruoyu Wang]

bnxt_aux_devices_init() calls auxiliary_device_init() before all fields
used by bnxt_aux_dev_release() are initialized.  After
auxiliary_device_init() succeeds, later errors must unwind with
auxiliary_device_uninit(), which invokes the release callback.

The release callback assumes that aux_priv->id, aux_priv->edev,
edev->net and edev->ulp_tbl are all populated.  If allocation fails
after auxiliary_device_init(), the release path can otherwise dereference
or clear partially initialized state.

Allocate and attach the bnxt_en_dev and ULP table before calling
auxiliary_device_init(), so the release callback only sees a fully
initialized auxiliary private object.  If auxiliary_device_init() itself
fails, free those allocations directly because device_initialize() has not
run and the release callback will not be invoked.

This issue was found by a static analysis checker and confirmed by manual
source review.

Fixes: 194fad5b2781 ("bnxt_en: Refactor bnxt_rdma_aux_device_init/uninit functions")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
v2:
- Allocate edev and ulp before auxiliary_device_init(), as suggested by
  Pavan Chebbi.
v1: https://lore.kernel.org/netdev/20260708143401.3167477-1-ruoyuw560@gmail.com/

 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 39 ++++++++++---------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index 5c75193..a515c36 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -566,6 +566,18 @@ void bnxt_aux_devices_init(struct bnxt *bp)
 		if (!aux_priv)
 			goto next_auxdev;
 
+		edev = kzalloc_obj(*edev);
+		if (!edev)
+			goto aux_priv_free;
+		aux_priv->edev = edev;
+		bnxt_set_edev_info(edev, bp);
+
+		ulp = kzalloc_obj(*ulp);
+		if (!ulp)
+			goto edev_free;
+		edev->ulp_tbl = ulp;
+		aux_priv->id = idx;
+
 		aux_dev = &aux_priv->aux_dev;
 		aux_dev->id = bp->auxdev_id;
 		aux_dev->name = bnxt_aux_devices[idx].name;
@@ -573,37 +585,26 @@ void bnxt_aux_devices_init(struct bnxt *bp)
 		aux_dev->dev.release = bnxt_aux_dev_release;
 
 		rc = auxiliary_device_init(aux_dev);
-		if (rc) {
-			kfree(aux_priv);
-			goto next_auxdev;
-		}
+		if (rc)
+			goto ulp_free;
 		bp->aux_priv[idx] = aux_priv;
 
 		/* From this point, all cleanup will happen via the .release
 		 * callback & any error unwinding will need to include a call
 		 * to auxiliary_device_uninit.
 		 */
-		edev = kzalloc_obj(*edev);
-		if (!edev)
-			goto aux_dev_uninit;
-
-		aux_priv->edev = edev;
-		bnxt_set_edev_info(edev, bp);
-
-		ulp = kzalloc_obj(*ulp);
-		if (!ulp)
-			goto aux_dev_uninit;
-
-		edev->ulp_tbl = ulp;
 		bp->edev[idx] = edev;
 		if (idx == BNXT_AUXDEV_RDMA)
 			bp->ulp_num_msix_want = bnxt_set_dflt_ulp_msix(bp);
-		aux_priv->id = idx;
 		bnxt_auxdev_set_state(bp, idx, BNXT_ADEV_STATE_INIT);
 
 		continue;
-aux_dev_uninit:
-		auxiliary_device_uninit(aux_dev);
+ulp_free:
+		kfree(ulp);
+edev_free:
+		kfree(edev);
+aux_priv_free:
+		kfree(aux_priv);
 next_auxdev:
 		if (idx == BNXT_AUXDEV_RDMA)
 			bp->flags &= ~BNXT_FLAG_ROCE_CAP;
-- 
2.51.0

Re: [PATCH net v2] bnxt_en: Handle partially initialized auxiliary devices

[Pavan Chebbi]

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

On Sat, Jul 11, 2026 at 10:07 PM Ruoyu Wang <ruoyuw560@gmail.com> wrote:
>
> bnxt_aux_devices_init() calls auxiliary_device_init() before all fields
> used by bnxt_aux_dev_release() are initialized.  After
> auxiliary_device_init() succeeds, later errors must unwind with
> auxiliary_device_uninit(), which invokes the release callback.
>
> The release callback assumes that aux_priv->id, aux_priv->edev,
> edev->net and edev->ulp_tbl are all populated.  If allocation fails
> after auxiliary_device_init(), the release path can otherwise dereference
> or clear partially initialized state.
>
> Allocate and attach the bnxt_en_dev and ULP table before calling
> auxiliary_device_init(), so the release callback only sees a fully
> initialized auxiliary private object.  If auxiliary_device_init() itself
> fails, free those allocations directly because device_initialize() has not
> run and the release callback will not be invoked.
>
> This issue was found by a static analysis checker and confirmed by manual
> source review.
>
> Fixes: 194fad5b2781 ("bnxt_en: Refactor bnxt_rdma_aux_device_init/uninit functions")
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
> ---
> v2:
> - Allocate edev and ulp before auxiliary_device_init(), as suggested by
>   Pavan Chebbi.
> v1: https://lore.kernel.org/netdev/20260708143401.3167477-1-ruoyuw560@gmail.com/
>
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 39 ++++++++++---------
>  1 file changed, 20 insertions(+), 19 deletions(-)
>

Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Thank you.

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