diff mbox series

[U-Boot,v1] tools: mtk_image: replace strncpy(d, s, sizeof(d)) with snprintf()

Message ID 20181211144143.2122-1-andriy.shevchenko@linux.intel.com
State Accepted
Commit 74473ed0cf171acaf370b6ab2123b763107d5121
Delegated to: Tom Rini
Headers show
Series [U-Boot,v1] tools: mtk_image: replace strncpy(d, s, sizeof(d)) with snprintf() | expand

Commit Message

Andy Shevchenko Dec. 11, 2018, 2:41 p.m. UTC
Starting from version 8 the GCC, i.e. C compiler, starts complaining about
possible '\0' terminator loss or, as in this case, garbage copy.

In function ‘mtk_image_set_gen_header’,
    inlined from ‘mtk_image_set_header’ at tools/mtk_image.c:733:3:
tools/mtk_image.c:659:2: warning: ‘strncpy’ specified bound 12 equals destination size [-Wstringop-truncation]
  strncpy(hdr->boot.name, bootname, sizeof(hdr->boot.name));
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘mtk_brom_parse_imagename’,
    inlined from ‘mtk_image_check_params’ at tools/mtk_image.c:388:9:
tools/mtk_image.c:325:5: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation]
     strncpy(lk_name, val, sizeof(lk_name));
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Replace it with snprintf() to tell compiler how much room we have in the
destination buffer for source string.

Fixes: 3b975a147c3c ("tools: MediaTek: add MTK boot header generation to mkimage")
Cc: Ryder Lee <ryder.lee@mediatek.com>
Cc: Weijie Gao <weijie.gao@mediatek.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 tools/mtk_image.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Weijie Gao (高惟杰) Dec. 13, 2018, 4:22 a.m. UTC | #1
On Tue, 2018-12-11 at 16:41 +0200, Andy Shevchenko wrote:
> Starting from version 8 the GCC, i.e. C compiler, starts complaining about
> possible '\0' terminator loss or, as in this case, garbage copy.
> 
> In function ‘mtk_image_set_gen_header’,
>     inlined from ‘mtk_image_set_header’ at tools/mtk_image.c:733:3:
> tools/mtk_image.c:659:2: warning: ‘strncpy’ specified bound 12 equals destination size [-Wstringop-truncation]
>   strncpy(hdr->boot.name, bootname, sizeof(hdr->boot.name));
>   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In function ‘mtk_brom_parse_imagename’,
>     inlined from ‘mtk_image_check_params’ at tools/mtk_image.c:388:9:
> tools/mtk_image.c:325:5: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation]
>      strncpy(lk_name, val, sizeof(lk_name));
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Replace it with snprintf() to tell compiler how much room we have in the
> destination buffer for source string.
> 
> Fixes: 3b975a147c3c ("tools: MediaTek: add MTK boot header generation to mkimage")
> Cc: Ryder Lee <ryder.lee@mediatek.com>
> Cc: Weijie Gao <weijie.gao@mediatek.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  tools/mtk_image.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/mtk_image.c b/tools/mtk_image.c
> index 2706d2de32..2ca519483d 100644
> --- a/tools/mtk_image.c
> +++ b/tools/mtk_image.c
> @@ -322,7 +322,7 @@ static int mtk_brom_parse_imagename(const char *imagename)
>  				lk = val;
>  
>  			if (!strcmp(key, "lkname"))
> -				strncpy(lk_name, val, sizeof(lk_name));
> +				snprintf(lk_name, sizeof(lk_name), "%s", val);
>  		}
>  
>  		if (next)
> @@ -656,7 +656,7 @@ static void mtk_image_set_gen_header(void *ptr, off_t filesize,
>  		bootname = SDMMC_BOOT_NAME;
>  
>  	/* Generic device header */
> -	strncpy(hdr->boot.name, bootname, sizeof(hdr->boot.name));
> +	snprintf(hdr->boot.name, sizeof(hdr->boot.name), "%s", bootname);
>  	hdr->boot.version = cpu_to_le32(1);
>  	hdr->boot.size = cpu_to_le32(sizeof(hdr->boot));
>  

Hi Andy,

The snprintf adds a NULL char at the end of the string which gives
only 31 available bytes for the lk name filed. But it's OK for this
because almost no one would like to give a name to fill the whole
field. The same applies to the device header.

Reviewed-by: Weijie Gao <weijie.gao@mediatek.com>

Best Regards,

Weijie
Tom Rini Dec. 17, 2018, 12:11 p.m. UTC | #2
On Tue, Dec 11, 2018 at 04:41:43PM +0200, Andy Shevchenko wrote:

> Starting from version 8 the GCC, i.e. C compiler, starts complaining about
> possible '\0' terminator loss or, as in this case, garbage copy.
> 
> In function ‘mtk_image_set_gen_header’,
>     inlined from ‘mtk_image_set_header’ at tools/mtk_image.c:733:3:
> tools/mtk_image.c:659:2: warning: ‘strncpy’ specified bound 12 equals destination size [-Wstringop-truncation]
>   strncpy(hdr->boot.name, bootname, sizeof(hdr->boot.name));
>   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In function ‘mtk_brom_parse_imagename’,
>     inlined from ‘mtk_image_check_params’ at tools/mtk_image.c:388:9:
> tools/mtk_image.c:325:5: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation]
>      strncpy(lk_name, val, sizeof(lk_name));
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Replace it with snprintf() to tell compiler how much room we have in the
> destination buffer for source string.
> 
> Fixes: 3b975a147c3c ("tools: MediaTek: add MTK boot header generation to mkimage")
> Cc: Ryder Lee <ryder.lee@mediatek.com>
> Cc: Weijie Gao <weijie.gao@mediatek.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Weijie Gao <weijie.gao@mediatek.com>

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/tools/mtk_image.c b/tools/mtk_image.c
index 2706d2de32..2ca519483d 100644
--- a/tools/mtk_image.c
+++ b/tools/mtk_image.c
@@ -322,7 +322,7 @@  static int mtk_brom_parse_imagename(const char *imagename)
 				lk = val;
 
 			if (!strcmp(key, "lkname"))
-				strncpy(lk_name, val, sizeof(lk_name));
+				snprintf(lk_name, sizeof(lk_name), "%s", val);
 		}
 
 		if (next)
@@ -656,7 +656,7 @@  static void mtk_image_set_gen_header(void *ptr, off_t filesize,
 		bootname = SDMMC_BOOT_NAME;
 
 	/* Generic device header */
-	strncpy(hdr->boot.name, bootname, sizeof(hdr->boot.name));
+	snprintf(hdr->boot.name, sizeof(hdr->boot.name), "%s", bootname);
 	hdr->boot.version = cpu_to_le32(1);
 	hdr->boot.size = cpu_to_le32(sizeof(hdr->boot));