qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2] loader: Fix misaligned member access


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH v2] loader: Fix misaligned member access
Date: Mon, 23 Apr 2018 16:13:33 +0100

On 23 April 2018 at 15:14, Philippe Mathieu-Daudé <address@hidden> wrote:
> This fixes the following ASan warning:
>
>   $ mips64el-softmmu/qemu-system-mips64el -M boston -kernel vmlinux.gz.itb 
> -nographic
>   hw/core/loader-fit.c:108:17: runtime error: load of misaligned address 
> 0x7f95cd7e4264 for type 'fdt64_t', which requires 8 byte alignment
>   0x7f95cd7e4264: note: pointer points here
>     00 00 00 3e ff ff ff ff  80 7d 2a c0 00 00 00 01  68 61 73 68 40 30 00 00 
>  00 00 00 03 00 00 00 14
>                 ^
>
> Reported-by: AddressSanitizer
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
> v2: do not change the 32-bit access, add comments (David Gibson)
>
>  hw/core/loader-fit.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/loader-fit.c b/hw/core/loader-fit.c
> index 0c4a7207f4..c1c7c9c119 100644
> --- a/hw/core/loader-fit.c
> +++ b/hw/core/loader-fit.c
> @@ -94,6 +94,7 @@ static int fit_image_addr(const void *itb, int img, const 
> char *name,
>  {
>      const void *prop;
>      int len;
> +    fdt64_t fdt64;
>
>      prop = fdt_getprop(itb, img, name, &len);
>      if (!prop) {
> @@ -102,10 +103,18 @@ static int fit_image_addr(const void *itb, int img, 
> const char *name,
>
>      switch (len) {
>      case 4:
> +        /* Assuming the base of the fdt is aligned, then fdt_getprop()
> +         * returns 32-bit aligned properties, so this load is guaranteed
> +         * to be 32-bit aligned.
> +         */
>          *addr = fdt32_to_cpu(*(fdt32_t *)prop);
>          return 0;
>      case 8:
> -        *addr = fdt64_to_cpu(*(fdt64_t *)prop);
> +        /* Since the property is not guaranteed to be 64-bit aligned,
> +         * use the stack to avoid an unaligned load.
> +         */
> +        memcpy(&fdt64, prop, sizeof(fdt64));
> +        *addr = fdt64_to_cpu(fdt64);
>          return 0;
>      default:

No, don't use memcpy. This is what ldq_p() is for.

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]