[edk2] [PATCH V3 0/4] DXE Memory Protection

Jiewen Yao posted 4 patches 7 years, 1 month ago
Failed in applying to current master (apply log)
There is a newer version of this series
ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c              |   3 +-
ArmPkg/Drivers/CpuDxe/Arm/Mmu.c                  |  14 +-
ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c             |   5 +-
ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c |   3 +-
MdeModulePkg/Core/Dxe/DxeMain.h                  |  53 ++
MdeModulePkg/Core/Dxe/DxeMain.inf                |   5 +-
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c          |   3 +-
MdeModulePkg/Core/Dxe/Image/Image.c              |   7 +-
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c    | 735 ++++++++++++++++++
MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c     |  24 +-
MdeModulePkg/MdeModulePkg.dec                    |  10 +
UefiCpuPkg/CpuDxe/CpuDxe.c                       | 141 ++--
UefiCpuPkg/CpuDxe/CpuDxe.inf                     |   5 +-
UefiCpuPkg/CpuDxe/CpuPageTable.c                 | 779 ++++++++++++++++++++
UefiCpuPkg/CpuDxe/CpuPageTable.h                 | 113 +++
15 files changed, 1801 insertions(+), 99 deletions(-)
create mode 100644 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.c
create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.h
[edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Jiewen Yao 7 years, 1 month ago
==== V3 ====
1) Add PCD for policy control (feedback from Ard Biesheuvel)
(Discussed with Mike Kinney)
+  #    BIT0       - Image from unknown device. <BR>
+  #    BIT1       - Image from firmware volume.<BR>
+  # @Prompt Set image protection policy.
+  # @ValidRange 0x80000002 | 0x00000000 - 0x0000001F
+  gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x00000002|UINT32|0x00001047

2) Remove unused function in CpuDxe.(feedback from Liming Gao)
3) Add commit log on link option assumption (feedback from Feng Tian)

==== V2 ====
1) Clean up ArmPkg, (feedback from Leif Lindholm)

==== V1 ====
This series patch provides capability to protect PE/COFF image
in DXE memory.
If the UEFI image is page aligned, the image code section is set to read
only and the image data section is set to non-executable.

The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
the image.

Tested platform: NT32/Quark IA32/OVMF IA32/OVMF IA32X64/Intel internal X64/
Tested OS: UEFI Win10, UEFI Ubuntu 16.04.

Untested platform: ARM/AARCH64.
Can ARM/AARCH64 owner help to take a look and try the ARM platform?


Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>

Jiewen Yao (4):
  UefiCpuPkg/CpuDxe: Add memory attribute setting.
  ArmPkg/CpuDxe: Correct EFI_MEMORY_RO usage
  MdeModulePkg/dec: add PcdImageProtectionPolicy.
  MdeModulePkg/DxeCore: Add UEFI image protection.

 ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c              |   3 +-
 ArmPkg/Drivers/CpuDxe/Arm/Mmu.c                  |  14 +-
 ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c             |   5 +-
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c |   3 +-
 MdeModulePkg/Core/Dxe/DxeMain.h                  |  53 ++
 MdeModulePkg/Core/Dxe/DxeMain.inf                |   5 +-
 MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c          |   3 +-
 MdeModulePkg/Core/Dxe/Image/Image.c              |   7 +-
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c    | 735 ++++++++++++++++++
 MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c     |  24 +-
 MdeModulePkg/MdeModulePkg.dec                    |  10 +
 UefiCpuPkg/CpuDxe/CpuDxe.c                       | 141 ++--
 UefiCpuPkg/CpuDxe/CpuDxe.inf                     |   5 +-
 UefiCpuPkg/CpuDxe/CpuPageTable.c                 | 779 ++++++++++++++++++++
 UefiCpuPkg/CpuDxe/CpuPageTable.h                 | 113 +++
 15 files changed, 1801 insertions(+), 99 deletions(-)
 create mode 100644 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
 create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.c
 create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.h

-- 
2.7.4.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Yao, Jiewen 7 years, 1 month ago
I forget mentioning the V3 update also include below 2 feedback:
=============================
4) Rename file PageTableLib.h/.c to CpuPageTable.h/.c file (from Jeff Fan)
5) Remove multi-entrypoint usage (from Liming Gao/Mike Kinney)
=============================

Thank you
Yao Jiewen

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jiewen
> Yao
> Sent: Wednesday, February 8, 2017 11:20 PM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng,
> Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH V3 0/4] DXE Memory Protection
> 
> ==== V3 ====
> 1) Add PCD for policy control (feedback from Ard Biesheuvel)
> (Discussed with Mike Kinney)
> +  #    BIT0       - Image from unknown device. <BR>
> +  #    BIT1       - Image from firmware volume.<BR>
> +  # @Prompt Set image protection policy.
> +  # @ValidRange 0x80000002 | 0x00000000 - 0x0000001F
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x00000002|UIN
> T32|0x00001047
> 
> 2) Remove unused function in CpuDxe.(feedback from Liming Gao)
> 3) Add commit log on link option assumption (feedback from Feng Tian)
> 
> ==== V2 ====
> 1) Clean up ArmPkg, (feedback from Leif Lindholm)
> 
> ==== V1 ====
> This series patch provides capability to protect PE/COFF image
> in DXE memory.
> If the UEFI image is page aligned, the image code section is set to read
> only and the image data section is set to non-executable.
> 
> The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
> the image.
> 
> Tested platform: NT32/Quark IA32/OVMF IA32/OVMF IA32X64/Intel internal X64/
> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
> 
> Untested platform: ARM/AARCH64.
> Can ARM/AARCH64 owner help to take a look and try the ARM platform?
> 
> 
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> 
> Jiewen Yao (4):
>   UefiCpuPkg/CpuDxe: Add memory attribute setting.
>   ArmPkg/CpuDxe: Correct EFI_MEMORY_RO usage
>   MdeModulePkg/dec: add PcdImageProtectionPolicy.
>   MdeModulePkg/DxeCore: Add UEFI image protection.
> 
>  ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c              |   3 +-
>  ArmPkg/Drivers/CpuDxe/Arm/Mmu.c                  |  14 +-
>  ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c             |   5 +-
>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c |   3 +-
>  MdeModulePkg/Core/Dxe/DxeMain.h                  |  53 ++
>  MdeModulePkg/Core/Dxe/DxeMain.inf                |   5 +-
>  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c          |   3 +-
>  MdeModulePkg/Core/Dxe/Image/Image.c              |   7 +-
>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c    | 735
> ++++++++++++++++++
>  MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c     |  24 +-
>  MdeModulePkg/MdeModulePkg.dec                    |  10 +
>  UefiCpuPkg/CpuDxe/CpuDxe.c                       | 141 ++--
>  UefiCpuPkg/CpuDxe/CpuDxe.inf                     |   5 +-
>  UefiCpuPkg/CpuDxe/CpuPageTable.c                 | 779
> ++++++++++++++++++++
>  UefiCpuPkg/CpuDxe/CpuPageTable.h                 | 113 +++
>  15 files changed, 1801 insertions(+), 99 deletions(-)
>  create mode 100644 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
>  create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.c
>  create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.h
> 
> --
> 2.7.4.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Yao, Jiewen 7 years, 1 month ago
Hi Lindholm/Ard
This version 3 contains both of your feedback before.

If you can do me a favor to evaluated the impact to ARM, that will be great.

Thank you
Yao Jiewen

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jiewen
> Yao
> Sent: Wednesday, February 8, 2017 11:20 PM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng,
> Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH V3 0/4] DXE Memory Protection
> 
> ==== V3 ====
> 1) Add PCD for policy control (feedback from Ard Biesheuvel)
> (Discussed with Mike Kinney)
> +  #    BIT0       - Image from unknown device. <BR>
> +  #    BIT1       - Image from firmware volume.<BR>
> +  # @Prompt Set image protection policy.
> +  # @ValidRange 0x80000002 | 0x00000000 - 0x0000001F
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x00000002|UIN
> T32|0x00001047
> 
> 2) Remove unused function in CpuDxe.(feedback from Liming Gao)
> 3) Add commit log on link option assumption (feedback from Feng Tian)
> 
> ==== V2 ====
> 1) Clean up ArmPkg, (feedback from Leif Lindholm)
> 
> ==== V1 ====
> This series patch provides capability to protect PE/COFF image
> in DXE memory.
> If the UEFI image is page aligned, the image code section is set to read
> only and the image data section is set to non-executable.
> 
> The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
> the image.
> 
> Tested platform: NT32/Quark IA32/OVMF IA32/OVMF IA32X64/Intel internal X64/
> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
> 
> Untested platform: ARM/AARCH64.
> Can ARM/AARCH64 owner help to take a look and try the ARM platform?
> 
> 
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
> 
> Jiewen Yao (4):
>   UefiCpuPkg/CpuDxe: Add memory attribute setting.
>   ArmPkg/CpuDxe: Correct EFI_MEMORY_RO usage
>   MdeModulePkg/dec: add PcdImageProtectionPolicy.
>   MdeModulePkg/DxeCore: Add UEFI image protection.
> 
>  ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c              |   3 +-
>  ArmPkg/Drivers/CpuDxe/Arm/Mmu.c                  |  14 +-
>  ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c             |   5 +-
>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c |   3 +-
>  MdeModulePkg/Core/Dxe/DxeMain.h                  |  53 ++
>  MdeModulePkg/Core/Dxe/DxeMain.inf                |   5 +-
>  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c          |   3 +-
>  MdeModulePkg/Core/Dxe/Image/Image.c              |   7 +-
>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c    | 735
> ++++++++++++++++++
>  MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c     |  24 +-
>  MdeModulePkg/MdeModulePkg.dec                    |  10 +
>  UefiCpuPkg/CpuDxe/CpuDxe.c                       | 141 ++--
>  UefiCpuPkg/CpuDxe/CpuDxe.inf                     |   5 +-
>  UefiCpuPkg/CpuDxe/CpuPageTable.c                 | 779
> ++++++++++++++++++++
>  UefiCpuPkg/CpuDxe/CpuPageTable.h                 | 113 +++
>  15 files changed, 1801 insertions(+), 99 deletions(-)
>  create mode 100644 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
>  create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.c
>  create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.h
> 
> --
> 2.7.4.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Ard Biesheuvel 7 years, 1 month ago
On 9 February 2017 at 07:43, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> Hi Lindholm/Ard
> This version 3 contains both of your feedback before.
>
> If you can do me a favor to evaluated the impact to ARM, that will be great.
>

I will take a look right away.

