Re: [Qemu-devel] [PATCH 1/4] load_image_targphys() should enforce the max size

[David Gibson <david@gibson.dropbear.id.au>]

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 <benh@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 <benh@kernel.crashing.org>
> >Signed-off-by: David Gibson <david@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