bnx2x_nic_load() hands the VF status blocks and statistics buffer to the PF with bnx2x_vfpf_init(), and later its queues with SETUP_Q. If the load fails after that, the unwind frees all of them without sending TEARDOWN_Q or CLOSE to the PF. The PF keeps the VF enabled and keeps issuing statistics ramrods that write to the freed statistics buffer, and every later load of the VF fails, as the PF only accepts INIT from a VF in the acquired state. This is easy to reach: if the host has set the VF MAC address with "ip link set vf mac " and the guest brings the interface up with a different address, the PF rejects it in bnx2x_filters_validate_mac() after the queues have been set up. Close the VF in the unwind before the memory is freed, as bnx2x_nic_unload() does. Call bnx2x_vfpf_init() after bnx2x_set_real_num_queues() so that every failure after INIT goes through load_error1, where NAPI and IRQs have already been released and bnx2x_vfpf_close_vf() only has to talk to the PF. Fixes: 8d9ac297d18d ("bnx2x: Add init, setup_q, set_mac to VF <-> PF channel") Assisted-by: LLM Signed-off-by: Yuho Choi --- This was found with a static analysis tool from ongoing, not yet published research, and confirmed by code inspection. It has not been reproduced on hardware; compile-tested with CONFIG_BNX2X_SRIOV=y, W=1. .../net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c index 5a9742fd3ddf..5463de650f70 100644 --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c @@ -2689,13 +2689,6 @@ int bnx2x_nic_load(struct bnx2x *bp, int load_mode) if (rc) LOAD_ERROR_EXIT(bp, load_error0); - /* request pf to initialize status blocks */ - if (IS_VF(bp)) { - rc = bnx2x_vfpf_init(bp); - if (rc) - LOAD_ERROR_EXIT(bp, load_error0); - } - /* As long as bnx2x_alloc_mem() may possibly update * bp->num_queues, bnx2x_set_real_num_queues() should always * come after it. At this stage cnic queues are not counted. @@ -2706,6 +2699,13 @@ int bnx2x_nic_load(struct bnx2x *bp, int load_mode) LOAD_ERROR_EXIT(bp, load_error0); } + /* request pf to initialize status blocks */ + if (IS_VF(bp)) { + rc = bnx2x_vfpf_init(bp); + if (rc) + LOAD_ERROR_EXIT(bp, load_error0); + } + /* configure multi cos mappings in kernel. * this configuration may be overridden by a multi class queue * discipline or by a dcbx negotiation result. @@ -2967,6 +2967,15 @@ int bnx2x_nic_load(struct bnx2x *bp, int load_mode) /* clear pf_load status, as it was already set */ if (IS_PF(bp)) bnx2x_clear_pf_load(bp); + + /* The PF may still use the status blocks, queues and statistics + * buffer it got from bnx2x_vfpf_init(). Close the VF, as + * bnx2x_nic_unload() does, before they are freed below. + */ + if (IS_VF(bp)) { + bnx2x_clear_vlan_info(bp); + bnx2x_vfpf_close_vf(bp); + } load_error0: bnx2x_free_fw_stats_mem(bp); bnx2x_free_fp_mem(bp); base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222 -- 2.43.0