[U-Boot] [PATCH 1/4] tools: imx8image: check lseek return value
Check lseek return value.
Fix Coverity CID: 184236 184235 184232
Signed-off-by: Peng Fan peng.fan@nxp.com --- tools/imx8image.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/tools/imx8image.c b/tools/imx8image.c index e6b0a146b6..eb039b583d 100644 --- a/tools/imx8image.c +++ b/tools/imx8image.c @@ -333,7 +333,12 @@ static void copy_file_aligned(int ifd, const char *datafile, int offset, }
size = sbuf.st_size; - lseek(ifd, offset, SEEK_SET); + if (lseek(ifd, offset, SEEK_SET) == -1) { + fprintf(stderr, "%s: lseek error %s\n", + __func__, strerror(errno)); + exit(EXIT_FAILURE); + } + if (write(ifd, ptr, size) != size) { fprintf(stderr, "Write error %s\n", strerror(errno)); exit(EXIT_FAILURE); @@ -387,7 +392,12 @@ static void copy_file (int ifd, const char *datafile, int pad, int offset) }
size = sbuf.st_size; - lseek(ifd, offset, SEEK_SET); + if (lseek(ifd, offset, SEEK_SET) == -1) { + fprintf(stderr, "%s: lseek error %s\n", + __func__, strerror(errno)); + exit(EXIT_FAILURE); + } + if (write(ifd, ptr, size) != size) { fprintf(stderr, "Write error %s\n", strerror(errno)); @@ -883,7 +893,11 @@ static int build_container(soc_type_t soc, uint32_t sector_size, } while (img_sp->option != NO_IMG);
/* Add padding or skip appended container */ - lseek(ofd, file_padding, SEEK_SET); + if (lseek(ofd, file_padding, SEEK_SET) == -1) { + fprintf(stderr, "%s: lseek error %s\n", + __func__, strerror(errno)); + exit(EXIT_FAILURE); + }
/* Note: Image offset are not contained in the image */ tmp = flatten_container_header(&imx_header, container + 1, &size,
Fix: CID 184234: (TAINTED_SCALAR) Using tainted variable "header.num_images - 1" as an index into an array "header.img".
Signed-off-by: Peng Fan peng.fan@nxp.com --- tools/imx8image.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/imx8image.c b/tools/imx8image.c index eb039b583d..06b72ea989 100644 --- a/tools/imx8image.c +++ b/tools/imx8image.c @@ -663,8 +663,10 @@ static int get_container_image_start_pos(image_t *image_stack, uint32_t align) }
ret = fread(&header, sizeof(header), 1, fd); - if (ret != 1) + if (ret != 1) { printf("Failure Read header %d\n", ret); + exit(EXIT_FAILURE); + }
fclose(fd);
Fix: CID 184233: (NEGATIVE_RETURNS) Using variable "container" as an index to array "imx_header.fhdr".
Signed-off-by: Peng Fan peng.fan@nxp.com --- tools/imx8image.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/tools/imx8image.c b/tools/imx8image.c index 06b72ea989..f3d1658f11 100644 --- a/tools/imx8image.c +++ b/tools/imx8image.c @@ -808,6 +808,10 @@ static int build_container(soc_type_t soc, uint32_t sector_size, case SCFW: case DATA: case MSG_BLOCK: + if (container < 0) { + fprintf(stderr, "No container found\n"); + exit(EXIT_FAILURE); + } check_file(&sbuf, img_sp->filename); tmp_filename = img_sp->filename; set_image_array_entry(&imx_header.fhdr[container], @@ -821,6 +825,10 @@ static int build_container(soc_type_t soc, uint32_t sector_size, break;
case SECO: + if (container < 0) { + fprintf(stderr, "No container found\n"); + exit(EXIT_FAILURE); + } check_file(&sbuf, img_sp->filename); tmp_filename = img_sp->filename; set_image_array_entry(&imx_header.fhdr[container],
If there is no CONTAINER entry, there is no need to flatten container header.
Signed-off-by: Peng Fan peng.fan@nxp.com --- tools/imx8image.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/tools/imx8image.c b/tools/imx8image.c index f3d1658f11..6ed5781979 100644 --- a/tools/imx8image.c +++ b/tools/imx8image.c @@ -909,17 +909,19 @@ static int build_container(soc_type_t soc, uint32_t sector_size, exit(EXIT_FAILURE); }
- /* Note: Image offset are not contained in the image */ - tmp = flatten_container_header(&imx_header, container + 1, &size, - file_padding); - /* Write image header */ - if (write(ofd, tmp, size) != size) { - fprintf(stderr, "error writing image hdr\n"); - exit(EXIT_FAILURE); - } + if (container >= 0) { + /* Note: Image offset are not contained in the image */ + tmp = flatten_container_header(&imx_header, container + 1, + &size, file_padding); + /* Write image header */ + if (write(ofd, tmp, size) != size) { + fprintf(stderr, "error writing image hdr\n"); + exit(EXIT_FAILURE); + }
- /* Clean-up memory used by the headers */ - free(tmp); + /* Clean-up memory used by the headers */ + free(tmp); + }
/* * step through the image stack again this time copying
Hi Peng,
On Thu, Nov 1, 2018 at 3:42 AM Peng Fan peng.fan@nxp.com wrote:
size = sbuf.st_size;
lseek(ifd, offset, SEEK_SET);
if (lseek(ifd, offset, SEEK_SET) == -1) {fprintf(stderr, "%s: lseek error %s\n",__func__, strerror(errno));exit(EXIT_FAILURE);}
What about something like this instead?
ret = lseek(ifd, offset, SEEK_SET); if (ret < 0) { fprintf(stderr, "%s: lseek error: %d\n", __func__, ret); exit(EXIT_FAILURE); }
Hi Fabio,
-----Original Message----- From: Fabio Estevam [mailto:festevam@gmail.com] Sent: 2018年11月1日 18:18 To: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de; Fabio Estevam fabio.estevam@nxp.com; U-Boot-Denx u-boot@lists.denx.de; dl-linux-imx linux-imx@nxp.com Subject: Re: [U-Boot] [PATCH 1/4] tools: imx8image: check lseek return value Hi Peng,
On Thu, Nov 1, 2018 at 3:42 AM Peng Fan peng.fan@nxp.com wrote:
size = sbuf.st_size;
lseek(ifd, offset, SEEK_SET);
if (lseek(ifd, offset, SEEK_SET) == -1) {fprintf(stderr, "%s: lseek error %s\n",__func__, strerror(errno));exit(EXIT_FAILURE);}What about something like this instead?
ret = lseek(ifd, offset, SEEK_SET); if (ret < 0) { fprintf(stderr, "%s: lseek error: %d\n", __func__, ret); exit(EXIT_FAILURE); }
lseek only returns -1 when error. Is the coding style you proposed is preferred than what this patch has? I saw fit_image use if (xxx < 0) and fw_env.c use if (xxx == -1), there are also others use ret = lseek().
Thanks, Peng.
Hi Peng,
On Thu, Nov 1, 2018 at 9:15 AM Peng Fan peng.fan@nxp.com wrote:
lseek only returns -1 when error. Is the coding style you proposed is preferred than what this patch has?
In case lseek changes someday to return something else, checking for (ret < 0) will still work, so it is a more robust error handling.
Hi Fabio,
-----Original Message----- From: Fabio Estevam [mailto:festevam@gmail.com] Sent: 2018年11月1日 20:18 To: Peng Fan peng.fan@nxp.com Cc: Stefano Babic sbabic@denx.de; Fabio Estevam fabio.estevam@nxp.com; U-Boot-Denx u-boot@lists.denx.de; dl-linux-imx linux-imx@nxp.com Subject: Re: [U-Boot] [PATCH 1/4] tools: imx8image: check lseek return value
Hi Peng,
On Thu, Nov 1, 2018 at 9:15 AM Peng Fan peng.fan@nxp.com wrote:
lseek only returns -1 when error. Is the coding style you proposed is preferred than what this patch has?
In case lseek changes someday to return something else, checking for (ret < 0) will still work, so it is a more robust error handling.
lseek prototype, return value, in the spec http://pubs.opengroup.org/onlinepubs/9699919799/ Understand (ret < 0) is a good practice, but I do not think lseek return value will change.
Thanks, Peng.
participants (2)
-
Fabio Estevam -
Peng Fan