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

[Ard Biesheuvel]

On 8 September 2017 at 15:56, Gregory CLEMENT
<gregory.clement at free-electrons.com> wrote:
> Hi Ard,
>
>  On ven., sept. 08 2017, Ard Biesheuvel <ard.biesheuvel at linaro.org> wrote:
>
>> On 8 September 2017 at 15:33, Gregory CLEMENT
>> <gregory.clement at free-electrons.com> wrote:
>>> 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
>>>
>>
>>
>> How about this?
>
> It fixed the bug! (I tested with and without your previous patch and it
> worked in both case)
>
> When you will send your patch, you can add my:
> Tested-by: Gregory CLEMENT <gregory.clement at free-electrons.com>
>
> Thanks,
>
> Gregory
>

Thanks a lot. I will send it out today.