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

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

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;

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

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.d...
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

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.d...
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
participants (4)
-
Michal Suchanek
-
Michal Suchánek
-
Rasmus Villemoes
-
Simon Glass