[PATCH] net/bnxt: fix truflow out-of-bounds accesses

[Manish Kurup <manish.kurup@broadcom.com>]

From: Joey Zhong <xiaozhen.zhong@broadcom.com>

Harden several TruFlow paths that trusted unvalidated indices or input.

dpool_free() and dpool_set_entry_data() checked only the lower bound
(start < 0) of the computed pool index before dereferencing
dpool->entry[start] and, in dpool_free(), running a zero-write loop.
The EM index originates from the HWRM EM-delete/move response
(tf_msg_delete_em_entry() stores resp.em_index into parms->index) and
is handed to dpool_free() by tf_em_hash_delete_int_entry(). Firmware
that returns an out-of-range em_index could drive a read and
conditional zero-write up to ~1 MB past the EM dpool allocation on
every flow delete. Add an upper-bound check ((uint32_t)start >=
dpool->size) to both functions. dpool_free() also needs the write
loop itself bounded: start + size can still exceed dpool->size when
the stale flags byte at a corrupted start encodes a nonzero size, so
reject that case too instead of only validating start in isolation.

tf_em_hash_delete_int_entry() also indexed pool->entry[] directly in
the TF_FLOW_SCALE_QUERY usage-update path using the same untrusted
index with no bound check. Guard that read with the same range test.

tf_attach_session() required the control and attach channel names to
match a strict 4-field PCI form ("%x:%x:%x.%u") and returned -EINVAL
otherwise, unlike tf_open_session() which falls back to the
domain-omitted 3-field form ("%x:%x.%u", domain forced to 0) that DPDK
permits. Names without the domain prefix therefore failed to attach
even though the same name opened successfully. Add the same fallback
to both channel parses.

Fixes: 05b405d58148 ("net/bnxt: add dpool allocator for EM allocation")
Fixes: 19f3ac618ab2 ("net/bnxt/tf_core: support flow scale query")
Fixes: a46bbb57605b ("net/bnxt: update multi device design")
Cc: stable@dpdk.org

Signed-off-by: Joey Zhong <xiaozhen.zhong@broadcom.com>
Signed-off-by: Manish Kurup <manish.kurup@broadcom.com>
---
 drivers/net/bnxt/tf_core/dpool.c              |  7 ++--
 drivers/net/bnxt/tf_core/tf_core.c            | 38 ++++++++++++++++---
 .../net/bnxt/tf_core/tf_em_hash_internal.c    | 17 ++++++---
 3 files changed, 47 insertions(+), 15 deletions(-)

diff --git a/drivers/net/bnxt/tf_core/dpool.c b/drivers/net/bnxt/tf_core/dpool.c
index f60c04e949..65d5239680 100644
--- a/drivers/net/bnxt/tf_core/dpool.c
+++ b/drivers/net/bnxt/tf_core/dpool.c
@@ -345,12 +345,13 @@ int dpool_free(struct dpool *dpool,
 	int start = (index - dpool->start_index);
 	uint32_t size;
 
-	if (start < 0)
+	if (start < 0 || (uint32_t)start >= dpool->size)
 		return -1;
 
 	if (DP_IS_START(dpool->entry[start].flags)) {
 		size = DP_FLAGS_SIZE(dpool->entry[start].flags);
-		if (size > dpool->max_alloc_size || size == 0)
+		if (size > dpool->max_alloc_size || size == 0 ||
+		    (uint32_t)start + size > dpool->size)
 			return -1;
 
 		for (i = start; i < (start + size); i++)
@@ -376,7 +377,7 @@ int dpool_set_entry_data(struct dpool *dpool,
 {
 	int start = (index - dpool->start_index);
 
-	if (start < 0)
+	if (start < 0 || (uint32_t)start >= dpool->size)
 		return -1;
 
 	if (DP_IS_START(dpool->entry[start].flags)) {
diff --git a/drivers/net/bnxt/tf_core/tf_core.c b/drivers/net/bnxt/tf_core/tf_core.c
index f1b3be48aa..3e60c2aa8c 100644
--- a/drivers/net/bnxt/tf_core/tf_core.c
+++ b/drivers/net/bnxt/tf_core/tf_core.c
@@ -109,9 +109,22 @@ tf_attach_session(struct tf *tfp,
 		    &slot,
 		    &device);
 	if (rc != 4) {
-		TFP_DRV_LOG(ERR,
-			    "Failed to scan device ctrl_chan_name\n");
-		return -EINVAL;
+		/* PCI Domain not provided (optional in DPDK), thus we
+		 * force domain to 0 and recheck.
+		 */
+		domain = 0;
+
+		/* Check parsing of bus/slot/device */
+		rc = sscanf(parms->ctrl_chan_name,
+			    "%x:%x.%u",
+			    &bus,
+			    &slot,
+			    &device);
+		if (rc != 3) {
+			TFP_DRV_LOG(ERR,
+				    "Failed to scan device ctrl_chan_name\n");
+			return -EINVAL;
+		}
 	}
 
 	/* Verify 'attach' channel */
@@ -122,9 +135,22 @@ tf_attach_session(struct tf *tfp,
 		    &slot,
 		    &device);
 	if (rc != 4) {
-		TFP_DRV_LOG(ERR,
-			    "Failed to scan device attach_chan_name\n");
-		return -EINVAL;
+		/* PCI Domain not provided (optional in DPDK), thus we
+		 * force domain to 0 and recheck.
+		 */
+		domain = 0;
+
+		/* Check parsing of bus/slot/device */
+		rc = sscanf(parms->attach_chan_name,
+			    "%x:%x.%u",
+			    &bus,
+			    &slot,
+			    &device);
+		if (rc != 3) {
+			TFP_DRV_LOG(ERR,
+				    "Failed to scan device attach_chan_name\n");
+			return -EINVAL;
+		}
 	}
 
 	/* Prepare return value of session_id, using ctrl_chan_name
diff --git a/drivers/net/bnxt/tf_core/tf_em_hash_internal.c b/drivers/net/bnxt/tf_core/tf_em_hash_internal.c
index 0212abd05d..e83e7c34f8 100644
--- a/drivers/net/bnxt/tf_core/tf_em_hash_internal.c
+++ b/drivers/net/bnxt/tf_core/tf_em_hash_internal.c
@@ -153,12 +153,17 @@ tf_em_hash_delete_int_entry(struct tf *tfp,
 	pool = (struct dpool *)tfs->em_pool[parms->dir];
 
 #ifdef TF_FLOW_SCALE_QUERY
-	/* Update usage state buffer for EM */
-	size = DP_FLAGS_SIZE(pool->entry[parms->index - pool->start_index].flags);
-	tf_em_usage_update(tfp,
-			   parms->dir,
-			   size,
-			   TF_RESC_FREE);
+	/* Update usage state buffer for EM.
+	 * parms->index is populated from the (untrusted) HWRM response by
+	 * tf_msg_delete_em_entry(); validate it before indexing the pool.
+	 */
+	if ((parms->index - pool->start_index) < pool->size) {
+		size = DP_FLAGS_SIZE(pool->entry[parms->index - pool->start_index].flags);
+		tf_em_usage_update(tfp,
+				   parms->dir,
+				   size,
+				   TF_RESC_FREE);
+	}
 #endif /* TF_FLOW_SCALE_QUERY */
 
 	dpool_free(pool, parms->index);
-- 
2.31.1