Re: [PATCH v3] of/fdt: Don't calculate initrd size from DT if start > end

[Rob Herring]

On Fri, 09 Sep 2022 02:33:57 +0000, Marek Bykowski wrote:
> If the properties 'linux,initrd-start' and 'linux,initrd-end' of
> the chosen node populated from the bootloader, eg. U-Boot, are so that
> start > end, then the phys_initrd_size calculated from end - start is
> negative that subsequently gets converted to a high positive value for
> being unsigned long long. Then, the memory region with the (invalid)
> size is added to the bootmem and attempted being paged in paging_init()
> that results in the kernel fault.
> 
> For example, on the FVP ARM64 system I'm running, the U-Boot populates
> the 'linux,initrd-start' with 8800_0000 and 'linux,initrd-end' with 0.
> The phys_initrd_size calculated is then ffff_ffff_7800_0000
> (= 0 - 8800_0000 = -8800_0000 + ULLONG_MAX + 1). paging_init() then
> attempts to map the address 8800_0000 + ffff_ffff_7800_0000 and oops'es
> as below.
> 
> It should be stressed, it is generally a fault of the bootloader's with
> the kernel relying on it, however we should not allow the bootloader's
> misconfiguration to lead to the kernel oops. Not only the kernel should be
> bullet proof against it but also finding the root cause of the paging
> fault spanning over the bootloader, DT, and kernel may happen is not so
> easy.
> 
>   Unable to handle kernel paging request at virtual address fffffffefe43c000
>   Mem abort info:
>     ESR = 0x96000007
>     EC = 0x25: DABT (current EL), IL = 32 bits
>     SET = 0, FnV = 0
>     EA = 0, S1PTW = 0
>   Data abort info:
>     ISV = 0, ISS = 0x00000007
>     CM = 0, WnR = 0
>   swapper pgtable: 4k pages, 39-bit VAs, pgdp=0000000080e3d000
>   [fffffffefe43c000] pgd=0000000080de9003, pud=0000000080de9003
>   Unable to handle kernel paging request at virtual address ffffff8000de9f90
>   Mem abort info:
>     ESR = 0x96000005
>     EC = 0x25: DABT (current EL), IL = 32 bits
>     SET = 0, FnV = 0
>     EA = 0, S1PTW = 0
>   Data abort info:
>     ISV = 0, ISS = 0x00000005
>     CM = 0, WnR = 0
>   swapper pgtable: 4k pages, 39-bit VAs, pgdp=0000000080e3d000
>   [ffffff8000de9f90] pgd=0000000000000000, pud=0000000000000000
>   Internal error: Oops: 96000005 [#1] PREEMPT SMP
>   Modules linked in:
>   CPU: 0 PID: 0 Comm: swapper Not tainted 5.4.51-yocto-standard #1
>   Hardware name: FVP Base (DT)
>   pstate: 60000085 (nZCv daIf -PAN -UAO)
>   pc : show_pte+0x12c/0x1b4
>   lr : show_pte+0x100/0x1b4
>   sp : ffffffc010ce3b30
>   x29: ffffffc010ce3b30 x28: ffffffc010ceed80
>   x27: fffffffefe43c000 x26: fffffffefe43a028
>   x25: 0000000080bf0000 x24: 0000000000000025
>   x23: ffffffc010b8d000 x22: ffffffc010e3d000
>   x23: ffffffc010b8d000 x22: ffffffc010e3d000
>   x21: 0000000080de9000 x20: ffffff7f80000f90
>   x19: fffffffefe43c000 x18: 0000000000000030
>   x17: 0000000000001400 x16: 0000000000001c00
>   x15: ffffffc010cef1b8 x14: ffffffffffffffff
>   x13: ffffffc010df1f40 x12: ffffffc010df1b70
>   x11: ffffffc010ce3b30 x10: ffffffc010ce3b30
>   x9 : 00000000ffffffc8 x8 : 0000000000000000
>   x7 : 000000000000000f x6 : ffffffc010df16e8
>   x5 : 0000000000000000 x4 : 0000000000000000
>   x3 : 00000000ffffffff x2 : 0000000000000000
>   x1 : 0000008080000000 x0 : ffffffc010af1d68
>   Call trace:
>    show_pte+0x12c/0x1b4
>    die_kernel_fault+0x54/0x78
>    __do_kernel_fault+0x11c/0x128
>    do_translation_fault+0x58/0xac
>    do_mem_abort+0x50/0xb0
>    el1_da+0x1c/0x90
>    __create_pgd_mapping+0x348/0x598
>    paging_init+0x3f0/0x70d0
>    setup_arch+0x2c0/0x5d4
>    start_kernel+0x94/0x49c
>   Code: 92748eb5 900052a0 9135a000 cb010294 (f8756a96)
> 
> Signed-off-by: Marek Bykowski <marek.bykowski@xxxxxxxxx>
> ---
> v2 -> v3:
> - I confused the description I fixed now. I put that we should
>   complain if start < end and it should be the other way around
>   if end > start
> v1 -> v2:
> - followed Rob's suggestion that we check on start > end instead of
>   for end being 0
> ---
>  drivers/of/fdt.c | 2 ++
>  1 file changed, 2 insertions(+)
> 

Applied, thanks!