DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Manish Kurup <manish.kurup@broadcom.com>
To: dev@dpdk.org
Cc: kishore.padmanabha@broadcom.com,
	Joey Zhong <xiaozhen.zhong@broadcom.com>,
	stable@dpdk.org
Subject: [PATCH] net/bnxt: fix -Warray-bounds in blob bit-stream pull helpers
Date: Mon,  5 Oct 2026 15:25:26 -0500	[thread overview]
Message-ID: <20261005202526.17981-1-manish.kurup@broadcom.com> (raw)

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

GCC inlines ulp_bs_pull_lsb() and flags the remainder branch:
  if (len)
      ulp_bs_get_lsb(src, offset, len, &dst[size - 1 - idx]);
After the full-byte loop exits, idx == cnt. When cnt == size with a
non-zero bit remainder (e.g., len = 33 with size = 4), the subscript
size - 1 - idx underflows to UINT32_MAX, producing a -Warray-bounds
false positive (and a genuine out-of-bounds write if len/size are
ever miscalculated upstream of the check that normally prevents
this). ulp_blob_pull() rejects this state via its
ULP_BYTE_2_BITS(data_size) < len check, but GCC cannot carry that
constraint through the inlined call chain.

Changes:
- ulp_bs_pull_lsb / ulp_bs_pull_msb: add unlikely(cnt > size) entry
  guard and an explicit likely(idx < size) check on the remainder
  branch so GCC can prove all subscripts are in range.
- ulp_bs_pull_msb: add the missing uint32_t size parameter (matching
  ulp_bs_pull_lsb) and update its three call sites to pass the
  destination buffer size.
- ulp_bs_get_lsb / ulp_bs_get_msb: add unlikely(!bitlen) early exit
  to make the zero-length contract explicit.

Fixes: f634204b7ad8 ("net/bnxt: support generic table processing")
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_ulp/ulp_gen_tbl.c |  2 +-
 drivers/net/bnxt/tf_ulp/ulp_mapper.c  |  1 +
 drivers/net/bnxt/tf_ulp/ulp_utils.h   | 44 +++++++++++++++++++++++----
 3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/drivers/net/bnxt/tf_ulp/ulp_gen_tbl.c b/drivers/net/bnxt/tf_ulp/ulp_gen_tbl.c
index ac0a7e6db1..535849b1f9 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_gen_tbl.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_gen_tbl.c
@@ -309,7 +309,7 @@ ulp_mapper_gen_tbl_entry_data_get(struct ulp_mapper_gen_tbl_entry *entry,
 	if (entry->byte_order == BNXT_ULP_BYTE_ORDER_LE)
 		ulp_bs_pull_lsb(entry->byte_data, data, data_size, offset, len);
 	else
-		ulp_bs_pull_msb(entry->byte_data, data, offset, len);
+		ulp_bs_pull_msb(entry->byte_data, data, data_size, offset, len);
 
 	return 0;
 }
diff --git a/drivers/net/bnxt/tf_ulp/ulp_mapper.c b/drivers/net/bnxt/tf_ulp/ulp_mapper.c
index 960cdda311..168f05a5a4 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_mapper.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_mapper.c
@@ -701,6 +701,7 @@ ulp_mapper_tbl_ident_scan_ext(struct bnxt_ulp_mapper_parms *parms,
 					idents[i].ident_bit_size);
 		else
 			ulp_bs_pull_msb(byte_data, (uint8_t *)&val64,
+					sizeof(val64),
 					idents[i].ident_bit_pos,
 					idents[i].ident_bit_size);
 
diff --git a/drivers/net/bnxt/tf_ulp/ulp_utils.h b/drivers/net/bnxt/tf_ulp/ulp_utils.h
index ce20f1916f..5cb6e48b37 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_utils.h
+++ b/drivers/net/bnxt/tf_ulp/ulp_utils.h
@@ -663,6 +663,11 @@ ulp_bs_get_lsb(uint8_t *src, uint16_t bitpos, uint8_t bitlen, uint8_t *dst)
 	uint16_t index  = ULP_BITS_2_BYTE_NR(bitpos);
 	uint8_t mask, partial, shift;
 
