[PATCH 0/5] net/bnxt: fix flow, Rx and naming bounds issues

[PATCH 0/5] net/bnxt: fix flow, Rx and naming bounds issues

[Mohammad Shuab Siddique]

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

This series fixes five independent out-of-bounds issues in flow,
Rx-datapath and naming code in the bnxt PMD:

 - two stack-allocated variable-length arrays in flow-stats sizing
   that risked stack exhaustion,
 - a TPA aggregation ID read from a completion and used to index
   rxr->tpa_info[] without a bounds check,
 - three separate sprintf() calls into fixed-size buffers with no
   bound on the formatted string length, plus three related bugs
   (a leak, a stale flag, and a lock left held) introduced by this
   change's own new early-return paths and fixed here,
 - a caller-supplied MAC pool index used before being validated
   against bp->max_vnics, and an unbounded flow item/action skip
   loop that could walk off the end of the pattern array, and
 - a firmware-supplied Rx completion opaque value used unmasked as
   an rx_buf_ring[] index, an aggregation-segment count guarded only
   by a release-mode-compiled-out RTE_ASSERT, and an unclamped VF
   VNIC-count from firmware.

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.

Chenna Arnoori (1):
  net/bnxt: fix bounds in MAC pool index and flow parsing

Joseph Wong (1):
  net/bnxt: fix stack exhaustion in flow stats

Keegan Freyhof (1):
  net/bnxt: harden sprintf bounds for device memory names

Kishore Padmanabha (1):
  net/bnxt: fix TPA agg Rx descriptor and VNIC query bounds

Mohammad Shuab Siddique (1):
  net/bnxt: fix bounds on TPA aggregation ID from completions

 drivers/net/bnxt/bnxt.h        | 15 +++++++
 drivers/net/bnxt/bnxt_ethdev.c | 43 +++++++++++++-------
 drivers/net/bnxt/bnxt_flow.c   | 28 +++++++++----
 drivers/net/bnxt/bnxt_hwrm.c   | 74 +++++++++++++++++++++++++---------
 drivers/net/bnxt/bnxt_rxr.c    | 57 ++++++++++++++++++++------
 drivers/net/bnxt/bnxt_stats.c  | 18 ++++-----
 6 files changed, 173 insertions(+), 62 deletions(-)

-- 
2.47.3

[PATCH 1/5] net/bnxt: fix stack exhaustion in flow stats

[Mohammad Shuab Siddique]

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

In bnxt_flow_stats_cnt(), two variable-length arrays are allocated
on the stack. These arrays are allocated merely to be used to
calculate the dimensions via RTE_DIM(). Replace with the
mathematical equivalent to avoid potential stack exhaustion.

Fixes: 1e2f8aca2cc1 ("net/bnxt: fix allocation of flow stat related structs")
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_stats.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_stats.c b/drivers/net/bnxt/bnxt_stats.c
index ba858710a5..c4efcb4b17 100644
--- a/drivers/net/bnxt/bnxt_stats.c
+++ b/drivers/net/bnxt/bnxt_stats.c
@@ -1095,12 +1095,8 @@ int bnxt_flow_stats_cnt(struct bnxt *bp)
 {
 	if (bp->fw_cap & BNXT_FW_CAP_ADV_FLOW_COUNTERS &&
 	    bp->fw_cap & BNXT_FW_CAP_ADV_FLOW_MGMT &&
-	    BNXT_FLOW_XSTATS_EN(bp)) {
-		struct bnxt_xstats_name_off flow_bytes[bp->max_l2_ctx];
-		struct bnxt_xstats_name_off flow_pkts[bp->max_l2_ctx];
-
-		return RTE_DIM(flow_bytes) + RTE_DIM(flow_pkts);
-	}
+	    BNXT_FLOW_XSTATS_EN(bp))
+		return 2 * bp->max_l2_ctx;
 
 	return 0;
 }
-- 
2.47.3

[PATCH 2/5] net/bnxt: fix bounds on TPA aggregation ID from completions

[Mohammad Shuab Siddique]

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

TPA start/end and TPA v2 abuf paths indexed rxr->tpa_info[] using
the aggregation ID from the completion without checking
BNXT_TPA_MAX_AGGS. Add bounds checks and schedule a ring reset on
failure. Also fix abuf aggregation ID decode to use
rte_le_to_cpu_16() when reading the device field.

Fixes: 0958d8b6435 ("net/bnxt: support LRO")
Fixes: b150a7e7ee6 ("net/bnxt: support LRO on Thor adapters")
Cc: stable@dpdk.org

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

diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 0fab4ddf78..87640eaa79 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -236,12 +236,19 @@ static void bnxt_tpa_start(struct bnxt_rx_queue *rxq,
 			   struct rx_tpa_start_cmpl_hi *tpa_start1)
 {
 	struct bnxt_rx_ring_info *rxr = rxq->rx_ring;
-	uint16_t agg_id;
-	uint16_t data_cons;
 	struct bnxt_tpa_info *tpa_info;
+	uint32_t data_cons, agg_id;
+	struct bnxt *bp = rxq->bp;
 	struct rte_mbuf *mbuf;
 
-	agg_id = bnxt_tpa_start_agg_id(rxq->bp, tpa_start);
+	agg_id = bnxt_tpa_start_agg_id(bp, tpa_start);
+	if (unlikely(agg_id >= BNXT_TPA_MAX_AGGS(bp))) {
+		PMD_DRV_LOG_LINE(ERR,
+				 "TPA start: invalid agg_id %u (max %u)",
+				 agg_id, BNXT_TPA_MAX_AGGS(bp));
+		bnxt_sched_ring_reset(rxq);
+		return;
+	}
 
 	data_cons = tpa_start->opaque;
 	tpa_info = &rxr->tpa_info[agg_id];
@@ -268,7 +275,7 @@ static void bnxt_tpa_start(struct bnxt_rx_queue *rxq,
 	mbuf->port = rxq->port_id;
 	mbuf->ol_flags = RTE_MBUF_F_RX_LRO;
 
-	bnxt_tpa_get_metadata(rxq->bp, tpa_info, tpa_start, tpa_start1);
+	bnxt_tpa_get_metadata(bp, tpa_info, tpa_start, tpa_start1);
 
 	if (likely(tpa_info->hash_valid)) {
 		mbuf->hash.rss = tpa_info->rss_hash;
@@ -278,7 +285,7 @@ static void bnxt_tpa_start(struct bnxt_rx_queue *rxq,
 		mbuf->ol_flags |= RTE_MBUF_F_RX_FDIR | RTE_MBUF_F_RX_FDIR_ID;
 	}
 
-	if (tpa_info->vlan_valid && BNXT_RX_VLAN_STRIP_EN(rxq->bp)) {
+	if (tpa_info->vlan_valid && BNXT_RX_VLAN_STRIP_EN(bp)) {
 		mbuf->vlan_tci = tpa_info->vlan;
 		mbuf->ol_flags |= RTE_MBUF_F_RX_VLAN | RTE_MBUF_F_RX_VLAN_STRIPPED;
 	}
@@ -421,20 +428,21 @@ static inline struct rte_mbuf *bnxt_tpa_end(
 {
 	struct bnxt_cp_ring_info *cpr = rxq->cp_ring;
 	struct bnxt_rx_ring_info *rxr = rxq->rx_ring;
-	uint16_t agg_id;
+	struct bnxt_tpa_info *tpa_info;
+	struct bnxt *bp = rxq->bp;
+	uint8_t payload_offset;
 	struct rte_mbuf *mbuf;
 	uint8_t agg_bufs;
-	uint8_t payload_offset;
-	struct bnxt_tpa_info *tpa_info;
+	uint32_t agg_id;
 
 	if (unlikely(rxq->in_reset)) {
 		PMD_DRV_LOG_LINE(ERR, "rxq->in_reset: raw_cp_cons:%d",
 			    *raw_cp_cons);
-		bnxt_discard_rx(rxq->bp, cpr, raw_cp_cons, tpa_end);
+		bnxt_discard_rx(bp, cpr, raw_cp_cons, tpa_end);
 		return NULL;
 	}
 
-	if (BNXT_CHIP_P5_P7(rxq->bp)) {
+	if (BNXT_CHIP_P5_P7(bp)) {
 		struct rx_tpa_v2_end_cmpl *th_tpa_end;
 		struct rx_tpa_v2_end_cmpl_hi *th_tpa_end1;
 
@@ -451,6 +459,14 @@ static inline struct rte_mbuf *bnxt_tpa_end(
 		payload_offset = tpa_end->payload_offset;
 	}
 
+	if (unlikely(agg_id >= BNXT_TPA_MAX_AGGS(bp))) {
+		PMD_DRV_LOG_LINE(ERR,
+				 "TPA end: invalid agg_id %u (max %u)",
+				 agg_id, BNXT_TPA_MAX_AGGS(bp));
+		bnxt_sched_ring_reset(rxq);
+		return NULL;
+	}
+
 	tpa_info = &rxr->tpa_info[agg_id];
 	mbuf = tpa_info->mbuf;
 	RTE_ASSERT(mbuf != NULL);
@@ -1135,9 +1151,18 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
 
 	if (cmp_type == RX_TPA_V2_ABUF_CMPL_TYPE_RX_TPA_AGG) {
 		struct rx_tpa_v2_abuf_cmpl *rx_agg = (void *)rxcmp;
-		uint16_t agg_id = rte_cpu_to_le_16(rx_agg->agg_id);
+		uint32_t agg_id = rte_le_to_cpu_16(rx_agg->agg_id);
 		struct bnxt_tpa_info *tpa_info;
 
+		if (unlikely(agg_id >= BNXT_TPA_MAX_AGGS(bp))) {
+			PMD_DRV_LOG_LINE(ERR,
+					 "TPA abuf: invalid agg_id %u (max %u)",
+					 agg_id, BNXT_TPA_MAX_AGGS(bp));
+			bnxt_sched_ring_reset(rxq);
+			rc = -EINVAL;
+			goto next_rx;
+		}
+
 		tpa_info = &rxr->tpa_info[agg_id];
 		RTE_ASSERT(tpa_info->agg_count < 16);
 		tpa_info->agg_arr[tpa_info->agg_count++] = *rx_agg;
-- 
2.47.3

[PATCH 3/5] net/bnxt: harden sprintf bounds for device memory names

[Mohammad Shuab Siddique]

From: Keegan Freyhof <keegan.freyhof@broadcom.com>

sprintf() into fixed-size stack buffers such as
char type[RTE_MEMZONE_NAMESIZE] does not bound the write to the
buffer, so a long enough formatted string (e.g. from PCI address
fields) overflows it.

Add check_snprintf_rc(), a helper that logs and returns an error on a
failed snprintf() call and logs (without failing) a truncated one.
Convert sprintf() calls building a memzone/malloc name to snprintf()
plus this check, and add the same check to the existing snprintf()
calls building HWRM CFA pair_name request fields. Unlike a truncated
memzone/malloc label, a truncated pair_name would be sent to firmware
and could match the wrong pair or none at all, so
bnxt_hwrm_cfa_pair_exists()/_alloc()/_free() additionally reject a
truncated pair_name outright instead of proceeding.

This change introduces and corrects three bugs of its own.

In bnxt_hwrm_ver_get(), free bp->hwrm_short_cmd_req_addr (and null it)
before checking the new snprintf's return, instead of after, so an
early return on a snprintf failure doesn't leak the previous
allocation; also clear BNXT_FLAG_SHORT_CMD on that same early return,
since it may already have been set a few lines above and would
otherwise claim short-command support with no buffer allocated. In
bnxt_hwrm_cfa_pair_exists()/_alloc()/_free(), the new early-return
paths exited without releasing bp->hwrm_lock (held since the
preceding HWRM_PREP()), which would deadlock every later HWRM call;
added the missing HWRM_UNLOCK() before each return.

Signed-off-by: Keegan Freyhof <keegan.freyhof@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
 drivers/net/bnxt/bnxt.h        | 15 +++++++
 drivers/net/bnxt/bnxt_ethdev.c | 24 ++++++++----
 drivers/net/bnxt/bnxt_hwrm.c   | 71 +++++++++++++++++++++++++---------
 drivers/net/bnxt/bnxt_stats.c  | 10 +++--
 4 files changed, 92 insertions(+), 28 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 336de75da0..bfd6cf15b7 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -1285,6 +1285,21 @@ extern int bnxt_logtype_driver;
 						       BNXT_LINK_SPEEDS_V2_VF((bp))))
 #define BNXT_MAX_SPEED_LANES 8
 #define BNXT_SUPPORTS_TPA(bp)  (!BNXT_CHIP_P5_P7(bp) || (bp)->max_tpa_v2)
+
+static inline int
+check_snprintf_rc(int rc, size_t max_size, const char *ctx)
+{
+	if (rc < 0) {
+		PMD_DRV_LOG_LINE(ERR, "Error when creating string for %s", ctx);
+		return rc;
+	}
+
+	if (rc >= (int)max_size)
+		PMD_DRV_LOG_LINE(INFO, "String truncated when creating string for %s", ctx);
+
+	return 0;
+}
+
 extern const struct rte_flow_ops bnxt_ulp_rte_flow_ops;
 int32_t bnxt_ulp_port_init(struct bnxt *bp);
 void bnxt_ulp_port_deinit(struct bnxt *bp);
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 8e8ead8f61..27cf67c04f 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -647,13 +647,15 @@ static int bnxt_init_fc_ctx_mem(struct bnxt *bp)
 {
 	struct rte_pci_device *pdev = bp->pdev;
 	char type[RTE_MEMZONE_NAMESIZE];
+	int rc = 0, snp_rc = 0;
 	uint16_t max_fc;
-	int rc = 0;
 
 	max_fc = bp->flow_stat->max_fc;
 
-	sprintf(type, "bnxt_rx_fc_in_" PCI_PRI_FMT, pdev->addr.domain,
+	snp_rc = snprintf(type, sizeof(type), "bnxt_rx_fc_in_" PCI_PRI_FMT, pdev->addr.domain,
 		pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
+	if (check_snprintf_rc(snp_rc, sizeof(type), "bnxt_rx_fc_in_") < 0)
+		return snp_rc;
 	/* 4 bytes for each counter-id */
 	rc = bnxt_alloc_ctx_mem_buf(bp, type,
 				    max_fc * 4,
@@ -661,8 +663,10 @@ static int bnxt_init_fc_ctx_mem(struct bnxt *bp)
 	if (rc)
 		return rc;
 
-	sprintf(type, "bnxt_rx_fc_out_" PCI_PRI_FMT, pdev->addr.domain,
+	snp_rc = snprintf(type, sizeof(type), "bnxt_rx_fc_out_" PCI_PRI_FMT, pdev->addr.domain,
 		pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
+	if (check_snprintf_rc(snp_rc, sizeof(type), "bnxt_rx_fc_out_") < 0)
+		return snp_rc;
 	/* 16 bytes for each counter - 8 bytes pkt_count, 8 bytes byte_count */
 	rc = bnxt_alloc_ctx_mem_buf(bp, type,
 				    max_fc * 16,
@@ -670,8 +674,10 @@ static int bnxt_init_fc_ctx_mem(struct bnxt *bp)
 	if (rc)
 		return rc;
 
-	sprintf(type, "bnxt_tx_fc_in_" PCI_PRI_FMT, pdev->addr.domain,
+	snp_rc = snprintf(type, sizeof(type), "bnxt_tx_fc_in_" PCI_PRI_FMT, pdev->addr.domain,
 		pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
+	if (check_snprintf_rc(snp_rc, sizeof(type), "bnxt_tx_fc_in_") < 0)
+		return snp_rc;
 	/* 4 bytes for each counter-id */
 	rc = bnxt_alloc_ctx_mem_buf(bp, type,
 				    max_fc * 4,
@@ -679,8 +685,10 @@ static int bnxt_init_fc_ctx_mem(struct bnxt *bp)
 	if (rc)
 		return rc;
 
-	sprintf(type, "bnxt_tx_fc_out_" PCI_PRI_FMT, pdev->addr.domain,
+	snp_rc = snprintf(type, sizeof(type), "bnxt_tx_fc_out_" PCI_PRI_FMT, pdev->addr.domain,
 		pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
+	if (check_snprintf_rc(snp_rc, sizeof(type), "bnxt_tx_fc_out_") < 0)
+		return snp_rc;
 	/* 16 bytes for each counter - 8 bytes pkt_count, 8 bytes byte_count */
 	rc = bnxt_alloc_ctx_mem_buf(bp, type,
 				    max_fc * 16,
@@ -5226,8 +5234,8 @@ int bnxt_alloc_ctx_pg_tbls(struct bnxt *bp)
 {
 	struct bnxt_ctx_mem_info *ctx = bp->ctx;
 	struct bnxt_ctx_mem *ctx2;
+	int rc = 0, snp_rc = 0;
 	uint16_t type;
-	int rc = 0;
 
 	ctx2 = &ctx->ctx_arr[0];
 	for (type = 0; type < ctx->types && rc == 0; type++) {
@@ -5248,7 +5256,9 @@ int bnxt_alloc_ctx_pg_tbls(struct bnxt *bp)
 		for (i = 0; i < w && rc == 0; i++) {
 			char name[RTE_MEMZONE_NAMESIZE] = {0};
 
-			sprintf(name, "_%d_%d", i, type);
+			snp_rc = snprintf(name, sizeof(name), "_%d_%d", i, type);
+			if (check_snprintf_rc(snp_rc, sizeof(name), "index and type.") < 0)
+				return snp_rc;
 
 			if (ctxm->entry_multiple)
 				entries = bnxt_roundup(ctxm->max_entries,
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 1615b36aae..0143da8789 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1583,12 +1583,12 @@ int bnxt_hwrm_func_resc_qcaps(struct bnxt *bp)
 
 int bnxt_hwrm_ver_get(struct bnxt *bp, uint32_t timeout)
 {
-	int rc = 0;
 	struct hwrm_ver_get_input req = {.req_type = 0 };
 	struct hwrm_ver_get_output *resp = bp->hwrm_cmd_resp_addr;
 	uint32_t fw_version;
 	uint16_t max_resp_len;
 	char type[RTE_MEMZONE_NAMESIZE];
+	int rc = 0, snp_rc = 0;
 	uint32_t dev_caps_cfg;
 
 	bp->max_req_len = HWRM_MAX_REQ_LEN;
@@ -1669,11 +1669,15 @@ int bnxt_hwrm_ver_get(struct bnxt *bp, uint32_t timeout)
 	     (dev_caps_cfg &
 	      HWRM_VER_GET_OUTPUT_DEV_CAPS_CFG_SHORT_CMD_REQUIRED)) ||
 	    bp->hwrm_max_ext_req_len > HWRM_MAX_REQ_LEN) {
-		sprintf(type, "bnxt_hwrm_short_" PCI_PRI_FMT,
-			bp->pdev->addr.domain, bp->pdev->addr.bus,
-			bp->pdev->addr.devid, bp->pdev->addr.function);
-
+		snp_rc = snprintf(type, sizeof(type), "bnxt_hwrm_short_" PCI_PRI_FMT,
+				  bp->pdev->addr.domain, bp->pdev->addr.bus,
+				  bp->pdev->addr.devid, bp->pdev->addr.function);
 		rte_free(bp->hwrm_short_cmd_req_addr);
+		bp->hwrm_short_cmd_req_addr = NULL;
+		if (check_snprintf_rc(snp_rc, sizeof(type), "bnxt_hwrm_short_") < 0) {
+			bp->flags &= ~BNXT_FLAG_SHORT_CMD;
+			return snp_rc;
+		}
 
 		bp->hwrm_short_cmd_req_addr =
 				rte_malloc(type, bp->hwrm_max_ext_req_len, 0);
@@ -3541,9 +3545,12 @@ int bnxt_alloc_hwrm_resources(struct bnxt *bp)
 {
 	struct rte_pci_device *pdev = bp->pdev;
 	char type[RTE_MEMZONE_NAMESIZE];
+	int snp_rc = 0;
 
-	sprintf(type, "bnxt_hwrm_" PCI_PRI_FMT, pdev->addr.domain,
-		pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
+	snp_rc = snprintf(type, sizeof(type), "bnxt_hwrm_" PCI_PRI_FMT, pdev->addr.domain,
+			  pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
+	if (check_snprintf_rc(snp_rc, sizeof(type), "bnxt_hwrm_") < 0)
+		return snp_rc;
 	bp->max_resp_len = BNXT_PAGE_SIZE;
 	bp->hwrm_cmd_resp_addr = rte_malloc(type, bp->max_resp_len, 0);
 	if (bp->hwrm_cmd_resp_addr == NULL)
@@ -6780,6 +6787,7 @@ static int bnxt_alloc_all_ctx_pg_info(struct bnxt *bp)
 {
 	struct bnxt_ctx_mem_info *ctx = bp->ctx;
 	char name[RTE_MEMZONE_NAMESIZE];
+	int snp_rc = 0;
 	uint16_t type;
 
 	for (type = 0; type < ctx->types; type++) {
@@ -6792,8 +6800,10 @@ static int bnxt_alloc_all_ctx_pg_info(struct bnxt *bp)
 		if (ctxm->instance_bmap)
 			n = hweight32(ctxm->instance_bmap);
 
-		sprintf(name, "bnxt_ctx_pgmem_%d_%d",
-			bp->eth_dev->data->port_id, type);
+		snp_rc = snprintf(name, sizeof(name), "bnxt_ctx_pgmem_%d_%d",
+				  bp->eth_dev->data->port_id, type);
+		if (check_snprintf_rc(snp_rc, sizeof(name), "bnxt_ctx_pgmem_") < 0)
+			return snp_rc;
 		ctxm->pg_info = rte_malloc(name, sizeof(*ctxm->pg_info) * n,
 					   RTE_CACHE_LINE_SIZE);
 		if (!ctxm->pg_info)
@@ -7751,7 +7761,7 @@ int bnxt_hwrm_cfa_pair_exists(struct bnxt *bp, struct bnxt_representor *rep_bp)
 {
 	struct hwrm_cfa_pair_info_output *resp = bp->hwrm_cmd_resp_addr;
 	struct hwrm_cfa_pair_info_input req = {0};
-	int rc = 0;
+	int rc = 0, snp_rc = 0;
 
 	if (!(BNXT_PF(bp) || BNXT_VF_IS_TRUSTED(bp))) {
 		PMD_DRV_LOG_LINE(DEBUG,
@@ -7760,8 +7770,16 @@ int bnxt_hwrm_cfa_pair_exists(struct bnxt *bp, struct bnxt_representor *rep_bp)
 	}
 
 	HWRM_PREP(&req, HWRM_CFA_PAIR_INFO, BNXT_USE_CHIMP_MB);
-	snprintf(req.pair_name, sizeof(req.pair_name), "%svfr%d",
-		 bp->eth_dev->data->name, rep_bp->vf_id);
+	snp_rc = snprintf(req.pair_name, sizeof(req.pair_name), "%svfr%d",
+			  bp->eth_dev->data->name, rep_bp->vf_id);
+	if (check_snprintf_rc(snp_rc, sizeof(req.pair_name), "svfr") < 0) {
+		HWRM_UNLOCK();
+		return snp_rc;
+	}
+	if (snp_rc >= (int)sizeof(req.pair_name)) {
+		HWRM_UNLOCK();
+		return -EINVAL;
+	}
 	req.flags =
 		rte_cpu_to_le_32(HWRM_CFA_PAIR_INFO_INPUT_FLAGS_LOOKUP_TYPE);
 
@@ -7779,7 +7797,7 @@ int bnxt_hwrm_cfa_pair_alloc(struct bnxt *bp, struct bnxt_representor *rep_bp)
 {
 	struct hwrm_cfa_pair_alloc_output *resp = bp->hwrm_cmd_resp_addr;
 	struct hwrm_cfa_pair_alloc_input req = {0};
-	int rc;
+	int rc, snp_rc = 0;
 
 	if (!(BNXT_PF(bp) || BNXT_VF_IS_TRUSTED(bp))) {
 		PMD_DRV_LOG_LINE(DEBUG,
@@ -7789,8 +7807,16 @@ int bnxt_hwrm_cfa_pair_alloc(struct bnxt *bp, struct bnxt_representor *rep_bp)
 
 	HWRM_PREP(&req, HWRM_CFA_PAIR_ALLOC, BNXT_USE_CHIMP_MB);
 	req.pair_mode = HWRM_CFA_PAIR_FREE_INPUT_PAIR_MODE_REP2FN_TRUFLOW;
-	snprintf(req.pair_name, sizeof(req.pair_name), "%svfr%d",
-		 bp->eth_dev->data->name, rep_bp->vf_id);
+	snp_rc = snprintf(req.pair_name, sizeof(req.pair_name), "%svfr%d",
+			  bp->eth_dev->data->name, rep_bp->vf_id);
+	if (check_snprintf_rc(snp_rc, sizeof(req.pair_name), "svfr") < 0) {
+		HWRM_UNLOCK();
+		return snp_rc;
+	}
+	if (snp_rc >= (int)sizeof(req.pair_name)) {
+		HWRM_UNLOCK();
+		return -EINVAL;
+	}
 
 	req.pf_b_id = rep_bp->parent_pf_idx;
 	req.vf_b_id = BNXT_REP_PF(rep_bp) ? rte_cpu_to_le_16(((uint16_t)-1)) :
@@ -7825,7 +7851,7 @@ int bnxt_hwrm_cfa_pair_free(struct bnxt *bp, struct bnxt_representor *rep_bp)
 {
 	struct hwrm_cfa_pair_free_output *resp = bp->hwrm_cmd_resp_addr;
 	struct hwrm_cfa_pair_free_input req = {0};
-	int rc;
+	int rc, snp_rc = 0;
 
 	if (!(BNXT_PF(bp) || BNXT_VF_IS_TRUSTED(bp))) {
 		PMD_DRV_LOG_LINE(DEBUG,
@@ -7834,8 +7860,17 @@ int bnxt_hwrm_cfa_pair_free(struct bnxt *bp, struct bnxt_representor *rep_bp)
 	}
 
 	HWRM_PREP(&req, HWRM_CFA_PAIR_FREE, BNXT_USE_CHIMP_MB);
-	snprintf(req.pair_name, sizeof(req.pair_name), "%svfr%d",
-		 bp->eth_dev->data->name, rep_bp->vf_id);
+	snp_rc = snprintf(req.pair_name, sizeof(req.pair_name), "%svfr%d",
+			  bp->eth_dev->data->name, rep_bp->vf_id);
+	if (check_snprintf_rc(snp_rc, sizeof(req.pair_name), "svfr") < 0) {
+		HWRM_UNLOCK();
+		return snp_rc;
+	}
+	if (snp_rc >= (int)sizeof(req.pair_name)) {
+		HWRM_UNLOCK();
+		return -EINVAL;
+	}
+
 	req.pf_b_id = rep_bp->parent_pf_idx;
 	req.pair_mode = HWRM_CFA_PAIR_FREE_INPUT_PAIR_MODE_REP2FN_TRUFLOW;
 	req.vf_id = BNXT_REP_PF(rep_bp) ? rte_cpu_to_le_16(((uint16_t)-1)) :
diff --git a/drivers/net/bnxt/bnxt_stats.c b/drivers/net/bnxt/bnxt_stats.c
index c4efcb4b17..19153f96ae 100644
--- a/drivers/net/bnxt/bnxt_stats.c
+++ b/drivers/net/bnxt/bnxt_stats.c
@@ -1108,7 +1108,7 @@ int bnxt_dev_xstats_get_names_op(struct rte_eth_dev *eth_dev,
 	struct bnxt *bp = (struct bnxt *)eth_dev->data->dev_private;
 	unsigned int stat_cnt;
 	unsigned int i, count = 0, sz;
-	int rc;
+	int rc, snp_rc = 0;
 
 	rc = is_bnxt_in_error(bp);
 	if (rc)
@@ -1183,12 +1183,16 @@ int bnxt_dev_xstats_get_names_op(struct rte_eth_dev *eth_dev,
 		for (i = 0; i < bp->max_l2_ctx; i++) {
 			char buf[RTE_ETH_XSTATS_NAME_SIZE];
 
-			sprintf(buf, "flow_%d_bytes", i);
+			snp_rc = snprintf(buf, sizeof(buf), "flow_%d_bytes", i);
+			if (check_snprintf_rc(snp_rc, sizeof(buf), "flow_%d_bytes") < 0)
+				return snp_rc;
 			strlcpy(xstats_names[count].name, buf,
 				sizeof(xstats_names[count].name));
 			count++;
 
-			sprintf(buf, "flow_%d_packets", i);
+			snp_rc = snprintf(buf, sizeof(buf), "flow_%d_packets", i);
+			if (check_snprintf_rc(snp_rc, sizeof(buf), "flow_%d_packets") < 0)
+				return snp_rc;
 			strlcpy(xstats_names[count].name, buf,
 				sizeof(xstats_names[count].name));
 
-- 
2.47.3

[PATCH 4/5] net/bnxt: fix bounds in MAC pool index and flow parsing

[Mohammad Shuab Siddique]

From: Chenna Arnoori <chenna.arnoori@broadcom.com>

Two independent out-of-bounds issues in the driver:

- bnxt_mac_addr_add_op() indexed bp->vnic_info[pool] with a
  caller-supplied pool before validating it against bp->max_vnics, and
  before checking bp->vnic_info was even allocated yet (it is NULL
  until the port is started). The existing "if (!vnic)" check was
  always false, since vnic held the address of an array element and
  is never NULL. Reorder to check dev_started/vnic_info first, then
  bounds-check pool against max_vnics before indexing.

- bnxt_flow_non_void_item()/bnxt_flow_non_void_action() looped
  unconditionally until a non-VOID item/action was found, walking off
  the end of a pattern/actions array that lacked a terminating END
  item. Bound the skip loop and stop advancing once the limit is hit.

Fixes: 51fafb89a9a ("net/bnxt: get rid of ff pools and use VNIC info array")
Fixes: 5c1171c972 ("net/bnxt: refactor filter/flow")
Cc: stable@dpdk.org

Signed-off-by: Chenna Arnoori <chenna.arnoori@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 16 ++++++++++------
 drivers/net/bnxt/bnxt_flow.c   | 28 ++++++++++++++++++++--------
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 27cf67c04f..db9b49238a 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -2113,7 +2113,7 @@ static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 				uint32_t index, uint32_t pool)
 {
 	struct bnxt *bp = eth_dev->data->dev_private;
-	struct bnxt_vnic_info *vnic = &bp->vnic_info[pool];
+	struct bnxt_vnic_info *vnic;
 	int rc = 0;
 
 	rc = is_bnxt_in_error(bp);
@@ -2125,15 +2125,19 @@ static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 		return -ENOTSUP;
 	}
 
-	if (!vnic) {
-		PMD_DRV_LOG_LINE(ERR, "VNIC not found for pool %d!", pool);
-		return -EINVAL;
-	}
-
 	/* Filter settings will get applied when port is started */
 	if (!eth_dev->data->dev_started)
 		return 0;
 
+	if (bp->vnic_info == NULL)
+		return 0;
+
+	if (pool >= bp->max_vnics) {
+		PMD_DRV_LOG_LINE(ERR, "Pool %u exceeds VNIC count %u!", pool, bp->max_vnics);
+		return -EINVAL;
+	}
+	vnic = &bp->vnic_info[pool];
+
 	rc = bnxt_add_mac_filter(bp, vnic, mac_addr, index, pool);
 
 	return rc;
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index a2e590540b..14d52e1818 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -58,24 +58,36 @@ bnxt_flow_args_validate(const struct rte_flow_attr *attr,
 	return 0;
 }
 
+#define BNXT_MAX_FLOW_ITEMS 256
+
 static const struct rte_flow_item *
 bnxt_flow_non_void_item(const struct rte_flow_item *cur)
 {
-	while (1) {
-		if (cur->type != RTE_FLOW_ITEM_TYPE_VOID)
-			return cur;
+	int i = 0;
+
+	if (!cur)
+		return NULL;
+
+	while (cur->type == RTE_FLOW_ITEM_TYPE_VOID && i < BNXT_MAX_FLOW_ITEMS) {
 		cur++;
+		i++;
 	}
+	return cur;
 }
 
 static const struct rte_flow_action *
 bnxt_flow_non_void_action(const struct rte_flow_action *cur)
 {
-	while (1) {
-		if (cur->type != RTE_FLOW_ACTION_TYPE_VOID)
-			return cur;
+	int i = 0;
+
+	if (!cur)
+		return NULL;
+
+	while (cur->type == RTE_FLOW_ACTION_TYPE_VOID && i < BNXT_MAX_FLOW_ITEMS) {
 		cur++;
+		i++;
 	}
+	return cur;
 }
 
 static int
@@ -109,7 +121,7 @@ bnxt_filter_type_check(const struct rte_flow_item pattern[],
 			PMD_DRV_LOG_LINE(DEBUG, "Unknown Flow type");
 			use_ntuple |= 0;
 		}
-		item++;
+		item = bnxt_flow_non_void_item(item + 1);
 	}
 
 	if (has_vlan && use_ntuple) {
@@ -680,7 +692,7 @@ bnxt_validate_and_parse_flow_type(const struct rte_flow_attr *attr,
 		default:
 			break;
 		}
-		item++;
+		item = bnxt_flow_non_void_item(item + 1);
 	}
 	filter->enables = en;
 	filter->valid_flags = valid_flags;
-- 
2.47.3

[PATCH 5/5] net/bnxt: fix TPA agg Rx descriptor and VNIC query bounds

[Mohammad Shuab Siddique]

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

Three independent out-of-bounds issues:

- bnxt_rx_pkt() incremented tpa_info->agg_count and indexed
  tpa_info->agg_arr[] with only an RTE_ASSERT (compiled out in release
  builds) guarding the array bound, allowing an out-of-bounds write if
  firmware sent more aggregation segments than TPA_MAX_NUM_SEGS.

- bnxt_rx_descriptor_status_op() used the firmware-supplied completion
  opaque value directly as an rx_buf_ring[] index without masking it to
  the ring size first.

- bnxt_hwrm_func_vf_vnic_query() returned the firmware-reported
  vnic_id_cnt unclamped; a value exceeding bp->pf->total_vnics would
  cause the caller to iterate past the end of its VNIC ID buffer.

Fixes: b150a7e7ee ("net/bnxt: support LRO on Thor adapters")
Fixes: 0fe613bb87 ("net/bnxt: support Rx descriptor status")
Fixes: cbcd375d37 ("net/bnxt: fix HWRM macros and locking")
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_ethdev.c |  3 ++-
 drivers/net/bnxt/bnxt_hwrm.c   |  3 ++-
 drivers/net/bnxt/bnxt_rxr.c    | 10 +++++++++-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index db9b49238a..d21ebac0c2 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -3631,7 +3631,8 @@ bnxt_rx_descriptor_status_op(void *rx_queue, uint16_t offset)
 		case CMPL_BASE_TYPE_RX_L2:
 		case CMPL_BASE_TYPE_RX_L2_V2:
 			if (desc == offset) {
-				cons = rxcmp->opaque;
+				cons = RING_IDX(rxr->rx_ring_struct,
+						 rxcmp->opaque);
 				if (rxr->rx_buf_ring[cons])
 					return RTE_ETH_RX_DESC_DONE;
 				else
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 0143da8789..aae50c1fea 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -6242,7 +6242,8 @@ static int bnxt_hwrm_func_vf_vnic_query(struct bnxt *bp, uint16_t vf,
 	}
 	rc = bnxt_hwrm_send_message(bp, &req, sizeof(req), BNXT_USE_CHIMP_MB);
 	HWRM_CHECK_RESULT();
-	rc = rte_le_to_cpu_32(resp->vnic_id_cnt);
+	rc = RTE_MIN(rte_le_to_cpu_32(resp->vnic_id_cnt),
+		     (uint32_t)bp->pf->total_vnics);
 
 	HWRM_UNLOCK();
 
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 87640eaa79..cd4e93bdb3 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -1164,7 +1164,15 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
 		}
 
 		tpa_info = &rxr->tpa_info[agg_id];
-		RTE_ASSERT(tpa_info->agg_count < 16);
+		if (unlikely(tpa_info->agg_count >= TPA_MAX_NUM_SEGS)) {
+			PMD_DRV_LOG_LINE(ERR,
+					 "TPA abuf: agg_count %u exceeds max %u",
+					 tpa_info->agg_count, TPA_MAX_NUM_SEGS);
+			tpa_info->agg_count = 0;
+			bnxt_sched_ring_reset(rxq);
+			rc = -EINVAL;
+			goto next_rx;
+		}
 		tpa_info->agg_arr[tpa_info->agg_count++] = *rx_agg;
 		rc = -EINVAL; /* Continue w/o new mbuf */
 		goto next_rx;
-- 
2.47.3

[PATCH v2 0/5] net/bnxt: fix flow, Rx and naming bounds issues

[Mohammad Shuab Siddique]

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

This series fixes five independent out-of-bounds issues in flow,
Rx-datapath and naming code in the bnxt PMD:

 - two stack-allocated variable-length arrays in flow-stats sizing
   that risked stack exhaustion,
 - a TPA aggregation ID read from a completion and used to index
   rxr->tpa_info[] without a bounds check,
 - three separate sprintf() calls into fixed-size buffers with no
   bound on the formatted string length, plus four related bugs
   (a leak, a stale flag, and two locks left held) introduced by
   this change's own new early-return paths and fixed here,
 - a caller-supplied MAC pool index used before being validated
   against bp->max_vnics, and an unbounded flow item/action skip
   loop that could walk off the end of the pattern array, and
 - a firmware-supplied Rx completion opaque value used unmasked as
   an rx_buf_ring[] index, an aggregation-segment count guarded only
   by a release-mode-compiled-out RTE_ASSERT, and an unclamped VF
   VNIC-count from firmware.

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 ("fix bounds on TPA aggregation ID from completions"):
  corrected its Fixes: tag SHA1s to the full 12-character form, and
  fixed a real bug an AI-review pass caught -- the legacy (non-Thor)
  TPA end path's new bounds-check-failure branch wasn't draining
  pending aggregation-buffer completions before returning, unlike
  the sibling in_reset early return right above it, which could
  desync the CQ consumer index. See that patch's own changelog.
* Patch 3/5 ("harden sprintf bounds for device memory names"): fixed
  a real bug an AI-review pass caught -- bnxt_hwrm_ver_get()'s new
  early return on a snprintf failure also skipped HWRM_UNLOCK(),
  unlike this same function's other error exits and the sibling
  cfa_pair_*() fixes in this same patch, which would leak
  bp->hwrm_lock and deadlock every later HWRM call. See that patch's
  own changelog.
* Patch 4/5 ("fix bounds in MAC pool index and flow parsing"): fixed
  both Fixes: tag SHA1s to the full 12-character form.
* Patch 5/5 ("fix TPA agg Rx descriptor and VNIC query bounds"):
  fixed all three Fixes: tag SHA1s to the full 12-character form.
  Also addressed (no code change needed, see that patch's changelog)
  a reviewer question about a possible leak in the agg_count-overflow
  error path.
* Patch 1/5 is unchanged from v1.

Chenna Arnoori (1):
  net/bnxt: fix bounds in MAC pool index and flow parsing

Joseph Wong (1):
  net/bnxt: fix stack exhaustion in flow stats

Keegan Freyhof (1):
  net/bnxt: harden sprintf bounds for device memory names

Kishore Padmanabha (1):
  net/bnxt: fix TPA agg Rx descriptor and VNIC query bounds

Mohammad Shuab Siddique (1):
  net/bnxt: fix bounds on TPA aggregation ID from completions

 drivers/net/bnxt/bnxt.h        | 15 +++++++
 drivers/net/bnxt/bnxt_ethdev.c | 43 +++++++++++++-------
 drivers/net/bnxt/bnxt_flow.c   | 28 +++++++++----
 drivers/net/bnxt/bnxt_hwrm.c   | 75 ++++++++++++++++++++++++++---------
 drivers/net/bnxt/bnxt_rxr.c    | 58 +++++++++++++++++++++-------
 drivers/net/bnxt/bnxt_stats.c  | 18 ++++-----
 6 files changed, 175 insertions(+), 62 deletions(-)

--
2.47.3

[PATCH v2 1/5] net/bnxt: fix stack exhaustion in flow stats

[Mohammad Shuab Siddique]

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

In bnxt_flow_stats_cnt(), two variable-length arrays are allocated
on the stack. These arrays are allocated merely to be used to
calculate the dimensions via RTE_DIM(). Replace with the
mathematical equivalent to avoid potential stack exhaustion.

Fixes: 1e2f8aca2cc1 ("net/bnxt: fix allocation of flow stat related structs")
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_stats.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_stats.c b/drivers/net/bnxt/bnxt_stats.c
index ba858710a5..c4efcb4b17 100644
--- a/drivers/net/bnxt/bnxt_stats.c
+++ b/drivers/net/bnxt/bnxt_stats.c
@@ -1095,12 +1095,8 @@ int bnxt_flow_stats_cnt(struct bnxt *bp)
 {
 	if (bp->fw_cap & BNXT_FW_CAP_ADV_FLOW_COUNTERS &&
 	    bp->fw_cap & BNXT_FW_CAP_ADV_FLOW_MGMT &&
-	    BNXT_FLOW_XSTATS_EN(bp)) {
-		struct bnxt_xstats_name_off flow_bytes[bp->max_l2_ctx];
-		struct bnxt_xstats_name_off flow_pkts[bp->max_l2_ctx];
-
-		return RTE_DIM(flow_bytes) + RTE_DIM(flow_pkts);
-	}
+	    BNXT_FLOW_XSTATS_EN(bp))
+		return 2 * bp->max_l2_ctx;
 
 	return 0;
 }
-- 
2.47.3

[PATCH v2 2/5] net/bnxt: fix bounds on TPA aggregation ID from completions

[Mohammad Shuab Siddique]

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

TPA start/end and TPA v2 abuf paths indexed rxr->tpa_info[] using
the aggregation ID from the completion without checking
BNXT_TPA_MAX_AGGS. Add bounds checks and schedule a ring reset on
failure. Also fix abuf aggregation ID decode to use
rte_le_to_cpu_16() when reading the device field.

Fixes: 0958d8b6435d ("net/bnxt: support LRO")
Fixes: b150a7e7ee66 ("net/bnxt: support LRO on Thor adapters")
Cc: stable@dpdk.org

Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
v2:
* Real bug found during v2 review: on the legacy (non-Thor) TPA end
  path, the new agg_id bounds-check-failure branch returned without
  draining the pending aggregation-buffer completions, unlike the
  sibling in_reset early return a few lines above it -- this would
  leave the CQ raw_cons out of sync with the ring even after the
  scheduled reset. Added the same bnxt_discard_rx() call the sibling
  path already uses.

 drivers/net/bnxt/bnxt_rxr.c | 48 ++++++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 11 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 0fab4ddf78..87640eaa79 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -236,12 +236,19 @@ static void bnxt_tpa_start(struct bnxt_rx_queue *rxq,
 			   struct rx_tpa_start_cmpl_hi *tpa_start1)
 {
 	struct bnxt_rx_ring_info *rxr = rxq->rx_ring;
-	uint16_t agg_id;
-	uint16_t data_cons;
 	struct bnxt_tpa_info *tpa_info;
+	uint32_t data_cons, agg_id;
+	struct bnxt *bp = rxq->bp;
 	struct rte_mbuf *mbuf;
 
-	agg_id = bnxt_tpa_start_agg_id(rxq->bp, tpa_start);
+	agg_id = bnxt_tpa_start_agg_id(bp, tpa_start);
+	if (unlikely(agg_id >= BNXT_TPA_MAX_AGGS(bp))) {
+		PMD_DRV_LOG_LINE(ERR,
+				 "TPA start: invalid agg_id %u (max %u)",
+				 agg_id, BNXT_TPA_MAX_AGGS(bp));
+		bnxt_sched_ring_reset(rxq);
+		return;
+	}
 
 	data_cons = tpa_start->opaque;
 	tpa_info = &rxr->tpa_info[agg_id];
@@ -268,7 +275,7 @@ static void bnxt_tpa_start(struct bnxt_rx_queue *rxq,
 	mbuf->port = rxq->port_id;
 	mbuf->ol_flags = RTE_MBUF_F_RX_LRO;
 
-	bnxt_tpa_get_metadata(rxq->bp, tpa_info, tpa_start, tpa_start1);
+	bnxt_tpa_get_metadata(bp, tpa_info, tpa_start, tpa_start1);
 
 	if (likely(tpa_info->hash_valid)) {
 		mbuf->hash.rss = tpa_info->rss_hash;
@@ -278,7 +285,7 @@ static void bnxt_tpa_start(struct bnxt_rx_queue *rxq,
 		mbuf->ol_flags |= RTE_MBUF_F_RX_FDIR | RTE_MBUF_F_RX_FDIR_ID;
 	}
 
-	if (tpa_info->vlan_valid && BNXT_RX_VLAN_STRIP_EN(rxq->bp)) {
+	if (tpa_info->vlan_valid && BNXT_RX_VLAN_STRIP_EN(bp)) {
 		mbuf->vlan_tci = tpa_info->vlan;
 		mbuf->ol_flags |= RTE_MBUF_F_RX_VLAN | RTE_MBUF_F_RX_VLAN_STRIPPED;
 	}
@@ -421,20 +428,21 @@ static inline struct rte_mbuf *bnxt_tpa_end(
 {
 	struct bnxt_cp_ring_info *cpr = rxq->cp_ring;
 	struct bnxt_rx_ring_info *rxr = rxq->rx_ring;
-	uint16_t agg_id;
+	struct bnxt_tpa_info *tpa_info;
+	struct bnxt *bp = rxq->bp;
+	uint8_t payload_offset;
 	struct rte_mbuf *mbuf;
 	uint8_t agg_bufs;
-	uint8_t payload_offset;
-	struct bnxt_tpa_info *tpa_info;
+	uint32_t agg_id;
 
 	if (unlikely(rxq->in_reset)) {
 		PMD_DRV_LOG_LINE(ERR, "rxq->in_reset: raw_cp_cons:%d",
 			    *raw_cp_cons);
-		bnxt_discard_rx(rxq->bp, cpr, raw_cp_cons, tpa_end);
+		bnxt_discard_rx(bp, cpr, raw_cp_cons, tpa_end);
 		return NULL;
 	}
 
-	if (BNXT_CHIP_P5_P7(rxq->bp)) {
+	if (BNXT_CHIP_P5_P7(bp)) {
 		struct rx_tpa_v2_end_cmpl *th_tpa_end;
 		struct rx_tpa_v2_end_cmpl_hi *th_tpa_end1;
 
@@ -451,6 +459,15 @@ static inline struct rte_mbuf *bnxt_tpa_end(
 		payload_offset = tpa_end->payload_offset;
 	}

+	if (unlikely(agg_id >= BNXT_TPA_MAX_AGGS(bp))) {
+		PMD_DRV_LOG_LINE(ERR,
+				 "TPA end: invalid agg_id %u (max %u)",
+				 agg_id, BNXT_TPA_MAX_AGGS(bp));
+		bnxt_discard_rx(bp, cpr, raw_cp_cons, tpa_end);
+		bnxt_sched_ring_reset(rxq);
+		return NULL;
+	}
+
 	tpa_info = &rxr->tpa_info[agg_id];
 	mbuf = tpa_info->mbuf;
 	RTE_ASSERT(mbuf != NULL);
@@ -1135,9 +1152,18 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
 
 	if (cmp_type == RX_TPA_V2_ABUF_CMPL_TYPE_RX_TPA_AGG) {
 		struct rx_tpa_v2_abuf_cmpl *rx_agg = (void *)rxcmp;
-		uint16_t agg_id = rte_cpu_to_le_16(rx_agg->agg_id);
+		uint32_t agg_id = rte_le_to_cpu_16(rx_agg->agg_id);
 		struct bnxt_tpa_info *tpa_info;
 
+		if (unlikely(agg_id >= BNXT_TPA_MAX_AGGS(bp))) {
+			PMD_DRV_LOG_LINE(ERR,
+					 "TPA abuf: invalid agg_id %u (max %u)",
+					 agg_id, BNXT_TPA_MAX_AGGS(bp));
+			bnxt_sched_ring_reset(rxq);
+			rc = -EINVAL;
+			goto next_rx;
+		}
+
 		tpa_info = &rxr->tpa_info[agg_id];
 		RTE_ASSERT(tpa_info->agg_count < 16);
 		tpa_info->agg_arr[tpa_info->agg_count++] = *rx_agg;
-- 
2.47.3

[PATCH v2 3/5] net/bnxt: harden sprintf bounds for device memory names

[Mohammad Shuab Siddique]

From: Keegan Freyhof <keegan.freyhof@broadcom.com>

sprintf() into fixed-size stack buffers such as
char type[RTE_MEMZONE_NAMESIZE] does not bound the write to the
buffer, so a long enough formatted string (e.g. from PCI address
fields) overflows it.

Add check_snprintf_rc(), a helper that logs and returns an error on a
failed snprintf() call and logs (without failing) a truncated one.
Convert sprintf() calls building a memzone/malloc name to snprintf()
plus this check, and add the same check to the existing snprintf()
calls building HWRM CFA pair_name request fields. Unlike a truncated
memzone/malloc label, a truncated pair_name would be sent to firmware
and could match the wrong pair or none at all, so
bnxt_hwrm_cfa_pair_exists()/_alloc()/_free() additionally reject a
truncated pair_name outright instead of proceeding.

Three bugs introduced by this change and fixed here: in
bnxt_hwrm_ver_get(), free bp->hwrm_short_cmd_req_addr (and null it)
before checking the new snprintf's return, instead of after, so an
early return on a snprintf failure doesn't leak the previous
allocation; also clear BNXT_FLAG_SHORT_CMD on that same early return,
since it may already have been set a few lines above and would
otherwise claim short-command support with no buffer allocated. In
bnxt_hwrm_cfa_pair_exists()/_alloc()/_free(), the new early-return
paths exited without releasing bp->hwrm_lock (held since the
preceding HWRM_PREP()), which would deadlock every later HWRM call;
added the missing HWRM_UNLOCK() before each return.

Signed-off-by: Keegan Freyhof <keegan.freyhof@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <shuab.siddique@broadcom.com>

---
v2:
* Fixed three bugs the v1 diff itself introduced, found in a second
  review pass: in bnxt_hwrm_ver_get(), free/null
  bp->hwrm_short_cmd_req_addr and clear BNXT_FLAG_SHORT_CMD before
  (not after) checking the new snprintf's return, so an early return
  on failure doesn't leak the previous allocation or leave the flag
  claiming short-command support with no buffer behind it; in
  bnxt_hwrm_cfa_pair_exists()/_alloc()/_free(), added the missing
  HWRM_UNLOCK() on the new early-return paths, which previously
  exited holding bp->hwrm_lock and would deadlock every later HWRM
  call.
---
 drivers/net/bnxt/bnxt.h        | 15 +++++++
 drivers/net/bnxt/bnxt_ethdev.c | 24 ++++++++----
 drivers/net/bnxt/bnxt_hwrm.c   | 72 +++++++++++++++++++++++++---------
 drivers/net/bnxt/bnxt_stats.c  | 10 +++--
 4 files changed, 93 insertions(+), 28 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 69455af31f9..ad0d5fe5dba 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -1288,6 +1288,21 @@ extern int bnxt_logtype_driver;
 						       BNXT_LINK_SPEEDS_V2_VF((bp))))
 #define BNXT_MAX_SPEED_LANES 8
 #define BNXT_SUPPORTS_TPA(bp)  (!BNXT_CHIP_P5_P7(bp) || (bp)->max_tpa_v2)
+
+static inline int
+check_snprintf_rc(int rc, size_t max_size, const char *ctx)
+{
+	if (rc < 0) {
+		PMD_DRV_LOG_LINE(ERR, "Error when creating string for %s", ctx);
+		return rc;
+	}
+
+	if (rc >= (int)max_size)
+		PMD_DRV_LOG_LINE(INFO, "String truncated when creating string for %s", ctx);
+
+	return 0;
+}
+
 extern const struct rte_flow_ops bnxt_ulp_rte_flow_ops;
 int32_t bnxt_ulp_port_init(struct bnxt *bp);
 void bnxt_ulp_port_deinit(struct bnxt *bp);
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index eefd99464e9..7b9d1c862d8 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -647,13 +647,15 @@ static int bnxt_init_fc_ctx_mem(struct bnxt *bp)
 {
 	struct rte_pci_device *pdev = bp->pdev;
 	char type[RTE_MEMZONE_NAMESIZE];
+	int rc = 0, snp_rc = 0;
 	uint16_t max_fc;
-	int rc = 0;
 
 	max_fc = bp->flow_stat->max_fc;
 
-	sprintf(type, "bnxt_rx_fc_in_" PCI_PRI_FMT, pdev->addr.domain,
+	snp_rc = snprintf(type, sizeof(type), "bnxt_rx_fc_in_" PCI_PRI_FMT, pdev->addr.domain,
 		pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
+	if (check_snprintf_rc(snp_rc, sizeof(type), "bnxt_rx_fc_in_") < 0)
+		return snp_rc;
 	/* 4 bytes for each counter-id */
 	rc = bnxt_alloc_ctx_mem_buf(bp, type,
 				    max_fc * 4,
@@ -661,8 +663,10 @@ static int bnxt_init_fc_ctx_mem(struct bnxt *bp)
 	if (rc)
 		return rc;
 
-	sprintf(type, "bnxt_rx_fc_out_" PCI_PRI_FMT, pdev->addr.domain,
+	snp_rc = snprintf(type, sizeof(type), "bnxt_rx_fc_out_" PCI_PRI_FMT, pdev->addr.domain,
 		pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
+	if (check_snprintf_rc(snp_rc, sizeof(type), "bnxt_rx_fc_out_") < 0)
+		return snp_rc;
 	/* 16 bytes for each counter - 8 bytes pkt_count, 8 bytes byte_count */
 	rc = bnxt_alloc_ctx_mem_buf(bp, type,
 				    max_fc * 16,
@@ -670,8 +674,10 @@ static int bnxt_init_fc_ctx_mem(struct bnxt *bp)
 	if (rc)
 		return rc;
 
-	sprintf(type, "bnxt_tx_fc_in_" PCI_PRI_FMT, pdev->addr.domain,
+	snp_rc = snprintf(type, sizeof(type), "bnxt_tx_fc_in_" PCI_PRI_FMT, pdev->addr.domain,
 		pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
+	if (check_snprintf_rc(snp_rc, sizeof(type), "bnxt_tx_fc_in_") < 0)
+		return snp_rc;
 	/* 4 bytes for each counter-id */
 	rc = bnxt_alloc_ctx_mem_buf(bp, type,
 				    max_fc * 4,
@@ -679,8 +685,10 @@ static int bnxt_init_fc_ctx_mem(struct bnxt *bp)
 	if (rc)
 		return rc;
 
-	sprintf(type, "bnxt_tx_fc_out_" PCI_PRI_FMT, pdev->addr.domain,
+	snp_rc = snprintf(type, sizeof(type), "bnxt_tx_fc_out_" PCI_PRI_FMT, pdev->addr.domain,
 		pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
+	if (check_snprintf_rc(snp_rc, sizeof(type), "bnxt_tx_fc_out_") < 0)
+		return snp_rc;
 	/* 16 bytes for each counter - 8 bytes pkt_count, 8 bytes byte_count */
 	rc = bnxt_alloc_ctx_mem_buf(bp, type,
 				    max_fc * 16,
@@ -5226,8 +5234,8 @@ int bnxt_alloc_ctx_pg_tbls(struct bnxt *bp)
 {
 	struct bnxt_ctx_mem_info *ctx = bp->ctx;
 	struct bnxt_ctx_mem *ctx2;
+	int rc = 0, snp_rc = 0;
 	uint16_t type;
-	int rc = 0;
 
 	ctx2 = &ctx->ctx_arr[0];
 	for (type = 0; type < ctx->types && rc == 0; type++) {
@@ -5248,7 +5256,9 @@ int bnxt_alloc_ctx_pg_tbls(struct bnxt *bp)
 		for (i = 0; i < w && rc == 0; i++) {
 			char name[RTE_MEMZONE_NAMESIZE] = {0};
 
-			sprintf(name, "_%d_%d", i, type);
+			snp_rc = snprintf(name, sizeof(name), "_%d_%d", i, type);
+			if (check_snprintf_rc(snp_rc, sizeof(name), "index and type.") < 0)
+				return snp_rc;
 
 			if (ctxm->entry_multiple)
 				entries = bnxt_roundup(ctxm->max_entries,
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 4e50ff7e1e0..7719084a6d3 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1591,12 +1591,12 @@ int bnxt_hwrm_func_resc_qcaps(struct bnxt *bp)
 
 int bnxt_hwrm_ver_get(struct bnxt *bp, uint32_t timeout)
 {
-	int rc = 0;
 	struct hwrm_ver_get_input req = {.req_type = 0 };
 	struct hwrm_ver_get_output *resp = bp->hwrm_cmd_resp_addr;
 	uint32_t fw_version;
 	uint16_t max_resp_len;
 	char type[RTE_MEMZONE_NAMESIZE];
+	int rc = 0, snp_rc = 0;
 	uint32_t dev_caps_cfg;
 
 	bp->max_req_len = HWRM_MAX_REQ_LEN;
@@ -1677,11 +1677,16 @@ int bnxt_hwrm_ver_get(struct bnxt *bp, uint32_t timeout)
 	     (dev_caps_cfg &
 	      HWRM_VER_GET_OUTPUT_DEV_CAPS_CFG_SHORT_CMD_REQUIRED)) ||
 	    bp->hwrm_max_ext_req_len > HWRM_MAX_REQ_LEN) {
-		sprintf(type, "bnxt_hwrm_short_" PCI_PRI_FMT,
-			bp->pdev->addr.domain, bp->pdev->addr.bus,
-			bp->pdev->addr.devid, bp->pdev->addr.function);
-
+		snp_rc = snprintf(type, sizeof(type), "bnxt_hwrm_short_" PCI_PRI_FMT,
+				  bp->pdev->addr.domain, bp->pdev->addr.bus,
+				  bp->pdev->addr.devid, bp->pdev->addr.function);
 		rte_free(bp->hwrm_short_cmd_req_addr);
+		bp->hwrm_short_cmd_req_addr = NULL;
+		if (check_snprintf_rc(snp_rc, sizeof(type), "bnxt_hwrm_short_") < 0) {
+			bp->flags &= ~BNXT_FLAG_SHORT_CMD;
+			rc = snp_rc;
+			goto error;
+		}
 
 		bp->hwrm_short_cmd_req_addr =
 				rte_malloc(type, bp->hwrm_max_ext_req_len, 0);
@@ -3526,9 +3531,12 @@ int bnxt_alloc_hwrm_resources(struct bnxt *bp)
 {
 	struct rte_pci_device *pdev = bp->pdev;
 	char type[RTE_MEMZONE_NAMESIZE];
+	int snp_rc = 0;
 
-	sprintf(type, "bnxt_hwrm_" PCI_PRI_FMT, pdev->addr.domain,
-		pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
+	snp_rc = snprintf(type, sizeof(type), "bnxt_hwrm_" PCI_PRI_FMT, pdev->addr.domain,
+			  pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
+	if (check_snprintf_rc(snp_rc, sizeof(type), "bnxt_hwrm_") < 0)
+		return snp_rc;
 	bp->max_resp_len = BNXT_PAGE_SIZE;
 	bp->hwrm_cmd_resp_addr = rte_malloc(type, bp->max_resp_len, 0);
 	if (bp->hwrm_cmd_resp_addr == NULL)
@@ -6769,6 +6777,7 @@ static int bnxt_alloc_all_ctx_pg_info(struct bnxt *bp)
 {
 	struct bnxt_ctx_mem_info *ctx = bp->ctx;
 	char name[RTE_MEMZONE_NAMESIZE];
+	int snp_rc = 0;
 	uint16_t type;
 
 	for (type = 0; type < ctx->types; type++) {
@@ -6781,8 +6790,10 @@ static int bnxt_alloc_all_ctx_pg_info(struct bnxt *bp)
 		if (ctxm->instance_bmap)
 			n = bnxt_hweight32(ctxm->instance_bmap);
 
-		sprintf(name, "bnxt_ctx_pgmem_%d_%d",
-			bp->eth_dev->data->port_id, type);
+		snp_rc = snprintf(name, sizeof(name), "bnxt_ctx_pgmem_%d_%d",
+				  bp->eth_dev->data->port_id, type);
+		if (check_snprintf_rc(snp_rc, sizeof(name), "bnxt_ctx_pgmem_") < 0)
+			return snp_rc;
 		ctxm->pg_info = rte_malloc(name, sizeof(*ctxm->pg_info) * n,
 					   RTE_CACHE_LINE_SIZE);
 		if (!ctxm->pg_info)
@@ -7740,7 +7751,7 @@ int bnxt_hwrm_cfa_pair_exists(struct bnxt *bp, struct bnxt_representor *rep_bp)
 {
 	struct hwrm_cfa_pair_info_output *resp = bp->hwrm_cmd_resp_addr;
 	struct hwrm_cfa_pair_info_input req = {0};
-	int rc = 0;
+	int rc = 0, snp_rc = 0;
 
 	if (!(BNXT_PF(bp) || BNXT_VF_IS_TRUSTED(bp))) {
 		PMD_DRV_LOG_LINE(DEBUG,
@@ -7749,8 +7760,16 @@ int bnxt_hwrm_cfa_pair_exists(struct bnxt *bp, struct bnxt_representor *rep_bp)
 	}
 
 	HWRM_PREP(&req, HWRM_CFA_PAIR_INFO, BNXT_USE_CHIMP_MB);
-	snprintf(req.pair_name, sizeof(req.pair_name), "%svfr%d",
-		 bp->eth_dev->data->name, rep_bp->vf_id);
+	snp_rc = snprintf(req.pair_name, sizeof(req.pair_name), "%svfr%d",
+			  bp->eth_dev->data->name, rep_bp->vf_id);
+	if (check_snprintf_rc(snp_rc, sizeof(req.pair_name), "svfr") < 0) {
+		HWRM_UNLOCK();
+		return snp_rc;
+	}
+	if (snp_rc >= (int)sizeof(req.pair_name)) {
+		HWRM_UNLOCK();
+		return -EINVAL;
+	}
 	req.flags =
 		rte_cpu_to_le_32(HWRM_CFA_PAIR_INFO_INPUT_FLAGS_LOOKUP_TYPE);
 
@@ -7768,7 +7787,7 @@ int bnxt_hwrm_cfa_pair_alloc(struct bnxt *bp, struct bnxt_representor *rep_bp)
 {
 	struct hwrm_cfa_pair_alloc_output *resp = bp->hwrm_cmd_resp_addr;
 	struct hwrm_cfa_pair_alloc_input req = {0};
-	int rc;
+	int rc, snp_rc = 0;
 
 	if (!(BNXT_PF(bp) || BNXT_VF_IS_TRUSTED(bp))) {
 		PMD_DRV_LOG_LINE(DEBUG,
@@ -7778,8 +7797,16 @@ int bnxt_hwrm_cfa_pair_alloc(struct bnxt *bp, struct bnxt_representor *rep_bp)
 
 	HWRM_PREP(&req, HWRM_CFA_PAIR_ALLOC, BNXT_USE_CHIMP_MB);
 	req.pair_mode = HWRM_CFA_PAIR_FREE_INPUT_PAIR_MODE_REP2FN_TRUFLOW;
-	snprintf(req.pair_name, sizeof(req.pair_name), "%svfr%d",
-		 bp->eth_dev->data->name, rep_bp->vf_id);
+	snp_rc = snprintf(req.pair_name, sizeof(req.pair_name), "%svfr%d",
+			  bp->eth_dev->data->name, rep_bp->vf_id);
+	if (check_snprintf_rc(snp_rc, sizeof(req.pair_name), "svfr") < 0) {
+		HWRM_UNLOCK();
+		return snp_rc;
+	}
+	if (snp_rc >= (int)sizeof(req.pair_name)) {
+		HWRM_UNLOCK();
+		return -EINVAL;
+	}
 
 	req.pf_b_id = rep_bp->parent_pf_idx;
 	req.vf_b_id = BNXT_REP_PF(rep_bp) ? rte_cpu_to_le_16(((uint16_t)-1)) :
@@ -7814,7 +7841,7 @@ int bnxt_hwrm_cfa_pair_free(struct bnxt *bp, struct bnxt_representor *rep_bp)
 {
 	struct hwrm_cfa_pair_free_output *resp = bp->hwrm_cmd_resp_addr;
 	struct hwrm_cfa_pair_free_input req = {0};
-	int rc;
+	int rc, snp_rc = 0;
 
 	if (!(BNXT_PF(bp) || BNXT_VF_IS_TRUSTED(bp))) {
 		PMD_DRV_LOG_LINE(DEBUG,
@@ -7823,8 +7850,17 @@ int bnxt_hwrm_cfa_pair_free(struct bnxt *bp, struct bnxt_representor *rep_bp)
 	}
 
 	HWRM_PREP(&req, HWRM_CFA_PAIR_FREE, BNXT_USE_CHIMP_MB);
-	snprintf(req.pair_name, sizeof(req.pair_name), "%svfr%d",
-		 bp->eth_dev->data->name, rep_bp->vf_id);
+	snp_rc = snprintf(req.pair_name, sizeof(req.pair_name), "%svfr%d",
+			  bp->eth_dev->data->name, rep_bp->vf_id);
+	if (check_snprintf_rc(snp_rc, sizeof(req.pair_name), "svfr") < 0) {
+		HWRM_UNLOCK();
+		return snp_rc;
+	}
+	if (snp_rc >= (int)sizeof(req.pair_name)) {
+		HWRM_UNLOCK();
+		return -EINVAL;
+	}
+
 	req.pf_b_id = rep_bp->parent_pf_idx;
 	req.pair_mode = HWRM_CFA_PAIR_FREE_INPUT_PAIR_MODE_REP2FN_TRUFLOW;
 	req.vf_id = BNXT_REP_PF(rep_bp) ? rte_cpu_to_le_16(((uint16_t)-1)) :
diff --git a/drivers/net/bnxt/bnxt_stats.c b/drivers/net/bnxt/bnxt_stats.c
index c4efcb4b179..19153f96ae6 100644
--- a/drivers/net/bnxt/bnxt_stats.c
+++ b/drivers/net/bnxt/bnxt_stats.c
@@ -1108,7 +1108,7 @@ int bnxt_dev_xstats_get_names_op(struct rte_eth_dev *eth_dev,
 	struct bnxt *bp = (struct bnxt *)eth_dev->data->dev_private;
 	unsigned int stat_cnt;
 	unsigned int i, count = 0, sz;
-	int rc;
+	int rc, snp_rc = 0;
 
 	rc = is_bnxt_in_error(bp);
 	if (rc)
@@ -1183,12 +1183,16 @@ int bnxt_dev_xstats_get_names_op(struct rte_eth_dev *eth_dev,
 		for (i = 0; i < bp->max_l2_ctx; i++) {
 			char buf[RTE_ETH_XSTATS_NAME_SIZE];
 
-			sprintf(buf, "flow_%d_bytes", i);
+			snp_rc = snprintf(buf, sizeof(buf), "flow_%d_bytes", i);
+			if (check_snprintf_rc(snp_rc, sizeof(buf), "flow_%d_bytes") < 0)
+				return snp_rc;
 			strlcpy(xstats_names[count].name, buf,
 				sizeof(xstats_names[count].name));
 			count++;
 
-			sprintf(buf, "flow_%d_packets", i);
+			snp_rc = snprintf(buf, sizeof(buf), "flow_%d_packets", i);
+			if (check_snprintf_rc(snp_rc, sizeof(buf), "flow_%d_packets") < 0)
+				return snp_rc;
 			strlcpy(xstats_names[count].name, buf,
 				sizeof(xstats_names[count].name));
 
-- 
2.47.3

Re: [PATCH v2 3/5] net/bnxt: harden sprintf bounds for device memory names

[Stephen Hemminger]

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

> From: Keegan Freyhof <keegan.freyhof@broadcom.com>
> 
> sprintf() into fixed-size stack buffers such as
> char type[RTE_MEMZONE_NAMESIZE] does not bound the write to the
> buffer, so a long enough formatted string (e.g. from PCI address
> fields) overflows it.
> 
> Add check_snprintf_rc(), a helper that logs and returns an error on a
> failed snprintf() call and logs (without failing) a truncated one.
> Convert sprintf() calls building a memzone/malloc name to snprintf()
> plus this check, and add the same check to the existing snprintf()
> calls building HWRM CFA pair_name request fields. Unlike a truncated
> memzone/malloc label, a truncated pair_name would be sent to firmware
> and could match the wrong pair or none at all, so
> bnxt_hwrm_cfa_pair_exists()/_alloc()/_free() additionally reject a
> truncated pair_name outright instead of proceeding.
> 
> Three bugs introduced by this change and fixed here: in
> bnxt_hwrm_ver_get(), free bp->hwrm_short_cmd_req_addr (and null it)
> before checking the new snprintf's return, instead of after, so an
> early return on a snprintf failure doesn't leak the previous
> allocation; also clear BNXT_FLAG_SHORT_CMD on that same early return,
> since it may already have been set a few lines above and would
> otherwise claim short-command support with no buffer allocated. In
> bnxt_hwrm_cfa_pair_exists()/_alloc()/_free(), the new early-return
> paths exited without releasing bp->hwrm_lock (held since the
> preceding HWRM_PREP()), which would deadlock every later HWRM call;
> added the missing HWRM_UNLOCK() before each return.
> 
> Signed-off-by: Keegan Freyhof <keegan.freyhof@broadcom.com>
> Signed-off-by: Mohammad Shuab Siddique <shuab.siddique@broadcom.com>
> 
> ---


[PATCH v2 3/5] net/bnxt: harden sprintf bounds for device memory
names

Error: does not apply to main (see summary).

Warning: the rc < 0 branch of check_snprintf_rc() is unreachable.
snprintf() only fails on encoding errors, which cannot happen with
these formats. Every converted name except pair_name is an
rte_malloc()/rte_zmalloc_socket() type label. That label is
informational only, so truncation is harmless.

The only real overflow is a PCI domain above 0xffff with
"bnxt_hwrm_short_": 16 + 8 + 8 = 32 chars plus NUL into 32 bytes.
Plain snprintf() fixes that. Drop the helper and the early-return
paths, including the flag clearing and unlock handling added for
unreachable code.

For pair_name, rejecting truncation is reasonable. A single
"if (snprintf(...) >= sizeof(req.pair_name))" with HWRM_UNLOCK()
covers it.

Warning: the commit body carries review history ("Three bugs
introduced by this change and fixed here..."). Move it below ---.

Info: the flow xstat names can be written with snprintf() directly
into xstats_names[count].name, dropping buf and strlcpy(). "_%d_%d"
cannot exceed 32 bytes, so it needs no check.

Info: if the PCI domain overflow is the motivation, add Fixes: and
Cc: stable@dpdk.org.

[PATCH v2 4/5] net/bnxt: fix bounds in MAC pool index and flow parsing

[Mohammad Shuab Siddique]

From: Chenna Arnoori <chenna.arnoori@broadcom.com>

Two independent out-of-bounds issues in the driver:

- bnxt_mac_addr_add_op() indexed bp->vnic_info[pool] with a
  caller-supplied pool before validating it against bp->max_vnics, and
  before checking bp->vnic_info was even allocated yet (it is NULL
  until the port is started). The existing "if (!vnic)" check was
  always false, since vnic held the address of an array element and
  is never NULL. Reorder to check dev_started/vnic_info first, then
  bounds-check pool against max_vnics before indexing.

- bnxt_flow_non_void_item()/bnxt_flow_non_void_action() looped
  unconditionally until a non-VOID item/action was found, walking off
  the end of a pattern/actions array that lacked a terminating END
  item. Bound the skip loop and stop advancing once the limit is hit.

Fixes: 51fafb89a9a0 ("net/bnxt: get rid of ff pools and use VNIC info array")
Fixes: 5c1171c97216 ("net/bnxt: refactor filter/flow")
Cc: stable@dpdk.org

Signed-off-by: Chenna Arnoori <chenna.arnoori@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
 drivers/net/bnxt/bnxt_ethdev.c | 16 ++++++++++------
 drivers/net/bnxt/bnxt_flow.c   | 28 ++++++++++++++++++++--------
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 27cf67c04f..db9b49238a 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -2113,7 +2113,7 @@ static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 				uint32_t index, uint32_t pool)
 {
 	struct bnxt *bp = eth_dev->data->dev_private;
-	struct bnxt_vnic_info *vnic = &bp->vnic_info[pool];
+	struct bnxt_vnic_info *vnic;
 	int rc = 0;
 
 	rc = is_bnxt_in_error(bp);
@@ -2125,15 +2125,19 @@ static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 		return -ENOTSUP;
 	}
 
-	if (!vnic) {
-		PMD_DRV_LOG_LINE(ERR, "VNIC not found for pool %d!", pool);
-		return -EINVAL;
-	}
-
 	/* Filter settings will get applied when port is started */
 	if (!eth_dev->data->dev_started)
 		return 0;
 
+	if (bp->vnic_info == NULL)
+		return 0;
+
+	if (pool >= bp->max_vnics) {
+		PMD_DRV_LOG_LINE(ERR, "Pool %u exceeds VNIC count %u!", pool, bp->max_vnics);
+		return -EINVAL;
+	}
+	vnic = &bp->vnic_info[pool];
+
 	rc = bnxt_add_mac_filter(bp, vnic, mac_addr, index, pool);
 
 	return rc;
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index a2e590540b..14d52e1818 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -58,24 +58,36 @@ bnxt_flow_args_validate(const struct rte_flow_attr *attr,
 	return 0;
 }
 
+#define BNXT_MAX_FLOW_ITEMS 256
+
 static const struct rte_flow_item *
 bnxt_flow_non_void_item(const struct rte_flow_item *cur)
 {
-	while (1) {
-		if (cur->type != RTE_FLOW_ITEM_TYPE_VOID)
-			return cur;
+	int i = 0;
+
+	if (!cur)
+		return NULL;
+
+	while (cur->type == RTE_FLOW_ITEM_TYPE_VOID && i < BNXT_MAX_FLOW_ITEMS) {
 		cur++;
+		i++;
 	}
+	return cur;
 }
 
 static const struct rte_flow_action *
 bnxt_flow_non_void_action(const struct rte_flow_action *cur)
 {
-	while (1) {
-		if (cur->type != RTE_FLOW_ACTION_TYPE_VOID)
-			return cur;
+	int i = 0;
+
+	if (!cur)
+		return NULL;
+
+	while (cur->type == RTE_FLOW_ACTION_TYPE_VOID && i < BNXT_MAX_FLOW_ITEMS) {
 		cur++;
+		i++;
 	}
+	return cur;
 }
 
 static int
@@ -109,7 +121,7 @@ bnxt_filter_type_check(const struct rte_flow_item pattern[],
 			PMD_DRV_LOG_LINE(DEBUG, "Unknown Flow type");
 			use_ntuple |= 0;
 		}
-		item++;
+		item = bnxt_flow_non_void_item(item + 1);
 	}
 
 	if (has_vlan && use_ntuple) {
@@ -680,7 +692,7 @@ bnxt_validate_and_parse_flow_type(const struct rte_flow_attr *attr,
 		default:
 			break;
 		}
-		item++;
+		item = bnxt_flow_non_void_item(item + 1);
 	}
 	filter->enables = en;
 	filter->valid_flags = valid_flags;
-- 
2.47.3

Re: [PATCH v2 4/5] net/bnxt: fix bounds in MAC pool index and flow parsing

[Stephen Hemminger]

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

> From: Chenna Arnoori <chenna.arnoori@broadcom.com>
> 
> Two independent out-of-bounds issues in the driver:
> 
> - bnxt_mac_addr_add_op() indexed bp->vnic_info[pool] with a
>   caller-supplied pool before validating it against bp->max_vnics, and
>   before checking bp->vnic_info was even allocated yet (it is NULL
>   until the port is started). The existing "if (!vnic)" check was
>   always false, since vnic held the address of an array element and
>   is never NULL. Reorder to check dev_started/vnic_info first, then
>   bounds-check pool against max_vnics before indexing.
> 
> - bnxt_flow_non_void_item()/bnxt_flow_non_void_action() looped
>   unconditionally until a non-VOID item/action was found, walking off
>   the end of a pattern/actions array that lacked a terminating END
>   item. Bound the skip loop and stop advancing once the limit is hit.
> 
> Fixes: 51fafb89a9a0 ("net/bnxt: get rid of ff pools and use VNIC info array")
> Fixes: 5c1171c97216 ("net/bnxt: refactor filter/flow")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chenna Arnoori <chenna.arnoori@broadcom.com>
> Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
> ---




[PATCH v2 4/5] net/bnxt: fix bounds in MAC pool index and flow
parsing

Warning: the bounded VOID skip does not bound the walk. The outer
loops in bnxt_filter_type_check() and
bnxt_validate_and_parse_flow_type() run while type != END. After 256
VOIDs the helper returns a VOID item, and the loop calls it again at
item + 1. A pattern without END is still walked off the end, and
non-VOID items are never counted. The rte_flow API requires the END
terminator. Drop this half of the patch.

The added "if (!cur) return NULL" paths return a value that no
caller checks.

Warning: in bnxt_mac_addr_add_op() the pool bounds check comes after
the dev_started early return. An invalid pool added while the port
is stopped returns 0, is recorded in mac_pool_sel, and then fails
later in bnxt_restore_mac_filters() at dev_start. Validate pool
against bp->max_vnics before the dev_started test. The
bp->vnic_info == NULL test after dev_started is dead code.

[PATCH v2 5/5] net/bnxt: fix TPA agg Rx descriptor and VNIC query bounds

[Mohammad Shuab Siddique]

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

Three independent out-of-bounds issues:

- bnxt_rx_pkt() incremented tpa_info->agg_count and indexed
  tpa_info->agg_arr[] with only an RTE_ASSERT (compiled out in release
  builds) guarding the array bound, allowing an out-of-bounds write if
  firmware sent more aggregation segments than TPA_MAX_NUM_SEGS.

- bnxt_rx_descriptor_status_op() used the firmware-supplied completion
  opaque value directly as an rx_buf_ring[] index without masking it to
  the ring size first.

- bnxt_hwrm_func_vf_vnic_query() returned the firmware-reported
  vnic_id_cnt unclamped; a value exceeding bp->pf->total_vnics would
  cause the caller to iterate past the end of its VNIC ID buffer.

Fixes: b150a7e7ee66 ("net/bnxt: support LRO on Thor adapters")
Fixes: 0fe613bb87b2 ("net/bnxt: support Rx descriptor status")
Fixes: cbcd375d37d2 ("net/bnxt: fix HWRM macros and locking")
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_ethdev.c |  3 ++-
 drivers/net/bnxt/bnxt_hwrm.c   |  3 ++-
 drivers/net/bnxt/bnxt_rxr.c    | 10 +++++++++-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index db9b49238a..d21ebac0c2 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -3631,7 +3631,8 @@ bnxt_rx_descriptor_status_op(void *rx_queue, uint16_t offset)
 		case CMPL_BASE_TYPE_RX_L2:
 		case CMPL_BASE_TYPE_RX_L2_V2:
 			if (desc == offset) {
-				cons = rxcmp->opaque;
+				cons = RING_IDX(rxr->rx_ring_struct,
+						 rxcmp->opaque);
 				if (rxr->rx_buf_ring[cons])
 					return RTE_ETH_RX_DESC_DONE;
 				else
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 0143da8789..aae50c1fea 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -6242,7 +6242,8 @@ static int bnxt_hwrm_func_vf_vnic_query(struct bnxt *bp, uint16_t vf,
 	}
 	rc = bnxt_hwrm_send_message(bp, &req, sizeof(req), BNXT_USE_CHIMP_MB);
 	HWRM_CHECK_RESULT();
-	rc = rte_le_to_cpu_32(resp->vnic_id_cnt);
+	rc = RTE_MIN(rte_le_to_cpu_32(resp->vnic_id_cnt),
+		     (uint32_t)bp->pf->total_vnics);
 
 	HWRM_UNLOCK();
 
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 87640eaa79..cd4e93bdb3 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -1164,7 +1164,15 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
 		}
 
 		tpa_info = &rxr->tpa_info[agg_id];
-		RTE_ASSERT(tpa_info->agg_count < 16);
+		if (unlikely(tpa_info->agg_count >= TPA_MAX_NUM_SEGS)) {
+			PMD_DRV_LOG_LINE(ERR,
+					 "TPA abuf: agg_count %u exceeds max %u",
+					 tpa_info->agg_count, TPA_MAX_NUM_SEGS);
+			tpa_info->agg_count = 0;
+			bnxt_sched_ring_reset(rxq);
+			rc = -EINVAL;
+			goto next_rx;
+		}
 		tpa_info->agg_arr[tpa_info->agg_count++] = *rx_agg;
 		rc = -EINVAL; /* Continue w/o new mbuf */
 		goto next_rx;
-- 
2.47.3

[PATCH v3 0/5] net/bnxt: fix flow, Rx and naming bounds issues

[Mohammad Shuab Siddique]

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

This series fixes five independent out-of-bounds issues in flow,
Rx-datapath and naming code in the bnxt PMD:

 - two stack-allocated variable-length arrays in flow-stats sizing
   that risked stack exhaustion,
 - a TPA aggregation ID read from a completion and used to index
   rxr->tpa_info[] without a bounds check,
 - sprintf() calls into fixed-size buffers with no bound on the
   formatted string length,
 - a caller-supplied MAC pool index used before being validated
   against bp->max_vnics, and
 - a firmware-supplied Rx completion opaque value used unmasked as
   an rx_buf_ring[] index, an aggregation-segment count guarded only
   by a release-mode-compiled-out RTE_ASSERT, and an unclamped VF
   VNIC-count from firmware.

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 3/5 ("harden sprintf bounds for device memory names"):
  dropped check_snprintf_rc() and the early-return paths on the CFA
  pair_name sites entirely -- Stephen Hemminger noted the rc < 0
  branch is unreachable and plain snprintf() is enough. See that
  patch's own changelog.
* Patch 4/5: retitled from "fix bounds in MAC pool index and flow
  parsing" to "fix bounds in MAC address pool index" and dropped the
  flow-parsing half entirely -- Stephen Hemminger pointed out
  rte_flow patterns/actions are always END-terminated by API
  contract, so that bound guarded nothing reachable. Also reordered
  the remaining fix to check pool-vs-max_vnics before dev_started.
  See that patch's own changelog.
* Patches 1/5, 2/5 and 5/5 are unchanged from v2.
v2:
* Patch 2/5 ("fix bounds on TPA aggregation ID from completions"):
  corrected its Fixes: tag SHA1s to the full 12-character form, and
  fixed a real bug an AI-review pass caught -- the legacy (non-Thor)
  TPA end path's new bounds-check-failure branch wasn't draining
  pending aggregation-buffer completions before returning, unlike
  the sibling in_reset early return right above it, which could
  desync the CQ consumer index.
* Patch 3/5: fixed a real bug an AI-review pass caught --
  bnxt_hwrm_ver_get()'s new early return on a snprintf failure also
  skipped HWRM_UNLOCK(), which would leak bp->hwrm_lock and deadlock
  every later HWRM call.
* Patch 4/5: fixed both Fixes: tag SHA1s to the full 12-character
  form.
* Patch 5/5: fixed all three Fixes: tag SHA1s to the full
  12-character form.
* Patch 1/5 is unchanged from v1.

Chenna Arnoori (1):
  net/bnxt: fix bounds in MAC address pool index

Joseph Wong (1):
  net/bnxt: fix stack exhaustion in flow stats

Keegan Freyhof (1):
  net/bnxt: harden sprintf bounds for device memory names

Kishore Padmanabha (1):
  net/bnxt: fix TPA agg Rx descriptor and VNIC query bounds

Mohammad Shuab Siddique (1):
  net/bnxt: validate TPA aggregation ID from completions

 drivers/net/bnxt/bnxt.h        |  1 +
 drivers/net/bnxt/bnxt_ethdev.c | 28 +++++++++-------
 drivers/net/bnxt/bnxt_flow.c   |  1 +
 drivers/net/bnxt/bnxt_hwrm.c   | 13 ++++----
 drivers/net/bnxt/bnxt_rxr.c    | 58 +++++++++++++++++++++++++++-------
 drivers/net/bnxt/bnxt_stats.c  | 23 +++++---------
 6 files changed, 80 insertions(+), 44 deletions(-)

--
2.47.3

[PATCH v3 1/5] net/bnxt: fix stack exhaustion in flow stats

[Mohammad Shuab Siddique]

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

In bnxt_flow_stats_cnt(), two variable-length arrays are allocated
on the stack. These arrays are allocated merely to be used to
calculate the dimensions via RTE_DIM(). Replace with the
mathematical equivalent to avoid potential stack exhaustion.

Fixes: 1e2f8aca2cc1 ("net/bnxt: fix allocation of flow stat related structs")
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_stats.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_stats.c b/drivers/net/bnxt/bnxt_stats.c
index 37b33f0505..4074613eb0 100644
--- a/drivers/net/bnxt/bnxt_stats.c
+++ b/drivers/net/bnxt/bnxt_stats.c
@@ -1079,12 +1079,8 @@ int bnxt_flow_stats_cnt(struct bnxt *bp)
 {
 	if (bp->fw_cap & BNXT_FW_CAP_ADV_FLOW_COUNTERS &&
 	    bp->fw_cap & BNXT_FW_CAP_ADV_FLOW_MGMT &&
-	    BNXT_FLOW_XSTATS_EN(bp)) {
-		struct bnxt_xstats_name_off flow_bytes[bp->max_l2_ctx];
-		struct bnxt_xstats_name_off flow_pkts[bp->max_l2_ctx];
-
-		return RTE_DIM(flow_bytes) + RTE_DIM(flow_pkts);
-	}
+	    BNXT_FLOW_XSTATS_EN(bp))
+		return 2 * bp->max_l2_ctx;
 
 	return 0;
 }
-- 
2.47.3

[PATCH v3 2/5] net/bnxt: validate TPA aggregation ID from completions

[Mohammad Shuab Siddique]

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

TPA start/end and TPA v2 abuf paths indexed rxr->tpa_info[] using
the aggregation ID from the completion without checking
BNXT_TPA_MAX_AGGS. Add bounds checks and schedule a ring reset on
failure. Also fix abuf aggregation ID decode to use
rte_le_to_cpu_16() when reading the device field. On the legacy
(non-Thor) TPA end path, also discard the pending agg bufs before
returning on an invalid agg_id, matching the existing in_reset early
return a few lines above; otherwise those agg buf completions are
left unconsumed and the CQ raw_cons goes out of sync with the ring
even after the scheduled reset.

Fixes: 0958d8b6435d ("net/bnxt: support LRO")
Fixes: b150a7e7ee66 ("net/bnxt: support LRO on Thor adapters")
Cc: stable@dpdk.org

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

diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 98bdbc136a..1790ce839f 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -227,12 +227,19 @@ static void bnxt_tpa_start(struct bnxt_rx_queue *rxq,
 			   struct rx_tpa_start_cmpl_hi *tpa_start1)
 {
 	struct bnxt_rx_ring_info *rxr = rxq->rx_ring;
-	uint16_t agg_id;
-	uint16_t data_cons;
 	struct bnxt_tpa_info *tpa_info;
+	uint32_t data_cons, agg_id;
+	struct bnxt *bp = rxq->bp;
 	struct rte_mbuf *mbuf;
 
-	agg_id = bnxt_tpa_start_agg_id(rxq->bp, tpa_start);
+	agg_id = bnxt_tpa_start_agg_id(bp, tpa_start);
+	if (unlikely(agg_id >= BNXT_TPA_MAX_AGGS(bp))) {
+		PMD_DRV_LOG_LINE(ERR,
+				 "TPA start: invalid agg_id %u (max %u)",
+				 agg_id, BNXT_TPA_MAX_AGGS(bp));
+		bnxt_sched_ring_reset(rxq);
+		return;
+	}
 
 	data_cons = tpa_start->opaque;
 	tpa_info = &rxr->tpa_info[agg_id];
@@ -259,7 +266,7 @@ static void bnxt_tpa_start(struct bnxt_rx_queue *rxq,
 	mbuf->port = rxq->port_id;
 	mbuf->ol_flags = RTE_MBUF_F_RX_LRO;
 
-	bnxt_tpa_get_metadata(rxq->bp, tpa_info, tpa_start, tpa_start1);
+	bnxt_tpa_get_metadata(bp, tpa_info, tpa_start, tpa_start1);
 
 	if (likely(tpa_info->hash_valid)) {
 		mbuf->hash.rss = tpa_info->rss_hash;
@@ -269,7 +276,7 @@ static void bnxt_tpa_start(struct bnxt_rx_queue *rxq,
 		mbuf->ol_flags |= RTE_MBUF_F_RX_FDIR | RTE_MBUF_F_RX_FDIR_ID;
 	}
 
-	if (tpa_info->vlan_valid && BNXT_RX_VLAN_STRIP_EN(rxq->bp)) {
+	if (tpa_info->vlan_valid && BNXT_RX_VLAN_STRIP_EN(bp)) {
 		mbuf->vlan_tci = tpa_info->vlan;
 		mbuf->ol_flags |= RTE_MBUF_F_RX_VLAN | RTE_MBUF_F_RX_VLAN_STRIPPED;
 	}
@@ -412,20 +419,21 @@ static inline struct rte_mbuf *bnxt_tpa_end(
 {
 	struct bnxt_cp_ring_info *cpr = rxq->cp_ring;
 	struct bnxt_rx_ring_info *rxr = rxq->rx_ring;
-	uint16_t agg_id;
+	struct bnxt_tpa_info *tpa_info;
+	struct bnxt *bp = rxq->bp;
+	uint8_t payload_offset;
 	struct rte_mbuf *mbuf;
 	uint8_t agg_bufs;
-	uint8_t payload_offset;
-	struct bnxt_tpa_info *tpa_info;
+	uint32_t agg_id;
 
 	if (unlikely(rxq->in_reset)) {
 		PMD_DRV_LOG_LINE(ERR, "rxq->in_reset: raw_cp_cons:%d",
 			    *raw_cp_cons);
-		bnxt_discard_rx(rxq->bp, cpr, raw_cp_cons, tpa_end);
+		bnxt_discard_rx(bp, cpr, raw_cp_cons, tpa_end);
 		return NULL;
 	}
 
-	if (BNXT_CHIP_P5_P7(rxq->bp)) {
+	if (BNXT_CHIP_P5_P7(bp)) {
 		struct rx_tpa_v2_end_cmpl *th_tpa_end;
 		struct rx_tpa_v2_end_cmpl_hi *th_tpa_end1;
 
@@ -442,6 +450,15 @@ static inline struct rte_mbuf *bnxt_tpa_end(
 		payload_offset = tpa_end->payload_offset;
 	}
 
+	if (unlikely(agg_id >= BNXT_TPA_MAX_AGGS(bp))) {
+		PMD_DRV_LOG_LINE(ERR,
+				 "TPA end: invalid agg_id %u (max %u)",
+				 agg_id, BNXT_TPA_MAX_AGGS(bp));
+		bnxt_discard_rx(bp, cpr, raw_cp_cons, tpa_end);
+		bnxt_sched_ring_reset(rxq);
+		return NULL;
+	}
+
 	tpa_info = &rxr->tpa_info[agg_id];
 	mbuf = tpa_info->mbuf;
 	RTE_ASSERT(mbuf != NULL);
@@ -1126,9 +1143,18 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
 
 	if (cmp_type == RX_TPA_V2_ABUF_CMPL_TYPE_RX_TPA_AGG) {
 		struct rx_tpa_v2_abuf_cmpl *rx_agg = (void *)rxcmp;
-		uint16_t agg_id = rte_cpu_to_le_16(rx_agg->agg_id);
+		uint32_t agg_id = rte_le_to_cpu_16(rx_agg->agg_id);
 		struct bnxt_tpa_info *tpa_info;
 
+		if (unlikely(agg_id >= BNXT_TPA_MAX_AGGS(bp))) {
+			PMD_DRV_LOG_LINE(ERR,
+					 "TPA abuf: invalid agg_id %u (max %u)",
+					 agg_id, BNXT_TPA_MAX_AGGS(bp));
+			bnxt_sched_ring_reset(rxq);
+			rc = -EINVAL;
+			goto next_rx;
+		}
+
 		tpa_info = &rxr->tpa_info[agg_id];
 		RTE_ASSERT(tpa_info->agg_count < 16);
 		tpa_info->agg_arr[tpa_info->agg_count++] = *rx_agg;
-- 
2.47.3

[PATCH v3 3/5] net/bnxt: harden sprintf bounds for device memory names

[Mohammad Shuab Siddique]

From: Keegan Freyhof <keegan.freyhof@broadcom.com>

sprintf() into fixed-size stack buffers such as
char type[RTE_MEMZONE_NAMESIZE] does not bound the write to the
buffer. bnxt_hwrm_ver_get()'s "bnxt_hwrm_short_" PCI_PRI_FMT label
overflows a 32-byte RTE_MEMZONE_NAMESIZE buffer once the PCI domain
needs more than 4 hex digits: rte_pci_addr.domain is a uint32_t, and
PCI_PRI_FMT's %.4x is a minimum width, not a maximum, so an 8-digit
domain plus the 16-character prefix plus the rest of the PCI address
string plus the terminating NUL is 33 bytes into 32.

Convert this and the other sprintf() calls building memzone/malloc
names and HWRM CFA pair_name request fields to snprintf(), which
bounds the write and truncates instead of overflowing. The return
value is not checked: none of these format strings involve locale or
multibyte conversion, so a negative return is not reachable, and a
truncated debug label or pair_name is not itself a memory-safety
concern -- a truncated pair_name simply fails to match at the
firmware side, handled the same way as any other not-found pair.

Also write the per-flow xstat names directly into
xstats_names[count].name via snprintf() instead of through an
intermediate buf plus strlcpy(), since the destination is already
exactly sized for that format string.

Fixes: 79cc1efd99c6 ("net/bnxt: support extended HWRM request sizes")
Cc: stable@dpdk.org

Signed-off-by: Keegan Freyhof <keegan.freyhof@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
v3:
* Dropped check_snprintf_rc() and the early-return paths on the CFA
  pair_name sites entirely -- Stephen Hemminger noted the rc < 0
  branch is unreachable and plain snprintf() is enough for the PCI
  domain overflow this patch actually targets. All ten snprintf()
  sites now behave the same way: bounded, unchecked, truncate on
  overflow. This also removes the three bugs (a leak, a stale flag,
  and missing HWRM_UNLOCK()s) v2's early-return paths had introduced
  and fixed, since there is no early-return path left to have them.

 drivers/net/bnxt/bnxt.h        |  1 +
 drivers/net/bnxt/bnxt_ethdev.c | 14 +++++++-------
 drivers/net/bnxt/bnxt_hwrm.c   | 10 +++++-----
 drivers/net/bnxt/bnxt_stats.c  | 15 ++++++---------
 4 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
index 336de75da0..f3edfb9530 100644
--- a/drivers/net/bnxt/bnxt.h
+++ b/drivers/net/bnxt/bnxt.h
@@ -1285,6 +1285,7 @@ extern int bnxt_logtype_driver;
 						       BNXT_LINK_SPEEDS_V2_VF((bp))))
 #define BNXT_MAX_SPEED_LANES 8
 #define BNXT_SUPPORTS_TPA(bp)  (!BNXT_CHIP_P5_P7(bp) || (bp)->max_tpa_v2)
+
 extern const struct rte_flow_ops bnxt_ulp_rte_flow_ops;
 int32_t bnxt_ulp_port_init(struct bnxt *bp);
 void bnxt_ulp_port_deinit(struct bnxt *bp);
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 4d4349457c..11e8f7908d 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -647,12 +647,12 @@ static int bnxt_init_fc_ctx_mem(struct bnxt *bp)
 {
 	struct rte_pci_device *pdev = bp->pdev;
 	char type[RTE_MEMZONE_NAMESIZE];
-	uint16_t max_fc;
 	int rc = 0;
+	uint16_t max_fc;
 
 	max_fc = bp->flow_stat->max_fc;
 
-	sprintf(type, "bnxt_rx_fc_in_" PCI_PRI_FMT, pdev->addr.domain,
+	snprintf(type, sizeof(type), "bnxt_rx_fc_in_" PCI_PRI_FMT, pdev->addr.domain,
 		pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
 	/* 4 bytes for each counter-id */
 	rc = bnxt_alloc_ctx_mem_buf(bp, type,
@@ -661,7 +661,7 @@ static int bnxt_init_fc_ctx_mem(struct bnxt *bp)
 	if (rc)
 		return rc;
 
-	sprintf(type, "bnxt_rx_fc_out_" PCI_PRI_FMT, pdev->addr.domain,
+	snprintf(type, sizeof(type), "bnxt_rx_fc_out_" PCI_PRI_FMT, pdev->addr.domain,
 		pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
 	/* 16 bytes for each counter - 8 bytes pkt_count, 8 bytes byte_count */
 	rc = bnxt_alloc_ctx_mem_buf(bp, type,
@@ -670,7 +670,7 @@ static int bnxt_init_fc_ctx_mem(struct bnxt *bp)
 	if (rc)
 		return rc;
 
-	sprintf(type, "bnxt_tx_fc_in_" PCI_PRI_FMT, pdev->addr.domain,
+	snprintf(type, sizeof(type), "bnxt_tx_fc_in_" PCI_PRI_FMT, pdev->addr.domain,
 		pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
 	/* 4 bytes for each counter-id */
 	rc = bnxt_alloc_ctx_mem_buf(bp, type,
@@ -679,7 +679,7 @@ static int bnxt_init_fc_ctx_mem(struct bnxt *bp)
 	if (rc)
 		return rc;
 
-	sprintf(type, "bnxt_tx_fc_out_" PCI_PRI_FMT, pdev->addr.domain,
+	snprintf(type, sizeof(type), "bnxt_tx_fc_out_" PCI_PRI_FMT, pdev->addr.domain,
 		pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
 	/* 16 bytes for each counter - 8 bytes pkt_count, 8 bytes byte_count */
 	rc = bnxt_alloc_ctx_mem_buf(bp, type,
@@ -5232,8 +5232,8 @@ int bnxt_alloc_ctx_pg_tbls(struct bnxt *bp)
 {
 	struct bnxt_ctx_mem_info *ctx = bp->ctx;
 	struct bnxt_ctx_mem *ctx2;
-	uint16_t type;
 	int rc = 0;
+	uint16_t type;
 
 	ctx2 = &ctx->ctx_arr[0];
 	for (type = 0; type < ctx->types && rc == 0; type++) {
@@ -5254,7 +5254,7 @@ int bnxt_alloc_ctx_pg_tbls(struct bnxt *bp)
 		for (i = 0; i < w && rc == 0; i++) {
 			char name[RTE_MEMZONE_NAMESIZE] = {0};
 
-			sprintf(name, "_%d_%d", i, type);
+			snprintf(name, sizeof(name), "_%d_%d", i, type);
 
 			if (ctxm->entry_multiple)
 				entries = bnxt_roundup(ctxm->max_entries,
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 1615b36aae..361402b36f 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -1583,12 +1583,12 @@ int bnxt_hwrm_func_resc_qcaps(struct bnxt *bp)
 
 int bnxt_hwrm_ver_get(struct bnxt *bp, uint32_t timeout)
 {
-	int rc = 0;
 	struct hwrm_ver_get_input req = {.req_type = 0 };
 	struct hwrm_ver_get_output *resp = bp->hwrm_cmd_resp_addr;
 	uint32_t fw_version;
 	uint16_t max_resp_len;
 	char type[RTE_MEMZONE_NAMESIZE];
+	int rc = 0;
 	uint32_t dev_caps_cfg;
 
 	bp->max_req_len = HWRM_MAX_REQ_LEN;
@@ -1669,10 +1669,9 @@ int bnxt_hwrm_ver_get(struct bnxt *bp, uint32_t timeout)
 	     (dev_caps_cfg &
 	      HWRM_VER_GET_OUTPUT_DEV_CAPS_CFG_SHORT_CMD_REQUIRED)) ||
 	    bp->hwrm_max_ext_req_len > HWRM_MAX_REQ_LEN) {
-		sprintf(type, "bnxt_hwrm_short_" PCI_PRI_FMT,
+		snprintf(type, sizeof(type), "bnxt_hwrm_short_" PCI_PRI_FMT,
 			bp->pdev->addr.domain, bp->pdev->addr.bus,
 			bp->pdev->addr.devid, bp->pdev->addr.function);
-
 		rte_free(bp->hwrm_short_cmd_req_addr);
 
 		bp->hwrm_short_cmd_req_addr =
@@ -3542,7 +3541,7 @@ int bnxt_alloc_hwrm_resources(struct bnxt *bp)
 	struct rte_pci_device *pdev = bp->pdev;
 	char type[RTE_MEMZONE_NAMESIZE];
 
-	sprintf(type, "bnxt_hwrm_" PCI_PRI_FMT, pdev->addr.domain,
+	snprintf(type, sizeof(type), "bnxt_hwrm_" PCI_PRI_FMT, pdev->addr.domain,
 		pdev->addr.bus, pdev->addr.devid, pdev->addr.function);
 	bp->max_resp_len = BNXT_PAGE_SIZE;
 	bp->hwrm_cmd_resp_addr = rte_malloc(type, bp->max_resp_len, 0);
@@ -6792,7 +6791,7 @@ static int bnxt_alloc_all_ctx_pg_info(struct bnxt *bp)
 		if (ctxm->instance_bmap)
 			n = hweight32(ctxm->instance_bmap);
 
-		sprintf(name, "bnxt_ctx_pgmem_%d_%d",
+		snprintf(name, sizeof(name), "bnxt_ctx_pgmem_%d_%d",
 			bp->eth_dev->data->port_id, type);
 		ctxm->pg_info = rte_malloc(name, sizeof(*ctxm->pg_info) * n,
 					   RTE_CACHE_LINE_SIZE);
@@ -7836,6 +7835,7 @@ int bnxt_hwrm_cfa_pair_free(struct bnxt *bp, struct bnxt_representor *rep_bp)
 	HWRM_PREP(&req, HWRM_CFA_PAIR_FREE, BNXT_USE_CHIMP_MB);
 	snprintf(req.pair_name, sizeof(req.pair_name), "%svfr%d",
 		 bp->eth_dev->data->name, rep_bp->vf_id);
+
 	req.pf_b_id = rep_bp->parent_pf_idx;
 	req.pair_mode = HWRM_CFA_PAIR_FREE_INPUT_PAIR_MODE_REP2FN_TRUFLOW;
 	req.vf_id = BNXT_REP_PF(rep_bp) ? rte_cpu_to_le_16(((uint16_t)-1)) :
diff --git a/drivers/net/bnxt/bnxt_stats.c b/drivers/net/bnxt/bnxt_stats.c
index 4074613eb0..05e8944f47 100644
--- a/drivers/net/bnxt/bnxt_stats.c
+++ b/drivers/net/bnxt/bnxt_stats.c
@@ -1165,17 +1165,14 @@ int bnxt_dev_xstats_get_names_op(struct rte_eth_dev *eth_dev,
 	    bp->fw_cap & BNXT_FW_CAP_ADV_FLOW_MGMT &&
 	    BNXT_FLOW_XSTATS_EN(bp)) {
 		for (i = 0; i < bp->max_l2_ctx; i++) {
-			char buf[RTE_ETH_XSTATS_NAME_SIZE];
-
-			sprintf(buf, "flow_%d_bytes", i);
-			strlcpy(xstats_names[count].name, buf,
-				sizeof(xstats_names[count].name));
+			snprintf(xstats_names[count].name,
+				sizeof(xstats_names[count].name),
+				"flow_%d_bytes", i);
 			count++;
 
-			sprintf(buf, "flow_%d_packets", i);
-			strlcpy(xstats_names[count].name, buf,
-				sizeof(xstats_names[count].name));
-
+			snprintf(xstats_names[count].name,
+				sizeof(xstats_names[count].name),
+				"flow_%d_packets", i);
 			count++;
 		}
 	}
-- 
2.47.3

[PATCH v3 4/5] net/bnxt: fix bounds in MAC address pool index

[Mohammad Shuab Siddique]

From: Chenna Arnoori <chenna.arnoori@broadcom.com>

bnxt_mac_addr_add_op() indexed bp->vnic_info[pool] with a
caller-supplied pool before validating it against bp->max_vnics, and
before checking bp->vnic_info was even allocated yet (it is NULL
until the port is started). The existing "if (!vnic)" check was
always false, since vnic held the address of an array element and
is never NULL. bp->max_vnics is known from probe onward, so the pool
bound is checked first and unconditionally; the dev_started and
vnic_info checks, which gate whether there is anything to index yet,
follow it.

Fixes: 51fafb89a9a0 ("net/bnxt: get rid of ff pools and use VNIC info array")
Cc: stable@dpdk.org

Signed-off-by: Chenna Arnoori <chenna.arnoori@broadcom.com>
Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
---
v3:
* Retitled from "fix bounds in MAC pool index and flow parsing" and
  dropped the flow-parsing half entirely (the bounded VOID-item skip
  in bnxt_flow_non_void_item()/bnxt_flow_non_void_action()) --
  Stephen Hemminger pointed out rte_flow patterns/actions are always
  END-terminated by API contract, so that bound did not guard a
  reachable path; reverted to the original unbounded skip loop
  rather than keep unnecessary defensive code. Dropped the
  corresponding Fixes: 5c1171c97216 tag.
* Reordered the remaining MAC-pool fix so the pool-vs-max_vnics bound
  check runs before the dev_started/vnic_info checks, not after --
  also per Stephen Hemminger.

 drivers/net/bnxt/bnxt_ethdev.c | 11 ++++++++---
 drivers/net/bnxt/bnxt_flow.c   |  1 +
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 11e8f7908d..20084826d3 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -2105,7 +2105,7 @@ static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 				uint32_t index, uint32_t pool)
 {
 	struct bnxt *bp = eth_dev->data->dev_private;
-	struct bnxt_vnic_info *vnic = &bp->vnic_info[pool];
+	struct bnxt_vnic_info *vnic;
 	int rc = 0;
 
 	rc = is_bnxt_in_error(bp);
@@ -2117,8 +2117,8 @@ static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 		return -ENOTSUP;
 	}
 
-	if (!vnic) {
-		PMD_DRV_LOG_LINE(ERR, "VNIC not found for pool %d!", pool);
+	if (pool >= bp->max_vnics) {
+		PMD_DRV_LOG_LINE(ERR, "Pool %u exceeds VNIC count %u!", pool, bp->max_vnics);
 		return -EINVAL;
 	}
 
@@ -2126,6 +2126,11 @@ static int bnxt_mac_addr_add_op(struct rte_eth_dev *eth_dev,
 	if (!eth_dev->data->dev_started)
 		return 0;
 
+	if (bp->vnic_info == NULL)
+		return 0;
+
+	vnic = &bp->vnic_info[pool];
+
 	rc = bnxt_add_mac_filter(bp, vnic, mac_addr, index, pool);
 
 	return rc;
diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
index 4bf38043b5..7b27d62697 100644
--- a/drivers/net/bnxt/bnxt_flow.c
+++ b/drivers/net/bnxt/bnxt_flow.c
@@ -1697,6 +1697,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
 	while (act->type != RTE_FLOW_ACTION_TYPE_END)
 		goto start;
 
+
 	return rc;
 ret:
 
-- 
2.47.3

[PATCH v3 5/5] net/bnxt: fix TPA agg Rx descriptor and VNIC query bounds

[Mohammad Shuab Siddique]

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

Three independent out-of-bounds issues:

- bnxt_rx_pkt() incremented tpa_info->agg_count and indexed
  tpa_info->agg_arr[] with only an RTE_ASSERT (compiled out in release
  builds) guarding the array bound, allowing an out-of-bounds write if
  firmware sent more aggregation segments than TPA_MAX_NUM_SEGS.

- bnxt_rx_descriptor_status_op() used the firmware-supplied completion
  opaque value directly as an rx_buf_ring[] index without masking it to
  the ring size first.

- bnxt_hwrm_func_vf_vnic_query() returned the firmware-reported
  vnic_id_cnt unclamped; a value exceeding bp->pf->total_vnics would
  cause the caller to iterate past the end of its VNIC ID buffer.

Fixes: b150a7e7ee66 ("net/bnxt: support LRO on Thor adapters")
Fixes: 0fe613bb87b2 ("net/bnxt: support Rx descriptor status")
Fixes: cbcd375d37d2 ("net/bnxt: fix HWRM macros and locking")
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_ethdev.c |  3 ++-
 drivers/net/bnxt/bnxt_hwrm.c   |  3 ++-
 drivers/net/bnxt/bnxt_rxr.c    | 10 +++++++++-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 20084826d3..3a530c31aa 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -3624,7 +3624,8 @@ bnxt_rx_descriptor_status_op(void *rx_queue, uint16_t offset)
 		case CMPL_BASE_TYPE_RX_L2:
 		case CMPL_BASE_TYPE_RX_L2_V2:
 			if (desc == offset) {
-				cons = rxcmp->opaque;
+				cons = RING_IDX(rxr->rx_ring_struct,
+						 rxcmp->opaque);
 				if (rxr->rx_buf_ring[cons])
 					return RTE_ETH_RX_DESC_DONE;
 				else
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 361402b36f..42ed6e5ae1 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -6234,7 +6234,8 @@ static int bnxt_hwrm_func_vf_vnic_query(struct bnxt *bp, uint16_t vf,
 	}
 	rc = bnxt_hwrm_send_message(bp, &req, sizeof(req), BNXT_USE_CHIMP_MB);
 	HWRM_CHECK_RESULT();
-	rc = rte_le_to_cpu_32(resp->vnic_id_cnt);
+	rc = RTE_MIN(rte_le_to_cpu_32(resp->vnic_id_cnt),
+		     (uint32_t)bp->pf->total_vnics);
 
 	HWRM_UNLOCK();
 
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index 1790ce839f..fa10446c27 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -1156,7 +1156,15 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
 		}
 
 		tpa_info = &rxr->tpa_info[agg_id];
-		RTE_ASSERT(tpa_info->agg_count < 16);
+		if (unlikely(tpa_info->agg_count >= TPA_MAX_NUM_SEGS)) {
+			PMD_DRV_LOG_LINE(ERR,
+					 "TPA abuf: agg_count %u exceeds max %u",
+					 tpa_info->agg_count, TPA_MAX_NUM_SEGS);
+			tpa_info->agg_count = 0;
+			bnxt_sched_ring_reset(rxq);
+			rc = -EINVAL;
+			goto next_rx;
+		}
 		tpa_info->agg_arr[tpa_info->agg_count++] = *rx_agg;
 		rc = -EINVAL; /* Continue w/o new mbuf */
 		goto next_rx;
-- 
2.47.3