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

[Peter Maydell]

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.

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.

> Reported-by: Kurtis Miller <address@hidden>
> Signed-off-by: Prasad J Pandit <address@hidden>
> ---
>  device_tree.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/device_tree.c b/device_tree.c
> index 296278e12a..9059ee5545 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -79,9 +79,9 @@ void *load_device_tree(const char *filename_path, int 
> *sizep)
>
>      *sizep = 0;
>      dt_size = get_image_size(filename_path);
> -    if (dt_size < 0) {
> -        error_report("Unable to get size of device tree file '%s'",
> -                     filename_path);
> +    if (dt_size < 0 || dt_size > FDT_MAX_SIZE) {
> +        error_report("Invalid size of device tree file: %s: %d",
> +                     filename_path, dt_size);
>          goto fail;
>      }

thanks
-- PMM