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

[Michael Neuling]

On Mon, 2019-03-18 at 12:41 +1100, Stewart Smith wrote:
> 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>

Tested-by: Michael Neuling <mikey at neuling.org>

> ---
>  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);