Re: [PATCH net v2] bnxt_en: don't leave an XDP program installed when the open fails

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: bnxt_en XDP detach memory leak and state desynchronization on NIC open failure
--

commit a5efe83c5d84fe92f5143236d2e982d0ce3eaec4
Author: Vaibhav Nagare <vnagare@redhat.com>

bnxt_en: don't leave an XDP program installed when the open fails

This commit restores the previous XDP program and ring configuration when
bnxt_open_nic() fails. This prevents the driver from leaving a freed program
in bp->xdp_prog on a failed XDP update.

Cc: stable@vger.kernel.org

> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
> index 9e5009be8e989..edfd751a601ab 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c
[ ... ]
> @@ -418,25 +438,21 @@ static int bnxt_xdp_set(struct bnxt *bp, struct bpf_prog *prog)
>  	if (netif_running(dev))
>  		bnxt_close_nic(bp, true, false);
>  
> +	old_tx_xdp = bp->tx_nr_rings_xdp;
>  	old = xchg(&bp->xdp_prog, prog);
> +	bnxt_xdp_apply_cfg(bp, tx_xdp);
> +
> +	if (netif_running(dev)) {
> +		rc = bnxt_open_nic(bp, true, false);
> +		if (rc) {
> +			WRITE_ONCE(bp->xdp_prog, old);
> +			bnxt_xdp_apply_cfg(bp, old_tx_xdp);
> +			return rc;

[Severity: High]
Does this error path introduce a memory leak and state desynchronization if
the user was trying to detach the XDP program?

If bnxt_open_nic() fails during a detach operation where prog is NULL, the
core stack's dev_xdp_detach_link() ignores the error returned by the driver
and forcefully unlinks the program anyway:

net/core/dev.c:dev_xdp_detach_link() {
    ...
    WARN_ON(dev_xdp_install(dev, mode, bpf_op, NULL, 0, NULL));
    dev_xdp_set_link(dev, mode, NULL);
    ...
}

Because this error path restores old to bp->xdp_prog and returns early
without calling bpf_prog_put(old), does the driver maintain a leaked
reference to the BPF program? 

Would this cause the driver to continue executing a 'detached' XDP program
when the NIC is brought up later, while the core stack considers it fully
detached?

> +		}
>  	}
>  
> -	if (old)
> -		bpf_prog_put(old);
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930070901.1218980-1-vnagare@redhat.com?part=1