[edk2-devel] [PATCH v3 0/4] Enable BTI support in memory attributes table

Ard Biesheuvel posted 4 patches 1 year ago
Failed in applying to current master (apply log)
BaseTools/Source/C/GenFw/Elf64Convert.c               | 104 +++++++++++++++++---
BaseTools/Source/C/GenFw/GenFw.c                      |   3 +-
BaseTools/Source/C/GenFw/elf_common.h                 |   9 ++
BaseTools/Source/C/Include/IndustryStandard/PeImage.h |  13 ++-
MdeModulePkg/Core/Dxe/DxeMain.h                       |   2 +
MdeModulePkg/Core/Dxe/Image/Image.c                   |  10 ++
MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c    |   8 +-
MdePkg/Include/IndustryStandard/PeImage.h             |  13 ++-
MdePkg/Include/Library/PeCoffLib.h                    |   6 ++
MdePkg/Library/BasePeCoffLib/BasePeCoff.c             |  46 ++++++---
10 files changed, 186 insertions(+), 28 deletions(-)
[edk2-devel] [PATCH v3 0/4] Enable BTI support in memory attributes table
Posted by Ard Biesheuvel 1 year ago
Implement version 2 of the memory attributes table, which now contains a
flag informing the OS whether or not code regions may be mapped with CFI
mitigations such as IBT or BTI enabled.

This series covers the remaining parts after the AArch64 specific
changes were merged:

- Update the BaseTools to emit the appropriate PE/COFF annotation when a
  BTI/IBT compatible ELF executable is converted to PE/COFF
- Take this PE/COFF annotation into account when populating the memory
  attributes table in the DXE core

TODO:
- X64 changes to make the code IBT compatible and emit the ELF note
- Figure out how to generate such executables with native PE toolchains
- Implement BTI/IBT enforcement at boot time - this is something I
  intend to look into next.

Can be tested with the CLANG38 toolchain (both Clang compiler and LLD
linker, version 3.8 or newer) with the following build options.

[BuildOptions]
  GCC:*_*_AARCH64_PP_FLAGS = -mbranch-protection=bti
  GCC:*_*_AARCH64_CC_FLAGS = -mbranch-protection=bti
  GCC:*_*_AARCH64_DLINK_FLAGS = -fuse-ld=lld -Wl,--no-relax,--no-pie,-z,bti-report=error

Changes since v2:
- increase DllCharacteristicsEx field to 4 bytes
- add Oliver's Rb

If no comments or objections have been raised by the end of the week, I
will go ahead and merge this - thanks.

Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Rebecca Cran <quic_rcran@quicinc.com>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Taylor Beebe <t@taylorbeebe.com>
Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Oliver Smith-Denny <osde@linux.microsoft.com>

Ard Biesheuvel (4):
  BaseTools/GenFw: Parse IBT/BTI support status from ELF note
  BaseTools/GenFw: Add DllCharacteristicsEx field to debug data
  MdePkg/PeCoffLib: Capture DLL characteristics fields in image context
  MdeModulePkg: Enable forward edge CFI in mem attributes table

 BaseTools/Source/C/GenFw/Elf64Convert.c               | 104 +++++++++++++++++---
 BaseTools/Source/C/GenFw/GenFw.c                      |   3 +-
 BaseTools/Source/C/GenFw/elf_common.h                 |   9 ++
 BaseTools/Source/C/Include/IndustryStandard/PeImage.h |  13 ++-
 MdeModulePkg/Core/Dxe/DxeMain.h                       |   2 +
 MdeModulePkg/Core/Dxe/Image/Image.c                   |  10 ++
 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c    |   8 +-
 MdePkg/Include/IndustryStandard/PeImage.h             |  13 ++-
 MdePkg/Include/Library/PeCoffLib.h                    |   6 ++
 MdePkg/Library/BasePeCoffLib/BasePeCoff.c             |  46 ++++++---
 10 files changed, 186 insertions(+), 28 deletions(-)

-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102491): https://edk2.groups.io/g/devel/message/102491
Mute This Topic: https://groups.io/mt/98062730/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 0/4] Enable BTI support in memory attributes table
Posted by Marvin Häuser 1 year ago
FWIW, Reviewed-by: Marvin Häuser <mhaeuser@posteo.de>

