[PATCH 0/5] net/bnxt: fix VF and firmware-facing bounds/leak issues

[PATCH 0/5] net/bnxt: fix VF and firmware-facing bounds/leak issues

[Mohammad Shuab Siddique]

From: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>

This series fixes five independent out-of-bounds and memory-leak
issues in VF- and firmware-facing control-path code in the bnxt PMD:

 - an unvalidated, firmware-controlled VF ID used to index
   bp->pf->vf_info[] before range-checking it,
 - firmware-reported ring-group/L2-context counts that were not
   clamped before being cast down or summed,
 - a use-after-free in the VNIC filter list walk during cleanup
   (STAILQ_FOREACH() dereferencing a node this loop had already
   freed),
 - a memory leak on the VF VNIC query error path, and
 - a memory leak on the VF info allocation error path.

Each patch is independently bisectable and was validated with a
scoped net/bnxt build (and, for split points, an intermediate-commit
build) in addition to the full compliance gate.

Joseph Wong (3):
  net/bnxt: add VF ID boundary check before usage
  net/bnxt: fix bounds on firmware-reported resource counts
  net/bnxt: fix VF info alloc error path memory leak

Kishore Padmanabha (1):
  net/bnxt: fix memory leak in VF VNIC query error path

Mohammad Shuab Siddique (1):
  net/bnxt: fix use-after-free in VNIC filter cleanup

 drivers/net/bnxt/bnxt.h      |  3 +++
 drivers/net/bnxt/bnxt_cpr.c  | 25 ++++++++++++++-----------
 drivers/net/bnxt/bnxt_hwrm.c | 31 +++++++++++++++++++++----------
 3 files changed, 38 insertions(+), 21 deletions(-)

-- 
2.47.3

[PATCH 1/5] net/bnxt: add VF ID boundary check before usage

[Mohammad Shuab Siddique]

From: Joseph Wong <joseph.wong@broadcom.com>

bnxt_handle_fwd_req() computed vf_id from an unvalidated,
firmware-controlled source_id and used it to index
bp->pf->vf_info[] before checking that the ID falls within the
active VF range. An out-of-range VF ID could index past
vf_info[] and cause memory corruption.

Move the range check ahead of the vf_id computation and the
vf_info[] lookup. Guard the later fwd_cmd/req_len usage on the
reject path, since they are no longer set when the request is
rejected early. Also guard bnxt_hwrm_reject_fwd_resp()'s memcpy()
against a NULL encaped pointer for the same reason.

Fixes: f2a768d4d186 ("net/bnxt: add completion ring")
Cc: stable@dpdk.org

Signed-off-by: Joseph Wong <joseph.wong@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
 drivers/net/bnxt/bnxt_cpr.c  | 25 ++++++++++++++-----------
 drivers/net/bnxt/bnxt_hwrm.c |  3 ++-
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
index 5c255de59e..ac731ea72f 100644
--- a/drivers/net/bnxt/bnxt_cpr.c
+++ b/drivers/net/bnxt/bnxt_cpr.c
@@ -375,16 +375,6 @@ void bnxt_handle_fwd_req(struct bnxt *bp, struct cmpl_base *cmpl)
 
 	/* Qualify the fwd request */
 	fw_vf_id = rte_le_to_cpu_16(fwd_cmpl->source_id);
-	vf_id = fw_vf_id - bp->pf->first_vf_id;
-
-	req_len = (rte_le_to_cpu_16(fwd_cmpl->req_len_type) &
-		   HWRM_FWD_REQ_CMPL_REQ_LEN_MASK) >>
-		HWRM_FWD_REQ_CMPL_REQ_LEN_SFT;
-	if (req_len > sizeof(fwreq->encap_request))
-		req_len = sizeof(fwreq->encap_request);
-
-	/* Locate VF's forwarded command */
-	fwd_cmd = (struct input *)bp->pf->vf_info[vf_id].req_buf;
 
 	if (fw_vf_id < bp->pf->first_vf_id ||
 	    fw_vf_id >= bp->pf->first_vf_id + bp->pf->active_vfs) {
@@ -393,9 +383,22 @@ void bnxt_handle_fwd_req(struct bnxt *bp, struct cmpl_base *cmpl)
 			fw_vf_id, bp->pf->first_vf_id,
 			(bp->pf->first_vf_id) + bp->pf->active_vfs - 1,
 			bp->pf->first_vf_id, bp->pf->active_vfs);
+		fwd_cmd = NULL;
+		req_len = 0;
 		goto reject;
 	}
 
+	vf_id = fw_vf_id - bp->pf->first_vf_id;
+
+	req_len = (rte_le_to_cpu_16(fwd_cmpl->req_len_type) &
+		   HWRM_FWD_REQ_CMPL_REQ_LEN_MASK) >>
+		HWRM_FWD_REQ_CMPL_REQ_LEN_SFT;
+	if (req_len > sizeof(fwreq->encap_request))
+		req_len = sizeof(fwreq->encap_request);
+
+	/* Locate VF's forwarded command */
+	fwd_cmd = (struct input *)bp->pf->vf_info[vf_id].req_buf;
+
 	if (bnxt_rcv_msg_from_vf(bp, vf_id, fwd_cmd)) {
 		/*
 		 * In older firmware versions, the MAC had to be all zeros for
@@ -495,7 +498,7 @@ void bnxt_handle_fwd_req(struct bnxt *bp, struct cmpl_base *cmpl)
 		PMD_DRV_LOG_LINE(ERR,
 			"Failed to send REJECT req VF 0x%x, type 0x%x.",
 			fw_vf_id - bp->pf->first_vf_id,
-			rte_le_to_cpu_16(fwd_cmd->req_type));
+			fwd_cmd ? rte_le_to_cpu_16(fwd_cmd->req_type) : 0xFFFF);
 	}
 
 	return;
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 1615b36aae..aa8152eaa0 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -5520,7 +5520,8 @@ int bnxt_hwrm_reject_fwd_resp(struct bnxt *bp, uint16_t target_id,
 	HWRM_PREP(&req, HWRM_REJECT_FWD_RESP, BNXT_USE_CHIMP_MB);
 
 	req.encap_resp_target_id = rte_cpu_to_le_16(target_id);
-	memcpy(req.encap_request, encaped, ec_size);
+	if (encaped)
+		memcpy(req.encap_request, encaped, ec_size);
 
 	rc = bnxt_hwrm_send_message(bp, &req, sizeof(req), BNXT_USE_CHIMP_MB);
 
-- 
2.47.3

[PATCH 2/5] net/bnxt: fix bounds on firmware-reported resource counts

[Mohammad Shuab Siddique]

From: Joseph Wong <joseph.wong@broadcom.com>

When parsing max_ring_grps and max_l2_ctx values from firmware, clamp
values if they exceed a 16-bit value. max_hw_ring_grps is received as
32-bit in __bnxt_hwrm_func_qcaps() (the func_qcaps response) but cast
to 16-bit when used; add the clamp there and, as defense-in-depth,
also in bnxt_hwrm_func_resc_qcaps() in case that response's field ever
widens. max_l2_ctx is 16-bit in both responses, but its post-read
addition with max_rx_em_flows can overflow a 16-bit sum; widen the
addition to 32-bit and clamp the result.

Fixes: 2691827e82c ("net/bnxt: add HWRM VNIC alloc")
Fixes: 80bf6811fa0 ("net/bnxt: fix L2 context calculation for Thor")
Cc: stable@dpdk.org

Signed-off-by: Joseph Wong <joseph.wong@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
 drivers/net/bnxt/bnxt.h      |  3 +++
 drivers/net/bnxt/bnxt_hwrm.c | 16 ++++++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 336de75da0..69455af31f 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -863,6 +863,9 @@ struct bnxt {
 #define BNXT_P7_MAX_NQ_RING_CNT	512
 #define BNXT_P7_CQ_MAX_L2_ENT	8192
 
+#define BNXT_MAX_RING_GRPS	65535U
+#define BNXT_MAX_L2_CTX		65535U
+
 	uint32_t			flags2;
 #define BNXT_FLAGS2_PTP_TIMESYNC_ENABLED	BIT(0)
 #define BNXT_FLAGS2_PTP_ALARM_SCHEDULED		BIT(1)
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index aa8152eaa0..765aa7c452 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1125,6 +1125,8 @@ static int __bnxt_hwrm_func_qcaps(struct bnxt *bp)
 	HWRM_CHECK_RESULT();
 
 	bp->max_ring_grps = rte_le_to_cpu_32(resp->max_hw_ring_grps);
+	if (bp->max_ring_grps > BNXT_MAX_RING_GRPS)
+		bp->max_ring_grps = BNXT_MAX_RING_GRPS;
 	flags = rte_le_to_cpu_32(resp->flags);
 	flags_ext2 = rte_le_to_cpu_32(resp->flags_ext2);
 	flags_ext3 = rte_le_to_cpu_32(resp->flags_ext3);
@@ -1155,8 +1157,10 @@ static int __bnxt_hwrm_func_qcaps(struct bnxt *bp)
 	bp->first_vf_id = rte_le_to_cpu_16(resp->first_vf_id);
 	bp->max_rx_em_flows = rte_le_to_cpu_16(resp->max_rx_em_flows);
 	bp->max_l2_ctx = rte_le_to_cpu_16(resp->max_l2_ctxs);
-	if (!BNXT_CHIP_P5_P7(bp) && !bp->pdev->max_vfs)
-		bp->max_l2_ctx += bp->max_rx_em_flows;
+	if (!BNXT_CHIP_P5_P7(bp) && !bp->pdev->max_vfs) {
+		uint32_t l2_ctx = bp->max_l2_ctx + bp->max_rx_em_flows;
+		bp->max_l2_ctx = (uint16_t)RTE_MIN(l2_ctx, (uint32_t)BNXT_MAX_L2_CTX);
+	}
 	if (bp->vnic_cap_flags & BNXT_VNIC_CAP_COS_CLASSIFY)
 		bp->max_vnics = rte_le_to_cpu_16(BNXT_MAX_VNICS_COS_CLASSIFY);
 	else
@@ -1556,12 +1560,16 @@ int bnxt_hwrm_func_resc_qcaps(struct bnxt *bp)
 	bp->max_tx_rings = rte_le_to_cpu_16(resp->max_tx_rings);
 	bp->max_rx_rings = rte_le_to_cpu_16(resp->max_rx_rings);
 	bp->max_ring_grps = rte_le_to_cpu_32(resp->max_hw_ring_grps);
+	if (bp->max_ring_grps > BNXT_MAX_RING_GRPS)
+		bp->max_ring_grps = BNXT_MAX_RING_GRPS;
 	/* func_resource_qcaps does not return max_rx_em_flows.
 	 * So use the value provided by func_qcaps.
 	 */
 	bp->max_l2_ctx = rte_le_to_cpu_16(resp->max_l2_ctxs);
-	if (!BNXT_CHIP_P5_P7(bp) && !bp->pdev->max_vfs)
-		bp->max_l2_ctx += bp->max_rx_em_flows;
+	if (!BNXT_CHIP_P5_P7(bp) && !bp->pdev->max_vfs) {
+		uint32_t l2_ctx = bp->max_l2_ctx + bp->max_rx_em_flows;
+		bp->max_l2_ctx = (uint16_t)RTE_MIN(l2_ctx, (uint32_t)BNXT_MAX_L2_CTX);
+	}
 	if (bp->vnic_cap_flags & BNXT_VNIC_CAP_COS_CLASSIFY)
 		bp->max_vnics = rte_le_to_cpu_16(BNXT_MAX_VNICS_COS_CLASSIFY);
 	else
-- 
2.47.3

[PATCH 3/5] net/bnxt: fix use-after-free in VNIC filter cleanup

[Mohammad Shuab Siddique]

From: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>

STAILQ_FOREACH()'s own advance step dereferences the current node's
next field after the loop body runs. The loop body here frees that
same node (bnxt_free_filter()) before the macro dereferences it on
the next iteration, so the filter list walk in
bnxt_clear_hwrm_vnic_filters() reads freed memory to find the
following entry.

Walk the list with STAILQ_FIRST()/STAILQ_REMOVE_HEAD() instead,
removing each filter from the list before freeing it so nothing is
dereferenced after being freed.

Fixes: 20ef524432dd ("net/bnxt: set L2 filters")
Cc: stable@dpdk.org

Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 765aa7c452..99539d3705 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -3593,9 +3593,10 @@ bnxt_clear_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 	struct bnxt_filter_info *filter;
 	int rc = 0;
 
-	STAILQ_FOREACH(filter, &vnic->filter, next) {
+	while (!STAILQ_EMPTY(&vnic->filter)) {
+		filter = STAILQ_FIRST(&vnic->filter);
 		rc = bnxt_clear_one_vnic_filter(bp, filter);
-		STAILQ_REMOVE(&vnic->filter, filter, bnxt_filter_info, next);
+		STAILQ_REMOVE_HEAD(&vnic->filter, next);
 		bnxt_free_filter(bp, filter);
 	}
 	return rc;
-- 
2.47.3

[PATCH 4/5] net/bnxt: fix memory leak in VF VNIC query error path

[Mohammad Shuab Siddique]

From: Kishore Padmanabha <kishore.padmanabha@broadcom.com>

bnxt_hwrm_func_vf_vnic_query_and_config() allocates vnic_ids before
calling bnxt_hwrm_func_vf_vnic_query(), but returns without freeing it
when that call fails.

Free vnic_ids on the error path too.

Fixes: 49947a13ba9e ("net/bnxt: support Tx loopback, set VF MAC and queues drop")
Cc: stable@dpdk.org

Signed-off-by: Kishore Padmanabha <kishore.padmanabha@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 99539d3705..dee35758e1 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -6280,8 +6280,10 @@ int bnxt_hwrm_func_vf_vnic_query_and_config(struct bnxt *bp, uint16_t vf,
 
 	num_vnic_ids = bnxt_hwrm_func_vf_vnic_query(bp, vf, vnic_ids);
 
-	if (num_vnic_ids < 0)
+	if (num_vnic_ids < 0) {
+		rte_free(vnic_ids);
 		return num_vnic_ids;
+	}
 
 	/* Retrieve VNIC, update bd_stall then update */
 
-- 
2.47.3

[PATCH 5/5] net/bnxt: fix VF info alloc error path memory leak

[Mohammad Shuab Siddique]

From: Joseph Wong <joseph.wong@broadcom.com>

In bnxt_alloc_vf_info(), vf_info pointer needs to be assigned prior to
the nested allocation operations. If an error occurs during the nested
allocations, the cleanup function bnxt_free_vf_info() needs the vf_info
pointer to correctly free the resources.

Fixes: 01406837bf4 ("net/bnxt: fix VF info allocation")
Cc: stable@dpdk.org

Signed-off-by: Joseph Wong <joseph.wong@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index dee35758e1..52c75c64de 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1079,6 +1079,7 @@ static int bnxt_alloc_vf_info(struct bnxt *bp, uint16_t max_vfs)
 	}
 
 	bp->pf->max_vfs = max_vfs;
+	bp->pf->vf_info = vf_info;
 	for (i = 0; i < max_vfs; i++) {
 		vf_info[i].fid = bp->pf->first_vf_id + i;
 		vf_info[i].vlan_table = rte_zmalloc("VF VLAN table",
@@ -1100,8 +1101,6 @@ static int bnxt_alloc_vf_info(struct bnxt *bp, uint16_t max_vfs)
 		STAILQ_INIT(&vf_info[i].filter);
 	}
 
-	bp->pf->vf_info = vf_info;
-
 	return 0;
 err:
 	bnxt_free_vf_info(bp);
-- 
2.47.3

[PATCH v2 0/5] net/bnxt: fix VF and firmware-facing bounds/leak issues

[Mohammad Shuab Siddique]

From: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>

This series fixes five independent out-of-bounds and memory-leak
issues in VF- and firmware-facing control-path code in the bnxt PMD:

 - an unvalidated, firmware-controlled VF ID used to index
   bp->pf->vf_info[] before range-checking it,
 - firmware-reported ring-group/L2-context counts that were not
   clamped before being cast down or summed,
 - a use-after-free in the VNIC filter list walk during cleanup
   (STAILQ_FOREACH() dereferencing a node this loop had already
   freed),
 - a memory leak on the VF VNIC query error path, and
 - a memory leak on the VF info allocation error path.

Each patch is independently bisectable and was validated with a
scoped net/bnxt build (and, for split points, an intermediate-commit
build) in addition to the full compliance gate.

v2:
* Patch 2/5 and patch 5/5 had their Fixes: tag SHA1s corrected to the
  full 12-character form per checkpatch's BAD_FIXES_TAG warning. No
  functional/code changes anywhere in this series -- patches 1/5,
  3/5 and 4/5 are unchanged from v1.

Joseph Wong (3):
  net/bnxt: add VF ID boundary check before usage
  net/bnxt: fix bounds on firmware-reported resource counts
  net/bnxt: fix VF info alloc error path memory leak

Kishore Padmanabha (1):
  net/bnxt: fix memory leak in VF VNIC query error path

Mohammad Shuab Siddique (1):
  net/bnxt: fix use-after-free in VNIC filter cleanup

 drivers/net/bnxt/bnxt.h      |  3 +++
 drivers/net/bnxt/bnxt_cpr.c  | 25 ++++++++++++++-----------
 drivers/net/bnxt/bnxt_hwrm.c | 31 +++++++++++++++++++++----------
 3 files changed, 38 insertions(+), 21 deletions(-)

--
2.47.3

[PATCH v2 1/5] net/bnxt: add VF ID boundary check before usage

[Mohammad Shuab Siddique]

From: Joseph Wong <joseph.wong@broadcom.com>

bnxt_handle_fwd_req() computed vf_id from an unvalidated,
firmware-controlled source_id and used it to index
bp->pf->vf_info[] before checking that the ID falls within the
active VF range. An out-of-range VF ID could index past
vf_info[] and cause memory corruption.

Move the range check ahead of the vf_id computation and the
vf_info[] lookup. Initialize fwd_cmd/req_len to NULL/0 at
declaration so they're always safe to use on the reject path,
which also silences a GCC 8 false-positive uninitialized-variable
warning in non-debug builds. Also guard
bnxt_hwrm_reject_fwd_resp()'s memcpy() against a NULL encaped
pointer for the same reason.

Fixes: f2a768d4d186 ("net/bnxt: add completion ring")
Cc: stable@dpdk.org

Signed-off-by: Joseph Wong <joseph.wong@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <shuab.siddique@broadcom.com>

---
v2:
* Initialize fwd_cmd/req_len at declaration instead of only on the
  reject path, removing the now-redundant explicit reset there. This
  also silences a GCC 8 false-positive uninitialized-variable warning
  in non-debug builds (reported independently after v1).
---
 drivers/net/bnxt/bnxt_cpr.c  | 27 ++++++++++++++-------------
 drivers/net/bnxt/bnxt_hwrm.c |  3 ++-
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
index 5c255de59ea..edbfc14ec60 100644
--- a/drivers/net/bnxt/bnxt_cpr.c
+++ b/drivers/net/bnxt/bnxt_cpr.c
@@ -362,10 +362,10 @@ void bnxt_handle_fwd_req(struct bnxt *bp, struct cmpl_base *cmpl)
 {
 	struct hwrm_exec_fwd_resp_input *fwreq;
 	struct hwrm_fwd_req_cmpl *fwd_cmpl = (struct hwrm_fwd_req_cmpl *)cmpl;
-	struct input *fwd_cmd;
+	struct input *fwd_cmd = NULL;
 	uint16_t fw_vf_id;
 	uint16_t vf_id;
-	uint16_t req_len;
+	uint16_t req_len = 0;
 	int rc;
 
 	if (bp->pf->active_vfs <= 0) {
@@ -375,16 +375,6 @@ void bnxt_handle_fwd_req(struct bnxt *bp, struct cmpl_base *cmpl)
 
 	/* Qualify the fwd request */
 	fw_vf_id = rte_le_to_cpu_16(fwd_cmpl->source_id);
-	vf_id = fw_vf_id - bp->pf->first_vf_id;
-
-	req_len = (rte_le_to_cpu_16(fwd_cmpl->req_len_type) &
-		   HWRM_FWD_REQ_CMPL_REQ_LEN_MASK) >>
-		HWRM_FWD_REQ_CMPL_REQ_LEN_SFT;
-	if (req_len > sizeof(fwreq->encap_request))
-		req_len = sizeof(fwreq->encap_request);
-
-	/* Locate VF's forwarded command */
-	fwd_cmd = (struct input *)bp->pf->vf_info[vf_id].req_buf;
 
 	if (fw_vf_id < bp->pf->first_vf_id ||
 	    fw_vf_id >= bp->pf->first_vf_id + bp->pf->active_vfs) {
@@ -396,6 +386,17 @@ void bnxt_handle_fwd_req(struct bnxt *bp, struct cmpl_base *cmpl)
 		goto reject;
 	}
 
+	vf_id = fw_vf_id - bp->pf->first_vf_id;
+
+	req_len = (rte_le_to_cpu_16(fwd_cmpl->req_len_type) &
+		   HWRM_FWD_REQ_CMPL_REQ_LEN_MASK) >>
+		HWRM_FWD_REQ_CMPL_REQ_LEN_SFT;
+	if (req_len > sizeof(fwreq->encap_request))
+		req_len = sizeof(fwreq->encap_request);
+
+	/* Locate VF's forwarded command */
+	fwd_cmd = (struct input *)bp->pf->vf_info[vf_id].req_buf;
+
 	if (bnxt_rcv_msg_from_vf(bp, vf_id, fwd_cmd)) {
 		/*
 		 * In older firmware versions, the MAC had to be all zeros for
@@ -495,7 +496,7 @@ void bnxt_handle_fwd_req(struct bnxt *bp, struct cmpl_base *cmpl)
 		PMD_DRV_LOG_LINE(ERR,
 			"Failed to send REJECT req VF 0x%x, type 0x%x.",
 			fw_vf_id - bp->pf->first_vf_id,
-			rte_le_to_cpu_16(fwd_cmd->req_type));
+			fwd_cmd ? rte_le_to_cpu_16(fwd_cmd->req_type) : 0xFFFF);
 	}
 
 	return;
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index a2b0c280353..2b45f05bae9 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -5520,7 +5520,8 @@ int bnxt_hwrm_reject_fwd_resp(struct bnxt *bp, uint16_t target_id,
 	HWRM_PREP(&req, HWRM_REJECT_FWD_RESP, BNXT_USE_CHIMP_MB);
 
 	req.encap_resp_target_id = rte_cpu_to_le_16(target_id);
-	memcpy(req.encap_request, encaped, ec_size);
+	if (encaped)
+		memcpy(req.encap_request, encaped, ec_size);
 
 	rc = bnxt_hwrm_send_message(bp, &req, sizeof(req), BNXT_USE_CHIMP_MB);
 
-- 
2.47.3

[PATCH v2 2/5] net/bnxt: fix bounds on firmware-reported resource counts

[Mohammad Shuab Siddique]

From: Joseph Wong <joseph.wong@broadcom.com>

When parsing max_ring_grps and max_l2_ctx values from firmware, clamp
values if they exceed a 16-bit value. max_hw_ring_grps is received as
32-bit in __bnxt_hwrm_func_qcaps() (the func_qcaps response) but cast
to 16-bit when used; add the clamp there and, as defense-in-depth,
also in bnxt_hwrm_func_resc_qcaps() in case that response's field ever
widens. max_l2_ctx is 16-bit in both responses, but its post-read
addition with max_rx_em_flows can overflow a 16-bit sum; widen the
addition to 32-bit and clamp the result.

Fixes: 2691827e82c0 ("net/bnxt: add HWRM VNIC alloc")
Fixes: 80bf6811fa0f ("net/bnxt: fix L2 context calculation for Thor")
Cc: stable@dpdk.org

Signed-off-by: Joseph Wong <joseph.wong@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
 drivers/net/bnxt/bnxt.h      |  3 +++
 drivers/net/bnxt/bnxt_hwrm.c | 16 ++++++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 336de75da0..69455af31f 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -863,6 +863,9 @@ struct bnxt {
 #define BNXT_P7_MAX_NQ_RING_CNT	512
 #define BNXT_P7_CQ_MAX_L2_ENT	8192
 
+#define BNXT_MAX_RING_GRPS	65535U
+#define BNXT_MAX_L2_CTX		65535U
+
 	uint32_t			flags2;
 #define BNXT_FLAGS2_PTP_TIMESYNC_ENABLED	BIT(0)
 #define BNXT_FLAGS2_PTP_ALARM_SCHEDULED		BIT(1)
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index aa8152eaa0..765aa7c452 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1125,6 +1125,8 @@ static int __bnxt_hwrm_func_qcaps(struct bnxt *bp)
 	HWRM_CHECK_RESULT();
 
 	bp->max_ring_grps = rte_le_to_cpu_32(resp->max_hw_ring_grps);
+	if (bp->max_ring_grps > BNXT_MAX_RING_GRPS)
+		bp->max_ring_grps = BNXT_MAX_RING_GRPS;
 	flags = rte_le_to_cpu_32(resp->flags);
 	flags_ext2 = rte_le_to_cpu_32(resp->flags_ext2);
 	flags_ext3 = rte_le_to_cpu_32(resp->flags_ext3);
@@ -1155,8 +1157,10 @@ static int __bnxt_hwrm_func_qcaps(struct bnxt *bp)
 	bp->first_vf_id = rte_le_to_cpu_16(resp->first_vf_id);
 	bp->max_rx_em_flows = rte_le_to_cpu_16(resp->max_rx_em_flows);
 	bp->max_l2_ctx = rte_le_to_cpu_16(resp->max_l2_ctxs);
-	if (!BNXT_CHIP_P5_P7(bp) && !bp->pdev->max_vfs)
-		bp->max_l2_ctx += bp->max_rx_em_flows;
+	if (!BNXT_CHIP_P5_P7(bp) && !bp->pdev->max_vfs) {
+		uint32_t l2_ctx = bp->max_l2_ctx + bp->max_rx_em_flows;
+		bp->max_l2_ctx = (uint16_t)RTE_MIN(l2_ctx, (uint32_t)BNXT_MAX_L2_CTX);
+	}
 	if (bp->vnic_cap_flags & BNXT_VNIC_CAP_COS_CLASSIFY)
 		bp->max_vnics = rte_le_to_cpu_16(BNXT_MAX_VNICS_COS_CLASSIFY);
 	else
@@ -1556,12 +1560,16 @@ int bnxt_hwrm_func_resc_qcaps(struct bnxt *bp)
 	bp->max_tx_rings = rte_le_to_cpu_16(resp->max_tx_rings);
 	bp->max_rx_rings = rte_le_to_cpu_16(resp->max_rx_rings);
 	bp->max_ring_grps = rte_le_to_cpu_32(resp->max_hw_ring_grps);
+	if (bp->max_ring_grps > BNXT_MAX_RING_GRPS)
+		bp->max_ring_grps = BNXT_MAX_RING_GRPS;
 	/* func_resource_qcaps does not return max_rx_em_flows.
 	 * So use the value provided by func_qcaps.
 	 */
 	bp->max_l2_ctx = rte_le_to_cpu_16(resp->max_l2_ctxs);
-	if (!BNXT_CHIP_P5_P7(bp) && !bp->pdev->max_vfs)
-		bp->max_l2_ctx += bp->max_rx_em_flows;
+	if (!BNXT_CHIP_P5_P7(bp) && !bp->pdev->max_vfs) {
+		uint32_t l2_ctx = bp->max_l2_ctx + bp->max_rx_em_flows;
+		bp->max_l2_ctx = (uint16_t)RTE_MIN(l2_ctx, (uint32_t)BNXT_MAX_L2_CTX);
+	}
 	if (bp->vnic_cap_flags & BNXT_VNIC_CAP_COS_CLASSIFY)
 		bp->max_vnics = rte_le_to_cpu_16(BNXT_MAX_VNICS_COS_CLASSIFY);
 	else
-- 
2.47.3

Re: [PATCH v2 2/5] net/bnxt: fix bounds on firmware-reported resource counts

[Stephen Hemminger]

On Sun, 20 Sep 2026 20:19:59 -0600
Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com> wrote:

> From: Joseph Wong <joseph.wong@broadcom.com>
> 
> When parsing max_ring_grps and max_l2_ctx values from firmware, clamp
> values if they exceed a 16-bit value. max_hw_ring_grps is received as
> 32-bit in __bnxt_hwrm_func_qcaps() (the func_qcaps response) but cast
> to 16-bit when used; add the clamp there and, as defense-in-depth,
> also in bnxt_hwrm_func_resc_qcaps() in case that response's field ever
> widens. max_l2_ctx is 16-bit in both responses, but its post-read
> addition with max_rx_em_flows can overflow a 16-bit sum; widen the
> addition to 32-bit and clamp the result.
> 
> Fixes: 2691827e82c0 ("net/bnxt: add HWRM VNIC alloc")
> Fixes: 80bf6811fa0f ("net/bnxt: fix L2 context calculation for Thor")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Joseph Wong <joseph.wong@broadcom.com>
> Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
> ---

Better  AI review flagged:



[PATCH v2 2/5] net/bnxt: fix bounds on firmware-reported resource
counts

Warning: in bnxt_hwrm_func_resc_qcaps(), resp->max_hw_ring_grps is
uint16_t in hwrm_func_resource_qcaps_output but is read with
rte_le_to_cpu_32(). On little endian the new clamp is dead code. On
big endian the 32-bit swap of a 16-bit field yields value << 16,
which the clamp turns into 65535. The fix is rte_le_to_cpu_16(); the
"in case the field ever widens" clamp should go.

[PATCH v2 3/5] net/bnxt: fix use-after-free in VNIC filter cleanup

[Mohammad Shuab Siddique]

From: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>

STAILQ_FOREACH()'s own advance step dereferences the current node's
next field after the loop body runs. The loop body here frees that
same node (bnxt_free_filter()) before the macro dereferences it on
the next iteration, so the filter list walk in
bnxt_clear_hwrm_vnic_filters() reads freed memory to find the
following entry.

Walk the list with STAILQ_FIRST()/STAILQ_REMOVE_HEAD() instead,
removing each filter from the list before freeing it so nothing is
dereferenced after being freed.

Fixes: 20ef524432dd ("net/bnxt: set L2 filters")
Cc: stable@dpdk.org

Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 765aa7c452..99539d3705 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -3593,9 +3593,10 @@ bnxt_clear_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 	struct bnxt_filter_info *filter;
 	int rc = 0;
 
-	STAILQ_FOREACH(filter, &vnic->filter, next) {
+	while (!STAILQ_EMPTY(&vnic->filter)) {
+		filter = STAILQ_FIRST(&vnic->filter);
 		rc = bnxt_clear_one_vnic_filter(bp, filter);
-		STAILQ_REMOVE(&vnic->filter, filter, bnxt_filter_info, next);
+		STAILQ_REMOVE_HEAD(&vnic->filter, next);
 		bnxt_free_filter(bp, filter);
 	}
 	return rc;
-- 
2.47.3

Re: [PATCH v2 3/5] net/bnxt: fix use-after-free in VNIC filter cleanup

[Stephen Hemminger]

On Sun, 20 Sep 2026 20:20:00 -0600
Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com> wrote:

> From: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
> 
> STAILQ_FOREACH()'s own advance step dereferences the current node's
> next field after the loop body runs. The loop body here frees that
> same node (bnxt_free_filter()) before the macro dereferences it on
> the next iteration, so the filter list walk in
> bnxt_clear_hwrm_vnic_filters() reads freed memory to find the
> following entry.
> 
> Walk the list with STAILQ_FIRST()/STAILQ_REMOVE_HEAD() instead,
> removing each filter from the list before freeing it so nothing is
> dereferenced after being freed.
> 
> Fixes: 20ef524432dd ("net/bnxt: set L2 filters")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
> ---

You could also use STAILQ_FOREACH_SAFE() instead.

AI flagged:

[PATCH v2 3/5] net/bnxt: fix use-after-free in VNIC filter cleanup

Warning: the commit message misdescribes the bug. bnxt_free_filter()
frees nothing; filters live in bp->filter_info[]. It memsets the
entry and inserts it on free_filter_list, leaving next == NULL. The
old STAILQ_FOREACH therefore stopped after the first filter. The
remaining filters were never cleared in HW and stayed on
vnic->filter. The code change is correct. Retitle, e.g. "fix VNIC
filter list walk", and describe the leaked filters.

[PATCH v2 4/5] net/bnxt: fix memory leak in VF VNIC query error path

[Mohammad Shuab Siddique]

From: Kishore Padmanabha <kishore.padmanabha@broadcom.com>

bnxt_hwrm_func_vf_vnic_query_and_config() allocates vnic_ids before
calling bnxt_hwrm_func_vf_vnic_query(), but returns without freeing it
when that call fails.

Free vnic_ids on the error path too.

Fixes: 49947a13ba9e ("net/bnxt: support Tx loopback, set VF MAC and queues drop")
Cc: stable@dpdk.org

Signed-off-by: Kishore Padmanabha <kishore.padmanabha@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 99539d3705..dee35758e1 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -6280,8 +6280,10 @@ int bnxt_hwrm_func_vf_vnic_query_and_config(struct bnxt *bp, uint16_t vf,
 
 	num_vnic_ids = bnxt_hwrm_func_vf_vnic_query(bp, vf, vnic_ids);
 
-	if (num_vnic_ids < 0)
+	if (num_vnic_ids < 0) {
+		rte_free(vnic_ids);
 		return num_vnic_ids;
+	}
 
 	/* Retrieve VNIC, update bd_stall then update */
 
-- 
2.47.3

[PATCH v2 5/5] net/bnxt: fix VF info alloc error path memory leak

[Mohammad Shuab Siddique]

From: Joseph Wong <joseph.wong@broadcom.com>

In bnxt_alloc_vf_info(), vf_info pointer needs to be assigned prior to
the nested allocation operations. If an error occurs during the nested
allocations, the cleanup function bnxt_free_vf_info() needs the vf_info
pointer to correctly free the resources.

Fixes: 01406837bf49 ("net/bnxt: fix VF info allocation")
Cc: stable@dpdk.org

Signed-off-by: Joseph Wong <joseph.wong@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index dee35758e1..52c75c64de 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1079,6 +1079,7 @@ static int bnxt_alloc_vf_info(struct bnxt *bp, uint16_t max_vfs)
 	}
 
 	bp->pf->max_vfs = max_vfs;
+	bp->pf->vf_info = vf_info;
 	for (i = 0; i < max_vfs; i++) {
 		vf_info[i].fid = bp->pf->first_vf_id + i;
 		vf_info[i].vlan_table = rte_zmalloc("VF VLAN table",
@@ -1100,8 +1101,6 @@ static int bnxt_alloc_vf_info(struct bnxt *bp, uint16_t max_vfs)
 		STAILQ_INIT(&vf_info[i].filter);
 	}
 
-	bp->pf->vf_info = vf_info;
-
 	return 0;
 err:
 	bnxt_free_vf_info(bp);
-- 
2.47.3

[PATCH v3 0/5] net/bnxt: fix VF and firmware-facing bounds/leak issues

[Mohammad Shuab Siddique]

From: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>

This series fixes five independent out-of-bounds and memory-leak
issues in VF- and firmware-facing control-path code in the bnxt PMD:

 - an unvalidated, firmware-controlled VF ID used to index
   bp->pf->vf_info[] before range-checking it,
 - firmware-reported ring-group/L2-context counts that were not
   clamped before being cast down or summed,
 - a VNIC filter list walk during cleanup that stopped after the
   first entry (STAILQ_FOREACH()'s advance step reading a next
   pointer this same loop had already zeroed via memset()),
 - a memory leak on the VF VNIC query error path, and
 - a memory leak on the VF info allocation error path.

Each patch is independently bisectable and was validated with a
scoped net/bnxt build (and, for split points, an intermediate-commit
build) in addition to the full compliance gate.

v3:
* Patch 2/5 ("fix bounds on firmware-reported resource counts"):
  removed the defense-in-depth clamp v2 added to
  bnxt_hwrm_func_resc_qcaps() and fixed the actual bug Stephen
  Hemminger found instead -- that function read a uint16_t firmware
  field with rte_le_to_cpu_32(). See that patch's own changelog.
* Patch 3/5: retitled from "fix use-after-free in VNIC filter
  cleanup" to "fix VNIC filter list walk stopping early" and
  rewrote its description -- Stephen Hemminger pointed out
  bnxt_free_filter() frees nothing, so the original description was
  wrong; the real bug is the early-termination leak. Tried Stephen's
  suggested STAILQ_FOREACH_SAFE(), but it isn't portable to this
  build (no STAILQ _SAFE variant in glibc's sys/queue.h or in DPDK's
  own RTE_TAILQ_FOREACH_SAFE() wrapper); kept v2's
  STAILQ_FIRST()/STAILQ_REMOVE_HEAD() loop, which is equivalent for
  this always-drain-the-head pattern. No code change.
* Patches 1/5, 4/5 and 5/5 are unchanged from v2.
v2:
* Patch 2/5 and patch 5/5 had their Fixes: tag SHA1s corrected to the
  full 12-character form per checkpatch's BAD_FIXES_TAG warning. No
  functional/code changes anywhere in this series -- patches 1/5,
  3/5 and 4/5 were unchanged from v1.

Joseph Wong (3):
  net/bnxt: add VF ID boundary check before usage
  net/bnxt: fix bounds on firmware-reported resource counts
  net/bnxt: fix VF info alloc error path memory leak

Kishore Padmanabha (1):
  net/bnxt: fix memory leak in VF VNIC query error path

Mohammad Shuab Siddique (1):
  net/bnxt: fix VNIC filter list walk stopping early

 drivers/net/bnxt/bnxt.h      |  3 +++
 drivers/net/bnxt/bnxt_cpr.c  | 27 ++++++++++++++-------------
 drivers/net/bnxt/bnxt_hwrm.c | 31 ++++++++++++++++++++-----------
 3 files changed, 37 insertions(+), 24 deletions(-)

--
2.47.3

[PATCH v3 1/5] net/bnxt: add VF ID boundary check before usage

[Mohammad Shuab Siddique]

From: Joseph Wong <joseph.wong@broadcom.com>

bnxt_handle_fwd_req() computed vf_id from an unvalidated,
firmware-controlled source_id and used it to index
bp->pf->vf_info[] before checking that the ID falls within the
active VF range. An out-of-range VF ID could index past
vf_info[] and cause memory corruption.

Move the range check ahead of the vf_id computation and the
vf_info[] lookup. Initialize fwd_cmd/req_len to NULL/0 at
declaration so they're always safe to use on the reject path,
which also silences a GCC 8 false-positive uninitialized-variable
warning in non-debug builds. Also guard
bnxt_hwrm_reject_fwd_resp()'s memcpy() against a NULL encaped
pointer for the same reason.

Fixes: f2a768d4d186 ("net/bnxt: add completion ring")
Cc: stable@dpdk.org

Signed-off-by: Joseph Wong <joseph.wong@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
 drivers/net/bnxt/bnxt_cpr.c  | 27 ++++++++++++++-------------
 drivers/net/bnxt/bnxt_hwrm.c |  3 ++-
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
index 5c255de59e..edbfc14ec6 100644
--- a/drivers/net/bnxt/bnxt_cpr.c
+++ b/drivers/net/bnxt/bnxt_cpr.c
@@ -362,10 +362,10 @@ void bnxt_handle_fwd_req(struct bnxt *bp, struct cmpl_base *cmpl)
 {
 	struct hwrm_exec_fwd_resp_input *fwreq;
 	struct hwrm_fwd_req_cmpl *fwd_cmpl = (struct hwrm_fwd_req_cmpl *)cmpl;
-	struct input *fwd_cmd;
+	struct input *fwd_cmd = NULL;
 	uint16_t fw_vf_id;
 	uint16_t vf_id;
-	uint16_t req_len;
+	uint16_t req_len = 0;
 	int rc;
 
 	if (bp->pf->active_vfs <= 0) {
@@ -375,16 +375,6 @@ void bnxt_handle_fwd_req(struct bnxt *bp, struct cmpl_base *cmpl)
 
 	/* Qualify the fwd request */
 	fw_vf_id = rte_le_to_cpu_16(fwd_cmpl->source_id);
-	vf_id = fw_vf_id - bp->pf->first_vf_id;
-
-	req_len = (rte_le_to_cpu_16(fwd_cmpl->req_len_type) &
-		   HWRM_FWD_REQ_CMPL_REQ_LEN_MASK) >>
-		HWRM_FWD_REQ_CMPL_REQ_LEN_SFT;
-	if (req_len > sizeof(fwreq->encap_request))
-		req_len = sizeof(fwreq->encap_request);
-
-	/* Locate VF's forwarded command */
-	fwd_cmd = (struct input *)bp->pf->vf_info[vf_id].req_buf;
 
 	if (fw_vf_id < bp->pf->first_vf_id ||
 	    fw_vf_id >= bp->pf->first_vf_id + bp->pf->active_vfs) {
@@ -396,6 +386,17 @@ void bnxt_handle_fwd_req(struct bnxt *bp, struct cmpl_base *cmpl)
 		goto reject;
 	}
 
+	vf_id = fw_vf_id - bp->pf->first_vf_id;
+
+	req_len = (rte_le_to_cpu_16(fwd_cmpl->req_len_type) &
+		   HWRM_FWD_REQ_CMPL_REQ_LEN_MASK) >>
+		HWRM_FWD_REQ_CMPL_REQ_LEN_SFT;
+	if (req_len > sizeof(fwreq->encap_request))
+		req_len = sizeof(fwreq->encap_request);
+
+	/* Locate VF's forwarded command */
+	fwd_cmd = (struct input *)bp->pf->vf_info[vf_id].req_buf;
+
 	if (bnxt_rcv_msg_from_vf(bp, vf_id, fwd_cmd)) {
 		/*
 		 * In older firmware versions, the MAC had to be all zeros for
@@ -495,7 +496,7 @@ void bnxt_handle_fwd_req(struct bnxt *bp, struct cmpl_base *cmpl)
 		PMD_DRV_LOG_LINE(ERR,
 			"Failed to send REJECT req VF 0x%x, type 0x%x.",
 			fw_vf_id - bp->pf->first_vf_id,
-			rte_le_to_cpu_16(fwd_cmd->req_type));
+			fwd_cmd ? rte_le_to_cpu_16(fwd_cmd->req_type) : 0xFFFF);
 	}
 
 	return;
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 1615b36aae..aa8152eaa0 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -5520,7 +5520,8 @@ int bnxt_hwrm_reject_fwd_resp(struct bnxt *bp, uint16_t target_id,
 	HWRM_PREP(&req, HWRM_REJECT_FWD_RESP, BNXT_USE_CHIMP_MB);
 
 	req.encap_resp_target_id = rte_cpu_to_le_16(target_id);
-	memcpy(req.encap_request, encaped, ec_size);
+	if (encaped)
+		memcpy(req.encap_request, encaped, ec_size);
 
 	rc = bnxt_hwrm_send_message(bp, &req, sizeof(req), BNXT_USE_CHIMP_MB);
 
-- 
2.47.3

[PATCH v3 2/5] net/bnxt: fix bounds on firmware-reported resource counts

[Mohammad Shuab Siddique]

From: Joseph Wong <joseph.wong@broadcom.com>

When parsing max_ring_grps and max_l2_ctx values from firmware, clamp
values if they exceed a 16-bit value. max_hw_ring_grps is received as
32-bit in __bnxt_hwrm_func_qcaps() (the func_qcaps response) but cast
to 16-bit when used; add the clamp there. max_l2_ctx is 16-bit in both
responses, but its post-read addition with max_rx_em_flows can
overflow a 16-bit sum; widen the addition to 32-bit and clamp the
result.

bnxt_hwrm_func_resc_qcaps() reads max_hw_ring_grps from a response
where that field is declared uint16_t, not uint32_t like func_qcaps's.
Reading it with rte_le_to_cpu_32() instead of rte_le_to_cpu_16() swaps
a 16-bit value as if it were 32-bit; on a big-endian host this shifts
the value into the upper 16 bits, which the clamp then silently forces
down to 65535 instead of the real value. Fixed the accessor to
rte_le_to_cpu_16() and dropped the now-unneeded clamp on this path,
since a correctly-read 16-bit value can never exceed 65535.

Fixes: 2691827e82c0 ("net/bnxt: add HWRM VNIC alloc")
Fixes: 80bf6811fa0f ("net/bnxt: fix L2 context calculation for Thor")
Cc: stable@dpdk.org

Signed-off-by: Joseph Wong <joseph.wong@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
v3:
* Removed the defense-in-depth clamp v2 had added to
  bnxt_hwrm_func_resc_qcaps() and fixed the actual bug instead:
  max_hw_ring_grps is uint16_t in that response (unlike func_qcaps's
  uint32_t field), but was read with rte_le_to_cpu_32(). On a
  big-endian host that reads the value into the wrong half of the
  register, which the clamp then silently forced down to 65535
  instead of surfacing the real value. Stephen Hemminger caught this.
  Switched to rte_le_to_cpu_16() and dropped the clamp on this path,
  since a correctly-read 16-bit value can never exceed 65535.

 drivers/net/bnxt/bnxt.h      |  3 +++
 drivers/net/bnxt/bnxt_hwrm.c | 16 +++++++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 336de75da0..69455af31f 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -863,6 +863,9 @@ struct bnxt {
 #define BNXT_P7_MAX_NQ_RING_CNT	512
 #define BNXT_P7_CQ_MAX_L2_ENT	8192
 
+#define BNXT_MAX_RING_GRPS	65535U
+#define BNXT_MAX_L2_CTX		65535U
+
 	uint32_t			flags2;
 #define BNXT_FLAGS2_PTP_TIMESYNC_ENABLED	BIT(0)
 #define BNXT_FLAGS2_PTP_ALARM_SCHEDULED		BIT(1)
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index aa8152eaa0..45c11b58da 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1125,6 +1125,8 @@ static int __bnxt_hwrm_func_qcaps(struct bnxt *bp)
 	HWRM_CHECK_RESULT();
 
 	bp->max_ring_grps = rte_le_to_cpu_32(resp->max_hw_ring_grps);
+	if (bp->max_ring_grps > BNXT_MAX_RING_GRPS)
+		bp->max_ring_grps = BNXT_MAX_RING_GRPS;
 	flags = rte_le_to_cpu_32(resp->flags);
 	flags_ext2 = rte_le_to_cpu_32(resp->flags_ext2);
 	flags_ext3 = rte_le_to_cpu_32(resp->flags_ext3);
@@ -1155,8 +1157,10 @@ static int __bnxt_hwrm_func_qcaps(struct bnxt *bp)
 	bp->first_vf_id = rte_le_to_cpu_16(resp->first_vf_id);
 	bp->max_rx_em_flows = rte_le_to_cpu_16(resp->max_rx_em_flows);
 	bp->max_l2_ctx = rte_le_to_cpu_16(resp->max_l2_ctxs);
-	if (!BNXT_CHIP_P5_P7(bp) && !bp->pdev->max_vfs)
-		bp->max_l2_ctx += bp->max_rx_em_flows;
+	if (!BNXT_CHIP_P5_P7(bp) && !bp->pdev->max_vfs) {
+		uint32_t l2_ctx = bp->max_l2_ctx + bp->max_rx_em_flows;
+		bp->max_l2_ctx = (uint16_t)RTE_MIN(l2_ctx, (uint32_t)BNXT_MAX_L2_CTX);
+	}
 	if (bp->vnic_cap_flags & BNXT_VNIC_CAP_COS_CLASSIFY)
 		bp->max_vnics = rte_le_to_cpu_16(BNXT_MAX_VNICS_COS_CLASSIFY);
 	else
@@ -1555,13 +1559,15 @@ int bnxt_hwrm_func_resc_qcaps(struct bnxt *bp)
 	bp->max_cp_rings = rte_le_to_cpu_16(resp->max_cmpl_rings);
 	bp->max_tx_rings = rte_le_to_cpu_16(resp->max_tx_rings);
 	bp->max_rx_rings = rte_le_to_cpu_16(resp->max_rx_rings);
-	bp->max_ring_grps = rte_le_to_cpu_32(resp->max_hw_ring_grps);
+	bp->max_ring_grps = rte_le_to_cpu_16(resp->max_hw_ring_grps);
 	/* func_resource_qcaps does not return max_rx_em_flows.
 	 * So use the value provided by func_qcaps.
 	 */
 	bp->max_l2_ctx = rte_le_to_cpu_16(resp->max_l2_ctxs);
-	if (!BNXT_CHIP_P5_P7(bp) && !bp->pdev->max_vfs)
-		bp->max_l2_ctx += bp->max_rx_em_flows;
+	if (!BNXT_CHIP_P5_P7(bp) && !bp->pdev->max_vfs) {
+		uint32_t l2_ctx = bp->max_l2_ctx + bp->max_rx_em_flows;
+		bp->max_l2_ctx = (uint16_t)RTE_MIN(l2_ctx, (uint32_t)BNXT_MAX_L2_CTX);
+	}
 	if (bp->vnic_cap_flags & BNXT_VNIC_CAP_COS_CLASSIFY)
 		bp->max_vnics = rte_le_to_cpu_16(BNXT_MAX_VNICS_COS_CLASSIFY);
 	else
-- 
2.47.3

[PATCH v3 3/5] net/bnxt: fix VNIC filter list walk stopping early

[Mohammad Shuab Siddique]

From: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>

bnxt_clear_hwrm_vnic_filters() used STAILQ_FOREACH() to walk
vnic->filter, clearing and freeing each filter as it went. This is
not a use-after-free: bnxt_free_filter() never actually releases the
underlying memory, it memsets the filter object (clearing its next
link too) and returns it to bp->free_filter_list for reuse, so the
object stays live and mapped.

The real bug is that memset() zeroing the filter's next field means
STAILQ_FOREACH()'s own advance step sees a NULL next pointer after
the first filter is freed, so the loop silently stops after removing
just one filter. Every other filter still linked in vnic->filter is
leaked: its hardware entry is never cleared and the filter object
itself never makes it back onto bp->free_filter_list.

Walk the list with STAILQ_FOREACH_SAFE(), which captures each
filter's next pointer before the body runs, so the walk does not
depend on a freed-and-reused object's link field. Each filter is
still unlinked from vnic->filter with STAILQ_REMOVE_HEAD() before
being freed.

Fixes: 20ef524432dd ("net/bnxt: set L2 filters")
Cc: stable@dpdk.org

Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
v3:
* Retitled from "fix use-after-free in VNIC filter cleanup" and
  rewrote the description -- Stephen Hemminger pointed out
  bnxt_free_filter() frees nothing (it recycles the object onto
  bp->free_filter_list), so this was never a use-after-free; the real
  bug is the early-termination leak described above.
* Tried STAILQ_FOREACH_SAFE(), Stephen Hemminger's suggestion, but it
  is not portable to this build: glibc's sys/queue.h (used on Linux)
  has no STAILQ _SAFE variant, and DPDK's own RTE_TAILQ_FOREACH_SAFE()
  portability wrapper has no STAILQ counterpart either. Kept the
  STAILQ_FIRST()/STAILQ_REMOVE_HEAD() loop from v2, which is
  functionally equivalent for this always-drain-the-head pattern.

 drivers/net/bnxt/bnxt_hwrm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 45c11b58da..5b07142770 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -3591,9 +3591,10 @@ bnxt_clear_hwrm_vnic_filters(struct bnxt *bp, struct bnxt_vnic_info *vnic)
 	struct bnxt_filter_info *filter;
 	int rc = 0;
 
-	STAILQ_FOREACH(filter, &vnic->filter, next) {
+	while (!STAILQ_EMPTY(&vnic->filter)) {
+		filter = STAILQ_FIRST(&vnic->filter);
 		rc = bnxt_clear_one_vnic_filter(bp, filter);
-		STAILQ_REMOVE(&vnic->filter, filter, bnxt_filter_info, next);
+		STAILQ_REMOVE_HEAD(&vnic->filter, next);
 		bnxt_free_filter(bp, filter);
 	}
 	return rc;
-- 
2.47.3

[PATCH v3 4/5] net/bnxt: fix memory leak in VF VNIC query error path

[Mohammad Shuab Siddique]

From: Kishore Padmanabha <kishore.padmanabha@broadcom.com>

bnxt_hwrm_func_vf_vnic_query_and_config() allocates vnic_ids before
calling bnxt_hwrm_func_vf_vnic_query(), but returns without freeing it
when that call fails.

Free vnic_ids on the error path too.

Fixes: 49947a13ba9e ("net/bnxt: support Tx loopback, set VF MAC and queues drop")
Cc: stable@dpdk.org

Signed-off-by: Kishore Padmanabha <kishore.padmanabha@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 5b07142770..f6623693fd 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -6278,8 +6278,10 @@ int bnxt_hwrm_func_vf_vnic_query_and_config(struct bnxt *bp, uint16_t vf,
 
 	num_vnic_ids = bnxt_hwrm_func_vf_vnic_query(bp, vf, vnic_ids);
 
-	if (num_vnic_ids < 0)
+	if (num_vnic_ids < 0) {
+		rte_free(vnic_ids);
 		return num_vnic_ids;
+	}
 
 	/* Retrieve VNIC, update bd_stall then update */
 
-- 
2.47.3

[PATCH v3 5/5] net/bnxt: fix VF info alloc error path memory leak

[Mohammad Shuab Siddique]

From: Joseph Wong <joseph.wong@broadcom.com>

In bnxt_alloc_vf_info(), vf_info pointer needs to be assigned prior to
the nested allocation operations. If an error occurs during the nested
allocations, the cleanup function bnxt_free_vf_info() needs the vf_info
pointer to correctly free the resources.

Fixes: 01406837bf49 ("net/bnxt: fix VF info allocation")
Cc: stable@dpdk.org

Signed-off-by: Joseph Wong <joseph.wong@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index f6623693fd..3a94838e3b 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1079,6 +1079,7 @@ static int bnxt_alloc_vf_info(struct bnxt *bp, uint16_t max_vfs)
 	}
 
 	bp->pf->max_vfs = max_vfs;
+	bp->pf->vf_info = vf_info;
 	for (i = 0; i < max_vfs; i++) {
 		vf_info[i].fid = bp->pf->first_vf_id + i;
 		vf_info[i].vlan_table = rte_zmalloc("VF VLAN table",
@@ -1100,8 +1101,6 @@ static int bnxt_alloc_vf_info(struct bnxt *bp, uint16_t max_vfs)
 		STAILQ_INIT(&vf_info[i].filter);
 	}
 
-	bp->pf->vf_info = vf_info;
-
 	return 0;
 err:
 	bnxt_free_vf_info(bp);
-- 
2.47.3