Re: [PATCH] image: Ensure image header name is null terminated

[John Keeping <john@metanate.com>]

On Tue, Aug 23, 2022 at 03:59:07PM +1000, Joel Stanley wrote:
> When building with GCC 12:
> 
> ../include/image.h:779:9: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation]
>   779 |         strncpy(image_get_name(hdr), name, IH_NMLEN);
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Ensure the copied string is null terminated by always setting the final
> byte to 0. Shorten the strncpy to IH_NMLEN-1 as we will always overwrite
> the last byte.
> 
> We can't use strlcpy as this is code is built on the host as well as the
> target.

Since this is in the header, isn't the point that it doesn't need to be
null-terminated?

When printing we're careful to use:

	"%.*s", IH_NMLEN, ...

so I think the warning is wrong here - we want both of the strncpy()
behaviours that are normally considered strange:

- it's okay not to null terminate as this is an explicitly sized field

- we want to pad the whole field with zeroes if the string is short

> Fixes: b97a2a0a21f2 ("[new uImage] Define a API for image handling operations")
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  include/image.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/image.h b/include/image.h
> index e4c6a50b885f..665b2278b7fb 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -776,7 +776,10 @@ image_set_hdr_b(comp)		/* image_set_comp */
>  
>  static inline void image_set_name(image_header_t *hdr, const char *name)
>  {
> -	strncpy(image_get_name(hdr), name, IH_NMLEN);
> +	char *hdr_name = image_get_name(hdr);
> +
> +	strncpy(hdr_name, name, IH_NMLEN - 1);
> +	hdr_name[IH_NMLEN - 1] = '\0';
>  }
>  
>  int image_check_hcrc(const image_header_t *hdr);
> -- 
> 2.35.1
>