An off-topic remark, but I find it ominous that when adding a hack like the DllCharacteristicsEx debug entry, the opportunity is not used to turn it into something that can be expanded in the future without introducing yet another hack like this (I know 31 more Bits look plenty now, but if an address, offset, or size will be needed… ouch).

Best regards,
Marvin

> On 4. Apr 2023, at 17:40, Ard Biesheuvel <ardb@kernel.org> wrote:
> Implement version 2 of the memory attributes table, which now contains a
> flag informing the OS whether or not code regions may be mapped with CFI
> mitigations such as IBT or BTI enabled.
> 
> This series covers the remaining parts after the AArch64 specific
> changes were merged:
> 
> - Update the BaseTools to emit the appropriate PE/COFF annotation when a
>  BTI/IBT compatible ELF executable is converted to PE/COFF
> - Take this PE/COFF annotation into account when populating the memory
>  attributes table in the DXE core
> 
> TODO:
> - X64 changes to make the code IBT compatible and emit the ELF note
> - Figure out how to generate such executables with native PE toolchains
> - Implement BTI/IBT enforcement at boot time - this is something I
>  intend to look into next.
> 
> Can be tested with the CLANG38 toolchain (both Clang compiler and LLD
> linker, version 3.8 or newer) with the following build options.
> 
> [BuildOptions]
>  GCC:*_*_AARCH64_PP_FLAGS = -mbranch-protection=bti
>  GCC:*_*_AARCH64_CC_FLAGS = -mbranch-protection=bti
>  GCC:*_*_AARCH64_DLINK_FLAGS = -fuse-ld=lld -Wl,--no-relax,--no-pie,-z,bti-report=error
> 
> Changes since v2:
> - increase DllCharacteristicsEx field to 4 bytes
> - add Oliver's Rb
> 
> If no comments or objections have been raised by the end of the week, I
> will go ahead and merge this - thanks.
> 
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Rebecca Cran <quic_rcran@quicinc.com>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Taylor Beebe <t@taylorbeebe.com>
> Cc: Marvin Häuser <mhaeuser@posteo.de>
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Oliver Smith-Denny <osde@linux.microsoft.com>
> 
> Ard Biesheuvel (4):
>  BaseTools/GenFw: Parse IBT/BTI support status from ELF note
>  BaseTools/GenFw: Add DllCharacteristicsEx field to debug data
>  MdePkg/PeCoffLib: Capture DLL characteristics fields in image context
>  MdeModulePkg: Enable forward edge CFI in mem attributes table
> 
> BaseTools/Source/C/GenFw/Elf64Convert.c               | 104 +++++++++++++++++---
> BaseTools/Source/C/GenFw/GenFw.c                      |   3 +-
> BaseTools/Source/C/GenFw/elf_common.h                 |   9 ++
> BaseTools/Source/C/Include/IndustryStandard/PeImage.h |  13 ++-
> MdeModulePkg/Core/Dxe/DxeMain.h                       |   2 +
> MdeModulePkg/Core/Dxe/Image/Image.c                   |  10 ++
> MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c    |   8 +-
> MdePkg/Include/IndustryStandard/PeImage.h             |  13 ++-
> MdePkg/Include/Library/PeCoffLib.h                    |   6 ++
> MdePkg/Library/BasePeCoffLib/BasePeCoff.c             |  46 ++++++---
> 10 files changed, 186 insertions(+), 28 deletions(-)
> 
> -- 
> 2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102498): https://edk2.groups.io/g/devel/message/102498
Mute This Topic: https://groups.io/mt/98062730/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 0/4] Enable BTI support in memory attributes table
Posted by Ard Biesheuvel 1 year ago
On Tue, 4 Apr 2023 at 18:19, Marvin Häuser <mhaeuser@posteo.de> wrote:
>
> FWIW, Reviewed-by: Marvin Häuser <mhaeuser@posteo.de>
>
> An off-topic remark, but I find it ominous that when adding a hack like the DllCharacteristicsEx debug entry, the opportunity is not used to turn it into something that can be expanded in the future without introducing yet another hack like this (I know 31 more Bits look plenty now, but if an address, offset, or size will be needed… ouch).
>

