Re: [Qemu-devel] [PATCH] device_tree: check device tree blob file size

[David Gibson]

On Fri, Mar 22, 2019 at 09:14:53AM +0000, Peter Maydell wrote:
> On Fri, 22 Mar 2019 at 07:38, P J P <address@hidden> wrote:
> >
> > From: Prasad J Pandit <address@hidden>
> >
> > Device tree blob(dtb) file can not be larger than 2MB in size.[*]
> > Add check to avoid loading large dtb files in load_device_tree(),
> > and potential integer(dt_size) overflow.
> >
> > [*] linux.git/tree/Documentation/arm64/booting.txt
> 
> This document is specific to aarch64, but the part of
> QEMU's device tree code being modified here is
> architecture independent.
> 
> Cc'ing David Gibson who will probably know if there is
> an architecture-independent limit on DTB size we should
> be enforcing, or whether we are better just to have a check
> that avoids the overflow.

The only inherent limit to dtb size should be 2^31-1 bytes (the format
uses signed 32-bit ints as offsets).

Indeed there shouldn't be any architecture (as in instruction set)
dependent limits either.  There may however be more specific platform
dependent limits.

> It's also worth noting in the commit message that this is
> not a security problem -- even if the "add 10000 and double"
> calculation overflows, the load_image_size() function will
> not load more data into the buffer than will fit, so the
> behaviour will be to truncate the DTB.

Yeah, you should probably make that hard error rather than truncating.
If a system works with a truncated tree, it can only be by sheer
accident.

-- 
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

signature.asc
Description: PGP signature