> Thank you
> Yao Jiewen
>
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jiewen
>> Yao
>> Sent: Wednesday, February 8, 2017 11:20 PM
>> To: edk2-devel@lists.01.org
>> Cc: Tian, Feng <feng.tian@intel.com>; Ard Biesheuvel
>> <ard.biesheuvel@linaro.org>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney,
>> Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng,
>> Star <star.zeng@intel.com>
>> Subject: [edk2] [PATCH V3 0/4] DXE Memory Protection
>>
>> ==== V3 ====
>> 1) Add PCD for policy control (feedback from Ard Biesheuvel)
>> (Discussed with Mike Kinney)
>> +  #    BIT0       - Image from unknown device. <BR>
>> +  #    BIT1       - Image from firmware volume.<BR>
>> +  # @Prompt Set image protection policy.
>> +  # @ValidRange 0x80000002 | 0x00000000 - 0x0000001F
>> +
>> gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x00000002|UIN
>> T32|0x00001047
>>
>> 2) Remove unused function in CpuDxe.(feedback from Liming Gao)
>> 3) Add commit log on link option assumption (feedback from Feng Tian)
>>
>> ==== V2 ====
>> 1) Clean up ArmPkg, (feedback from Leif Lindholm)
>>
>> ==== V1 ====
>> This series patch provides capability to protect PE/COFF image
>> in DXE memory.
>> If the UEFI image is page aligned, the image code section is set to read
>> only and the image data section is set to non-executable.
>>
>> The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
>> the image.
>>
>> Tested platform: NT32/Quark IA32/OVMF IA32/OVMF IA32X64/Intel internal X64/
>> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
>>
>> Untested platform: ARM/AARCH64.
>> Can ARM/AARCH64 owner help to take a look and try the ARM platform?
>>
>>
>> Cc: Jeff Fan <jeff.fan@intel.com>
>> Cc: Michael Kinney <michael.d.kinney@intel.com>
>> Cc: Leif Lindholm <leif.lindholm@linaro.org>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Star Zeng <star.zeng@intel.com>
>> Cc: Feng Tian <feng.tian@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
>>
>> Jiewen Yao (4):
>>   UefiCpuPkg/CpuDxe: Add memory attribute setting.
>>   ArmPkg/CpuDxe: Correct EFI_MEMORY_RO usage
>>   MdeModulePkg/dec: add PcdImageProtectionPolicy.
>>   MdeModulePkg/DxeCore: Add UEFI image protection.
>>
>>  ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c              |   3 +-
>>  ArmPkg/Drivers/CpuDxe/Arm/Mmu.c                  |  14 +-
>>  ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c             |   5 +-
>>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c |   3 +-
>>  MdeModulePkg/Core/Dxe/DxeMain.h                  |  53 ++
>>  MdeModulePkg/Core/Dxe/DxeMain.inf                |   5 +-
>>  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c          |   3 +-
>>  MdeModulePkg/Core/Dxe/Image/Image.c              |   7 +-
>>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c    | 735
>> ++++++++++++++++++
>>  MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c     |  24 +-
>>  MdeModulePkg/MdeModulePkg.dec                    |  10 +
>>  UefiCpuPkg/CpuDxe/CpuDxe.c                       | 141 ++--
>>  UefiCpuPkg/CpuDxe/CpuDxe.inf                     |   5 +-
>>  UefiCpuPkg/CpuDxe/CpuPageTable.c                 | 779
>> ++++++++++++++++++++
>>  UefiCpuPkg/CpuDxe/CpuPageTable.h                 | 113 +++
>>  15 files changed, 1801 insertions(+), 99 deletions(-)
>>  create mode 100644 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
>>  create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.c
>>  create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.h
>>
>> --
>> 2.7.4.windows.1
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Ard Biesheuvel 7 years, 1 month ago
On 9 February 2017 at 08:49, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 9 February 2017 at 07:43, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>> Hi Lindholm/Ard
>> This version 3 contains both of your feedback before.
>>
>> If you can do me a favor to evaluated the impact to ARM, that will be great.
>>
>
> I will take a look right away.
>

I am hitting the following assert as soon as ArmCpuDxe is loaded:

Loading driver at 0x0005BE15000 EntryPoint=0x0005BE16048 ArmCpuDxe.efi
InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 5B142398
ProtectUefiImageCommon - 0x5B142140
  - 0x000000005BE15000 - 0x0000000000010000
InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 5BE23008
InstallProtocolInterface: AD651C7D-3C22-4DBF-92E8-38A7CDAE87B2 5BE230B0
MemoryProtectionCpuArchProtocolNotify:
ProtectUefiImageCommon - 0x5EF02400
  - 0x000000005EEC4000 - 0x0000000000042000
SetUefiImageMemoryAttributes - 0x000000005EEC4000 - 0x0000000000001000
(0x0000000000004000)
EfiAttributeToArmAttribute: 0x4000 attributes is not supported.
ASSERT [ArmCpuDxe]
/home/ard/build/uefi-next/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c(220): 0

The reason appears to be that the GCD memory descriptor retrieved in
SetUefiImageMemoryAttributes() for this region has Attributes == 0

I am digging further ...
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Ard Biesheuvel 7 years, 1 month ago
On 9 February 2017 at 09:09, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 9 February 2017 at 08:49, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 9 February 2017 at 07:43, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>>> Hi Lindholm/Ard
>>> This version 3 contains both of your feedback before.
>>>
>>> If you can do me a favor to evaluated the impact to ARM, that will be great.
>>>
>>
>> I will take a look right away.
>>
>
> I am hitting the following assert as soon as ArmCpuDxe is loaded:
>
> Loading driver at 0x0005BE15000 EntryPoint=0x0005BE16048 ArmCpuDxe.efi
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 5B142398
> ProtectUefiImageCommon - 0x5B142140
>   - 0x000000005BE15000 - 0x0000000000010000
> InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 5BE23008
> InstallProtocolInterface: AD651C7D-3C22-4DBF-92E8-38A7CDAE87B2 5BE230B0
> MemoryProtectionCpuArchProtocolNotify:
> ProtectUefiImageCommon - 0x5EF02400
>   - 0x000000005EEC4000 - 0x0000000000042000
> SetUefiImageMemoryAttributes - 0x000000005EEC4000 - 0x0000000000001000
> (0x0000000000004000)
> EfiAttributeToArmAttribute: 0x4000 attributes is not supported.
> ASSERT [ArmCpuDxe]
> /home/ard/build/uefi-next/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c(220): 0
>
> The reason appears to be that the GCD memory descriptor retrieved in
> SetUefiImageMemoryAttributes() for this region has Attributes == 0
>
> I am digging further ...

Attached is the output of DEBUG_GCD
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Yao, Jiewen 7 years, 1 month ago
Thank you Ard. I check the ARM code again.

It seems the ARM CPU driver is similar as X86 CPU driver - it installs CpuArch protocol, then SyncCacheConfig.
========================
  Status = gBS->InstallMultipleProtocolInterfaces (
                &mCpuHandle,
                &gEfiCpuArchProtocolGuid,           &mCpu,
                &gVirtualUncachedPagesProtocolGuid, &gVirtualUncachedPages,
                NULL
                );

  //
  // Make sure GCD and MMU settings match. This API calls gDS->SetMemorySpaceAttributes ()
  // and that calls EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes, so this code needs to go
  // after the protocol is installed
  //
  SyncCacheConfig (&mCpu);
========================

In my update, the DxeCore registers a CPU ARCH callback function to setup PE code/data section attribute there.
At that time, the GCD attribute is not ready. As such, the cache attribute is all 0.

After SyncCacheConfig(), the rest attribute setting will carry expected GCD update.

Here is my thought:

1)       How about I set default PcdImageProtectionPolicy to be 0 for ARM in DEC file? As such the ARM platform is not impacted.

2)       At same time, someone from ARM can help to enhance the CPU driver to make it work.
In my opinion, ASSERT() here is not the best idea. According to PI spec V2, 12.3, EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes(), the EFI_UNSUPPORTED is expected.



Just for your reference, below is the OVMF log for X86 CPU:
It sets page attribute only before GCD sync, then set both page attribute and cache attributes after GCD sync.
============================
Loading driver at 0x00007605000 EntryPoint=0x000076066E9 CpuDxe.efi
InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 70D9410
ProtectUefiImageCommon - 0x70D91A8
  - 0x0000000007605000 - 0x000000000000E000
CurrentPagingContext:
  MachineType   - 0x14C
  PageTableBase - 0x76A9000
  Attributes    - 0xC0000002
InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 7611128
MemoryProtectionCpuArchProtocolNotify: ImageRecordCount - 0xA
SetUefiImageMemoryAttributes - 0x00000000076CE000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x76CE023->0x80000000076CE023
SetUefiImageMemoryAttributes - 0x00000000076CF000 - 0x0000000000016000 (0x0000000000020000)
ConvertPageEntryAttribute 0x76CF023->0x76CF021
ConvertPageEntryAttribute 0x76D0023->0x76D0021
ConvertPageEntryAttribute 0x76D1023->0x76D1021
ConvertPageEntryAttribute 0x76D2023->0x76D2021
ConvertPageEntryAttribute 0x76D3023->0x76D3021
ConvertPageEntryAttribute 0x76D4023->0x76D4021
ConvertPageEntryAttribute 0x76D5023->0x76D5021
ConvertPageEntryAttribute 0x76D6023->0x76D6021
ConvertPageEntryAttribute 0x76D7023->0x76D7021
ConvertPageEntryAttribute 0x76D8023->0x76D8021
ConvertPageEntryAttribute 0x76D9023->0x76D9021
ConvertPageEntryAttribute 0x76DA023->0x76DA021
ConvertPageEntryAttribute 0x76DB023->0x76DB021
ConvertPageEntryAttribute 0x76DC023->0x76DC021
ConvertPageEntryAttribute 0x76DD023->0x76DD021
ConvertPageEntryAttribute 0x76DE023->0x76DE021
ConvertPageEntryAttribute 0x76DF023->0x76DF021
ConvertPageEntryAttribute 0x76E0023->0x76E0021
ConvertPageEntryAttribute 0x76E1023->0x76E1021
ConvertPageEntryAttribute 0x76E2003->0x76E2001
ConvertPageEntryAttribute 0x76E3023->0x76E3021
ConvertPageEntryAttribute 0x76E4023->0x76E4021
SetUefiImageMemoryAttributes - 0x00000000076E5000 - 0x000000000000B000 (0x0000000000004000)
ConvertPageEntryAttribute 0x76E5023->0x80000000076E5023
ConvertPageEntryAttribute 0x76E6023->0x80000000076E6023
ConvertPageEntryAttribute 0x76E7023->0x80000000076E7023
ConvertPageEntryAttribute 0x76E8023->0x80000000076E8023
ConvertPageEntryAttribute 0x76E9023->0x80000000076E9023
ConvertPageEntryAttribute 0x76EA023->0x80000000076EA023
ConvertPageEntryAttribute 0x76EB063->0x80000000076EB063
ConvertPageEntryAttribute 0x76EC063->0x80000000076EC063
ConvertPageEntryAttribute 0x76ED063->0x80000000076ED063
ConvertPageEntryAttribute 0x76EE003->0x80000000076EE003
ConvertPageEntryAttribute 0x76EF003->0x80000000076EF003
SetUefiImageMemoryAttributes - 0x0000000007637000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7637063->0x8000000007637063
SetUefiImageMemoryAttributes - 0x0000000007638000 - 0x0000000000006000 (0x0000000000020000)
ConvertPageEntryAttribute 0x7638063->0x7638061
ConvertPageEntryAttribute 0x7639063->0x7639061
ConvertPageEntryAttribute 0x763A063->0x763A061
ConvertPageEntryAttribute 0x763B063->0x763B061
ConvertPageEntryAttribute 0x763C063->0x763C061
ConvertPageEntryAttribute 0x763D063->0x763D061
SetUefiImageMemoryAttributes - 0x000000000763E000 - 0x0000000000005000 (0x0000000000004000)
ConvertPageEntryAttribute 0x763E063->0x800000000763E063
ConvertPageEntryAttribute 0x763F063->0x800000000763F063
ConvertPageEntryAttribute 0x7640063->0x8000000007640063
ConvertPageEntryAttribute 0x7641063->0x8000000007641063
ConvertPageEntryAttribute 0x7642063->0x8000000007642063
SetUefiImageMemoryAttributes - 0x000000000762E000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x762E063->0x800000000762E063
SetUefiImageMemoryAttributes - 0x000000000762F000 - 0x0000000000004000 (0x0000000000020000)
ConvertPageEntryAttribute 0x762F063->0x762F061
ConvertPageEntryAttribute 0x7630063->0x7630061
ConvertPageEntryAttribute 0x7631063->0x7631061
ConvertPageEntryAttribute 0x7632063->0x7632061
SetUefiImageMemoryAttributes - 0x0000000007633000 - 0x0000000000004000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7633063->0x8000000007633063
ConvertPageEntryAttribute 0x7634063->0x8000000007634063
ConvertPageEntryAttribute 0x7635063->0x8000000007635063
ConvertPageEntryAttribute 0x7636063->0x8000000007636063
SetUefiImageMemoryAttributes - 0x000000000766C000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x766C063->0x800000000766C063
SetUefiImageMemoryAttributes - 0x000000000766D000 - 0x0000000000002000 (0x0000000000020000)
ConvertPageEntryAttribute 0x766D063->0x766D061
ConvertPageEntryAttribute 0x766E063->0x766E061
SetUefiImageMemoryAttributes - 0x000000000766F000 - 0x0000000000004000 (0x0000000000004000)
ConvertPageEntryAttribute 0x766F063->0x800000000766F063
ConvertPageEntryAttribute 0x7670063->0x8000000007670063
ConvertPageEntryAttribute 0x7671063->0x8000000007671063
ConvertPageEntryAttribute 0x7672063->0x8000000007672063
SetUefiImageMemoryAttributes - 0x0000000007666000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7666063->0x8000000007666063
SetUefiImageMemoryAttributes - 0x0000000007667000 - 0x0000000000002000 (0x0000000000020000)
ConvertPageEntryAttribute 0x7667063->0x7667061
ConvertPageEntryAttribute 0x7668063->0x7668061
SetUefiImageMemoryAttributes - 0x0000000007669000 - 0x0000000000003000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7669063->0x8000000007669063
ConvertPageEntryAttribute 0x766A063->0x800000000766A063
ConvertPageEntryAttribute 0x766B063->0x800000000766B063
SetUefiImageMemoryAttributes - 0x0000000007627000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7627063->0x8000000007627063
SetUefiImageMemoryAttributes - 0x0000000007628000 - 0x0000000000002000 (0x0000000000020000)
ConvertPageEntryAttribute 0x7628063->0x7628061
ConvertPageEntryAttribute 0x7629063->0x7629061
SetUefiImageMemoryAttributes - 0x000000000762A000 - 0x0000000000004000 (0x0000000000004000)
ConvertPageEntryAttribute 0x762A063->0x800000000762A063
ConvertPageEntryAttribute 0x762B063->0x800000000762B063
ConvertPageEntryAttribute 0x762C063->0x800000000762C063
ConvertPageEntryAttribute 0x762D063->0x800000000762D063
SetUefiImageMemoryAttributes - 0x000000000761F000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x761F063->0x800000000761F063
SetUefiImageMemoryAttributes - 0x0000000007620000 - 0x0000000000004000 (0x0000000000020000)
ConvertPageEntryAttribute 0x7620063->0x7620061
ConvertPageEntryAttribute 0x7621063->0x7621061
ConvertPageEntryAttribute 0x7622063->0x7622061
ConvertPageEntryAttribute 0x7623063->0x7623061
SetUefiImageMemoryAttributes - 0x0000000007624000 - 0x0000000000003000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7624063->0x8000000007624063
ConvertPageEntryAttribute 0x7625063->0x8000000007625063
ConvertPageEntryAttribute 0x7626063->0x8000000007626063
SetUefiImageMemoryAttributes - 0x0000000007619000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7619063->0x8000000007619063
SetUefiImageMemoryAttributes - 0x000000000761A000 - 0x0000000000002000 (0x0000000000020000)
ConvertPageEntryAttribute 0x761A063->0x761A061
ConvertPageEntryAttribute 0x761B063->0x761B061
SetUefiImageMemoryAttributes - 0x000000000761C000 - 0x0000000000003000 (0x0000000000004000)
ConvertPageEntryAttribute 0x761C063->0x800000000761C063
ConvertPageEntryAttribute 0x761D063->0x800000000761D063
ConvertPageEntryAttribute 0x761E063->0x800000000761E063
SetUefiImageMemoryAttributes - 0x0000000007613000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7613063->0x8000000007613063
SetUefiImageMemoryAttributes - 0x0000000007614000 - 0x0000000000002000 (0x0000000000020000)
ConvertPageEntryAttribute 0x7614063->0x7614061
ConvertPageEntryAttribute 0x7615063->0x7615061
SetUefiImageMemoryAttributes - 0x0000000007616000 - 0x0000000000003000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7616063->0x8000000007616063
ConvertPageEntryAttribute 0x7617063->0x8000000007617063
ConvertPageEntryAttribute 0x7618063->0x8000000007618063
SetUefiImageMemoryAttributes - 0x0000000007605000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7605063->0x8000000007605063
SetUefiImageMemoryAttributes - 0x0000000007606000 - 0x0000000000008000 (0x0000000000020000)
ConvertPageEntryAttribute 0x7606063->0x7606061
ConvertPageEntryAttribute 0x7607063->0x7607061
ConvertPageEntryAttribute 0x7608063->0x7608061
ConvertPageEntryAttribute 0x7609063->0x7609061
ConvertPageEntryAttribute 0x760A063->0x760A061
ConvertPageEntryAttribute 0x760B063->0x760B061
ConvertPageEntryAttribute 0x760C063->0x760C061
ConvertPageEntryAttribute 0x760D063->0x760D061
SetUefiImageMemoryAttributes - 0x000000000760E000 - 0x0000000000005000 (0x0000000000004000)
ConvertPageEntryAttribute 0x760E063->0x800000000760E063
ConvertPageEntryAttribute 0x760F063->0x800000000760F063
ConvertPageEntryAttribute 0x7610063->0x8000000007610063
ConvertPageEntryAttribute 0x7611063->0x8000000007611063
ConvertPageEntryAttribute 0x7612063->0x8000000007612063
  Flushing GCD
  Flushing GCD
  Flushing GCD
  Flushing GCD
  Flushing GCD
  Flushing GCD
  Flushing GCD
  Flushing GCD
  Flushing GCD
  Flushing GCD
  Flushing GCD
AP Loop Mode is 1
Detect CPU count: 4
InstallProtocolInterface: 3FDDA605-A76E-4F46-AD29-12F4531B3D08 76111D4
Loading driver F6697AC4-A776-4EE1-B643-1FEFF2B615BB
InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 70D8CA8
Loading driver at 0x000075FF000 EntryPoint=0x000076000B5 IncompatiblePciDeviceSupportDxe.efi
InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 70D9010
ProtectUefiImageCommon - 0x70D8CA8
  - 0x00000000075FF000 - 0x0000000000006000
SetUefiImageMemoryAttributes - 0x00000000075FF000 - 0x0000000000001000 (0x0000000000004008)
Split - 0x6CA7000
ConvertPageEntryAttribute 0x75FF063->0x80000000075FF063
SetUefiImageMemoryAttributes - 0x0000000007600000 - 0x0000000000002000 (0x0000000000020008)
ConvertPageEntryAttribute 0x7600063->0x7600061
ConvertPageEntryAttribute 0x7601063->0x7601061
SetUefiImageMemoryAttributes - 0x0000000007602000 - 0x0000000000003000 (0x0000000000004008)
ConvertPageEntryAttribute 0x7602063->0x8000000007602063
ConvertPageEntryAttribute 0x7603063->0x8000000007603063
ConvertPageEntryAttribute 0x7604063->0x8000000007604063
============================





From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Thursday, February 9, 2017 1:22 AM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection

On 9 February 2017 at 09:09, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> wrote:
> On 9 February 2017 at 08:49, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> wrote:
>> On 9 February 2017 at 07:43, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
>>> Hi Lindholm/Ard
>>> This version 3 contains both of your feedback before.
>>>
>>> If you can do me a favor to evaluated the impact to ARM, that will be great.
>>>
>>
>> I will take a look right away.
>>
>
> I am hitting the following assert as soon as ArmCpuDxe is loaded:
>
> Loading driver at 0x0005BE15000 EntryPoint=0x0005BE16048 ArmCpuDxe.efi
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 5B142398
> ProtectUefiImageCommon - 0x5B142140
>   - 0x000000005BE15000 - 0x0000000000010000
> InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 5BE23008
> InstallProtocolInterface: AD651C7D-3C22-4DBF-92E8-38A7CDAE87B2 5BE230B0
> MemoryProtectionCpuArchProtocolNotify:
> ProtectUefiImageCommon - 0x5EF02400
>   - 0x000000005EEC4000 - 0x0000000000042000
> SetUefiImageMemoryAttributes - 0x000000005EEC4000 - 0x0000000000001000
> (0x0000000000004000)
> EfiAttributeToArmAttribute: 0x4000 attributes is not supported.
> ASSERT [ArmCpuDxe]
> /home/ard/build/uefi-next/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c(220): 0
>
> The reason appears to be that the GCD memory descriptor retrieved in
> SetUefiImageMemoryAttributes() for this region has Attributes == 0
>
> I am digging further ...

Attached is the output of DEBUG_GCD
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Ard Biesheuvel 7 years, 1 month ago
On 9 February 2017 at 13:19, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> Thank you Ard. I check the ARM code again.
>
>
>
> It seems the ARM CPU driver is similar as X86 CPU driver – it installs
> CpuArch protocol, then SyncCacheConfig.
>
> ========================
>
>   Status = gBS->InstallMultipleProtocolInterfaces (
>
>                 &mCpuHandle,
>
>                 &gEfiCpuArchProtocolGuid,           &mCpu,
>
>                 &gVirtualUncachedPagesProtocolGuid, &gVirtualUncachedPages,
>
>                 NULL
>
>                 );
>
>
>
>   //
>
>   // Make sure GCD and MMU settings match. This API calls
> gDS->SetMemorySpaceAttributes ()
>
>   // and that calls EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes, so this code
> needs to go
>
>   // after the protocol is installed
>
>   //
>
>   SyncCacheConfig (&mCpu);
>
> ========================
>
>


Thanks a lot for taking the time to look at this.

>
> In my update, the DxeCore registers a CPU ARCH callback function to setup PE
> code/data section attribute there.
>
> At that time, the GCD attribute is not ready. As such, the cache attribute
> is all 0.
>
>
>
> After SyncCacheConfig(), the rest attribute setting will carry expected GCD
> update.
>

So how does x86 deal with this situation if it follows the same basic approach?