It *can* be expanded in the future. The debug directory entry includes
a size field, and once flags get defined that are not present, they
just default to unset.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102500): https://edk2.groups.io/g/devel/message/102500
Mute This Topic: https://groups.io/mt/98062730/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 0/4] Enable BTI support in memory attributes table
Posted by Marvin Häuser 1 year ago
> On 4. Apr 2023, at 18:29, Ard Biesheuvel <ardb@kernel.org> wrote:
> On Tue, 4 Apr 2023 at 18:19, Marvin Häuser <mhaeuser@posteo.de> wrote:
>> 
>> FWIW, Reviewed-by: Marvin Häuser <mhaeuser@posteo.de>
>> 
>> An off-topic remark, but I find it ominous that when adding a hack like the DllCharacteristicsEx debug entry, the opportunity is not used to turn it into something that can be expanded in the future without introducing yet another hack like this (I know 31 more Bits look plenty now, but if an address, offset, or size will be needed… ouch).
> 
> It *can* be expanded in the future. The debug directory entry includes
> a size field, and once flags get defined that are not present, they
> just default to unset.

Ugh, I should have known this… sorry for my terrible remark! :)

Though doesn’t that mean the size field should ideally be checked in 3/4 (not regarding the extensibility point, but well-formedness of the data)?

Best regards,
Marvin


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102502): https://edk2.groups.io/g/devel/message/102502
Mute This Topic: https://groups.io/mt/98062730/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 0/4] Enable BTI support in memory attributes table
Posted by Ard Biesheuvel 1 year ago
On Tue, 4 Apr 2023 at 18:42, Marvin Häuser <mhaeuser@posteo.de> wrote:
>
>
> > On 4. Apr 2023, at 18:29, Ard Biesheuvel <ardb@kernel.org> wrote:
> > On Tue, 4 Apr 2023 at 18:19, Marvin Häuser <mhaeuser@posteo.de> wrote:
> >>
> >> FWIW, Reviewed-by: Marvin Häuser <mhaeuser@posteo.de>
> >>
> >> An off-topic remark, but I find it ominous that when adding a hack like the DllCharacteristicsEx debug entry, the opportunity is not used to turn it into something that can be expanded in the future without introducing yet another hack like this (I know 31 more Bits look plenty now, but if an address, offset, or size will be needed… ouch).
> >
> > It *can* be expanded in the future. The debug directory entry includes
> > a size field, and once flags get defined that are not present, they
> > just default to unset.
>
> Ugh, I should have known this… sorry for my terrible remark! :)
>
> Though doesn’t that mean the size field should ideally be checked in 3/4 (not regarding the extensibility point, but well-formedness of the data)?
>

Ideally, yes, we shouldn't read more bytes than the debug entry
describes, and zero out the rest.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102508): https://edk2.groups.io/g/devel/message/102508
Mute This Topic: https://groups.io/mt/98062730/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
回复: [edk2-devel] [PATCH v3 0/4] Enable BTI support in memory attributes table
Posted by gaoliming via groups.io 1 year ago
Ard:
 Can you submit one BZ for this new feature? I will add it into the stable tag feature planning. 

 For this patch set, Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Ard
