Re: [PATCH] MdeModulePkg/DxeCore: base code protection on permission attributes

[Laszlo Ersek <lersek@redhat.com>]

On 02/26/17 15:00, Ard Biesheuvel wrote:
> On 25 February 2017 at 04:04, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>> Hi Ard
>> I agree with you on this enhancement.
>>
>> I prefer to adding the description as comment in the code, so that people
>> can get clear picture when he/she reads the code.
>>
>> //
>> // Instead of assuming that a PE/COFF section of type EFI_IMAGE_SCN_CNT_CODE
>> // can always be mapped read-only, classify a section as a code section only
>> // if it has the executable attribute set and the writable attribute
>> cleared.
>> //
>> // This adheres more closely to the PE/COFF spec, and avoids issues with
>> // Linux OS loaders that consists of a single read/write/execute section.
>> //
>> if ((Section[Index].Characteristics & (EFI_IMAGE_SCN_MEM_WRITE |
>> EFI_IMAGE_SCN_MEM_EXECUTE)) == EFI_IMAGE_SCN_MEM_EXECUTE) {
>>
>> With comment update, reviewed-by: Jiewen.yao@intel.com
>>
> 
> Thanks Jiewen
> 
> Pushed as a2ed40c02bf2

Is it possible that (recent?) Linux EFI stubs (aarch64) don't pass the
above check? I got a report from a colleague:

!!!!!!!!  ProtectUefiImageCommon - CodeSegmentCount is 0  !!!!!!!!
EFI stub: Booting Linux Kernel...
EFI stub: Using DTB from configuration table
EFI stub: Exiting boot services and installing virtual address map...

I tried to reproduce it with "4.5.0-15.el7.aarch64", and with
"4.8.7-300.fc25.aarch64", and I'm not seeing the message with either.

I asked him about the exact kernel version (no answer yet, his workday
hasn't started yet).

Any idea how I could validate the section headers of a (decompressed)
kernel image? I tried "aarch64-linux-gnu-objdump --section-headers", but
it doesn't recognize the image.

Thanks,
Laszlo

>>> -----Original Message-----
>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>>> Sent: Saturday, February 25, 2017 1:51 AM
>>> To: edk2-devel@lists.01.org; Yao, Jiewen <jiewen.yao@intel.com>
>>> Cc: Gao, Liming <liming.gao@intel.com>; Ard Biesheuvel
>>> <ard.biesheuvel@linaro.org>
>>> Subject: [PATCH] MdeModulePkg/DxeCore: base code protection on permission
>>> attributes
>>
>>
>>>
>>> Instead of assuming that a PE/COFF section of type EFI_IMAGE_SCN_CNT_CODE
>>> can always be mapped read-only, classify a section as a code section only
>>> if it has the executable attribute set and the writable attribute cleared.
>>>
>>> This adheres more closely to the PE/COFF spec, and avoids issues with
>>> Linux OS loaders that consists of a single read/write/execute section.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
>>> b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
>>> index 1142dcc5a83d..3e037607a6be 100644
>>> --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
>>> +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
>>> @@ -533,7 +533,7 @@ ProtectUefiImageCommon (
>>>        Name[7]
>>>        ));
>>>
>>> -    if ((Section[Index].Characteristics & EFI_IMAGE_SCN_CNT_CODE) != 0) {
>>> +    if ((Section[Index].Characteristics & (EFI_IMAGE_SCN_MEM_WRITE |
>>> EFI_IMAGE_SCN_MEM_EXECUTE)) == EFI_IMAGE_SCN_MEM_EXECUTE) {
>>>        DEBUG ((DEBUG_VERBOSE, "  VirtualSize          - 0x%08x\n",
>>> Section[Index].Misc.VirtualSize));
>>>        DEBUG ((DEBUG_VERBOSE, "  VirtualAddress       - 0x%08x\n",
>>> Section[Index].VirtualAddress));
>>>        DEBUG ((DEBUG_VERBOSE, "  SizeOfRawData        - 0x%08x\n",
>>> Section[Index].SizeOfRawData));
>>> --
>>> 2.7.4
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>