+	if (unlikely(!bitlen)) {
+		*dst = 0;
+		return;
+	}
+
 	shift = bitoffs;
 	partial = ULP_BLOB_BYTE - bitoffs;
 	if (bitoffs + bitlen <= ULP_BLOB_BYTE) {
@@ -700,6 +705,16 @@ ulp_bs_pull_lsb(uint8_t *src, uint8_t *dst, uint32_t size,
 	uint32_t idx;
 	uint32_t cnt = ULP_BITS_2_BYTE_NR(len);
 
+	/*
+	 * cnt > size means len >= (size + 1) * 8; caller should have
+	 * rejected this. It does not catch size*8 < len < (size + 1)*8,
+	 * which also can't fit in dst: there cnt == size, so idx == size
+	 * after the loop below, which makes "idx < size" false and
+	 * silently drops the remainder instead of writing past dst.
+	 */
+	if (unlikely(cnt > size))
+		return;
+
 	/* iterate bytewise to get data */
 	for (idx = 0; idx < cnt; idx++) {
 		ulp_bs_get_lsb(src, offset, ULP_BLOB_BYTE,
@@ -708,8 +723,8 @@ ulp_bs_pull_lsb(uint8_t *src, uint8_t *dst, uint32_t size,
 		len -= ULP_BLOB_BYTE;
 	}
 
-	/* Extract the last reminder data that is not 8 byte boundary */
-	if (len)
+	/* Extract the last remainder data that is not 8 byte boundary */
+	if (len && likely(idx < size))
 		ulp_bs_get_lsb(src, offset, len, &dst[size - 1 - idx]);
 }
 
@@ -735,6 +750,11 @@ ulp_bs_get_msb(uint8_t *src, uint16_t bitpos, uint8_t bitlen, uint8_t *dst)
 	uint8_t mask;
 	int32_t shift;
 
+	if (unlikely(!bitlen)) {
+		*dst = 0;
+		return;
+	}
+
 	shift = ULP_BLOB_BYTE - bitoffs - bitlen;
 	if (shift >= 0) {
 		mask = 0xFF >> -bitlen;
@@ -752,6 +772,8 @@ ulp_bs_get_msb(uint8_t *src, uint16_t bitpos, uint8_t bitlen, uint8_t *dst)
  *
  * dst [out] The byte array where data is pulled into
  *
+ * size [in] The size of dst array in bytes
+ *
  * offset [in] The offset where data is pulled
  *
  * len [in] The number of bits to be extracted from the data array
@@ -759,12 +781,22 @@ ulp_bs_get_msb(uint8_t *src, uint16_t bitpos, uint8_t bitlen, uint8_t *dst)
  * returns None.
  */
 static inline void
-ulp_bs_pull_msb(uint8_t *src, uint8_t *dst,
+ulp_bs_pull_msb(uint8_t *src, uint8_t *dst, uint32_t size,
 		uint32_t offset, uint32_t len)
 {
 	uint32_t idx;
 	uint32_t cnt = ULP_BITS_2_BYTE_NR(len);
 
+	/*
+	 * cnt > size means len >= (size + 1) * 8; caller should have
+	 * rejected this. It does not catch size*8 < len < (size + 1)*8,
+	 * which also can't fit in dst: there cnt == size, so idx == size
+	 * after the loop below, which makes "idx < size" false and
+	 * silently drops the remainder instead of writing past dst.
+	 */
+	if (unlikely(cnt > size))
+		return;
+
 	/* iterate bytewise to get data */
 	for (idx = 0; idx < cnt; idx++) {
 		ulp_bs_get_msb(src, offset, ULP_BLOB_BYTE, &dst[idx]);
@@ -772,8 +804,8 @@ ulp_bs_pull_msb(uint8_t *src, uint8_t *dst,
 		len -= ULP_BLOB_BYTE;
 	}
 
-	/* Extract the last reminder data that is not 8 byte boundary */
-	if (len)
+	/* Extract the last remainder data that is not 8 byte boundary */
+	if (len && likely(idx < size))
 		ulp_bs_get_msb(src, offset, len, &dst[idx]);
 }
 
@@ -802,7 +834,7 @@ ulp_blob_pull(struct ulp_blob *blob, uint8_t *data, uint32_t data_size,
 	}
 
 	if (blob->byte_order == BNXT_ULP_BYTE_ORDER_BE)
-		ulp_bs_pull_msb(blob->data, data, offset, len);
+		ulp_bs_pull_msb(blob->data, data, data_size, offset, len);
 	else
 		ulp_bs_pull_lsb(blob->data, data, data_size, offset, len);
 	return 0;
-- 
2.31.1


                 reply	other threads:[~2026-10-05 20:25 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261005202526.17981-1-manish.kurup@broadcom.com \
    --to=manish.kurup@broadcom.com \
    --cc=dev@dpdk.org \
    --cc=kishore.padmanabha@broadcom.com \
    --cc=stable@dpdk.org \
    --cc=xiaozhen.zhong@broadcom.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox