Messages in this thread Patch in this message | | | From | Nick Desaulniers <> | Date | Wed, 16 Dec 2020 18:08:42 -0800 | Subject | Re: [PATCH] MIPS: boot: Fix unaligned access with CONFIG_MIPS_RAW_APPENDED_DTB |
| |
On Wed, Dec 16, 2020 at 3:40 PM Paul Cercueil <paul@crapouillou.net> wrote: > > The compressed payload is not necesarily 4-byte aligned, at least when > compiling with Clang. In that case, the 4-byte value appended to the > compressed payload that corresponds to the uncompressed kernel image > size must be read using get_unaligned_le().
Should it be get_unaligned_le32()?
> > This fixes Clang-built kernels not booting on MIPS (tested on a Ingenic > JZ4770 board). > > Fixes: b8f54f2cde78 ("MIPS: ZBOOT: copy appended dtb to the end of the kernel") > Cc: <stable@vger.kernel.org> # v4.7 > Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Hi Paul, thanks for the patch (and for testing with Clang)! Alternatively, we could re-align __image_end to the next 4B multiple via:
diff --git a/arch/mips/boot/compressed/ld.script b/arch/mips/boot/compressed/ld.script index 0ebb667274d6..349919eff5fb 100644 --- a/arch/mips/boot/compressed/ld.script +++ b/arch/mips/boot/compressed/ld.script @@ -27,6 +27,7 @@ SECTIONS /* Put the compressed image here */ __image_begin = .; *(.image) + . = ALIGN(4); __image_end = .; CONSTRUCTORS . = ALIGN(16); The tradeoff being up to 3 wasted bytes of padding in the compressed image, vs fetching one value slower (assuming unaligned loads are slower than aligned loads MIPS, IDK). I doubt decompress_kernel is called repeatedly, so let's take the byte saving approach of yours by using unaligned loads!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> --- > arch/mips/boot/compressed/decompress.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/mips/boot/compressed/decompress.c b/arch/mips/boot/compressed/decompress.c > index c61c641674e6..47c07990432b 100644 > --- a/arch/mips/boot/compressed/decompress.c > +++ b/arch/mips/boot/compressed/decompress.c > @@ -117,7 +117,7 @@ void decompress_kernel(unsigned long boot_heap_start) > dtb_size = fdt_totalsize((void *)&__appended_dtb); > > /* last four bytes is always image size in little endian */ > - image_size = le32_to_cpup((void *)&__image_end - 4); > + image_size = get_unaligned_le32((void *)&__image_end - 4); > > /* copy dtb to where the booted kernel will expect it */ > memcpy((void *)VMLINUX_LOAD_ADDRESS_ULL + image_size, > -- > 2.29.2 >
-- Thanks, ~Nick Desaulniers
| |