[PATCH] mkimage: clarify error message for empty input files

[PATCH] mkimage: clarify error message for empty input files

[Thomas Hebb]

Currently, an empty imput file causes `mmap()` to fail, and you get an
error like "mkimage: Can't read file.img: Invalid argument", which is
extremely unintuitive and hard to diagnose if you don't know what to
look for. Add an explicit check for an empty file and provide a clear
error message instead.

We already bounds check the image size when listing and re-signing
existing images, so we only need this check here, when opening data
files going into a image.

Signed-off-by: Thomas Hebb <tommyhebb@gmail.com>

---

 tools/mkimage.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/mkimage.c b/tools/mkimage.c
index 302bfcf971..fbe883ce36 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -732,6 +732,12 @@ copy_file (int ifd, const char *datafile, int pad)
 		exit (EXIT_FAILURE);
 	}
 
+	if (sbuf.st_size == 0) {
+		fprintf (stderr, "%s: Input file %s is empty, bailing out\n",
+			params.cmdname, datafile);
+		exit (EXIT_FAILURE);
+	}
+
 	ptr = mmap(0, sbuf.st_size, PROT_READ, MAP_SHARED, dfd, 0);
 	if (ptr == MAP_FAILED) {
 		fprintf (stderr, "%s: Can't read %s: %s\n",
-- 
2.32.0

Re: [PATCH] mkimage: clarify error message for empty input files

[Tom Hebb]

On Sun, Aug 1, 2021 at 3:23 PM Thomas Hebb <tommyhebb@gmail.com> wrote:
>
> Currently, an empty imput file causes `mmap()` to fail, and you get an
Whoops, s/imput/input/. Didn't catch this before sending it out.

> error like "mkimage: Can't read file.img: Invalid argument", which is
> extremely unintuitive and hard to diagnose if you don't know what to
> look for. Add an explicit check for an empty file and provide a clear
> error message instead.
>
> We already bounds check the image size when listing and re-signing
> existing images, so we only need this check here, when opening data
> files going into a image.
>
> Signed-off-by: Thomas Hebb <tommyhebb@gmail.com>
>
> ---
>
>  tools/mkimage.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index 302bfcf971..fbe883ce36 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -732,6 +732,12 @@ copy_file (int ifd, const char *datafile, int pad)
>                 exit (EXIT_FAILURE);
>         }
>
> +       if (sbuf.st_size == 0) {
> +               fprintf (stderr, "%s: Input file %s is empty, bailing out\n",
> +                       params.cmdname, datafile);
> +               exit (EXIT_FAILURE);
> +       }
> +
>         ptr = mmap(0, sbuf.st_size, PROT_READ, MAP_SHARED, dfd, 0);
>         if (ptr == MAP_FAILED) {
>                 fprintf (stderr, "%s: Can't read %s: %s\n",
> --
> 2.32.0
>

Re: [PATCH] mkimage: clarify error message for empty input files

[Simon Glass]

On Sun, 1 Aug 2021 at 16:23, Thomas Hebb <tommyhebb@gmail.com> wrote:
>
> Currently, an empty imput file causes `mmap()` to fail, and you get an
> error like "mkimage: Can't read file.img: Invalid argument", which is
> extremely unintuitive and hard to diagnose if you don't know what to
> look for. Add an explicit check for an empty file and provide a clear
> error message instead.
>
> We already bounds check the image size when listing and re-signing
> existing images, so we only need this check here, when opening data
> files going into a image.
>
> Signed-off-by: Thomas Hebb <tommyhebb@gmail.com>
>
> ---
>
>  tools/mkimage.c | 6 ++++++
>  1 file changed, 6 insertions(+)

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

(s/imput/input/ as you note)

Re: [PATCH] mkimage: clarify error message for empty input files

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 706 bytes --]

On Sun, Aug 01, 2021 at 03:23:13PM -0700, Thomas Hebb wrote:

> Currently, an empty imput file causes `mmap()` to fail, and you get an
> error like "mkimage: Can't read file.img: Invalid argument", which is
> extremely unintuitive and hard to diagnose if you don't know what to
> look for. Add an explicit check for an empty file and provide a clear
> error message instead.
> 
> We already bounds check the image size when listing and re-signing
> existing images, so we only need this check here, when opening data
> files going into a image.
> 
> Signed-off-by: Thomas Hebb <tommyhebb@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]