diff mbox

[v2,2/4] hw/arm/boot: pass an address limit to and return size from load_dtb()

Message ID 1410346790-31743-3-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show

Commit Message

Ard Biesheuvel Sept. 10, 2014, 10:59 a.m. UTC
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.biesheuvel@linaro.org>
---
 hw/arm/boot.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Comments

Peter Maydell Sept. 11, 2014, 11:05 a.m. UTC | #1
On 10 September 2014 11:59, Ard Biesheuvel <ard.biesheuvel@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.biesheuvel@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
diff mbox

Patch

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)
 {
     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
+         * 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;