[PATCH 0/2] net/bnxt: fix message layer and hot-upgrade issues

[PATCH 0/2] net/bnxt: fix message layer and hot-upgrade issues

[Manish Kurup]

This series fixes several bounds/pointer issues found by static
analysis in the tf_core message layer, plus a related validation
limit bug found while fixing them:

- Patch 1 caps a memcpy() to the caller's buffer size in
  tfc_msg_idx_tbl_get(), adds a NULL check on that function's
  caller-supplied dev_data/data_size pointers, fixes a fall-through
  in tfc_msg_if_tbl_get() that let a size-mismatch error still copy
  oversized data, and adds missing NULL checks on a caller-supplied
  pointer in tfc_msg_idx_tbl_alloc_set()/tfc_msg_idx_tbl_set().

- Patch 2 fixes the app-instance-ID validation limit used during
  hot-upgrade: it enforced a limit of 4 (the number of concurrent
  instances) instead of 8 (the actual valid ID range), incorrectly
  rejecting valid instance IDs 5-8.

Manish Kurup (1):
  net/bnxt: fix hot-upgrade app instance ID limit

Mohammad Shuab Siddique (1):
  net/bnxt: fix message-layer bounds and pointer checks

 .../bnxt/hcapi/cfa_v3/include/cfa_resources.h |  2 +-
 drivers/net/bnxt/tf_core/v3/tfc_msg.c         | 32 ++++++++++++++++---
 2 files changed, 29 insertions(+), 5 deletions(-)

-- 
2.31.1

[PATCH 1/2] net/bnxt: fix message-layer bounds and pointer checks

[Manish Kurup]

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

Fix four medium/low severity issues in the message layer, found by
static analysis.

tfc_msg_idx_tbl_get() used the raw FW-reported resp.data_size in a
memcpy() without capping it to the caller's DMA buffer size, and
dereferenced the caller-supplied dev_data/data_size pointers without
a NULL check. Decode resp.data_size and reject the call if it
exceeds the caller-supplied *data_size, and add an early return with
-EINVAL if dev_data or data_size is NULL.

tfc_msg_if_tbl_get() set rc=-EINVAL on a size mismatch but fell
through to *data_size = resp.data_size and then memcpy()'d that many
bytes into the caller's buffer anyway. Return early on error, and
decode resp.data_size before assigning it to *data_size.

tfc_msg_idx_tbl_alloc_set() and tfc_msg_idx_tbl_set() dereferenced
the caller-supplied dev_data pointer without a NULL check. Add an
early return with -EINVAL if dev_data is NULL.

Fixes: 80317ff6adfd ("net/bnxt/tf_core: support Thor2")
Cc: stable@dpdk.org

Signed-off-by: Mohammad Shuab Siddique <mohammad-shuab.siddique@broadcom.com>
Signed-off-by: Manish Kurup <manish.kurup@broadcom.com>
---
 drivers/net/bnxt/tf_core/v3/tfc_msg.c | 32 +++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bnxt/tf_core/v3/tfc_msg.c b/drivers/net/bnxt/tf_core/v3/tfc_msg.c