>
>
> Here is my thought:
>
> 1)       How about I set default PcdImageProtectionPolicy to be 0 for ARM in
> DEC file? As such the ARM platform is not impacted.
>
> 2)       At same time, someone from ARM can help to enhance the CPU driver
> to make it work.
>


> In my opinion, ASSERT() here is not the best idea. According to PI spec V2,
> 12.3, EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes(), the EFI_UNSUPPORTED is
> expected.
>
>
>
>
>
>
>
> Just for your reference, below is the OVMF log for X86 CPU:
>
> It sets page attribute only before GCD sync, then set both page attribute
> and cache attributes after GCD sync.
>
> ============================
>
> Loading driver at 0x00007605000 EntryPoint=0x000076066E9 CpuDxe.efi
>
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 70D9410
>
> ProtectUefiImageCommon - 0x70D91A8
>
>   - 0x0000000007605000 - 0x000000000000E000
>
> CurrentPagingContext:
>
>   MachineType   - 0x14C
>
>   PageTableBase - 0x76A9000
>
>   Attributes    - 0xC0000002
>
> InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 7611128
>
> MemoryProtectionCpuArchProtocolNotify: ImageRecordCount - 0xA
>
> SetUefiImageMemoryAttributes - 0x00000000076CE000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x76CE023->0x80000000076CE023
>
> SetUefiImageMemoryAttributes - 0x00000000076CF000 - 0x0000000000016000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x76CF023->0x76CF021
>
> ConvertPageEntryAttribute 0x76D0023->0x76D0021
>
> ConvertPageEntryAttribute 0x76D1023->0x76D1021
>
> ConvertPageEntryAttribute 0x76D2023->0x76D2021
>
> ConvertPageEntryAttribute 0x76D3023->0x76D3021
>
> ConvertPageEntryAttribute 0x76D4023->0x76D4021
>
> ConvertPageEntryAttribute 0x76D5023->0x76D5021
>
> ConvertPageEntryAttribute 0x76D6023->0x76D6021
>
> ConvertPageEntryAttribute 0x76D7023->0x76D7021
>
> ConvertPageEntryAttribute 0x76D8023->0x76D8021
>
> ConvertPageEntryAttribute 0x76D9023->0x76D9021
>
> ConvertPageEntryAttribute 0x76DA023->0x76DA021
>
> ConvertPageEntryAttribute 0x76DB023->0x76DB021
>
> ConvertPageEntryAttribute 0x76DC023->0x76DC021
>
> ConvertPageEntryAttribute 0x76DD023->0x76DD021
>
> ConvertPageEntryAttribute 0x76DE023->0x76DE021
>
> ConvertPageEntryAttribute 0x76DF023->0x76DF021
>
> ConvertPageEntryAttribute 0x76E0023->0x76E0021
>
> ConvertPageEntryAttribute 0x76E1023->0x76E1021
>
> ConvertPageEntryAttribute 0x76E2003->0x76E2001
>
> ConvertPageEntryAttribute 0x76E3023->0x76E3021
>
> ConvertPageEntryAttribute 0x76E4023->0x76E4021
>
> SetUefiImageMemoryAttributes - 0x00000000076E5000 - 0x000000000000B000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x76E5023->0x80000000076E5023
>
> ConvertPageEntryAttribute 0x76E6023->0x80000000076E6023
>
> ConvertPageEntryAttribute 0x76E7023->0x80000000076E7023
>
> ConvertPageEntryAttribute 0x76E8023->0x80000000076E8023
>
> ConvertPageEntryAttribute 0x76E9023->0x80000000076E9023
>
> ConvertPageEntryAttribute 0x76EA023->0x80000000076EA023
>
> ConvertPageEntryAttribute 0x76EB063->0x80000000076EB063
>
> ConvertPageEntryAttribute 0x76EC063->0x80000000076EC063
>
> ConvertPageEntryAttribute 0x76ED063->0x80000000076ED063
>
> ConvertPageEntryAttribute 0x76EE003->0x80000000076EE003
>
> ConvertPageEntryAttribute 0x76EF003->0x80000000076EF003
>
> SetUefiImageMemoryAttributes - 0x0000000007637000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7637063->0x8000000007637063
>
> SetUefiImageMemoryAttributes - 0x0000000007638000 - 0x0000000000006000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7638063->0x7638061
>
> ConvertPageEntryAttribute 0x7639063->0x7639061
>
> ConvertPageEntryAttribute 0x763A063->0x763A061
>
> ConvertPageEntryAttribute 0x763B063->0x763B061
>
> ConvertPageEntryAttribute 0x763C063->0x763C061
>
> ConvertPageEntryAttribute 0x763D063->0x763D061
>
> SetUefiImageMemoryAttributes - 0x000000000763E000 - 0x0000000000005000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x763E063->0x800000000763E063
>
> ConvertPageEntryAttribute 0x763F063->0x800000000763F063
>
> ConvertPageEntryAttribute 0x7640063->0x8000000007640063
>
> ConvertPageEntryAttribute 0x7641063->0x8000000007641063
>
> ConvertPageEntryAttribute 0x7642063->0x8000000007642063
>
> SetUefiImageMemoryAttributes - 0x000000000762E000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x762E063->0x800000000762E063
>
> SetUefiImageMemoryAttributes - 0x000000000762F000 - 0x0000000000004000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x762F063->0x762F061
>
> ConvertPageEntryAttribute 0x7630063->0x7630061
>
> ConvertPageEntryAttribute 0x7631063->0x7631061
>
> ConvertPageEntryAttribute 0x7632063->0x7632061
>
> SetUefiImageMemoryAttributes - 0x0000000007633000 - 0x0000000000004000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7633063->0x8000000007633063
>
> ConvertPageEntryAttribute 0x7634063->0x8000000007634063
>
> ConvertPageEntryAttribute 0x7635063->0x8000000007635063
>
> ConvertPageEntryAttribute 0x7636063->0x8000000007636063
>
> SetUefiImageMemoryAttributes - 0x000000000766C000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x766C063->0x800000000766C063
>
> SetUefiImageMemoryAttributes - 0x000000000766D000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x766D063->0x766D061
>
> ConvertPageEntryAttribute 0x766E063->0x766E061
>
> SetUefiImageMemoryAttributes - 0x000000000766F000 - 0x0000000000004000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x766F063->0x800000000766F063
>
> ConvertPageEntryAttribute 0x7670063->0x8000000007670063
>
> ConvertPageEntryAttribute 0x7671063->0x8000000007671063
>
> ConvertPageEntryAttribute 0x7672063->0x8000000007672063
>
> SetUefiImageMemoryAttributes - 0x0000000007666000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7666063->0x8000000007666063
>
> SetUefiImageMemoryAttributes - 0x0000000007667000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7667063->0x7667061
>
> ConvertPageEntryAttribute 0x7668063->0x7668061
>
> SetUefiImageMemoryAttributes - 0x0000000007669000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7669063->0x8000000007669063
>
> ConvertPageEntryAttribute 0x766A063->0x800000000766A063
>
> ConvertPageEntryAttribute 0x766B063->0x800000000766B063
>
> SetUefiImageMemoryAttributes - 0x0000000007627000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7627063->0x8000000007627063
>
> SetUefiImageMemoryAttributes - 0x0000000007628000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7628063->0x7628061
>
> ConvertPageEntryAttribute 0x7629063->0x7629061
>
> SetUefiImageMemoryAttributes - 0x000000000762A000 - 0x0000000000004000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x762A063->0x800000000762A063
>
> ConvertPageEntryAttribute 0x762B063->0x800000000762B063
>
> ConvertPageEntryAttribute 0x762C063->0x800000000762C063
>
> ConvertPageEntryAttribute 0x762D063->0x800000000762D063
>
> SetUefiImageMemoryAttributes - 0x000000000761F000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x761F063->0x800000000761F063
>
> SetUefiImageMemoryAttributes - 0x0000000007620000 - 0x0000000000004000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7620063->0x7620061
>
> ConvertPageEntryAttribute 0x7621063->0x7621061
>
> ConvertPageEntryAttribute 0x7622063->0x7622061
>
> ConvertPageEntryAttribute 0x7623063->0x7623061
>
> SetUefiImageMemoryAttributes - 0x0000000007624000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7624063->0x8000000007624063
>
> ConvertPageEntryAttribute 0x7625063->0x8000000007625063
>
> ConvertPageEntryAttribute 0x7626063->0x8000000007626063
>
> SetUefiImageMemoryAttributes - 0x0000000007619000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7619063->0x8000000007619063
>
> SetUefiImageMemoryAttributes - 0x000000000761A000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x761A063->0x761A061
>
> ConvertPageEntryAttribute 0x761B063->0x761B061
>
> SetUefiImageMemoryAttributes - 0x000000000761C000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x761C063->0x800000000761C063
>
> ConvertPageEntryAttribute 0x761D063->0x800000000761D063
>
> ConvertPageEntryAttribute 0x761E063->0x800000000761E063
>
> SetUefiImageMemoryAttributes - 0x0000000007613000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7613063->0x8000000007613063
>
> SetUefiImageMemoryAttributes - 0x0000000007614000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7614063->0x7614061
>
> ConvertPageEntryAttribute 0x7615063->0x7615061
>
> SetUefiImageMemoryAttributes - 0x0000000007616000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7616063->0x8000000007616063
>
> ConvertPageEntryAttribute 0x7617063->0x8000000007617063
>
> ConvertPageEntryAttribute 0x7618063->0x8000000007618063
>
> SetUefiImageMemoryAttributes - 0x0000000007605000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7605063->0x8000000007605063
>
> SetUefiImageMemoryAttributes - 0x0000000007606000 - 0x0000000000008000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7606063->0x7606061
>
> ConvertPageEntryAttribute 0x7607063->0x7607061
>
> ConvertPageEntryAttribute 0x7608063->0x7608061
>
> ConvertPageEntryAttribute 0x7609063->0x7609061
>
> ConvertPageEntryAttribute 0x760A063->0x760A061
>
> ConvertPageEntryAttribute 0x760B063->0x760B061
>
> ConvertPageEntryAttribute 0x760C063->0x760C061
>
> ConvertPageEntryAttribute 0x760D063->0x760D061
>
> SetUefiImageMemoryAttributes - 0x000000000760E000 - 0x0000000000005000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x760E063->0x800000000760E063
>
> ConvertPageEntryAttribute 0x760F063->0x800000000760F063
>
> ConvertPageEntryAttribute 0x7610063->0x8000000007610063
>
> ConvertPageEntryAttribute 0x7611063->0x8000000007611063
>
> ConvertPageEntryAttribute 0x7612063->0x8000000007612063
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
> AP Loop Mode is 1
>
> Detect CPU count: 4
>
> InstallProtocolInterface: 3FDDA605-A76E-4F46-AD29-12F4531B3D08 76111D4
>
> Loading driver F6697AC4-A776-4EE1-B643-1FEFF2B615BB
>
> InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 70D8CA8
>
> Loading driver at 0x000075FF000 EntryPoint=0x000076000B5
> IncompatiblePciDeviceSupportDxe.efi
>
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 70D9010
>
> ProtectUefiImageCommon - 0x70D8CA8
>
>   - 0x00000000075FF000 - 0x0000000000006000
>
> SetUefiImageMemoryAttributes - 0x00000000075FF000 - 0x0000000000001000
> (0x0000000000004008)
>
> Split - 0x6CA7000
>
> ConvertPageEntryAttribute 0x75FF063->0x80000000075FF063
>
> SetUefiImageMemoryAttributes - 0x0000000007600000 - 0x0000000000002000
> (0x0000000000020008)
>
> ConvertPageEntryAttribute 0x7600063->0x7600061
>
> ConvertPageEntryAttribute 0x7601063->0x7601061
>
> SetUefiImageMemoryAttributes - 0x0000000007602000 - 0x0000000000003000
> (0x0000000000004008)
>
> ConvertPageEntryAttribute 0x7602063->0x8000000007602063
>
> ConvertPageEntryAttribute 0x7603063->0x8000000007603063
>
> ConvertPageEntryAttribute 0x7604063->0x8000000007604063
>
> ============================
>
>
>
>
>
>
>
>
>
>
>
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard
> Biesheuvel
> Sent: Thursday, February 9, 2017 1:22 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm
> <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
>
>
>
> On 9 February 2017 at 09:09, Ard Biesheuvel <ard.biesheuvel@linaro.org>
> wrote:
>
>> On 9 February 2017 at 08:49, Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> wrote:
>>> On 9 February 2017 at 07:43, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>>>> Hi Lindholm/Ard
>>>> This version 3 contains both of your feedback before.
>>>>
>>>> If you can do me a favor to evaluated the impact to ARM, that will be
>>>> great.
>>>>
>>>
>>> I will take a look right away.
>>>
>>
>> I am hitting the following assert as soon as ArmCpuDxe is loaded:
>>
>> Loading driver at 0x0005BE15000 EntryPoint=0x0005BE16048 ArmCpuDxe.efi
>> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 5B142398
>> ProtectUefiImageCommon - 0x5B142140
>>   - 0x000000005BE15000 - 0x0000000000010000
>> InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 5BE23008
>> InstallProtocolInterface: AD651C7D-3C22-4DBF-92E8-38A7CDAE87B2 5BE230B0
>> MemoryProtectionCpuArchProtocolNotify:
>> ProtectUefiImageCommon - 0x5EF02400
>>   - 0x000000005EEC4000 - 0x0000000000042000
>> SetUefiImageMemoryAttributes - 0x000000005EEC4000 - 0x0000000000001000
>> (0x0000000000004000)
>> EfiAttributeToArmAttribute: 0x4000 attributes is not supported.
>> ASSERT [ArmCpuDxe]
>> /home/ard/build/uefi-next/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c(220): 0
>>
>> The reason appears to be that the GCD memory descriptor retrieved in
>> SetUefiImageMemoryAttributes() for this region has Attributes == 0
>>
>> I am digging further ...
>
> Attached is the output of DEBUG_GCD
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Yao, Jiewen 7 years, 1 month ago
For X86 CPU, the memory protection attribute goes to page table, the cache attribute goes to MTRR register.
Those are 2 difference resource, and can be set separately.

