[PATCH 1/1] riscv: efi: Set NX compat flag in PE/COFF header

[Alexandre Ghiti]

Hi Heinrich,

On 01/10/2024 17:24, Heinrich Schuchardt wrote:
> On 01.10.24 15:51, Alexandre Ghiti wrote:
>> Hi Heinrich,
>>
>> On 29/09/2024 16:02, Heinrich Schuchardt wrote:
>>> The IMAGE_DLLCHARACTERISTICS_NX_COMPAT informs the firmware that the
>>> EFI binary does not rely on pages that are both executable and
>>> writable.
>>>
>>> The flag is used by some distro versions of GRUB to decide if the EFI
>>> binary may be executed.
>>>
>>> As the Linux kernel neither has RWX sections nor needs RWX pages for
>>> relocation we should set the flag.
>>>
>>> Cc: Ard Biesheuvel <ardb at kernel.org>
>>> Cc: <stable at vger.kernel.org>
>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
>>> ---
>>>   arch/riscv/kernel/efi-header.S | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/riscv/kernel/efi-header.S b/arch/riscv/kernel/efi- 
>>> header.S
>>> index 515b2dfbca75..c5f17c2710b5 100644
>>> --- a/arch/riscv/kernel/efi-header.S
>>> +++ b/arch/riscv/kernel/efi-header.S
>>> @@ -64,7 +64,7 @@ extra_header_fields:
>>>       .long    efi_header_end - _start            // SizeOfHeaders
>>>       .long    0                    // CheckSum
>>>       .short    IMAGE_SUBSYSTEM_EFI_APPLICATION        // Subsystem
>>> -    .short    0                    // DllCharacteristics
>>> +    .short    IMAGE_DLL_CHARACTERISTICS_NX_COMPAT    // 
>>> DllCharacteristics
>>>       .quad    0                    // SizeOfStackReserve
>>>       .quad    0                    // SizeOfStackCommit
>>>       .quad    0                    // SizeOfHeapReserve
>>
>>
>> I don't understand if this fixes something or not: what could go 
>> wrong if we don't do this?
>>
>> Thanks,
>>
>> Alex
>>
>
>
> Hello Alexandre,
>
> https://learn.microsoft.com/en-us/windows-hardware/drivers/bringup/uefi-ca-memory-mitigation-requirements 
>
> describes Microsoft's effort to improve security by avoiding memory 
> pages that are both executable and writable.
>
> IMAGE_DLL_CHARACTERISTICS_NX_COMPAT is an assertion by the EFI binary 
> that it does not use RWX pages. It may use the 
> EFI_MEMORY_ATTRIBUTE_PROTOCOL to set whether a page is writable or 
> executable (but not both).
>
> When using secure boot, compliant firmware will not allow loading a 
> binary if the flag is not set.


Great, so that's a necessary fix, it will get merged in the next rc or so:

Fixes: cb7d2dd5612a ("RISC-V: Add PE/COFF header for EFI stub")

Thanks,

Alex


>
> Best regards
>
> Heinrich
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv