[PATCH net v1] bnx2x: Close the VF when loading it fails after INIT

[PATCH net v1] bnx2x: Close the VF when loading it fails after INIT

[Yuho Choi]

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 <pf> vf <n> mac <addr>" 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 <oss.patchbox@gmail.com>
---
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