| Message ID | 7c0f1ff7f8f77dd94cd74e7907b488d3a0a1e492.1627856591.git.tommyhebb@gmail.com |
|---|---|
| State | Accepted |
| Commit | eaa6442e4f4883418f595127b5bb12355ca9817d |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | mkimage: clarify error message for empty input files | expand |
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 >
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)
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!
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",
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(+)