[PATCH v2 1/2] image: Fix 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); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Michal Suchanek msuchanek@suse.de ---
Changes in v2: Use __nonstring instead of suppressing the warning
--- include/image.h | 3 ++- tools/kwbimage.h | 6 ------ 2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/include/image.h b/include/image.h index d7d6a3fe5b..82b447aeb7 100644 --- a/include/image.h +++ b/include/image.h @@ -18,6 +18,7 @@ #include "compiler.h" #include <asm/byteorder.h> #include <stdbool.h> +#include <linux/compiler_attributes.h>
/* Define this to avoid #ifdefs later on */ struct lmb; @@ -275,7 +276,7 @@ struct legacy_img_hdr { uint8_t ih_arch; /* CPU architecture */ uint8_t ih_type; /* Image Type */ uint8_t ih_comp; /* Compression Type */ - uint8_t ih_name[IH_NMLEN]; /* Image Name */ + uint8_t ih_name[IH_NMLEN] __nonstring; /* Image Name */ };
struct image_info { diff --git a/tools/kwbimage.h b/tools/kwbimage.h index 505522332b..327ca34494 100644 --- a/tools/kwbimage.h +++ b/tools/kwbimage.h @@ -11,12 +11,6 @@ #include <compiler.h> #include <stdint.h>
-#ifdef __GNUC__ -#define __packed __attribute((packed)) -#else -#define __packed -#endif - #define KWBIMAGE_MAX_CONFIG ((0x1dc - 0x20)/sizeof(struct reg_config)) #define MAX_TEMPBUF_LEN 32

image_set_name does not terminate the name in the image header.
Then is should not be assumed 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 Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
--- 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 Thu, Oct 13, 2022 at 10:16:56AM +0200, 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); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Suggested-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Signed-off-by: Michal Suchanek msuchanek@suse.de
Changes in v2: Use __nonstring instead of suppressing the warning
include/image.h | 3 ++- tools/kwbimage.h | 6 ------ 2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/include/image.h b/include/image.h index d7d6a3fe5b..82b447aeb7 100644 --- a/include/image.h +++ b/include/image.h @@ -18,6 +18,7 @@ #include "compiler.h" #include <asm/byteorder.h> #include <stdbool.h> +#include <linux/compiler_attributes.h>
/* Define this to avoid #ifdefs later on */ struct lmb; @@ -275,7 +276,7 @@ struct legacy_img_hdr { uint8_t ih_arch; /* CPU architecture */ uint8_t ih_type; /* Image Type */ uint8_t ih_comp; /* Compression Type */
- uint8_t ih_name[IH_NMLEN]; /* Image Name */
- uint8_t ih_name[IH_NMLEN] __nonstring; /* Image Name */
};
struct image_info { diff --git a/tools/kwbimage.h b/tools/kwbimage.h index 505522332b..327ca34494 100644 --- a/tools/kwbimage.h +++ b/tools/kwbimage.h @@ -11,12 +11,6 @@ #include <compiler.h> #include <stdint.h>
-#ifdef __GNUC__ -#define __packed __attribute((packed)) -#else -#define __packed -#endif
#define KWBIMAGE_MAX_CONFIG ((0x1dc - 0x20)/sizeof(struct reg_config)) #define MAX_TEMPBUF_LEN 32
-- 2.37.3

Hi,
On Thu, 13 Oct 2022 at 07:50, Michal Suchánek msuchanek@suse.de wrote:
On Thu, Oct 13, 2022 at 10:16:56AM +0200, 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); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Suggested-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Signed-off-by: Michal Suchanek msuchanek@suse.de
Changes in v2: Use __nonstring instead of suppressing the warning
include/image.h | 3 ++- tools/kwbimage.h | 6 ------ 2 files changed, 2 insertions(+), 7 deletions(-)
+Tom Rini you can drop my one then:
https://patchwork.ozlabs.org/project/uboot/patch/20221013122927.636867-30-sj...
Reviewed-by: Simon Glass sjg@chromium.org

On Thu, Oct 13, 2022 at 10:16:56AM +0200, 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); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Michal Suchanek msuchanek@suse.de Reviewed-by: Simon Glass sjg@chromium.org
Changes in v2: Use __nonstring instead of suppressing the warning
This breaks a ton of boards such as crs305-1g-4s in tools/kwbimage.h.
participants (4)
-
Michal Suchanek
-
Michal Suchánek
-
Simon Glass
-
Tom Rini