From: Joseph Wong <[email protected]>
bnxt_handle_fwd_req() computed vf_id from an unvalidated,
firmware-controlled source_id and used it to index
bp->pf->vf_info[] before checking that the ID falls within the
active VF range. An out-of-range VF ID could index past
vf_info[] and cause memory corruption.
Move the range check ahead of the vf_id computation and the
vf_info[] lookup. Initialize fwd_cmd/req_len to NULL/0 at
declaration so they're always safe to use on the reject path,
which also silences a GCC 8 false-positive uninitialized-variable
warning in non-debug builds. Also guard
bnxt_hwrm_reject_fwd_resp()'s memcpy() against a NULL encaped
pointer for the same reason.
Fixes: f2a768d4d186 ("net/bnxt: add completion ring")
Cc: [email protected]
Signed-off-by: Joseph Wong <[email protected]>
Signed-off-by: Mohammad Shuab Siddique <[email protected]>
---
v2:
* Initialize fwd_cmd/req_len at declaration instead of only on the
reject path, removing the now-redundant explicit reset there. This
also silences a GCC 8 false-positive uninitialized-variable warning
in non-debug builds (reported independently after v1).
---
drivers/net/bnxt/bnxt_cpr.c | 27 ++++++++++++++-------------
drivers/net/bnxt/bnxt_hwrm.c | 3 ++-
2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/net/bnxt/bnxt_cpr.c b/drivers/net/bnxt/bnxt_cpr.c
index 5c255de59ea..edbfc14ec60 100644
--- a/drivers/net/bnxt/bnxt_cpr.c
+++ b/drivers/net/bnxt/bnxt_cpr.c
@@ -362,10 +362,10 @@ void bnxt_handle_fwd_req(struct bnxt *bp, struct
cmpl_base *cmpl)
{
struct hwrm_exec_fwd_resp_input *fwreq;
struct hwrm_fwd_req_cmpl *fwd_cmpl = (struct hwrm_fwd_req_cmpl *)cmpl;
- struct input *fwd_cmd;
+ struct input *fwd_cmd = NULL;
uint16_t fw_vf_id;
uint16_t vf_id;
- uint16_t req_len;
+ uint16_t req_len = 0;
int rc;
if (bp->pf->active_vfs <= 0) {
@@ -375,16 +375,6 @@ void bnxt_handle_fwd_req(struct bnxt *bp, struct cmpl_base
*cmpl)
/* Qualify the fwd request */
fw_vf_id = rte_le_to_cpu_16(fwd_cmpl->source_id);
- vf_id = fw_vf_id - bp->pf->first_vf_id;
-
- req_len = (rte_le_to_cpu_16(fwd_cmpl->req_len_type) &
- HWRM_FWD_REQ_CMPL_REQ_LEN_MASK) >>
- HWRM_FWD_REQ_CMPL_REQ_LEN_SFT;
- if (req_len > sizeof(fwreq->encap_request))
- req_len = sizeof(fwreq->encap_request);
-
- /* Locate VF's forwarded command */
- fwd_cmd = (struct input *)bp->pf->vf_info[vf_id].req_buf;
if (fw_vf_id < bp->pf->first_vf_id ||
fw_vf_id >= bp->pf->first_vf_id + bp->pf->active_vfs) {
@@ -396,6 +386,17 @@ void bnxt_handle_fwd_req(struct bnxt *bp, struct cmpl_base
*cmpl)
goto reject;
}
+ vf_id = fw_vf_id - bp->pf->first_vf_id;
+
+ req_len = (rte_le_to_cpu_16(fwd_cmpl->req_len_type) &
+ HWRM_FWD_REQ_CMPL_REQ_LEN_MASK) >>
+ HWRM_FWD_REQ_CMPL_REQ_LEN_SFT;
+ if (req_len > sizeof(fwreq->encap_request))
+ req_len = sizeof(fwreq->encap_request);
+
+ /* Locate VF's forwarded command */
+ fwd_cmd = (struct input *)bp->pf->vf_info[vf_id].req_buf;
+
if (bnxt_rcv_msg_from_vf(bp, vf_id, fwd_cmd)) {
/*
* In older firmware versions, the MAC had to be all zeros for
@@ -495,7 +496,7 @@ void bnxt_handle_fwd_req(struct bnxt *bp, struct cmpl_base
*cmpl)
PMD_DRV_LOG_LINE(ERR,
"Failed to send REJECT req VF 0x%x, type 0x%x.",
fw_vf_id - bp->pf->first_vf_id,
- rte_le_to_cpu_16(fwd_cmd->req_type));
+ fwd_cmd ? rte_le_to_cpu_16(fwd_cmd->req_type) : 0xFFFF);
}
return;
diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index a2b0c280353..2b45f05bae9 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -5520,7 +5520,8 @@ int bnxt_hwrm_reject_fwd_resp(struct bnxt *bp, uint16_t
target_id,
HWRM_PREP(&req, HWRM_REJECT_FWD_RESP, BNXT_USE_CHIMP_MB);
req.encap_resp_target_id = rte_cpu_to_le_16(target_id);
- memcpy(req.encap_request, encaped, ec_size);
+ if (encaped)
+ memcpy(req.encap_request, encaped, ec_size);
rc = bnxt_hwrm_send_message(bp, &req, sizeof(req), BNXT_USE_CHIMP_MB);
--
2.47.3
