On Tue, Apr 24, 2018 at 07:21:03PM -0300, Philippe Mathieu-Daudé wrote: > The libfdt does not guarantee than fdt_getprop() returns a pointer > aligned to the property size. > > Assuming the base of the fdt is aligned, a 32-bit property returns > a 32-bit aligned pointer. This is however not guaranteed for 64-bit > properties, where 64-bit loads might trigger unaligned access. > > Fix the 64-bit access using the ldst (host) API, which uses a local > copy on the stack, thus guaranteeing a safe aligned access. > > This fixes the following ASan warning: > > $ 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 > Suggested-by: Peter Maydell <peter.mayd...@linaro.org> > Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
Reviewed-by: David Gibson <da...@gibson.dropbear.id.au> I'll look at adding a hlper for this to libfdt, but no promises on timeframe. > --- > v4: do not change the 32-bit access, use ldq_he_p() for the 64-bit access > v3: do not use memcpy(), incorrectly change ldl_he_p() > v2: do not change the 32-bit access, use memcpy(), add comments (David Gibson) > v1: use memcpy() > > hw/core/loader-fit.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/hw/core/loader-fit.c b/hw/core/loader-fit.c > index 0c4a7207f4..ed4140061b 100644 > --- a/hw/core/loader-fit.c > +++ b/hw/core/loader-fit.c > @@ -102,10 +102,17 @@ 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 ldq_he_p()'s stack to avoid an unaligned load. > + */ > + *addr = fdt64_to_cpu(ldq_he_p(prop)); > return 0; > default: > error_printf("invalid %s address length %d\n", name, len); -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature