From: Mohammad Shuab Siddique <[email protected]>
Fix four medium/low severity issues in the message layer, found by
static analysis. Also fix the hot-upgrade app instance ID limit.
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. Also reject
a FW-reported size larger than the response data array, which the
caller's buffer size alone does not bound, and add an early return
with -EINVAL if data or data_size is NULL.
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.
Despite its name, CFA_HOT_UPGRADE_APP_INSTANCE_MAX (4) is used as the
highest valid app instance ID, and valid IDs are 1-8 (0 disables the
feature), matching the existing devargs validation.
tfc_hot_upgrade_validate() therefore rejected valid IDs 5-8 with an
"Invalid app instance" error. Raise the constant to 8.
Fixes: 80317ff6adfd ("net/bnxt/tf_core: support Thor2")
Fixes: eeefaecba000 ("net/bnxt/tf_ulp: support hot upgrade")
Cc: [email protected]
Signed-off-by: Mohammad Shuab Siddique <[email protected]>
Signed-off-by: Manish Kurup <[email protected]>
---
v2:
- Fold in the hot-upgrade app instance ID limit fix that was sent as 2/2 (4 ->
8), with its own Fixes tag; this patch supersedes both patches of the v1 series
- In tfc_msg_if_tbl_get(), also bound the FW-reported size against the
response data array, read the uint8_t data_size directly, and check
data/data_size for NULL
- New subject
.../bnxt/hcapi/cfa_v3/include/cfa_resources.h | 2 +-
drivers/net/bnxt/tf_core/v3/tfc_msg.c | 48 ++++++++++++++++---
2 files changed, 43 insertions(+), 7 deletions(-)
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 ada9741dfa1..98b96e0a11e 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,
diff --git a/drivers/net/bnxt/tf_core/v3/tfc_msg.c
b/drivers/net/bnxt/tf_core/v3/tfc_msg.c
index df11910dc3b..89c256b8109 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:
@@ -1274,10 +1298,16 @@ tfc_msg_if_tbl_get(struct tfc *tfcp, uint16_t fid,
uint16_t sid,
uint16_t index, uint8_t *data_size, uint8_t *data)
{
int rc = 0;
+ uint8_t resp_size;
struct bnxt *bp = tfcp->bp;
struct hwrm_tfc_if_tbl_get_input req = { 0 };
struct hwrm_tfc_if_tbl_get_output resp = { 0 };
+ if (!data || !data_size) {
+ PMD_DRV_LOG_LINE(ERR, "invalid input");
+ return -EINVAL;
+ }
+
req.flags = (dir == CFA_DIR_TX ?
HWRM_TFC_IF_TBL_GET_INPUT_FLAGS_DIR_TX :
HWRM_TFC_IF_TBL_GET_INPUT_FLAGS_DIR_RX);
@@ -1296,14 +1326,20 @@ tfc_msg_if_tbl_get(struct tfc *tfcp, uint16_t fid,
uint16_t sid,
if (rc)
return rc;
- if (*data_size < rte_le_to_cpu_16(resp.data_size)) {
+ resp_size = resp.data_size;
+ if (resp_size > sizeof(resp.data)) {
+ PMD_DRV_LOG_LINE(ERR, "Invalid table response size %u",
+ resp_size);
+ return -EINVAL;
+ }
+ if (*data_size < resp_size) {
PMD_DRV_LOG_LINE(ERR, "Table buffer is too small, rc:%s",
strerror(EINVAL));
- rc = -EINVAL;
+ return -EINVAL;
}
- *data_size = resp.data_size;
- memcpy(data, resp.data, *data_size);
+ *data_size = resp_size;
+ memcpy(data, resp.data, resp_size);
return rc;
}
--
2.47.3
