On Wed, Jan 11, 2012 at 07:19:00AM +0100, Stefan Weil wrote:
> Am 11.01.2012 06:44, schrieb David Gibson:
> >From: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> >
> >load_image_targphys() gets passed a max size for the file, but
> >doesn't enforce it at all. Add a check and return -1 (error) if
> >the file is too big, without loading it.
> >
> >Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> >Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> >---
> >hw/loader.c | 2 ++
> >1 files changed, 2 insertions(+), 0 deletions(-)
> >
> >diff --git a/hw/loader.c b/hw/loader.c
> >index 446b628..7ad9e22 100644
> >--- a/hw/loader.c
> >+++ b/hw/loader.c
> >@@ -108,6 +108,8 @@ int load_image_targphys(const char *filename,
> >int size;
> >
> >size = get_image_size(filename);
> >+ if (size > max_sz)
> >+ return -1;
> >if (size > 0)
> >rom_add_file_fixed(filename, addr, -1);
> >return size;
>
> Even if this file is full of block statements without braces,
> we should not add more of them. See CODING_STYLE and
> scripts/checkpatch.pl.
I know the coding style, I thought the counterexample right next to it
would take precedence. Corrected in a respin.
> There remains an additional problem:
> Using 'int' for the size of files was sufficient 10 years ago,
> but it is that no longer. get_image_size() silently reduced the
> return value from lseek() to an 'int' value. So even with your
> patch, very large files will be loaded (partially)!
Well, sure, but that's an independent bug. I'll fix it later if I get
to it, but that kind of leads to changing the types of a whole bunch
of things.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
