From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ma1-aaemail-dr-lapp03.apple.com (ma1-aaemail-dr-lapp03.apple.com [17.171.2.72]) by mx.groups.io with SMTP id smtpd.web09.3990.1628570414073399933 for ; Mon, 09 Aug 2021 21:40:14 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=Yj0G9Ojh; spf=pass (domain: apple.com, ip: 17.171.2.72, mailfrom: afish@apple.com) Received: from pps.filterd (ma1-aaemail-dr-lapp03.apple.com [127.0.0.1]) by ma1-aaemail-dr-lapp03.apple.com (8.16.0.42/8.16.0.42) with SMTP id 17A4bpdr004171; Mon, 9 Aug 2021 21:40:12 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=from : message-id : content-type : mime-version : subject : date : in-reply-to : cc : to : references; s=20180706; bh=DKrXnwUvMZ/MK2oW+am/9HJkgV/KPyJ01VJKBq9byNI=; b=Yj0G9OjhiRr0vly4nfp+s1WKolMX0glb4sCrMV0PVq6ovZr7Km59HHct/1CQUbmI6+nQ h05tg2og4hyfZ+z5WS5Jri4nEI0WENBjWweTuLkFklDVFIrtvb44c+1mmPW7aD06mznT /tKAOzEPhRyfgEHR52dwWc3MbSebqFbtoJ6srOt6YRsQjEJt+f6v8hjlp0E3nwzs3VfV 0898G34RGgdLO8TYck5553+AqwrY5ddL0oxh6jYBcope+A/cFaA7hKLCLqphp5OtOYTM 0c2tZJ8Oemarc6ZKuYfFnDNQfPb6PVWtwqkXwKqdCMuTICNJaLaJ6Ku/VJMmAGw279aK rw== Received: from rn-mailsvcp-mta-lapp01.rno.apple.com (rn-mailsvcp-mta-lapp01.rno.apple.com [10.225.203.149]) by ma1-aaemail-dr-lapp03.apple.com with ESMTP id 3a9s5vh7ft-9 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Mon, 09 Aug 2021 21:40:12 -0700 Received: from rn-mailsvcp-mmp-lapp04.rno.apple.com (rn-mailsvcp-mmp-lapp04.rno.apple.com [17.179.253.17]) by rn-mailsvcp-mta-lapp01.rno.apple.com (Oracle Communications Messaging Server 8.1.0.9.20210415 64bit (built Apr 15 2021)) with ESMTPS id <0QXL00TI6WZ0MO80@rn-mailsvcp-mta-lapp01.rno.apple.com>; Mon, 09 Aug 2021 21:40:12 -0700 (PDT) Received: from process_milters-daemon.rn-mailsvcp-mmp-lapp04.rno.apple.com by rn-mailsvcp-mmp-lapp04.rno.apple.com (Oracle Communications Messaging Server 8.1.0.9.20210415 64bit (built Apr 15 2021)) id <0QXL00M00WORM100@rn-mailsvcp-mmp-lapp04.rno.apple.com>; Mon, 09 Aug 2021 21:40:12 -0700 (PDT) X-Va-A: X-Va-T-CD: 23e0047370634caa524bca754240c109 X-Va-E-CD: fcd1927ce0fde6ca7b96266ab11e23fb X-Va-R-CD: 29b203c945f29c0235ef86afdd3cd929 X-Va-CD: 0 X-Va-ID: 3f86a475-8ab6-424b-b834-f3202b0d6fec X-V-A: X-V-T-CD: 23e0047370634caa524bca754240c109 X-V-E-CD: fcd1927ce0fde6ca7b96266ab11e23fb X-V-R-CD: 29b203c945f29c0235ef86afdd3cd929 X-V-CD: 0 X-V-ID: 65d56ac3-60d0-498e-8dc1-8dc39da9096e X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391,18.0.790 definitions=2021-08-10_01:2021-08-06,2021-08-10 signatures=0 Received: from [17.235.18.207] (unknown [17.235.18.207]) by rn-mailsvcp-mmp-lapp04.rno.apple.com (Oracle Communications Messaging Server 8.1.0.9.20210415 64bit (built Apr 15 2021)) with ESMTPSA id <0QXL00X3JWYYPT00@rn-mailsvcp-mmp-lapp04.rno.apple.com>; Mon, 09 Aug 2021 21:40:12 -0700 (PDT) From: "Andrew Fish" Message-id: <170EF66A-C746-4590-A317-0195B53E000A@apple.com> MIME-version: 1.0 (Mac OS X Mail 14.0 \(3654.20.0.2.1\)) Subject: Re: [edk2-devel] [PATCH v2 2/2] UefiCpuPkg/BaseUefiCpuLib: Use toolchain-specific rodata section name Date: Mon, 09 Aug 2021 21:40:10 -0700 In-reply-to: Cc: =?utf-8?Q?Marvin_H=C3=A4user?= , "Dong, Eric" , "Kumar, Rahul1" , Vitaly Cheptsov To: edk2-devel-groups-io , Ray Ni References: <252525969122e83d9fb9b83edc95c4f6dfd233b4.1628502434.git.mhaeuser@posteo.de> <34c8c87e673ce08bef460179c5cb7b3ea9271208.1628502434.git.mhaeuser@posteo.de> X-Mailer: Apple Mail (2.3654.20.0.2.1) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.391,18.0.790 definitions=2021-08-10_01:2021-08-06,2021-08-10 signatures=0 Content-type: multipart/alternative; boundary="Apple-Mail=_52A579C1-ABC5-4903-B2A5-1CF9C4CD062F" --Apple-Mail=_52A579C1-ABC5-4903-B2A5-1CF9C4CD062F Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On Aug 9, 2021, at 7:43 PM, Ni, Ray wrote: >=20 > Acked-by: Ray Ni > >=20 > I will depend on tool owner to review the tool configuration change makin= g sure that the correct section name is chosen for different C compilers. >=20 Ray, I made a detailed response about Mach-O with Xcode/clang and I don=E2=80=99= t think patch works. Not sure if it breaks anything, but it puts things in = the .data PE/COFF section.=20 I=E2=80=99m also worried it is broken for any toolchain that generates ELF = and use GenFw. I don=E2=80=99t think the GenFw tool creates a PE/COFF .roda= ta 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.=20 I=E2=80=99m guessing it likely does the correct thing for toolchains that g= enerate PE/COFF directly?=20 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 t= he .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?=20 It might be possible to cheat and use this concept to force code into the t= ext section for ELF and Mach-O, but I=E2=80=99m not sure if that hits the c= orrect 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.=20 [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 (".rel= oc", 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 (".rel= oc", mRelocOffset, mCoffOffset - mRelocOffset, BaseTools/Source/C/GenFw/ElfConvert.c:125:CreateSectionHeader ( BaseTools/Source/C/GenFw/ElfConvert.h:74:CreateSectionHeader ( Thanks, Andrew Fish > Thanks, > Ray >=20 >> -----Original Message----- >> From: Marvin H=C3=A4user = > >> Sent: Monday, August 9, 2021 5:51 PM >> To: devel@edk2.groups.io >> Cc: Dong, Eric >; Ni, R= ay >; Kumar, Rahul1 >; Vitaly Cheptsov >> > >> Subject: [PATCH v2 2/2] UefiCpuPkg/BaseUefiCpuLib: Use toolchain-specifi= c rodata section name >>=20 >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3318 >>=20 >> Correctly define the read-only data sections with the >> toolchain-specific section name. This hardens image permission >> security and may save image space. >>=20 >> Cc: Eric Dong >> Cc: Ray Ni >> Cc: Rahul Kumar >> Cc: Vitaly Cheptsov >> Signed-off-by: Marvin H=C3=A4user >> --- >> UefiCpuPkg/Library/BaseUefiCpuLib/Ia32/InitializeFpu.nasm | 2 +- >> UefiCpuPkg/Library/BaseUefiCpuLib/X64/InitializeFpu.nasm | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >>=20 >> 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 @@ >> ;* >>=20 >> ;-----------------------------------------------------------------------= ------- >>=20 >>=20 >>=20 >> - SECTION .rodata >>=20 >> + SECTION RODATA_SECTION_NAME >>=20 >>=20 >>=20 >> ; >>=20 >> ; Float control word initial value: >>=20 >> 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 @@ >> ;* >>=20 >> ;-----------------------------------------------------------------------= ------- >>=20 >>=20 >>=20 >> - SECTION .rodata >>=20 >> + SECTION RODATA_SECTION_NAME >>=20 >> ; >>=20 >> ; Float control word initial value: >>=20 >> ; all exceptions masked, double-extended-precision, round-to-nearest >>=20 >> -- >> 2.31.1 >=20 >=20 >=20 >=20 --Apple-Mail=_52A579C1-ABC5-4903-B2A5-1CF9C4CD062F Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8

On Aug 9, 202= 1, at 7:43 PM, Ni, Ray <r= ay.ni@intel.com> wrote:

Acked-by: Ray Ni= <ray.ni@intel.com>

I will d= epend on tool owner to review the tool configuration change making sure tha= t 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=E2=80=99t think patch works. Not sure if it breaks anything, but it p= uts things in the .data PE/COFF section. 

I=E2=80=99m also worried it is broken for any toolchain that genera= tes ELF and use GenFw. I don=E2=80=99t think the GenFw tool creates a PE/CO= FF .rodata section [1] so if things work they will end up in the .data sect= ion, or things might break? Some one who knows that tool better than me sho= uld take a detailed look. 

I=E2=80= =99m 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 toolch= ains. For Xcode it may be easier to just dump this stuff in the .text secti= on (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 f= orce code into the text section for ELF and Mach-O, but I=E2=80=99m not sur= e if that hits the correct security bar. But the last thing we want is to c= laim something is in a read only section when it is in a read write section= . 

[1]  git grep CreateSectionHeader<= /span>
BaseTools/Source/C/GenF= w/Elf32Convert.c:602:  &nb= sp; CreateSectionHeader (".text", m= TextOffset, mDataOffset - mTextOffset,
BaseTools/Source/C/GenFw/Elf32Convert.c:612:    CreateSectionHeader (".data", mDataOffset, mHiiRsrcOffset - mD= ataOffset,
BaseTools/So= urce/C/GenFw/Elf32Convert.c:622:    CreateSectionHeader = (".rsrc", mHiiRsrcOffset, mRelocOffset - mHiiRsrcOffset,
BaseTools/Source/C/GenFw/Elf32Convert.c:1107:    CreateSectionHeader (".reloc", mRelocOffset, mCo= ffOffset - mRelocOffset,
BaseTools/Source/C/GenFw/Elf64Convert.c:929<= span style=3D"font-variant-ligatures: no-common-ligatures; color: #2eaebb" = class=3D"">:    CreateSection= Header (".text", mTextOffset, mDataOffset - mTextOffset,
BaseTools/Source/C/GenFw/Elf64Conv= ert.c:939:    = CreateSectionHeader (".data", mDataOffset,= mHiiRsrcOffset - mDataOffset,
BaseTools/Source/C/GenFw/Elf64Convert.c:<= span style=3D"font-variant-ligatures: no-common-ligatures" class=3D"">949:    CreateS= ectionHeader (".rsrc", mHiiRsrcOffset, mRelocOffset - mHiiRsrcOffset= ,
BaseTools/Source/C/Ge= nFw/Elf64Convert.c:1641:  =   CreateSectionHeader (".reloc= ", mRelocOffset, mCoffOffset - mRelocOffset,
BaseTools/Source/C/GenFw/ElfConvert.c:125:CreateSectio= nHeader (
BaseTo= ols/Source/C/GenFw/ElfConvert.h:74:CreateSectionHeader (

Thanks,

Andrew Fish

Thanks,
Ray

-----Original Message-----From: Marvin H=C3=A4user <mhaeuser@posteo.de>
Sent: Monday, Augu= st 9, 2021 5:51 PM
To:&= nbsp;devel@edk2.g= roups.io
Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul1 &= lt;rahul1.kumar@intel.= com>; Vitaly Cheptsov
<vit9696@protonmail.com>
Subje= ct: [PATCH v2 2/2] UefiCpuPkg/BaseUefiCpuLib: Use toolchain-specific rodata= section name

REF: https://bugzilla.tianocor= e.org/show_bug.cgi?id=3D3318

Correctly def= ine the read-only data sections with the
toolchain-specific s= ection name. This hardens image permission
security and may s= ave image space.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@= intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
= Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Marvin H=C3= =A4user <mhaeuser@poste= o.de>
---
UefiCpuPkg/Library/BaseUefiCpu= Lib/Ia32/InitializeFpu.nasm | 2 +-
UefiCpuPkg/Library/BaseUef= iCpuLib/X64/InitializeFpu.nasm  | 2 +-
2 files changed, = 2 insertions(+), 2 deletions(-)

diff --git a/U= efiCpuPkg/Library/BaseUefiCpuLib/Ia32/InitializeFpu.nasm
b/Ue= fiCpuPkg/Library/BaseUefiCpuLib/Ia32/InitializeFpu.nasm
index= 5e27cc325012..cfb8bf4a5ae0 100644
--- a/UefiCpuPkg/Library/B= aseUefiCpuLib/Ia32/InitializeFpu.nasm
+++ b/UefiCpuPkg/Librar= y/BaseUefiCpuLib/Ia32/InitializeFpu.nasm
@@ -6,7 +6,7 @@
;*

;--------------------------------= ----------------------------------------------



-    SECTION .rodata

+    SECTION RODATA_SECTION_NAME



;

; Float control word initial value:

d= iff --git a/UefiCpuPkg/Library/BaseUefiCpuLib/X64/InitializeFpu.nasm
b/UefiCpuPkg/Library/BaseUefiCpuLib/X64/InitializeFpu.nasm
index 8485b4713548..3c976a21e391 100644
--- a/UefiCpuPk= g/Library/BaseUefiCpuLib/X64/InitializeFpu.nasm
+++ b/UefiCpu= Pkg/Library/BaseUefiCpuLib/X64/InitializeFpu.nasm
@@ -6,7 +6,= 7 @@
;*

;-----------------------= -------------------------------------------------------



-    SECTION .rodata=

+    SECTION RODATA_SECTION_NA= ME

;

; Float cont= rol word initial value:

; all exceptions maske= d, double-extended-precision, round-to-nearest

--
2.31.1




--Apple-Mail=_52A579C1-ABC5-4903-B2A5-1CF9C4CD062F--