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

[Pavan Chebbi]

On Wed, Jul 8, 2026 at 8:04 PM Ruoyu Wang <ruoyuw560@xxxxxxxxx> 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@xxxxxxxxx>
> ---
>  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: smime.p7s
Description: S/MIME Cryptographic Signature