[PATCH net] bnxt_en: Handle partially initialized auxiliary devices

[PATCH net] bnxt_en: Handle partially initialized auxiliary devices

[Ruoyu Wang]

bnxt_aux_devices_init() calls auxiliary_device_init() before allocating
and attaching the bnxt_en_dev and ULP table.  After
auxiliary_device_init() succeeds, the auxiliary bus owns the embedded
device lifetime and the driver must unwind later errors with
auxiliary_device_uninit(), which runs bnxt_aux_dev_release().

The release callback currently assumes that aux_priv->id, aux_priv->edev
and edev->net were all populated.  If the bnxt_en_dev allocation fails,
release dereferences aux_priv->edev while it is still NULL.  If a later
failure happens before aux_priv->id is assigned, release uses the zeroed
id field and can clear the wrong auxiliary-device slot while unwinding a
partially initialized device.

Set aux_priv->id before auxiliary_device_init() so the release path can
identify the slot, make release tolerate missing partial state, and clear
bp->aux_priv[idx] on the unwind path where release cannot derive bp from
an edev.

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>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
index 5c751933da6a9..e85d1b6c9fb1f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
@@ -472,12 +472,17 @@ static void bnxt_aux_dev_release(struct device *dev)
 {
 	struct bnxt_aux_priv *aux_priv =
 		container_of(dev, struct bnxt_aux_priv, aux_dev.dev);
-	struct bnxt *bp = netdev_priv(aux_priv->edev->net);
+	struct bnxt_en_dev *edev = aux_priv->edev;
+	struct bnxt *bp = edev && edev->net ? netdev_priv(edev->net) : NULL;
 
-	kfree(aux_priv->edev->ulp_tbl);
-	bp->edev[aux_priv->id] = NULL;
-	kfree(aux_priv->edev);
-	bp->aux_priv[aux_priv->id] = NULL;
+	if (edev) {
+		kfree(edev->ulp_tbl);
+		if (bp)
+			bp->edev[aux_priv->id] = NULL;
+		kfree(edev);
+	}
+	if (bp)
+		bp->aux_priv[aux_priv->id] = NULL;
 	kfree(aux_priv);
 }
 
@@ -571,6 +576,7 @@ void bnxt_aux_devices_init(struct bnxt *bp)
 		aux_dev->name = bnxt_aux_devices[idx].name;
 		aux_dev->dev.parent = &bp->pdev->dev;
 		aux_dev->dev.release = bnxt_aux_dev_release;
+		aux_priv->id = idx;
 
 		rc = auxiliary_device_init(aux_dev);
 		if (rc) {
@@ -598,12 +604,12 @@ void bnxt_aux_devices_init(struct bnxt *bp)
 		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);
+		bp->aux_priv[idx] = NULL;
 next_auxdev:
 		if (idx == BNXT_AUXDEV_RDMA)
 			bp->flags &= ~BNXT_FLAG_ROCE_CAP;
-- 
2.51.0

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

[Pavan Chebbi]

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

On Wed, Jul 8, 2026 at 8:04 PM Ruoyu Wang <ruoyuw560@gmail.com> wrote:
>
> bnxt_aux_devices_init() calls auxiliary_device_init() before allocating
> and attaching the bnxt_en_dev and ULP table.  After
> auxiliary_device_init() succeeds, the auxiliary bus owns the embedded
> device lifetime and the driver must unwind later errors with
> auxiliary_device_uninit(), which runs bnxt_aux_dev_release().
>
> The release callback currently assumes that aux_priv->id, aux_priv->edev
> and edev->net were all populated.  If the bnxt_en_dev allocation fails,
> release dereferences aux_priv->edev while it is still NULL.  If a later
> failure happens before aux_priv->id is assigned, release uses the zeroed
> id field and can clear the wrong auxiliary-device slot while unwinding a
> partially initialized device.
>
> Set aux_priv->id before auxiliary_device_init() so the release path can
> identify the slot, make release tolerate missing partial state, and clear
> bp->aux_priv[idx] on the unwind path where release cannot derive bp from
> an edev.
>
> 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>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> index 5c751933da6a9..e85d1b6c9fb1f 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c
> @@ -472,12 +472,17 @@ static void bnxt_aux_dev_release(struct device *dev)
>  {
>         struct bnxt_aux_priv *aux_priv =
>                 container_of(dev, struct bnxt_aux_priv, aux_dev.dev);
> -       struct bnxt *bp = netdev_priv(aux_priv->edev->net);
> +       struct bnxt_en_dev *edev = aux_priv->edev;
> +       struct bnxt *bp = edev && edev->net ? netdev_priv(edev->net) : NULL;
>
> -       kfree(aux_priv->edev->ulp_tbl);
> -       bp->edev[aux_priv->id] = NULL;
> -       kfree(aux_priv->edev);
> -       bp->aux_priv[aux_priv->id] = NULL;
> +       if (edev) {

Instead of these checks, just allocate the memory for edev and ulp
before calling auxiliary_device_init().
That would be much cleaner.

> +               kfree(edev->ulp_tbl);
> +               if (bp)
> +                       bp->edev[aux_priv->id] = NULL;
> +               kfree(edev);
> +       }
> +       if (bp)
> +               bp->aux_priv[aux_priv->id] = NULL;
>         kfree(aux_priv);
>  }
>
> @@ -571,6 +576,7 @@ void bnxt_aux_devices_init(struct bnxt *bp)
>                 aux_dev->name = bnxt_aux_devices[idx].name;
>                 aux_dev->dev.parent = &bp->pdev->dev;
>                 aux_dev->dev.release = bnxt_aux_dev_release;
> +               aux_priv->id = idx;
>
>                 rc = auxiliary_device_init(aux_dev);
>                 if (rc) {
> @@ -598,12 +604,12 @@ void bnxt_aux_devices_init(struct bnxt *bp)
>                 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);
> +               bp->aux_priv[idx] = NULL;
>  next_auxdev:
>                 if (idx == BNXT_AUXDEV_RDMA)
>                         bp->flags &= ~BNXT_FLAG_ROCE_CAP;
> --
> 2.51.0

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