Skip to contentSkip to conversation

net/ixgbe/base: fix overflow in ACI debug dump

RFC New Stephen Hemminger 1 participant 1 message Sep 4 Network drivers

lore
Share & tools
lore

Stephen Hemminger

author
#0 lore raw

RFC 1/1 net/ixgbe/base: fix overflow in ACI debug dump

If doing ASAN build GCC warns:
ixgbe_e610.c:59:41: warning: 'strncat' output may be truncated copying between 43 and 118 bytes from a string of length 127

The truncation is bogus, debug_portion is always 5 characters. Rather than fighting with GCC rewrite the loop to use a single snprintf() at a running offset.

I know this is base/ code but even upstream code needs to be able to build clean with all compiler options.

Fixes 12648d0a5afc ("net/ixgbe/base: add admin interface debug printouts")
Cc [email protected]
Signed-off-by Stephen Hemminger <[email protected]>
---
 drivers/net/intel/ixgbe/base/ixgbe_e610.c | 57 ++++++++++-------------
 1 file changed, 24 insertions(+), 33 deletions(-)
drivers/net/intel/ixgbe/base/ixgbe_e610.c +24 -33
index 06a69d1d62..4e2dac989f 100644
@@ -42,41 +42,32 @@ void ixgbe_shutdown_aci(struct ixgbe_hw *hw)
 STATIC void ixgbe_aci_debug_array(struct ixgbe_hw *hw, u16 row_size, u8 *buf,
 				  u16 buf_size)
 {
-	char debug_portion[IXGBE_ACI_MAX_DEBUG_STRING_LENGTH] = {'\0'};
-	char debug_string[IXGBE_ACI_MAX_DEBUG_STRING_LENGTH] = {'\0'};
-	u16 i = 0, j = 0;
-	s16 remaining_space = IXGBE_ACI_MAX_DEBUG_STRING_LENGTH - 1;
-	s16 nbytes = 0;
-	if (!hw)
+	char debug_string[IXGBE_ACI_MAX_DEBUG_STRING_LENGTH];
+	u16 i, j, row;
+	int offset, nbytes;
+
+	if (!hw || !buf || !buf_size || !row_size)
 		return;
-	if (buf && buf_size && row_size) {
-		if (buf_size >= row_size) {
-			for (i = 0; i < (buf_size - row_size); i += row_size) {
-				nbytes = snprintf(debug_string, sizeof(debug_string), "0x%04X : ", i);
-				remaining_space = IXGBE_ACI_MAX_DEBUG_STRING_LENGTH - nbytes - 1;
-				for (j = 0; j < row_size; j++) {
-					nbytes = snprintf(debug_portion, sizeof(debug_portion), "0x%02X ", buf[i + j]);
-					strncat(debug_string, debug_portion, remaining_space);
-					remaining_space -= nbytes;
-					if (remaining_space <= 0) break;
-				}
-				strncat(debug_string, "\n", remaining_space);
-				DEBUGOUT1("%s", debug_string);
-				memset(debug_string, 0, IXGBE_ACI_MAX_DEBUG_STRING_LENGTH);
-			}
-		}
-		if (i < buf_size) {
-			nbytes = snprintf(debug_string, sizeof(debug_string), "0x%04X : ", i);
-			remaining_space = IXGBE_ACI_MAX_DEBUG_STRING_LENGTH - nbytes - 1;
-			for (j = 0; j < (buf_size - i); j++) {
-				nbytes = snprintf(debug_portion, sizeof(debug_portion), "0x%02X ", buf[i + j]);
-				strncat(debug_string, debug_portion, remaining_space);
-				remaining_space -= nbytes;
-				if (remaining_space <= 0) break;
-			}
-			strncat(debug_string, "\n", remaining_space);
-			DEBUGOUT1("%s", debug_string);
+
+	for (i = 0; i < buf_size; i += row_size) {
+		row = buf_size - i;
+		if (row > row_size)
+			row = row_size;
+
+		offset = snprintf(debug_string, sizeof(debug_string),
+				  "0x%04X : ", i);
+
+		for (j = 0; j < row; j++) {
+			nbytes = snprintf(debug_string + offset,
+					  sizeof(debug_string) - offset,
+					  "0x%02X ", buf[i + j]);
+			if (nbytes < 0 ||
+			    (size_t)nbytes >= sizeof(debug_string) - offset)
+				break;
+			offset += nbytes;
 		}
+
+		DEBUGOUT1("%s\n", debug_string);
 	}
 }
Signature · 1 line
2.53.0