[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