[Skiboot] [PATCH] hw/ipmi/test/run-fru: Fix string truncation warning, enhance test

Stewart Smith stewart at linux.ibm.com
Mon Mar 18 12:41:52 AEDT 2019


We've been getting this warning/error from recent GCC:

In file included from hw/ipmi/test/run-fru.c:22:
hw/ipmi/test/../ipmi-fru.c: In function ‘fru_add’:
hw/ipmi/test/../ipmi-fru.c:162:3: warning: ‘strncpy’ output truncated copying 32 bytes from a string of length 38 [-Wstringop-truncation]
   strncpy(info.version, version, MAX_STR_LEN + 1);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This patch does two things:
1) Re-arrange some code to shut GCC up.
2) Add extra fu to tests to ensure we're producing correct bytes.

Signed-off-by: Stewart Smith <stewart at linux.ibm.com>
---
 hw/ipmi/ipmi-fru.c     | 16 +++++++++-------
 hw/ipmi/test/run-fru.c | 11 ++++++++++-
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/hw/ipmi/ipmi-fru.c b/hw/ipmi/ipmi-fru.c
index ddcb3d6f8bc1..7e7f0a68c301 100644
--- a/hw/ipmi/ipmi-fru.c
+++ b/hw/ipmi/ipmi-fru.c
@@ -129,8 +129,8 @@ static int fru_fill_product_info(u8 *buf, struct product_info *info, size_t size
 static int fru_add(u8 *buf, int size)
 {
 	int len;
-	char short_version[MAX_STR_LEN + 1];
 	struct common_header common_hdr;
+	char *short_version;
 	struct product_info info = {
 		.manufacturer = (char *) "IBM",
 		.product = (char *) "skiboot",
@@ -155,17 +155,19 @@ static int fru_add(u8 *buf, int size)
 	common_hdr.checksum = fru_checksum((u8 *) &common_hdr, sizeof(common_hdr) - 1);
 	memcpy(buf, &common_hdr, sizeof(common_hdr));
 
+	short_version = strdup(version);
 	info.version = short_version;
 	if (!strncmp(version, "skiboot-", 8))
-		strncpy(info.version, &version[8], MAX_STR_LEN + 1);
-	else
-		strncpy(info.version, version, MAX_STR_LEN + 1);
+		info.version = &short_version[8];
 
-	if (info.version[MAX_STR_LEN] != '\0')
-		info.version[MAX_STR_LEN - 1] = '+';
-	info.version[MAX_STR_LEN] = '\0';
+	if (strlen(info.version) >= MAX_STR_LEN) {
+		if (info.version[MAX_STR_LEN] != '\0')
+			info.version[MAX_STR_LEN - 1] = '+';
+		info.version[MAX_STR_LEN] = '\0';
+	}
 
 	len = fru_fill_product_info(&buf[64], &info, size - 64);
+	free(short_version);
 	if (len < 0)
 		return OPAL_PARAMETER;
 
diff --git a/hw/ipmi/test/run-fru.c b/hw/ipmi/test/run-fru.c
index 9f65ed459a3b..ff8df3912648 100644
--- a/hw/ipmi/test/run-fru.c
+++ b/hw/ipmi/test/run-fru.c
@@ -21,6 +21,8 @@
 
 #include "../ipmi-fru.c"
 
+#include <string.h>
+
 int error = 0;
 
 const char version[] = "a-too-long-version-test-string-is-here";
@@ -88,7 +90,10 @@ int main(void)
 	buf = malloc(256);
 
 	len = fru_fill_product_info(buf, &info, 40);
-	assert(len > 0);
+	assert(len == 40);
+	assert(memcmp(buf, "\001\005\000\303IBM\307skiboot\305hello"
+		      "\30512345\30512345\304abcd\301-",len) == 0);
+
 
 	/* Make sure the checksum is right */
 	assert(!fru_checksum(buf, len));
@@ -106,6 +111,10 @@ int main(void)
 
 	memset(buf, 0, 256);
 	assert(fru_add(buf, 256) > 0);
+	assert(0 == memcmp(&buf[64], "\001\a\000\303IBM\307skiboot\300"
+			   "\337a-too-long-version-test-string+\300\300\301"
+			   "\0\0\0",54));
+
 
 	memset(buf, 0, 256);
 	assert(fru_add(buf, 1) == OPAL_PARAMETER);
-- 
2.20.1



More information about the Skiboot mailing list