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

[Simon Glass]

Hi John,

On Tue, 23 Aug 2022 at 03:46, John Keeping <john at metanate.com> wrote:
>
> 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

That's my understanding too. We are careful to avoid expecting a
terminator. I am not sure what to do with the warning though

Regards,
Simon


>
> > Fixes: b97a2a0a21f2 ("[new uImage] Define a API for image handling operations")
> > Signed-off-by: Joel Stanley <joel at 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
> >