index cf72d09184..6282449b44 100644
--- a/drivers/net/bnxt/tf_core/v3/tfc_msg.c
+++ b/drivers/net/bnxt/tf_core/v3/tfc_msg.c
@@ -548,6 +548,11 @@ tfc_msg_idx_tbl_alloc_set(struct tfc *tfcp, uint16_t fid, uint16_t sid,
 	struct tfc_msg_dma_buf buf = { 0 };
 	uint8_t *data = NULL;
 
+	if (!dev_data) {
+		PMD_DRV_LOG_LINE(ERR, "invalid input");
+		return -EINVAL;
+	}
+
 	if (dir == CFA_DIR_RX)
 		req.flags |= HWRM_TFC_IDX_TBL_ALLOC_SET_INPUT_FLAGS_DIR_RX &
 			     HWRM_TFC_IDX_TBL_ALLOC_SET_INPUT_FLAGS_DIR;
@@ -613,6 +618,11 @@ tfc_msg_idx_tbl_set(struct tfc *tfcp, uint16_t fid,
 	struct tfc_msg_dma_buf buf = { 0 };
 	uint8_t *data = NULL;
 
+	if (!dev_data) {
+		PMD_DRV_LOG_LINE(ERR, "invalid input");
+		return -EINVAL;
+	}
+
 	if (dir == CFA_DIR_RX)
 		req.flags |= HWRM_TFC_IDX_TBL_SET_INPUT_FLAGS_DIR_RX &
 			     HWRM_TFC_IDX_TBL_SET_INPUT_FLAGS_DIR;
@@ -671,6 +681,11 @@ tfc_msg_idx_tbl_get(struct tfc *tfcp, uint16_t fid,
 	struct hwrm_tfc_idx_tbl_get_output resp = { 0 };
 	struct tfc_msg_dma_buf buf = { 0 };
 
+	if (!dev_data || !data_size) {
+		PMD_DRV_LOG_LINE(ERR, "invalid input");
+		return -EINVAL;
+	}
+
 	if (dir == CFA_DIR_RX)
 		req.flags |= HWRM_TFC_IDX_TBL_GET_INPUT_FLAGS_DIR_RX &
 			     HWRM_TFC_IDX_TBL_GET_INPUT_FLAGS_DIR;
@@ -702,8 +717,17 @@ tfc_msg_idx_tbl_get(struct tfc *tfcp, uint16_t fid,
 					 &req, sizeof(req), &resp, sizeof(resp));
 
 	if (rc == 0) {
-		memcpy(dev_data, buf.va_addr, resp.data_size);
-		*data_size = rte_le_to_cpu_16(resp.data_size);
+		uint16_t resp_data_size = rte_le_to_cpu_16(resp.data_size);
+
+		if (resp_data_size > *data_size) {
+			PMD_DRV_LOG_LINE(ERR,
+					 "FW resp data_size(%u) > caller buf(%u)",
+					 resp_data_size, *data_size);
+			rc = -EINVAL;
+			goto cleanup;
+		}
+		memcpy(dev_data, buf.va_addr, resp_data_size);
+		*data_size = resp_data_size;
 	}
 
 cleanup:
@@ -1289,10 +1313,10 @@ tfc_msg_if_tbl_get(struct tfc *tfcp, uint16_t fid, uint16_t sid,
 	if (*data_size < rte_le_to_cpu_16(resp.data_size)) {
 		PMD_DRV_LOG_LINE(ERR, "Table buffer is too small, rc:%s",
 				 strerror(EINVAL));
-		rc = -EINVAL;
+		return -EINVAL;
 	}
 
-	*data_size = resp.data_size;
+	*data_size = rte_le_to_cpu_16(resp.data_size);
 	memcpy(data, resp.data, *data_size);
 
 	return rc;
-- 
2.31.1

[PATCH 2/2] net/bnxt: fix hot-upgrade app instance ID limit

[Manish Kurup]

CFA_HOT_UPGRADE_APP_INSTANCE_MAX (4) represents the max number of
concurrent hot-upgrade instances, not the max valid instance ID.
Valid instance IDs are 1-8, matching the existing devargs
validation, so tfc_hot_upgrade_validate()'s existing
app_inst_id > CFA_HOT_UPGRADE_APP_INSTANCE_MAX check rejects valid
IDs 5-8. Bump the constant to 8; the comparison itself is already
correct against the corrected limit.

Fixes: 80317ff6adfd ("net/bnxt/tf_core: support Thor2")
Cc: stable@dpdk.org

Signed-off-by: Manish Kurup <manish.kurup@broadcom.com>
---
 drivers/net/bnxt/hcapi/cfa_v3/include/cfa_resources.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/hcapi/cfa_v3/include/cfa_resources.h b/drivers/net/bnxt/hcapi/cfa_v3/include/cfa_resources.h
index ada9741dfa..98b96e0a11 100644
--- a/drivers/net/bnxt/hcapi/cfa_v3/include/cfa_resources.h
+++ b/drivers/net/bnxt/hcapi/cfa_v3/include/cfa_resources.h
@@ -194,7 +194,7 @@ enum cfa_resource_subtype_gim {
 	 CFA_RSUBTYPE_SM_MAX + CFA_RSUBTYPE_TSM_MAX + CFA_RSUBTYPE_TIM_MAX +   \
 	 CFA_RSUBTYPE_GIM_MAX)
 
-#define CFA_HOT_UPGRADE_APP_INSTANCE_MAX 4
+#define CFA_HOT_UPGRADE_APP_INSTANCE_MAX 8
 enum cfa_hot_upgrade_cmd_op {
 	CFA_HOT_UPGRADE_CMD_ALLOC = 1,
 	CFA_HOT_UPGRADE_CMD_FREE,
-- 
2.31.1