The high level pseudo code is below:
=======================
CacheAttribute = Attribute & CACHE_ATTRIBUTE_MASK
MemoryProtectionAttribute = Attribute & MEMORY_PROTECTION_ATTRIBUTE_MASK
If (CacheAttribute != 0) {
     SetCacheAttribute(CacheAttribute)
}
SetMemoryProtectionAttribute(MemoryProtectionAttribute)
=======================

NOTE: we need compare CacheAttribute == 0, because the cache attribute is an individual mask. 0x1 means UN_CACHE, 0x8 means WRITE_BACK. 0 is meaningless.
We do not compare MemoryProtectionAttribute == 0, because 0 is a valid memory protection attribute, which means to disable protection.

Before GCD sync, the DxeCore does not know the cache attribute, so that it can only set memory attribute. The CPU driver only modifies page table based upon MemoryProtectionAttribute and keep cache attribute untouched.


Thank you
Yao Jiewen

From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Thursday, February 9, 2017 5:52 AM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection

On 9 February 2017 at 13:19, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
> Thank you Ard. I check the ARM code again.
>
>
>
> It seems the ARM CPU driver is similar as X86 CPU driver – it installs
> CpuArch protocol, then SyncCacheConfig.
>
> ========================
>
>   Status = gBS->InstallMultipleProtocolInterfaces (
>
>                 &mCpuHandle,
>
>                 &gEfiCpuArchProtocolGuid,           &mCpu,
>
>                 &gVirtualUncachedPagesProtocolGuid, &gVirtualUncachedPages,
>
>                 NULL
>
>                 );
>
>
>
>   //
>
>   // Make sure GCD and MMU settings match. This API calls
> gDS->SetMemorySpaceAttributes ()
>
>   // and that calls EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes, so this code
> needs to go
>
>   // after the protocol is installed
>
>   //
>
>   SyncCacheConfig (&mCpu);
>
> ========================
>
>


Thanks a lot for taking the time to look at this.

>
> In my update, the DxeCore registers a CPU ARCH callback function to setup PE
> code/data section attribute there.
>
> At that time, the GCD attribute is not ready. As such, the cache attribute
> is all 0.
>
>
>
> After SyncCacheConfig(), the rest attribute setting will carry expected GCD
> update.
>

So how does x86 deal with this situation if it follows the same basic approach?

>
>
> Here is my thought:
>
> 1)       How about I set default PcdImageProtectionPolicy to be 0 for ARM in
> DEC file? As such the ARM platform is not impacted.
>
> 2)       At same time, someone from ARM can help to enhance the CPU driver
> to make it work.
>