> Biesheuvel
> 发送时间: 2023年4月4日 23:40
> 收件人: devel@edk2.groups.io
> 抄送: Ard Biesheuvel <ardb@kernel.org>; Michael Kinney
> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
> Jiewen Yao <jiewen.yao@intel.com>; Michael Kubacki
> <michael.kubacki@microsoft.com>; Sean Brogan
> <sean.brogan@microsoft.com>; Rebecca Cran <quic_rcran@quicinc.com>;
> Leif Lindholm <quic_llindhol@quicinc.com>; Sami Mujawar
> <sami.mujawar@arm.com>; Taylor Beebe <t@taylorbeebe.com>; Marvin
> Häuser <mhaeuser@posteo.de>; Bob Feng <bob.c.feng@intel.com>; Oliver
> Smith-Denny <osde@linux.microsoft.com>
> 主题: [edk2-devel] [PATCH v3 0/4] Enable BTI support in memory attributes
> table
> 
> Implement version 2 of the memory attributes table, which now contains a
> 
> flag informing the OS whether or not code regions may be mapped with CFI
> 
> mitigations such as IBT or BTI enabled.
> 
> 
> 
> This series covers the remaining parts after the AArch64 specific
> 
> changes were merged:
> 
> 
> 
> - Update the BaseTools to emit the appropriate PE/COFF annotation when a
> 
>   BTI/IBT compatible ELF executable is converted to PE/COFF
> 
> - Take this PE/COFF annotation into account when populating the memory
> 
>   attributes table in the DXE core
> 
> 
> 
> TODO:
> 
> - X64 changes to make the code IBT compatible and emit the ELF note
> 
> - Figure out how to generate such executables with native PE toolchains
> 
> - Implement BTI/IBT enforcement at boot time - this is something I
> 
>   intend to look into next.
> 
> 
> 
> Can be tested with the CLANG38 toolchain (both Clang compiler and LLD
> 
> linker, version 3.8 or newer) with the following build options.
> 
> 
> 
> [BuildOptions]
> 
>   GCC:*_*_AARCH64_PP_FLAGS = -mbranch-protection=bti
> 
>   GCC:*_*_AARCH64_CC_FLAGS = -mbranch-protection=bti
> 
>   GCC:*_*_AARCH64_DLINK_FLAGS = -fuse-ld=lld
> -Wl,--no-relax,--no-pie,-z,bti-report=error
> 
> 
> 
> Changes since v2:
> 
> - increase DllCharacteristicsEx field to 4 bytes
> 
> - add Oliver's Rb
> 
> 
> 
> If no comments or objections have been raised by the end of the week, I
> 
> will go ahead and merge this - thanks.
> 
> 
> 
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> 
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> 
> Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> 
> Cc: Rebecca Cran <quic_rcran@quicinc.com>
> 
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> 
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> 
> Cc: Taylor Beebe <t@taylorbeebe.com>
> 
> Cc: Marvin Häuser <mhaeuser@posteo.de>
> 
> Cc: Bob Feng <bob.c.feng@intel.com>
> 
> Cc: Oliver Smith-Denny <osde@linux.microsoft.com>
> 
> 
> 
> Ard Biesheuvel (4):
> 
>   BaseTools/GenFw: Parse IBT/BTI support status from ELF note
> 
>   BaseTools/GenFw: Add DllCharacteristicsEx field to debug data
> 
>   MdePkg/PeCoffLib: Capture DLL characteristics fields in image context
> 
>   MdeModulePkg: Enable forward edge CFI in mem attributes table
> 
> 
> 
>  BaseTools/Source/C/GenFw/Elf64Convert.c               | 104
> +++++++++++++++++---
> 
>  BaseTools/Source/C/GenFw/GenFw.c                      |   3 +-
> 
>  BaseTools/Source/C/GenFw/elf_common.h                 |   9 ++
> 
>  BaseTools/Source/C/Include/IndustryStandard/PeImage.h |  13 ++-
> 
>  MdeModulePkg/Core/Dxe/DxeMain.h                       |   2 +
> 
>  MdeModulePkg/Core/Dxe/Image/Image.c                   |  10 ++
> 
>  MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c    |   8 +-
> 
>  MdePkg/Include/IndustryStandard/PeImage.h             |  13 ++-
> 
>  MdePkg/Include/Library/PeCoffLib.h                    |   6 ++
> 
>  MdePkg/Library/BasePeCoffLib/BasePeCoff.c             |  46
> ++++++---
> 
>  10 files changed, 186 insertions(+), 28 deletions(-)
> 
> 
> 
> --
> 
> 2.39.2
> 
> 
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#102491):
> https://edk2.groups.io/g/devel/message/102491
> Mute This Topic: https://groups.io/mt/98062730/4905953
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [gaoliming@byosoft.com.cn]
> -=-=-=-=-=-=
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102582): https://edk2.groups.io/g/devel/message/102582
Mute This Topic: https://groups.io/mt/98096794/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 0/4] Enable BTI support in memory attributes table
Posted by Ard Biesheuvel 1 year ago
On Thu, 6 Apr 2023 at 03:34, gaoliming via groups.io
<gaoliming=byosoft.com.cn@groups.io> wrote:
>
> Ard:
>  Can you submit one BZ for this new feature? I will add it into the stable tag feature planning.
>

https://bugzilla.tianocore.org/show_bug.cgi?id=4405

>  For this patch set, Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
>

Thanks, I've queued these up now.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102692): https://edk2.groups.io/g/devel/message/102692
Mute This Topic: https://groups.io/mt/98123487/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-