On 10 September 2014 11:59, Ard Biesheuvel <ard.biesheu...@linaro.org> wrote:
> Add an address limit input parameter to load_dtb() so that we can
> tell it how much memory the dtb is allowed to consume. If the dtb
> doesn't fit, return 0, otherwise return the actual size of the
> loaded dtb, or -1 on error.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> ---
> hw/arm/boot.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 50eca931e1a4..014fab347b09 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -312,7 +312,8 @@ static void set_kernel_args_old(const struct
> arm_boot_info *info)
> }
> }
>
> -static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
> +static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
> + hwaddr addr_limit)
> {
This patch looks good, but since you're going to respin the series
anyway we could use a doc comment before the function describing the
semantics of the return value. If you do that you can
add my Reviewed-by: tag.
> void *fdt = NULL;
> int size, rc;
> @@ -341,6 +342,15 @@ static int load_dtb(hwaddr addr, const struct
> arm_boot_info *binfo)
> }
> }
>
> + if (addr_limit > addr && size > (addr_limit - addr)) {
> + /* We have been given a non-zero address limit and we have exceeded
> + * it. Whether this is constitues a failure is up to the caller to
"constitutes"
> + * decide, so just return 0 as size, i.e., no error.
> + */
> + g_free(fdt);
> + return 0;
> + }
> +
> acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells");
> scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells");
> if (acells == 0 || scells == 0) {
> @@ -403,7 +413,7 @@ static int load_dtb(hwaddr addr, const struct
> arm_boot_info *binfo)
>
> g_free(fdt);
>
> - return 0;
> + return size;
>
> fail:
> g_free(fdt);
> @@ -572,7 +582,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
> *info)
> */
> hwaddr dtb_start = QEMU_ALIGN_UP(info->initrd_start +
> initrd_size,
> 4096);
> - if (load_dtb(dtb_start, info)) {
> + if (load_dtb(dtb_start, info, 0) < 0) {
> exit(1);
> }
> fixupcontext[FIXUP_ARGPTR] = dtb_start;
-- PMM