> In my opinion, ASSERT() here is not the best idea. According to PI spec V2,
> 12.3, EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes(), the EFI_UNSUPPORTED is
> expected.
>
>
>
>
>
>
>
> Just for your reference, below is the OVMF log for X86 CPU:
>
> It sets page attribute only before GCD sync, then set both page attribute
> and cache attributes after GCD sync.
>
> ============================
>
> Loading driver at 0x00007605000 EntryPoint=0x000076066E9 CpuDxe.efi
>
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 70D9410
>
> ProtectUefiImageCommon - 0x70D91A8
>
>   - 0x0000000007605000 - 0x000000000000E000
>
> CurrentPagingContext:
>
>   MachineType   - 0x14C
>
>   PageTableBase - 0x76A9000
>
>   Attributes    - 0xC0000002
>
> InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 7611128
>
> MemoryProtectionCpuArchProtocolNotify: ImageRecordCount - 0xA
>
> SetUefiImageMemoryAttributes - 0x00000000076CE000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x76CE023->0x80000000076CE023
>
> SetUefiImageMemoryAttributes - 0x00000000076CF000 - 0x0000000000016000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x76CF023->0x76CF021
>
> ConvertPageEntryAttribute 0x76D0023->0x76D0021
>
> ConvertPageEntryAttribute 0x76D1023->0x76D1021
>
> ConvertPageEntryAttribute 0x76D2023->0x76D2021
>
> ConvertPageEntryAttribute 0x76D3023->0x76D3021
>
> ConvertPageEntryAttribute 0x76D4023->0x76D4021
>
> ConvertPageEntryAttribute 0x76D5023->0x76D5021
>
> ConvertPageEntryAttribute 0x76D6023->0x76D6021
>
> ConvertPageEntryAttribute 0x76D7023->0x76D7021
>
> ConvertPageEntryAttribute 0x76D8023->0x76D8021
>
> ConvertPageEntryAttribute 0x76D9023->0x76D9021
>
> ConvertPageEntryAttribute 0x76DA023->0x76DA021
>
> ConvertPageEntryAttribute 0x76DB023->0x76DB021
>
> ConvertPageEntryAttribute 0x76DC023->0x76DC021
>
> ConvertPageEntryAttribute 0x76DD023->0x76DD021
>
> ConvertPageEntryAttribute 0x76DE023->0x76DE021
>
> ConvertPageEntryAttribute 0x76DF023->0x76DF021
>
> ConvertPageEntryAttribute 0x76E0023->0x76E0021
>
> ConvertPageEntryAttribute 0x76E1023->0x76E1021
>
> ConvertPageEntryAttribute 0x76E2003->0x76E2001
>
> ConvertPageEntryAttribute 0x76E3023->0x76E3021
>
> ConvertPageEntryAttribute 0x76E4023->0x76E4021
>
> SetUefiImageMemoryAttributes - 0x00000000076E5000 - 0x000000000000B000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x76E5023->0x80000000076E5023
>
> ConvertPageEntryAttribute 0x76E6023->0x80000000076E6023
>
> ConvertPageEntryAttribute 0x76E7023->0x80000000076E7023
>
> ConvertPageEntryAttribute 0x76E8023->0x80000000076E8023
>
> ConvertPageEntryAttribute 0x76E9023->0x80000000076E9023
>
> ConvertPageEntryAttribute 0x76EA023->0x80000000076EA023
>
> ConvertPageEntryAttribute 0x76EB063->0x80000000076EB063
>
> ConvertPageEntryAttribute 0x76EC063->0x80000000076EC063
>
> ConvertPageEntryAttribute 0x76ED063->0x80000000076ED063
>
> ConvertPageEntryAttribute 0x76EE003->0x80000000076EE003
>
> ConvertPageEntryAttribute 0x76EF003->0x80000000076EF003
>
> SetUefiImageMemoryAttributes - 0x0000000007637000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7637063->0x8000000007637063
>
> SetUefiImageMemoryAttributes - 0x0000000007638000 - 0x0000000000006000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7638063->0x7638061
>
> ConvertPageEntryAttribute 0x7639063->0x7639061
>
> ConvertPageEntryAttribute 0x763A063->0x763A061
>
> ConvertPageEntryAttribute 0x763B063->0x763B061
>
> ConvertPageEntryAttribute 0x763C063->0x763C061
>
> ConvertPageEntryAttribute 0x763D063->0x763D061
>
> SetUefiImageMemoryAttributes - 0x000000000763E000 - 0x0000000000005000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x763E063->0x800000000763E063
>
> ConvertPageEntryAttribute 0x763F063->0x800000000763F063
>
> ConvertPageEntryAttribute 0x7640063->0x8000000007640063
>
> ConvertPageEntryAttribute 0x7641063->0x8000000007641063
>
> ConvertPageEntryAttribute 0x7642063->0x8000000007642063
>
> SetUefiImageMemoryAttributes - 0x000000000762E000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x762E063->0x800000000762E063
>
> SetUefiImageMemoryAttributes - 0x000000000762F000 - 0x0000000000004000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x762F063->0x762F061
>
> ConvertPageEntryAttribute 0x7630063->0x7630061
>
> ConvertPageEntryAttribute 0x7631063->0x7631061
>
> ConvertPageEntryAttribute 0x7632063->0x7632061
>
> SetUefiImageMemoryAttributes - 0x0000000007633000 - 0x0000000000004000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7633063->0x8000000007633063
>
> ConvertPageEntryAttribute 0x7634063->0x8000000007634063
>
> ConvertPageEntryAttribute 0x7635063->0x8000000007635063
>
> ConvertPageEntryAttribute 0x7636063->0x8000000007636063
>
> SetUefiImageMemoryAttributes - 0x000000000766C000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x766C063->0x800000000766C063
>
> SetUefiImageMemoryAttributes - 0x000000000766D000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x766D063->0x766D061
>
> ConvertPageEntryAttribute 0x766E063->0x766E061
>
> SetUefiImageMemoryAttributes - 0x000000000766F000 - 0x0000000000004000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x766F063->0x800000000766F063
>
> ConvertPageEntryAttribute 0x7670063->0x8000000007670063
>
> ConvertPageEntryAttribute 0x7671063->0x8000000007671063
>
> ConvertPageEntryAttribute 0x7672063->0x8000000007672063
>
> SetUefiImageMemoryAttributes - 0x0000000007666000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7666063->0x8000000007666063
>
> SetUefiImageMemoryAttributes - 0x0000000007667000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7667063->0x7667061
>
> ConvertPageEntryAttribute 0x7668063->0x7668061
>
> SetUefiImageMemoryAttributes - 0x0000000007669000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7669063->0x8000000007669063
>
> ConvertPageEntryAttribute 0x766A063->0x800000000766A063
>
> ConvertPageEntryAttribute 0x766B063->0x800000000766B063
>
> SetUefiImageMemoryAttributes - 0x0000000007627000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7627063->0x8000000007627063
>
> SetUefiImageMemoryAttributes - 0x0000000007628000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7628063->0x7628061
>
> ConvertPageEntryAttribute 0x7629063->0x7629061
>
> SetUefiImageMemoryAttributes - 0x000000000762A000 - 0x0000000000004000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x762A063->0x800000000762A063
>
> ConvertPageEntryAttribute 0x762B063->0x800000000762B063
>
> ConvertPageEntryAttribute 0x762C063->0x800000000762C063
>
> ConvertPageEntryAttribute 0x762D063->0x800000000762D063
>
> SetUefiImageMemoryAttributes - 0x000000000761F000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x761F063->0x800000000761F063
>
> SetUefiImageMemoryAttributes - 0x0000000007620000 - 0x0000000000004000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7620063->0x7620061
>
> ConvertPageEntryAttribute 0x7621063->0x7621061
>
> ConvertPageEntryAttribute 0x7622063->0x7622061
>
> ConvertPageEntryAttribute 0x7623063->0x7623061
>
> SetUefiImageMemoryAttributes - 0x0000000007624000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7624063->0x8000000007624063
>
> ConvertPageEntryAttribute 0x7625063->0x8000000007625063
>
> ConvertPageEntryAttribute 0x7626063->0x8000000007626063
>
> SetUefiImageMemoryAttributes - 0x0000000007619000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7619063->0x8000000007619063
>
> SetUefiImageMemoryAttributes - 0x000000000761A000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x761A063->0x761A061
>
> ConvertPageEntryAttribute 0x761B063->0x761B061
>
> SetUefiImageMemoryAttributes - 0x000000000761C000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x761C063->0x800000000761C063
>
> ConvertPageEntryAttribute 0x761D063->0x800000000761D063
>
> ConvertPageEntryAttribute 0x761E063->0x800000000761E063
>
> SetUefiImageMemoryAttributes - 0x0000000007613000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7613063->0x8000000007613063
>
> SetUefiImageMemoryAttributes - 0x0000000007614000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7614063->0x7614061
>
> ConvertPageEntryAttribute 0x7615063->0x7615061
>
> SetUefiImageMemoryAttributes - 0x0000000007616000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7616063->0x8000000007616063
>
> ConvertPageEntryAttribute 0x7617063->0x8000000007617063
>
> ConvertPageEntryAttribute 0x7618063->0x8000000007618063
>
> SetUefiImageMemoryAttributes - 0x0000000007605000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7605063->0x8000000007605063
>
> SetUefiImageMemoryAttributes - 0x0000000007606000 - 0x0000000000008000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7606063->0x7606061
>
> ConvertPageEntryAttribute 0x7607063->0x7607061
>
> ConvertPageEntryAttribute 0x7608063->0x7608061
>
> ConvertPageEntryAttribute 0x7609063->0x7609061
>
> ConvertPageEntryAttribute 0x760A063->0x760A061
>
> ConvertPageEntryAttribute 0x760B063->0x760B061
>
> ConvertPageEntryAttribute 0x760C063->0x760C061
>
> ConvertPageEntryAttribute 0x760D063->0x760D061
>
> SetUefiImageMemoryAttributes - 0x000000000760E000 - 0x0000000000005000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x760E063->0x800000000760E063
>
> ConvertPageEntryAttribute 0x760F063->0x800000000760F063
>
> ConvertPageEntryAttribute 0x7610063->0x8000000007610063
>
> ConvertPageEntryAttribute 0x7611063->0x8000000007611063
>
> ConvertPageEntryAttribute 0x7612063->0x8000000007612063
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
>   Flushing GCD
>
> AP Loop Mode is 1
>
> Detect CPU count: 4
>
> InstallProtocolInterface: 3FDDA605-A76E-4F46-AD29-12F4531B3D08 76111D4
>
> Loading driver F6697AC4-A776-4EE1-B643-1FEFF2B615BB
>
> InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 70D8CA8
>
> Loading driver at 0x000075FF000 EntryPoint=0x000076000B5
> IncompatiblePciDeviceSupportDxe.efi
>
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 70D9010
>
> ProtectUefiImageCommon - 0x70D8CA8
>
>   - 0x00000000075FF000 - 0x0000000000006000
>
> SetUefiImageMemoryAttributes - 0x00000000075FF000 - 0x0000000000001000
> (0x0000000000004008)
>
> Split - 0x6CA7000
>
> ConvertPageEntryAttribute 0x75FF063->0x80000000075FF063
>
> SetUefiImageMemoryAttributes - 0x0000000007600000 - 0x0000000000002000
> (0x0000000000020008)
>
> ConvertPageEntryAttribute 0x7600063->0x7600061
>
> ConvertPageEntryAttribute 0x7601063->0x7601061
>
> SetUefiImageMemoryAttributes - 0x0000000007602000 - 0x0000000000003000
> (0x0000000000004008)
>
> ConvertPageEntryAttribute 0x7602063->0x8000000007602063
>
> ConvertPageEntryAttribute 0x7603063->0x8000000007603063
>
> ConvertPageEntryAttribute 0x7604063->0x8000000007604063
>
> ============================
>
>
>
>
>
>
>
>
>
>
>
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard
> Biesheuvel
> Sent: Thursday, February 9, 2017 1:22 AM
> To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Leif Lindholm
> <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>;
> Fan, Jeff <jeff.fan@intel.com<mailto:jeff.fan@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
>
>
>
> On 9 February 2017 at 09:09, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>
> wrote:
>
>> On 9 February 2017 at 08:49, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>
>> wrote:
>>> On 9 February 2017 at 07:43, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
>>>> Hi Lindholm/Ard
>>>> This version 3 contains both of your feedback before.
>>>>
>>>> If you can do me a favor to evaluated the impact to ARM, that will be
>>>> great.
>>>>
>>>
>>> I will take a look right away.
>>>
>>
>> I am hitting the following assert as soon as ArmCpuDxe is loaded:
>>
>> Loading driver at 0x0005BE15000 EntryPoint=0x0005BE16048 ArmCpuDxe.efi
>> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 5B142398
>> ProtectUefiImageCommon - 0x5B142140
>>   - 0x000000005BE15000 - 0x0000000000010000
>> InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 5BE23008
>> InstallProtocolInterface: AD651C7D-3C22-4DBF-92E8-38A7CDAE87B2 5BE230B0
>> MemoryProtectionCpuArchProtocolNotify:
>> ProtectUefiImageCommon - 0x5EF02400
>>   - 0x000000005EEC4000 - 0x0000000000042000
>> SetUefiImageMemoryAttributes - 0x000000005EEC4000 - 0x0000000000001000
>> (0x0000000000004000)
>> EfiAttributeToArmAttribute: 0x4000 attributes is not supported.
>> ASSERT [ArmCpuDxe]
>> /home/ard/build/uefi-next/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c(220): 0
>>
>> The reason appears to be that the GCD memory descriptor retrieved in
>> SetUefiImageMemoryAttributes() for this region has Attributes == 0
>>
>> I am digging further ...
>
> Attached is the output of DEBUG_GCD
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Ard Biesheuvel 7 years, 1 month ago
On 9 February 2017 at 14:08, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> For X86 CPU, the memory protection attribute goes to page table, the cache
> attribute goes to MTRR register.
>
> Those are 2 difference resource, and can be set separately.
>
>
>
> The high level pseudo code is below:
>
> =======================
>
> CacheAttribute = Attribute & CACHE_ATTRIBUTE_MASK
>
> MemoryProtectionAttribute = Attribute & MEMORY_PROTECTION_ATTRIBUTE_MASK
>
> If (CacheAttribute != 0) {
>
>      SetCacheAttribute(CacheAttribute)
>
> }
>
> SetMemoryProtectionAttribute(MemoryProtectionAttribute)
>
> =======================
>
>
>
> NOTE: we need compare CacheAttribute == 0, because the cache attribute is an
> individual mask. 0x1 means UN_CACHE, 0x8 means WRITE_BACK. 0 is meaningless.
>
> We do not compare MemoryProtectionAttribute == 0, because 0 is a valid
> memory protection attribute, which means to disable protection.
>
>
>
> Before GCD sync, the DxeCore does not know the cache attribute, so that it
> can only set memory attribute. The CPU driver only modifies page table based
> upon MemoryProtectionAttribute and keep cache attribute untouched.
>

OK, I think we should be able to work around this, although it is not
pretty. I will send it out as a separate patch.

I do have one other question though: would it be possible to round up
the end of the image to page size in this code? (in
SetUefiImageProtectionAttributes()) Otherwise, we may end up calling
SetMemoryAttributes() with a length that is not page aligned, which
hits an EFI_UNSUPPORTED on ARM. Given that the PE/COFF loader always
allocates full pages, we know the space after the image is not used,
so it is possible (and even more secure!) to clear the exec bit on it.

"""
  //
  // Last DATA
  //
  ASSERT (CurrentBase <= ImageEnd);
  if (CurrentBase < ImageEnd) {
    //
    // DATA
    //
    if (Protect) {
      Attribute = EFI_MEMORY_XP;
    } else {
      Attribute = 0;
    }
    SetUefiImageMemoryAttributes (
      CurrentBase,
      ImageEnd - CurrentBase,
      Attribute
      );
  }
"""
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Yao, Jiewen 7 years, 1 month ago
1)       That is great. I appreciate your quick response and help.
I will drop my patch for ARM 2/4, and wait for yours.


2)       For ImageEnd alignment issue, I agree with you.
I plan to round up with:
ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize, SectionAlignment);
before SetUefiImageProtectionAttributes (ImageRecord, Protect);

I will update it at V4.

Thank you
Yao Jiewen


From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Thursday, February 9, 2017 6:56 AM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection

