[edk2-devel] [PATCH v2 2/2] UefiCpuPkg/BaseUefiCpuLib: Use toolchain-specific rodata section name

[Andrew Fish via groups.io]

> On Aug 9, 2021, at 7:43 PM, Ni, Ray <ray.ni at intel.com> wrote:
> 
> Acked-by: Ray Ni <ray.ni at intel.com <mailto:ray.ni at intel.com>>
> 
> I will depend on tool owner to review the tool configuration change making sure that the correct section name is chosen for different C compilers.
> 

Ray,

I made a detailed response about Mach-O with Xcode/clang and I don’t think patch works. Not sure if it breaks anything, but it puts things in the .data PE/COFF section. 

I’m also worried it is broken for any toolchain that generates ELF and use GenFw. I don’t think the GenFw tool creates a PE/COFF .rodata section [1] so if things work they will end up in the .data section, or things might break? Some one who knows that tool better than me should take a detailed look. 

I’m guessing it likely does the correct thing for toolchains that generate PE/COFF directly? 

My vote is to not add this feature until we can prove it works properly on all the toolchains. For Xcode it may be easier to just dump this stuff in the .text section (see my other mail for more background). It looks like we might have to modify GenFw if we want to create a .rodata section? 

It might be possible to cheat and use this concept to force code into the text section for ELF and Mach-O, but I’m not sure if that hits the correct security bar. But the last thing we want is to claim something is in a read only section when it is in a read write section. 

[1]  git grep CreateSectionHeader
BaseTools/Source/C/GenFw/Elf32Convert.c:602:    CreateSectionHeader (".text", mTextOffset, mDataOffset - mTextOffset,
BaseTools/Source/C/GenFw/Elf32Convert.c:612:    CreateSectionHeader (".data", mDataOffset, mHiiRsrcOffset - mDataOffset,
BaseTools/Source/C/GenFw/Elf32Convert.c:622:    CreateSectionHeader (".rsrc", mHiiRsrcOffset, mRelocOffset - mHiiRsrcOffset,
BaseTools/Source/C/GenFw/Elf32Convert.c:1107:    CreateSectionHeader (".reloc", mRelocOffset, mCoffOffset - mRelocOffset,
BaseTools/Source/C/GenFw/Elf64Convert.c:929:    CreateSectionHeader (".text", mTextOffset, mDataOffset - mTextOffset,
BaseTools/Source/C/GenFw/Elf64Convert.c:939:    CreateSectionHeader (".data", mDataOffset, mHiiRsrcOffset - mDataOffset,
BaseTools/Source/C/GenFw/Elf64Convert.c:949:    CreateSectionHeader (".rsrc", mHiiRsrcOffset, mRelocOffset - mHiiRsrcOffset,
BaseTools/Source/C/GenFw/Elf64Convert.c:1641:    CreateSectionHeader (".reloc", mRelocOffset, mCoffOffset - mRelocOffset,
BaseTools/Source/C/GenFw/ElfConvert.c:125:CreateSectionHeader (
BaseTools/Source/C/GenFw/ElfConvert.h:74:CreateSectionHeader (

Thanks,

Andrew Fish

> Thanks,
> Ray
> 
>> -----Original Message-----
>> From: Marvin Häuser <mhaeuser at posteo.de <mailto:mhaeuser at posteo.de>>
>> Sent: Monday, August 9, 2021 5:51 PM
>> To: devel at edk2.groups.io <mailto:devel at edk2.groups.io>
>> Cc: Dong, Eric <eric.dong at intel.com <mailto:eric.dong at intel.com>>; Ni, Ray <ray.ni at intel.com <mailto:ray.ni at intel.com>>; Kumar, Rahul1 <rahul1.kumar at intel.com <mailto:rahul1.kumar at intel.com>>; Vitaly Cheptsov
>> <vit9696 at protonmail.com <mailto:vit9696 at protonmail.com>>
>> Subject: [PATCH v2 2/2] UefiCpuPkg/BaseUefiCpuLib: Use toolchain-specific rodata section name
>> 
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3318
>> 
>> Correctly define the read-only data sections with the
>> toolchain-specific section name. This hardens image permission
>> security and may save image space.
>> 
>> Cc: Eric Dong <eric.dong at intel.com>
>> Cc: Ray Ni <ray.ni at intel.com>
>> Cc: Rahul Kumar <rahul1.kumar at intel.com>
>> Cc: Vitaly Cheptsov <vit9696 at protonmail.com>
>> Signed-off-by: Marvin Häuser <mhaeuser at posteo.de>
>> ---
>> UefiCpuPkg/Library/BaseUefiCpuLib/Ia32/InitializeFpu.nasm | 2 +-
>> UefiCpuPkg/Library/BaseUefiCpuLib/X64/InitializeFpu.nasm  | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/UefiCpuPkg/Library/BaseUefiCpuLib/Ia32/InitializeFpu.nasm
>> b/UefiCpuPkg/Library/BaseUefiCpuLib/Ia32/InitializeFpu.nasm
>> index 5e27cc325012..cfb8bf4a5ae0 100644
>> --- a/UefiCpuPkg/Library/BaseUefiCpuLib/Ia32/InitializeFpu.nasm
>> +++ b/UefiCpuPkg/Library/BaseUefiCpuLib/Ia32/InitializeFpu.nasm
>> @@ -6,7 +6,7 @@
>> ;*
>> 
>> ;------------------------------------------------------------------------------
>> 
>> 
>> 
>> -    SECTION .rodata
>> 
>> +    SECTION RODATA_SECTION_NAME
>> 
>> 
>> 
>> ;
>> 
>> ; Float control word initial value:
>> 
>> diff --git a/UefiCpuPkg/Library/BaseUefiCpuLib/X64/InitializeFpu.nasm
>> b/UefiCpuPkg/Library/BaseUefiCpuLib/X64/InitializeFpu.nasm
>> index 8485b4713548..3c976a21e391 100644
>> --- a/UefiCpuPkg/Library/BaseUefiCpuLib/X64/InitializeFpu.nasm
>> +++ b/UefiCpuPkg/Library/BaseUefiCpuLib/X64/InitializeFpu.nasm
>> @@ -6,7 +6,7 @@
>> ;*
>> 
>> ;------------------------------------------------------------------------------
>> 
>> 
>> 
>> -    SECTION .rodata
>> 
>> +    SECTION RODATA_SECTION_NAME
>> 
>> ;
>> 
>> ; Float control word initial value:
>> 
>> ; all exceptions masked, double-extended-precision, round-to-nearest
>> 
>> --
>> 2.31.1
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79016): https://edk2.groups.io/g/devel/message/79016
Mute This Topic: https://groups.io/mt/84764907/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20210809/98b9eb8b/attachment.htm>