[PATCH 1/2] image: Suppress string truncation warning

[PATCH 1/2] image: Suppress string truncation warning

[Michal Suchanek]

In file included from ../tools/imagetool.h:24,
                 from ../tools/default_image.c:16:
In function ‘image_set_name’,
    inlined from ‘image_set_header’ at ../tools/default_image.c:133:2:
../include/image.h:786:9: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation]
  786 |         strncpy(image_get_name(hdr), name, IH_NMLEN);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

There is no standard function that can be used to make the copy without
a warning.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---

 include/image.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/image.h b/include/image.h
index d7d6a3fe5b..e08ef6ed55 100644
--- a/include/image.h
+++ b/include/image.h
@@ -783,7 +783,14 @@ image_set_hdr_b(comp)		/* image_set_comp */
 
 static inline void image_set_name(struct legacy_img_hdr *hdr, const char *name)
 {
+#if defined(__GNUC__) && (__GNUC__ > 7)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wstringop-truncation"
+#endif
 	strncpy(image_get_name(hdr), name, IH_NMLEN);
+#if defined(__GNUC__) && (__GNUC__ > 7)
+#pragma GCC diagnostic pop
+#endif
 }
 
 int image_check_hcrc(const struct legacy_img_hdr *hdr);
-- 
2.37.3

[PATCH 2/2] spl: spl_legacy: Fix printing image name

[Michal Suchanek]

image_set_name does not terminate the name in the image header.

Then is should not be assument it's nul terminated. image_print_contents
correctly print only IH_NMLEN characters.

Fix printing the unterminated field in spl_parse_legacy_header. The
format specifies the minimum printed length, not maximum.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---

 common/spl/spl_legacy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c
index b3624dfbb7..11c459254b 100644
--- a/common/spl/spl_legacy.c
+++ b/common/spl/spl_legacy.c
@@ -55,7 +55,7 @@ int spl_parse_legacy_header(struct spl_image_info *spl_image,
 	spl_image->os = image_get_os(header);
 	spl_image->name = image_get_name(header);
 	debug(SPL_TPL_PROMPT
-	      "payload image: %32s load addr: 0x%lx size: %d\n",
+	      "payload image: %.*s load addr: 0x%lx size: %d\n", IH_NMLEN,
 	      spl_image->name, spl_image->load_addr, spl_image->size);
 
 	return 0;
-- 
2.37.3

Re: [PATCH 2/2] spl: spl_legacy: Fix printing image name

[Simon Glass]

On Wed, 12 Oct 2022 at 13:47, Michal Suchanek <msuchanek@suse.de> wrote:
>
> image_set_name does not terminate the name in the image header.
>
> Then is should not be assument it's nul terminated. image_print_contents
> correctly print only IH_NMLEN characters.
>
> Fix printing the unterminated field in spl_parse_legacy_header. The
> format specifies the minimum printed length, not maximum.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>
>  common/spl/spl_legacy.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 1/2] image: Suppress string truncation warning

[Rasmus Villemoes]

On 12/10/2022 21.47, Michal Suchanek wrote:
> In file included from ../tools/imagetool.h:24,
>                  from ../tools/default_image.c:16:
> In function ‘image_set_name’,
>     inlined from ‘image_set_header’ at ../tools/default_image.c:133:2:
> ../include/image.h:786:9: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation]
>   786 |         strncpy(image_get_name(hdr), name, IH_NMLEN);
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> There is no standard function that can be used to make the copy without
> a warning.

True, but the compiler does give a way to inform that the destination is
not _supposed_ to be a nul-terminated string.

https://lore.kernel.org/u-boot/dad17a9f-d823-1e8b-3381-53961294521c@prevas.dk/

And our include/linux/compiler_attributes.h indeed already exposes that
__nonstring attribute. Perhaps try applying that to the ih_name member.

It may also be necessary to drop the image_get_name() indirection and
just use hdr->ih_name directly; though gcc obviously sees through it to
account for the size of the destination buffer (otherwise it couldn't
warn), I'm not sure it won't lose that attribute.

Rasmus

Re: [PATCH 1/2] image: Suppress string truncation warning

[Michal Suchánek]

On Thu, Oct 13, 2022 at 09:29:22AM +0200, Rasmus Villemoes wrote:
> On 12/10/2022 21.47, Michal Suchanek wrote:
> > In file included from ../tools/imagetool.h:24,
> >                  from ../tools/default_image.c:16:
> > In function ‘image_set_name’,
> >     inlined from ‘image_set_header’ at ../tools/default_image.c:133:2:
> > ../include/image.h:786:9: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation]
> >   786 |         strncpy(image_get_name(hdr), name, IH_NMLEN);
> >       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> > There is no standard function that can be used to make the copy without
> > a warning.
> 
> True, but the compiler does give a way to inform that the destination is
> not _supposed_ to be a nul-terminated string.
> 
> https://lore.kernel.org/u-boot/dad17a9f-d823-1e8b-3381-53961294521c@prevas.dk/
> 
> And our include/linux/compiler_attributes.h indeed already exposes that
> __nonstring attribute. Perhaps try applying that to the ih_name member.

That's better, that's an actual fix.

Thanks

Michal