On 9 February 2017 at 14:08, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
> For X86 CPU, the memory protection attribute goes to page table, the cache
> attribute goes to MTRR register.
>
> Those are 2 difference resource, and can be set separately.
>
>
>
> The high level pseudo code is below:
>
> =======================
>
> CacheAttribute = Attribute & CACHE_ATTRIBUTE_MASK
>
> MemoryProtectionAttribute = Attribute & MEMORY_PROTECTION_ATTRIBUTE_MASK
>
> If (CacheAttribute != 0) {
>
>      SetCacheAttribute(CacheAttribute)
>
> }
>
> SetMemoryProtectionAttribute(MemoryProtectionAttribute)
>
> =======================
>
>
>
> NOTE: we need compare CacheAttribute == 0, because the cache attribute is an
> individual mask. 0x1 means UN_CACHE, 0x8 means WRITE_BACK. 0 is meaningless.
>
> We do not compare MemoryProtectionAttribute == 0, because 0 is a valid
> memory protection attribute, which means to disable protection.
>
>
>
> Before GCD sync, the DxeCore does not know the cache attribute, so that it
> can only set memory attribute. The CPU driver only modifies page table based
> upon MemoryProtectionAttribute and keep cache attribute untouched.
>

OK, I think we should be able to work around this, although it is not
pretty. I will send it out as a separate patch.

I do have one other question though: would it be possible to round up
the end of the image to page size in this code? (in
SetUefiImageProtectionAttributes()) Otherwise, we may end up calling
SetMemoryAttributes() with a length that is not page aligned, which
hits an EFI_UNSUPPORTED on ARM. Given that the PE/COFF loader always
allocates full pages, we know the space after the image is not used,
so it is possible (and even more secure!) to clear the exec bit on it.

"""
  //
  // Last DATA
  //
  ASSERT (CurrentBase <= ImageEnd);
  if (CurrentBase < ImageEnd) {
    //
    // DATA
    //
    if (Protect) {
      Attribute = EFI_MEMORY_XP;
    } else {
      Attribute = 0;
    }
    SetUefiImageMemoryAttributes (
      CurrentBase,
      ImageEnd - CurrentBase,
      Attribute
      );
  }
"""
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Ard Biesheuvel 7 years, 1 month ago
On 9 February 2017 at 15:27, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> 1)       That is great. I appreciate your quick response and help.
>
> I will drop my patch for ARM 2/4, and wait for yours.
>

OK

>
>
> 2)       For ImageEnd alignment issue, I agree with you.
>
> I plan to round up with:
>
> ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
> SectionAlignment);
>
> before SetUefiImageProtectionAttributes (ImageRecord, Protect);
>

Great, that should fix my issue!

Thanks,
Ard.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Ard Biesheuvel 7 years, 1 month ago
On 9 February 2017 at 15:28, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 9 February 2017 at 15:27, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>> 1)       That is great. I appreciate your quick response and help.
>>
>> I will drop my patch for ARM 2/4, and wait for yours.
>>
>
> OK
>
>>
>>
>> 2)       For ImageEnd alignment issue, I agree with you.
>>
>> I plan to round up with:
>>
>> ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
>> SectionAlignment);
>>
>> before SetUefiImageProtectionAttributes (ImageRecord, Protect);
>>
>
> Great, that should fix my issue!
>

Actually, does that still work correctly with 64 KB section alignment?
I don't think the PE/COFF loader rounds up the allocation to section
alignment, does it?
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Yao, Jiewen 7 years, 1 month ago
Very good point.

Can ARCH64 set 4K paging for 64K aligned runtime memory?

If yes, how about we use
“ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize, EFI_PAGE_SIZE);”


Thank you
Yao Jiewen

From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Thursday, February 9, 2017 8:21 AM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection

On 9 February 2017 at 15:28, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> wrote:
> On 9 February 2017 at 15:27, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
>> 1)       That is great. I appreciate your quick response and help.
>>
>> I will drop my patch for ARM 2/4, and wait for yours.
>>
>
> OK
>
>>
>>
>> 2)       For ImageEnd alignment issue, I agree with you.
>>
>> I plan to round up with:
>>
>> ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
>> SectionAlignment);
>>
>> before SetUefiImageProtectionAttributes (ImageRecord, Protect);
>>
>
> Great, that should fix my issue!
>

Actually, does that still work correctly with 64 KB section alignment?
I don't think the PE/COFF loader rounds up the allocation to section
alignment, does it?
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Ard Biesheuvel 7 years, 1 month ago
On 9 February 2017 at 16:29, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> Very good point.
>
> Can ARCH64 set 4K paging for 64K aligned runtime memory?
>

UEFI always uses 4 KB, but the OS may use 64 KB, so to create the
virtual address map it needs the runtime regions to be 64 KB aligned.

>
>
> If yes, how about we use
>
> “ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
> EFI_PAGE_SIZE);”
>

Perfect, thanks.

>
>
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Thursday, February 9, 2017 8:21 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm
> <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
>
>
>
> On 9 February 2017 at 15:28, Ard Biesheuvel <ard.biesheuvel@linaro.org>
> wrote:
>
>
>> On 9 February 2017 at 15:27, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>>> 1)       That is great. I appreciate your quick response and help.
>>>
>>> I will drop my patch for ARM 2/4, and wait for yours.
>>>
>>
>> OK
>>
>>>
>>>
>>> 2)       For ImageEnd alignment issue, I agree with you.
>>>
>>> I plan to round up with:
>>>
>>> ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
>>> SectionAlignment);
>>>
>>> before SetUefiImageProtectionAttributes (ImageRecord, Protect);
>>>
>>
>> Great, that should fix my issue!
>>
>
> Actually, does that still work correctly with 64 KB section alignment?
> I don't think the PE/COFF loader rounds up the allocation to section
> alignment, does it?
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Ard Biesheuvel 7 years, 1 month ago
On 9 February 2017 at 16:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 9 February 2017 at 16:29, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>> Very good point.
>>
>> Can ARCH64 set 4K paging for 64K aligned runtime memory?
>>
>
> UEFI always uses 4 KB, but the OS may use 64 KB, so to create the
> virtual address map it needs the runtime regions to be 64 KB aligned.
>
>>
>>
>> If yes, how about we use
>>
>> “ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
>> EFI_PAGE_SIZE);”
>>
>

Another question: did you try SetVirtualAddressMap()? It looks like we
need to lift read-only permissions to allow the runtime PE/COFF
relocation to apply the fixups
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Yao, Jiewen 7 years, 1 month ago
Very good question.


1)       Yes, I did test UEFI OS boot, which is mentioned in V1 summary:
======
Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
======


2)       Star helps double confirm that OS already takes over the control of page table on SetVirtualAddressMap().
See below log on UEFI Win10.
======
DXEIPL CR3 0x88140000
RUNTIMEDXE CR3 0x1AB000
======

Thank you
Yao Jiewen

From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Thursday, February 9, 2017 8:48 AM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection

On 9 February 2017 at 16:30, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> wrote:
> On 9 February 2017 at 16:29, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
>> Very good point.
>>
>> Can ARCH64 set 4K paging for 64K aligned runtime memory?
>>
>
> UEFI always uses 4 KB, but the OS may use 64 KB, so to create the
> virtual address map it needs the runtime regions to be 64 KB aligned.
>
>>
>>
>> If yes, how about we use
>>
>> “ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
>> EFI_PAGE_SIZE);”
>>
>

Another question: did you try SetVirtualAddressMap()? It looks like we
need to lift read-only permissions to allow the runtime PE/COFF
relocation to apply the fixups
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Ard Biesheuvel 7 years, 1 month ago

> On 10 Feb 2017, at 02:26, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> 
> Very good question.
>  
> 1)       Yes, I did test UEFI OS boot, which is mentioned in V1 summary:
> ======
> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
> ======
>  
> 2)       Star helps double confirm that OS already takes over the control of page table on SetVirtualAddressMap().
> See below log on UEFI Win10.
> ======
> DXEIPL CR3 0x88140000
> RUNTIMEDXE CR3 0x1AB000
> ======
>  

Not on AArch64/ARM linux, and the spec does not mandate it, so we need to deal with this imo

> Thank you
> Yao Jiewen
>  
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
> Sent: Thursday, February 9, 2017 8:48 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
>  
> On 9 February 2017 at 16:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > On 9 February 2017 at 16:29, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> >> Very good point.
> >>
> >> Can ARCH64 set 4K paging for 64K aligned runtime memory?
> >>
> >
> > UEFI always uses 4 KB, but the OS may use 64 KB, so to create the
> > virtual address map it needs the runtime regions to be 64 KB aligned.
> >
> >>
> >>
> >> If yes, how about we use
> >>
> >> “ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
> >> EFI_PAGE_SIZE);”
> >>
> >
> 
> Another question: did you try SetVirtualAddressMap()? It looks like we
> need to lift read-only permissions to allow the runtime PE/COFF
> relocation to apply the fixups
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Ard Biesheuvel 7 years, 1 month ago

> On 10 Feb 2017, at 06:34, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 
> 
> 
>> On 10 Feb 2017, at 02:26, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>> 
>> Very good question.
>>  
>> 1)       Yes, I did test UEFI OS boot, which is mentioned in V1 summary:
>> ======
>> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
>> ======
>>  
>> 2)       Star helps double confirm that OS already takes over the control of page table on SetVirtualAddressMap().
>> See below log on UEFI Win10.
>> ======
>> DXEIPL CR3 0x88140000
>> RUNTIMEDXE CR3 0x1AB000
>> ======
>>  
> 
> Not on AArch64/ARM linux, and the spec does not mandate it, so we need to deal with this imo
> 

I think we should probably undo the protections for runtime drivers in EBS()


>> Thank you
>> Yao Jiewen
>>  
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
>> Sent: Thursday, February 9, 2017 8:48 AM
>> To: Yao, Jiewen <jiewen.yao@intel.com>
>> Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
>> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
>>  
>> On 9 February 2017 at 16:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> > On 9 February 2017 at 16:29, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>> >> Very good point.
>> >>
>> >> Can ARCH64 set 4K paging for 64K aligned runtime memory?
>> >>
>> >
>> > UEFI always uses 4 KB, but the OS may use 64 KB, so to create the
>> > virtual address map it needs the runtime regions to be 64 KB aligned.
>> >
>> >>
>> >>
>> >> If yes, how about we use
>> >>
>> >> “ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
>> >> EFI_PAGE_SIZE);”
>> >>
>> >
>> 
>> Another question: did you try SetVirtualAddressMap()? It looks like we
>> need to lift read-only permissions to allow the runtime PE/COFF
>> relocation to apply the fixups
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Yao, Jiewen 7 years, 1 month ago
Good to learn that ARM/ARCH64 behavior. That is interesting.

Yes, if that is the case, we need figure out a way to make it work.

IMHO, “Undo the protection” directly is also risky.
Maybe the protection is used or setup by OS purposely before EBS (WoW. ☺). Unprotecting them in BIOS might break the OS expectation.


Would you please provide more info on ARM Linux? When the OSLoader or OS takes over the page table? After EBS?

Thank you
Yao Jiewen

From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Thursday, February 9, 2017 10:42 PM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection



