[PATCH v2 5/7] arm: efi: split zImage code and data into separate PE/COFF sections

[Gregory CLEMENT]

Hi Ard,
 
 On ven., sept. 08 2017, Ard Biesheuvel <ard.biesheuvel at linaro.org> wrote:

> On 8 September 2017 at 14:54, Ard Biesheuvel <ard.biesheuvel at linaro.org> wrote:
>> On 8 September 2017 at 14:50, Gregory CLEMENT
>> <gregory.clement at free-electrons.com> wrote:
>>> Hi Ard,
>>>
>>>  On jeu., juin 29 2017, Ard Biesheuvel <ard.biesheuvel at linaro.org> wrote:
>>>
>>>> To prevent unintended modifications to the kernel text (malicious or
>>>> otherwise) while running the EFI stub, describe the kernel image as
>>>> two separate sections: a .text section with read-execute permissions,
>>>> covering .text, .rodata, .piggytext and the GOT sections (which the
>>>> stub does not care about anyway), and a .data section with read-write
>>>> permissions, covering .data and .bss.
>>>>
>>>> This relies on the firmware to actually take the section permission
>>>> flags into account, but this is something that is currently being
>>>> implemented in EDK2, which means we will likely start seeing it in
>>>> the wild between one and two years from now.
>>>
>>> This patch had been merged in mainline yesterday and now prevent the
>>> Marvell Armada 370 and the Armada XP based SoC to boot. I also suspect
>>> that more Socs are impacted because the number of boot fail exploded
>>> according to kci:
>>> https://kernelci.org/boot/all/job/mainline/branch/master/kernel/v4.13-8899-g8dc5b3a6cb2f/
>>>
>>
>> Ouch.
>>
>>> I found this patch after bisecting (I can provide the bisect log if
>>> needed).
>>>
>>> The kernel failed to boot only if CONFIG_EFI is enabled so it occurs in
>>> multi_v7_defconfig but not with mvebu_v7_defconfig.
>>>
>>> Currently the solution is to revert this patch.
>>>
>>> Have you a better option?
>>>
>>
>> I will investigate.
>
> I cannot reproduce this on QEMU or my Beaglebone white. I have tried a
> locally built zImage as well as the one built by kernelci.
>
> Could you please try whether this fixes things? It does not explain
> anything but it will help me figure out what is going on (hopefully)

I've just tested this change and it didn't fix anything.

Gregory

>
>
> diff --git a/arch/arm/boot/compressed/efi-header.S
> b/arch/arm/boot/compressed/efi-header.S
> index c94a88ae834d..671a6e5b7b99 100644
> --- a/arch/arm/boot/compressed/efi-header.S
> +++ b/arch/arm/boot/compressed/efi-header.S
> @@ -127,7 +127,7 @@ section_table:
>
>                 .set    section_count, (. - section_table) / 40
>
> -               .align  12
> +               .align  9
>  __efi_start:
>  #endif
>                 .endm

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com