> On 10 Feb 2017, at 06:34, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> wrote:
>
>
>
>> On 10 Feb 2017, at 02:26, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
>>
>> Very good question.
>>
>> 1)       Yes, I did test UEFI OS boot, which is mentioned in V1 summary:
>> ======
>> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
>> ======
>>
>> 2)       Star helps double confirm that OS already takes over the control of page table on SetVirtualAddressMap().
>> See below log on UEFI Win10.
>> ======
>> DXEIPL CR3 0x88140000
>> RUNTIMEDXE CR3 0x1AB000
>> ======
>>
>
> Not on AArch64/ARM linux, and the spec does not mandate it, so we need to deal with this imo
>

I think we should probably undo the protections for runtime drivers in EBS()


>> Thank you
>> Yao Jiewen
>>
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
>> Sent: Thursday, February 9, 2017 8:48 AM
>> To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
>> Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Leif Lindholm <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Fan, Jeff <jeff.fan@intel.com<mailto:jeff.fan@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
>> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
>>
>> On 9 February 2017 at 16:30, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> wrote:
>> > On 9 February 2017 at 16:29, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
>> >> Very good point.
>> >>
>> >> Can ARCH64 set 4K paging for 64K aligned runtime memory?
>> >>
>> >
>> > UEFI always uses 4 KB, but the OS may use 64 KB, so to create the
>> > virtual address map it needs the runtime regions to be 64 KB aligned.
>> >
>> >>
>> >>
>> >> If yes, how about we use
>> >>
>> >> “ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
>> >> EFI_PAGE_SIZE);”
>> >>
>> >
>>
>> Another question: did you try SetVirtualAddressMap()? It looks like we
>> need to lift read-only permissions to allow the runtime PE/COFF
>> relocation to apply the fixups
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
>> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Ard Biesheuvel 7 years, 1 month ago
> On 10 Feb 2017, at 11:32, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> 
> Good to learn that ARM/ARCH64 behavior. That is interesting.
>  
> Yes, if that is the case, we need figure out a way to make it work.
>  
> IMHO, “Undo the protection” directly is also risky.
> Maybe the protection is used or setup by OS purposely before EBS (WoW. J). Unprotecting them in BIOS might break the OS expectation.
>  
>  
> Would you please provide more info on ARM Linux? When the OSLoader or OS takes over the page table? After EBS?

On ARM, we call SetVirtualAddressMap directly after EBS, in the OS loader. The caches are disabled much later. It is the OS itself that reenables the MMU, and install the UEFI virtual mapping as per-process mapping, which is only live when a runtime services call is in progress.

Therefore, we use a low virtual mapping for runtime services, which may conflict with the 1:1 mapping, so we never map any uefi regions 1:1 under the os.

This implies that the virtual mapping we install is not yet live when we install it, and so the easiest way to do that is to install it immediately after ebs, when the firmware's 1:1 mapping is still live.



> Thank you
> Yao Jiewen
>  
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
> Sent: Thursday, February 9, 2017 10:42 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
>  
> 
> 
> > On 10 Feb 2017, at 06:34, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > 
> > 
> > 
> >> On 10 Feb 2017, at 02:26, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> >> 
> >> Very good question.
> >>  
> >> 1)       Yes, I did test UEFI OS boot, which is mentioned in V1 summary:
> >> ======
> >> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
> >> ======
> >>  
> >> 2)       Star helps double confirm that OS already takes over the control of page table on SetVirtualAddressMap().
> >> See below log on UEFI Win10.
> >> ======
> >> DXEIPL CR3 0x88140000
> >> RUNTIMEDXE CR3 0x1AB000
> >> ======
> >>  
> > 
> > Not on AArch64/ARM linux, and the spec does not mandate it, so we need to deal with this imo
> > 
> 
> I think we should probably undo the protections for runtime drivers in EBS()
> 
> 
> >> Thank you
> >> Yao Jiewen
> >>  
> >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
> >> Sent: Thursday, February 9, 2017 8:48 AM
> >> To: Yao, Jiewen <jiewen.yao@intel.com>
> >> Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
> >> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
> >>  
> >> On 9 February 2017 at 16:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> > On 9 February 2017 at 16:29, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> >> >> Very good point.
> >> >>
> >> >> Can ARCH64 set 4K paging for 64K aligned runtime memory?
> >> >>
> >> >
> >> > UEFI always uses 4 KB, but the OS may use 64 KB, so to create the
> >> > virtual address map it needs the runtime regions to be 64 KB aligned.
> >> >
> >> >>
> >> >>
> >> >> If yes, how about we use
> >> >>
> >> >> “ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
> >> >> EFI_PAGE_SIZE);”
> >> >>
> >> >
> >> 
> >> Another question: did you try SetVirtualAddressMap()? It looks like we
> >> need to lift read-only permissions to allow the runtime PE/COFF
> >> relocation to apply the fixups
> >> _______________________________________________
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Yao, Jiewen 7 years, 1 month ago
Thanks for the info.

Mike/Vincent also mentioned that FW does not own page tables after ExitBootServices(), so the OS would have to relax NX protection of RT code pages across SVA.
Or delay setting NX protections on RT code pages until after SVA.

I agree with you that we can mark RT region to be RW.
Here is the pseudo code I plan to put to CoreExitBootServices().

=======================
VOID
MemoryprotectionExitBootServicesCallback (
  VOID
  )
{
  EFI_RUNTIME_IMAGE_ENTRY       *RuntimeImage;
  LIST_ENTRY                    *Link;

  //
  // We need remove the RT protection, because RT relocation need write code segment
  // at SetVirtualAddressMap(). We cannot assume OS/Loader has taken over page table at that time.
  //
  // Firmware does not own page tables after ExitBootServices(), so the OS would
  // have to relax protection of RT code pages across SetVirtualAddressMap(), or
  // delay setting protections on RT code pages until after SetVirtualAddressMap().
  // OS may set protection on RT based upon EFI_MEMORY_ATTRIBUTES_TABLE later.
  //
  if (mImageProtectionPolicy != 0) {
    for (Link = gRuntime->ImageHead.ForwardLink; Link != &gRuntime->ImageHead; Link = Link->ForwardLink) {
      RuntimeImage = BASE_CR (Link, EFI_RUNTIME_IMAGE_ENTRY, Link);
      SetUefiImageMemoryAttributes ((UINT64)(UINTN)RuntimeImage->ImageBase, ALIGN_VALUE(RuntimeImage->ImageSize, EFI_PAGE_SIZE), 0);
    }
  }
}
=======================


From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Friday, February 10, 2017 3:43 AM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection


On 10 Feb 2017, at 11:32, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
Good to learn that ARM/ARCH64 behavior. That is interesting.

Yes, if that is the case, we need figure out a way to make it work.

IMHO, “Undo the protection” directly is also risky.
Maybe the protection is used or setup by OS purposely before EBS (WoW. ☺). Unprotecting them in BIOS might break the OS expectation.


Would you please provide more info on ARM Linux? When the OSLoader or OS takes over the page table? After EBS?

On ARM, we call SetVirtualAddressMap directly after EBS, in the OS loader. The caches are disabled much later. It is the OS itself that reenables the MMU, and install the UEFI virtual mapping as per-process mapping, which is only live when a runtime services call is in progress.

Therefore, we use a low virtual mapping for runtime services, which may conflict with the 1:1 mapping, so we never map any uefi regions 1:1 under the os.

This implies that the virtual mapping we install is not yet live when we install it, and so the easiest way to do that is to install it immediately after ebs, when the firmware's 1:1 mapping is still live.




Thank you
Yao Jiewen

From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Thursday, February 9, 2017 10:42 PM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Leif Lindholm <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Fan, Jeff <jeff.fan@intel.com<mailto:jeff.fan@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection



> On 10 Feb 2017, at 06:34, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> wrote:
>
>
>
>> On 10 Feb 2017, at 02:26, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
>>
>> Very good question.
>>
>> 1)       Yes, I did test UEFI OS boot, which is mentioned in V1 summary:
>> ======
>> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
>> ======
>>
>> 2)       Star helps double confirm that OS already takes over the control of page table on SetVirtualAddressMap().
>> See below log on UEFI Win10.
>> ======
>> DXEIPL CR3 0x88140000
>> RUNTIMEDXE CR3 0x1AB000
>> ======
>>
>
> Not on AArch64/ARM linux, and the spec does not mandate it, so we need to deal with this imo
>

I think we should probably undo the protections for runtime drivers in EBS()


>> Thank you
>> Yao Jiewen
>>
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
>> Sent: Thursday, February 9, 2017 8:48 AM
>> To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
>> Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Leif Lindholm <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Fan, Jeff <jeff.fan@intel.com<mailto:jeff.fan@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
>> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
>>
>> On 9 February 2017 at 16:30, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> wrote:
>> > On 9 February 2017 at 16:29, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
>> >> Very good point.
>> >>
>> >> Can ARCH64 set 4K paging for 64K aligned runtime memory?
>> >>
>> >
>> > UEFI always uses 4 KB, but the OS may use 64 KB, so to create the
>> > virtual address map it needs the runtime regions to be 64 KB aligned.
>> >
>> >>
>> >>
>> >> If yes, how about we use
>> >>
>> >> “ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
>> >> EFI_PAGE_SIZE);”
>> >>
>> >
>>
>> Another question: did you try SetVirtualAddressMap()? It looks like we
>> need to lift read-only permissions to allow the runtime PE/COFF
>> relocation to apply the fixups
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
>> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
Posted by Ard Biesheuvel 7 years, 1 month ago
On 10 February 2017 at 12:59, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> Thanks for the info.
>
>
>
> Mike/Vincent also mentioned that FW does not own page tables after
> ExitBootServices(), so the OS would have to relax NX protection of RT code
> pages across SVA.
>
> Or delay setting NX protections on RT code pages until after SVA.
>
>
>
> I agree with you that we can mark RT region to be RW.
>
> Here is the pseudo code I plan to put to CoreExitBootServices().
>
>
>
> =======================
>
> VOID
>
> MemoryprotectionExitBootServicesCallback (
>
>   VOID
>
>   )
>
> {
>
>   EFI_RUNTIME_IMAGE_ENTRY       *RuntimeImage;
>
>   LIST_ENTRY                    *Link;
>
>
>
>   //
>
>   // We need remove the RT protection, because RT relocation need write code
> segment
>
>   // at SetVirtualAddressMap(). We cannot assume OS/Loader has taken over
> page table at that time.
>
>   //
>
>   // Firmware does not own page tables after ExitBootServices(), so the OS
> would
>
>   // have to relax protection of RT code pages across
> SetVirtualAddressMap(), or
>
>   // delay setting protections on RT code pages until after
> SetVirtualAddressMap().
>
>   // OS may set protection on RT based upon EFI_MEMORY_ATTRIBUTES_TABLE
> later.
>
>   //
>
>   if (mImageProtectionPolicy != 0) {
>
>     for (Link = gRuntime->ImageHead.ForwardLink; Link !=
> &gRuntime->ImageHead; Link = Link->ForwardLink) {
>
>       RuntimeImage = BASE_CR (Link, EFI_RUNTIME_IMAGE_ENTRY, Link);
>
>       SetUefiImageMemoryAttributes ((UINT64)(UINTN)RuntimeImage->ImageBase,
> ALIGN_VALUE(RuntimeImage->ImageSize, EFI_PAGE_SIZE), 0);
>
>     }
>
>   }
>
> }


This looks correct to me, and it is the only place where we can deal
with this situation.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel