[edk2] [PATCH v4 0/7] MdeModulePkg/DxeCore: increased memory protection

Ard Biesheuvel posted 7 patches 7 years, 1 month ago
Failed in applying to current master (apply log)
ArmPkg/Drivers/CpuDxe/CpuDxe.c                     |   3 +
ArmPkg/Drivers/CpuDxe/CpuDxe.h                     |   1 +
ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c               |   4 +
ArmVirtPkg/ArmVirt.dsc.inc                         |   6 +
MdeModulePkg/Core/Dxe/DxeMain.h                    |  24 ++
MdeModulePkg/Core/Dxe/DxeMain.inf                  |   1 +
MdeModulePkg/Core/Dxe/Mem/Page.c                   |   7 +
MdeModulePkg/Core/Dxe/Mem/Pool.c                   |  65 +++-
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c      | 371 +++++++++++++++++++-
MdeModulePkg/Core/Pei/Image/Image.c                |  23 +-
MdeModulePkg/MdeModulePkg.dec                      |  32 ++
MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c |   2 +-
MdeModulePkg/Universal/EbcDxe/EbcInt.c             |  23 ++
MdeModulePkg/Universal/EbcDxe/EbcInt.h             |  14 +
MdeModulePkg/Universal/EbcDxe/Ia32/EbcSupport.c    |   2 +-
MdeModulePkg/Universal/EbcDxe/Ipf/EbcSupport.c     |   2 +-
MdeModulePkg/Universal/EbcDxe/X64/EbcSupport.c     |   2 +-
17 files changed, 558 insertions(+), 24 deletions(-)
[edk2] [PATCH v4 0/7] MdeModulePkg/DxeCore: increased memory protection
Posted by Ard Biesheuvel 7 years, 1 month ago
Hello all,

First of all, thanks for the reviews and regression testing. However, I did
not add the tested-by tags nor some of the R-b's, given the changes in this v4.

This series implements a memory protection policy that removes all executable
permissions from writable memory regions, which greatly enhances security.
It is based on Jiewen's recent work, which is a step in the right direction,
but still leaves most of memory exploitable due to the default R+W+X
permissions.

The idea is that the implementation of the CPU arch protocol goes over the
memory map and removes exec permissions from all regions that are not already
marked as 'code. This requires some preparatory work to ensure that the DxeCore
itself is covered by a BootServicesCode region, not a BootServicesData region.
Exec permissions are re-granted selectively, when the PE/COFF loader allocates
the space for it. Combined with Jiewen's code/data split, this removes all
RWX mapped regions.

Changes since v3:
- mandate that the same policy applies to EfiConventionalMemory regions and
  EfiBootServicesData regions: they are unlikely to differ in practice, and
  dealing with that corner case greatly complicates the implementation, given
  the way DxeCore allocates memory for itself in the implementation of the page
  and pool allocation routines.
- apply the EfiConventionalMemory policy to untested RAM regions in the GCD
  memory space map: without this, we may still have a large region of RAM that
  is exploitable, and it also removes the need to apply memory protections in
  PromoteMemoryResource (), which is very difficult to achieve without a major
  restructuring of the code due to the way locking is implemented here.
- add missing ApplyMemoryProtectionPolicy() call to CoreAddMemoryDescriptor()
- use CoreAcquireLockOrFail() on gMemoryLock for CoreAllocatePoolPages (#4)
- incorporate feedback from Liming (#2, #6)
- add patch to enable the NX memory protection policy for ArmVirtPkg (#7)

Changes since v2:
- added patch to make EBC use EfiBootServicesCode pool allocations for thunks
- redefine PCD according to Jiewen's feedback, including default value
- use sorted memory map and merge adjacent entries with the same policy, to
  prevent unnecessary page table splitting
- ignore policy when executing in SMM
- refactor the logic for managing permission attributes of pool allocations
- added some R-b's

Changes since v1:
- allocate code pages for PE/COFF images in PeiCore, so that DxeCore pages have
  the expected memory type (as suggested by Jiewen)
- add patch to inhibit page table updates while syncing the GCD memory space
  map with the page tables
- add PCD to set memory protection policy, which allows the policy for reserved
  and ACPI/NVS memory to be configured separately
- move attribute manipulation into DxeCore page allocation code: this way, we
  should be able to solve the EBC case by allocating BootServicesCode pool
  memory explicitly.

Series can be found here:
https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/log/?h=memprot-take2-v4

Ard Biesheuvel (7):
  ArmPkg/CpuDxe: ignore attribute changes during SyncCacheConfig()
  MdeModulePkg/PeiCore: allocate BootServicesCode memory for PE/COFF
    images
  MdeModulePkg/EbcDxe: use EfiBootServicesCode memory for thunks
  MdeModulePkg/DxeCore: use separate lock for pool allocations
  MdeModulePkg: define PCD for DXE memory protection policy
  MdeModulePkg/DxeCore: implement memory protection policy
  ArmVirtPkg/ArmVirt.dsc.inc: enable NX memory protection for all
    platforms

 ArmPkg/Drivers/CpuDxe/CpuDxe.c                     |   3 +
 ArmPkg/Drivers/CpuDxe/CpuDxe.h                     |   1 +
 ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c               |   4 +
 ArmVirtPkg/ArmVirt.dsc.inc                         |   6 +
 MdeModulePkg/Core/Dxe/DxeMain.h                    |  24 ++
 MdeModulePkg/Core/Dxe/DxeMain.inf                  |   1 +
 MdeModulePkg/Core/Dxe/Mem/Page.c                   |   7 +
 MdeModulePkg/Core/Dxe/Mem/Pool.c                   |  65 +++-
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c      | 371 +++++++++++++++++++-
 MdeModulePkg/Core/Pei/Image/Image.c                |  23 +-
 MdeModulePkg/MdeModulePkg.dec                      |  32 ++
 MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c |   2 +-
 MdeModulePkg/Universal/EbcDxe/EbcInt.c             |  23 ++
 MdeModulePkg/Universal/EbcDxe/EbcInt.h             |  14 +
 MdeModulePkg/Universal/EbcDxe/Ia32/EbcSupport.c    |   2 +-
 MdeModulePkg/Universal/EbcDxe/Ipf/EbcSupport.c     |   2 +-
 MdeModulePkg/Universal/EbcDxe/X64/EbcSupport.c     |   2 +-
 17 files changed, 558 insertions(+), 24 deletions(-)

-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4 0/7] MdeModulePkg/DxeCore: increased memory protection
Posted by Yao, Jiewen 7 years, 1 month ago
All series reviewed-by: jiewen.yao@intel.com
X86 platform regression tested-by: Jiewen.yao@intel.com

Thank you
Yao Jiewen


> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Monday, February 27, 2017 10:38 PM
> To: edk2-devel@lists.01.org; afish@apple.com; leif.lindholm@linaro.org; Kinney,
> Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>;
> Yao, Jiewen <jiewen.yao@intel.com>
> Cc: lersek@redhat.com; Tian, Feng <feng.tian@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: [PATCH v4 0/7] MdeModulePkg/DxeCore: increased memory protection
> 
> Hello all,
> 
> First of all, thanks for the reviews and regression testing. However, I did
> not add the tested-by tags nor some of the R-b's, given the changes in this v4.
> 
> This series implements a memory protection policy that removes all executable
> permissions from writable memory regions, which greatly enhances security.
> It is based on Jiewen's recent work, which is a step in the right direction,
> but still leaves most of memory exploitable due to the default R+W+X
> permissions.
> 
> The idea is that the implementation of the CPU arch protocol goes over the
> memory map and removes exec permissions from all regions that are not already
> marked as 'code. This requires some preparatory work to ensure that the
> DxeCore
> itself is covered by a BootServicesCode region, not a BootServicesData region.
> Exec permissions are re-granted selectively, when the PE/COFF loader allocates
> the space for it. Combined with Jiewen's code/data split, this removes all
> RWX mapped regions.
> 
> Changes since v3:
> - mandate that the same policy applies to EfiConventionalMemory regions and
>   EfiBootServicesData regions: they are unlikely to differ in practice, and
>   dealing with that corner case greatly complicates the implementation, given
>   the way DxeCore allocates memory for itself in the implementation of the
> page
>   and pool allocation routines.
> - apply the EfiConventionalMemory policy to untested RAM regions in the GCD
>   memory space map: without this, we may still have a large region of RAM that
>   is exploitable, and it also removes the need to apply memory protections in
>   PromoteMemoryResource (), which is very difficult to achieve without a major
>   restructuring of the code due to the way locking is implemented here.
> - add missing ApplyMemoryProtectionPolicy() call to
> CoreAddMemoryDescriptor()
> - use CoreAcquireLockOrFail() on gMemoryLock for CoreAllocatePoolPages (#4)
> - incorporate feedback from Liming (#2, #6)
> - add patch to enable the NX memory protection policy for ArmVirtPkg (#7)
> 
> Changes since v2:
> - added patch to make EBC use EfiBootServicesCode pool allocations for thunks
> - redefine PCD according to Jiewen's feedback, including default value
> - use sorted memory map and merge adjacent entries with the same policy, to
>   prevent unnecessary page table splitting
> - ignore policy when executing in SMM
> - refactor the logic for managing permission attributes of pool allocations
> - added some R-b's
> 
> Changes since v1:
> - allocate code pages for PE/COFF images in PeiCore, so that DxeCore pages have
>   the expected memory type (as suggested by Jiewen)
> - add patch to inhibit page table updates while syncing the GCD memory space
>   map with the page tables
> - add PCD to set memory protection policy, which allows the policy for reserved
>   and ACPI/NVS memory to be configured separately
> - move attribute manipulation into DxeCore page allocation code: this way, we
>   should be able to solve the EBC case by allocating BootServicesCode pool
>   memory explicitly.
> 
> Series can be found here:
> https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/log/?h=memprot-tak
> e2-v4
> 
> Ard Biesheuvel (7):
>   ArmPkg/CpuDxe: ignore attribute changes during SyncCacheConfig()
>   MdeModulePkg/PeiCore: allocate BootServicesCode memory for PE/COFF
>     images
>   MdeModulePkg/EbcDxe: use EfiBootServicesCode memory for thunks
>   MdeModulePkg/DxeCore: use separate lock for pool allocations
>   MdeModulePkg: define PCD for DXE memory protection policy
>   MdeModulePkg/DxeCore: implement memory protection policy
>   ArmVirtPkg/ArmVirt.dsc.inc: enable NX memory protection for all
>     platforms
> 
>  ArmPkg/Drivers/CpuDxe/CpuDxe.c                     |   3 +
>  ArmPkg/Drivers/CpuDxe/CpuDxe.h                     |   1 +
>  ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c               |   4 +
>  ArmVirtPkg/ArmVirt.dsc.inc                         |   6 +
>  MdeModulePkg/Core/Dxe/DxeMain.h                    |  24 ++
>  MdeModulePkg/Core/Dxe/DxeMain.inf                  |   1 +
>  MdeModulePkg/Core/Dxe/Mem/Page.c                   |   7 +
>  MdeModulePkg/Core/Dxe/Mem/Pool.c                   |  65 +++-
>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c      | 371
> +++++++++++++++++++-
>  MdeModulePkg/Core/Pei/Image/Image.c                |  23 +-
>  MdeModulePkg/MdeModulePkg.dec                      |  32 ++
>  MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c |   2 +-
>  MdeModulePkg/Universal/EbcDxe/EbcInt.c             |  23 ++
>  MdeModulePkg/Universal/EbcDxe/EbcInt.h             |  14 +
>  MdeModulePkg/Universal/EbcDxe/Ia32/EbcSupport.c    |   2 +-
>  MdeModulePkg/Universal/EbcDxe/Ipf/EbcSupport.c     |   2 +-
>  MdeModulePkg/Universal/EbcDxe/X64/EbcSupport.c     |   2 +-
>  17 files changed, 558 insertions(+), 24 deletions(-)
> 
> --
> 2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4 0/7] MdeModulePkg/DxeCore: increased memory protection
Posted by Ard Biesheuvel 7 years, 1 month ago
On 28 February 2017 at 05:48, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> All series reviewed-by: jiewen.yao@intel.com
> X86 platform regression tested-by: Jiewen.yao@intel.com
>

Thanks all for the reviews and testing.

I am going to proceed and merge this series, except for the last patch
enabling it for ArmVirtQemu. That allows us to test it more widely on
out of tree platforms.

In the mean time, the recommendation is to disable NX protection for
EfiLoaderData regions, on AArch64 as well as on X86. In the future, we
may want to promote the PCD to one that can be configured as dynamic,
and let the user decide which policy to use. I will follow up with a
separate patch if we intend to go down that road.

Regards,
Ard.


>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: Monday, February 27, 2017 10:38 PM
>> To: edk2-devel@lists.01.org; afish@apple.com; leif.lindholm@linaro.org; Kinney,
>> Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>;
>> Yao, Jiewen <jiewen.yao@intel.com>
>> Cc: lersek@redhat.com; Tian, Feng <feng.tian@intel.com>; Zeng, Star
>> <star.zeng@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Subject: [PATCH v4 0/7] MdeModulePkg/DxeCore: increased memory protection
>>
>> Hello all,
>>
>> First of all, thanks for the reviews and regression testing. However, I did
>> not add the tested-by tags nor some of the R-b's, given the changes in this v4.
>>
>> This series implements a memory protection policy that removes all executable
>> permissions from writable memory regions, which greatly enhances security.
>> It is based on Jiewen's recent work, which is a step in the right direction,
>> but still leaves most of memory exploitable due to the default R+W+X
>> permissions.
>>
>> The idea is that the implementation of the CPU arch protocol goes over the
>> memory map and removes exec permissions from all regions that are not already
>> marked as 'code. This requires some preparatory work to ensure that the
>> DxeCore
>> itself is covered by a BootServicesCode region, not a BootServicesData region.
>> Exec permissions are re-granted selectively, when the PE/COFF loader allocates
>> the space for it. Combined with Jiewen's code/data split, this removes all
>> RWX mapped regions.
>>
>> Changes since v3:
>> - mandate that the same policy applies to EfiConventionalMemory regions and
>>   EfiBootServicesData regions: they are unlikely to differ in practice, and
>>   dealing with that corner case greatly complicates the implementation, given
>>   the way DxeCore allocates memory for itself in the implementation of the
>> page
>>   and pool allocation routines.
>> - apply the EfiConventionalMemory policy to untested RAM regions in the GCD
>>   memory space map: without this, we may still have a large region of RAM that
>>   is exploitable, and it also removes the need to apply memory protections in
>>   PromoteMemoryResource (), which is very difficult to achieve without a major
>>   restructuring of the code due to the way locking is implemented here.
>> - add missing ApplyMemoryProtectionPolicy() call to
>> CoreAddMemoryDescriptor()
>> - use CoreAcquireLockOrFail() on gMemoryLock for CoreAllocatePoolPages (#4)
>> - incorporate feedback from Liming (#2, #6)
>> - add patch to enable the NX memory protection policy for ArmVirtPkg (#7)
>>
>> Changes since v2:
>> - added patch to make EBC use EfiBootServicesCode pool allocations for thunks
>> - redefine PCD according to Jiewen's feedback, including default value
>> - use sorted memory map and merge adjacent entries with the same policy, to
>>   prevent unnecessary page table splitting
>> - ignore policy when executing in SMM
>> - refactor the logic for managing permission attributes of pool allocations
>> - added some R-b's
>>
>> Changes since v1:
>> - allocate code pages for PE/COFF images in PeiCore, so that DxeCore pages have
>>   the expected memory type (as suggested by Jiewen)
>> - add patch to inhibit page table updates while syncing the GCD memory space
>>   map with the page tables
>> - add PCD to set memory protection policy, which allows the policy for reserved
>>   and ACPI/NVS memory to be configured separately
>> - move attribute manipulation into DxeCore page allocation code: this way, we
>>   should be able to solve the EBC case by allocating BootServicesCode pool
>>   memory explicitly.
>>
>> Series can be found here:
>> https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/log/?h=memprot-tak
>> e2-v4
>>
>> Ard Biesheuvel (7):
>>   ArmPkg/CpuDxe: ignore attribute changes during SyncCacheConfig()
>>   MdeModulePkg/PeiCore: allocate BootServicesCode memory for PE/COFF
>>     images
>>   MdeModulePkg/EbcDxe: use EfiBootServicesCode memory for thunks
>>   MdeModulePkg/DxeCore: use separate lock for pool allocations
>>   MdeModulePkg: define PCD for DXE memory protection policy
>>   MdeModulePkg/DxeCore: implement memory protection policy
>>   ArmVirtPkg/ArmVirt.dsc.inc: enable NX memory protection for all
>>     platforms
>>
>>  ArmPkg/Drivers/CpuDxe/CpuDxe.c                     |   3 +
>>  ArmPkg/Drivers/CpuDxe/CpuDxe.h                     |   1 +
>>  ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c               |   4 +
>>  ArmVirtPkg/ArmVirt.dsc.inc                         |   6 +
>>  MdeModulePkg/Core/Dxe/DxeMain.h                    |  24 ++
>>  MdeModulePkg/Core/Dxe/DxeMain.inf                  |   1 +
>>  MdeModulePkg/Core/Dxe/Mem/Page.c                   |   7 +
>>  MdeModulePkg/Core/Dxe/Mem/Pool.c                   |  65 +++-
>>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c      | 371
>> +++++++++++++++++++-
>>  MdeModulePkg/Core/Pei/Image/Image.c                |  23 +-
>>  MdeModulePkg/MdeModulePkg.dec                      |  32 ++
>>  MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c |   2 +-
>>  MdeModulePkg/Universal/EbcDxe/EbcInt.c             |  23 ++
>>  MdeModulePkg/Universal/EbcDxe/EbcInt.h             |  14 +
>>  MdeModulePkg/Universal/EbcDxe/Ia32/EbcSupport.c    |   2 +-
>>  MdeModulePkg/Universal/EbcDxe/Ipf/EbcSupport.c     |   2 +-
>>  MdeModulePkg/Universal/EbcDxe/X64/EbcSupport.c     |   2 +-
>>  17 files changed, 558 insertions(+), 24 deletions(-)
>>
>> --
>> 2.7.4
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4 0/7] MdeModulePkg/DxeCore: increased memory protection
Posted by Laszlo Ersek 7 years, 1 month ago
On 02/27/17 15:38, Ard Biesheuvel wrote:
> Hello all,
> 
> First of all, thanks for the reviews and regression testing. However, I did
> not add the tested-by tags nor some of the R-b's, given the changes in this v4.
> 
> This series implements a memory protection policy that removes all executable
> permissions from writable memory regions, which greatly enhances security.
> It is based on Jiewen's recent work, which is a step in the right direction,
> but still leaves most of memory exploitable due to the default R+W+X
> permissions.
> 
> The idea is that the implementation of the CPU arch protocol goes over the
> memory map and removes exec permissions from all regions that are not already
> marked as 'code. This requires some preparatory work to ensure that the DxeCore
> itself is covered by a BootServicesCode region, not a BootServicesData region.
> Exec permissions are re-granted selectively, when the PE/COFF loader allocates
> the space for it. Combined with Jiewen's code/data split, this removes all
> RWX mapped regions.
> 
> Changes since v3:
> - mandate that the same policy applies to EfiConventionalMemory regions and
>   EfiBootServicesData regions: they are unlikely to differ in practice, and
>   dealing with that corner case greatly complicates the implementation, given
>   the way DxeCore allocates memory for itself in the implementation of the page
>   and pool allocation routines.
> - apply the EfiConventionalMemory policy to untested RAM regions in the GCD
>   memory space map: without this, we may still have a large region of RAM that
>   is exploitable, and it also removes the need to apply memory protections in
>   PromoteMemoryResource (), which is very difficult to achieve without a major
>   restructuring of the code due to the way locking is implemented here.
> - add missing ApplyMemoryProtectionPolicy() call to CoreAddMemoryDescriptor()
> - use CoreAcquireLockOrFail() on gMemoryLock for CoreAllocatePoolPages (#4)
> - incorporate feedback from Liming (#2, #6)
> - add patch to enable the NX memory protection policy for ArmVirtPkg (#7)
> 
> Changes since v2:
> - added patch to make EBC use EfiBootServicesCode pool allocations for thunks
> - redefine PCD according to Jiewen's feedback, including default value
> - use sorted memory map and merge adjacent entries with the same policy, to
>   prevent unnecessary page table splitting
> - ignore policy when executing in SMM
> - refactor the logic for managing permission attributes of pool allocations
> - added some R-b's
> 
> Changes since v1:
> - allocate code pages for PE/COFF images in PeiCore, so that DxeCore pages have
>   the expected memory type (as suggested by Jiewen)
> - add patch to inhibit page table updates while syncing the GCD memory space
>   map with the page tables
> - add PCD to set memory protection policy, which allows the policy for reserved
>   and ACPI/NVS memory to be configured separately
> - move attribute manipulation into DxeCore page allocation code: this way, we
>   should be able to solve the EBC case by allocating BootServicesCode pool
>   memory explicitly.
> 
> Series can be found here:
> https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/log/?h=memprot-take2-v4
> 
> Ard Biesheuvel (7):
>   ArmPkg/CpuDxe: ignore attribute changes during SyncCacheConfig()
>   MdeModulePkg/PeiCore: allocate BootServicesCode memory for PE/COFF
>     images
>   MdeModulePkg/EbcDxe: use EfiBootServicesCode memory for thunks
>   MdeModulePkg/DxeCore: use separate lock for pool allocations
>   MdeModulePkg: define PCD for DXE memory protection policy
>   MdeModulePkg/DxeCore: implement memory protection policy
>   ArmVirtPkg/ArmVirt.dsc.inc: enable NX memory protection for all
>     platforms
> 
>  ArmPkg/Drivers/CpuDxe/CpuDxe.c                     |   3 +
>  ArmPkg/Drivers/CpuDxe/CpuDxe.h                     |   1 +
>  ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c               |   4 +
>  ArmVirtPkg/ArmVirt.dsc.inc                         |   6 +
>  MdeModulePkg/Core/Dxe/DxeMain.h                    |  24 ++
>  MdeModulePkg/Core/Dxe/DxeMain.inf                  |   1 +
>  MdeModulePkg/Core/Dxe/Mem/Page.c                   |   7 +
>  MdeModulePkg/Core/Dxe/Mem/Pool.c                   |  65 +++-
>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c      | 371 +++++++++++++++++++-
>  MdeModulePkg/Core/Pei/Image/Image.c                |  23 +-
>  MdeModulePkg/MdeModulePkg.dec                      |  32 ++
>  MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c |   2 +-
>  MdeModulePkg/Universal/EbcDxe/EbcInt.c             |  23 ++
>  MdeModulePkg/Universal/EbcDxe/EbcInt.h             |  14 +
>  MdeModulePkg/Universal/EbcDxe/Ia32/EbcSupport.c    |   2 +-
>  MdeModulePkg/Universal/EbcDxe/Ipf/EbcSupport.c     |   2 +-
>  MdeModulePkg/Universal/EbcDxe/X64/EbcSupport.c     |   2 +-
>  17 files changed, 558 insertions(+), 24 deletions(-)
> 

I regression-tested this series for x86 / OVMF as under v3, with the zero PCD default, and experienced no issues.

However, v4 breaks booting Fedora 24 on my Mustang (aarch64/KVM):

-----------------
[Bds]Booting Fedora
FSOpen: Open '\EFI\fedora\shim.efi' Success
[Bds] DevicePath expand: HD(1,MBR,0xDB4976D3,0x800,0x64000)/\EFI\fedora\shim.efi -> PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)/Scsi(0x1,0x0)/HD(1,MBR,0xDB4976D3,0x800,0x64000)/\EFI\fedora\shim.efi
[Security] 3rd party image[0] can be loaded after EndOfDxe: PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)/Scsi(0x1,0x0)/HD(1,MBR,0xDB4976D3,0x800,0x64000)/\EFI\fedora\shim.efi.
InstallProtocolInterface: [EfiLoadedImageProtocol] 13A6D2AC0
Loading driver at 0x001382F4000 EntryPoint=0x001382F4148
Loading driver at 0x001382F4000 EntryPoint=0x001382F4148 
InstallProtocolInterface: [EfiLoadedImageDevicePathProtocol] 138CDBD98
ProtectUefiImageCommon - 0x3A6D2AC0
  - 0x00000001382F4000 - 0x00000000000CBAE0
!!!!!!!!  ProtectUefiImageCommon - Section Alignment(0x20) is incorrect  !!!!!!!!
FSOpen: Open '\EFI\fedora\grubaa64.efi' Success


Synchronous Exception at 0x00000001380F7400

  X0 0x000000013A6EEA98   X1 0x000000013BFF0018   X2 0x00000001380F7400   X3 0x00000000000FD000
  X4 0x0000000000000000   X5 0x0000000000000000   X6 0x0000000138362AF4   X7 0x0000000000000000
  X8 0x000000013C01F548   X9 0x0000000200000000  X10 0x00000001380F6000  X11 0x00000001382F3FFF
 X12 0x0000000000000000  X13 0x0000000000000008  X14 0x0000000000000000  X15 0x0000000000000000
 X16 0x000000013EC6ABD0  X17 0x0000000000000000  X18 0x0000000000000000  X19 0x0000000138CDB698
 X20 0x000000013A746E18  X21 0x0000000000000000  X22 0x0000000000000000  X23 0x0000000000000000
 X24 0x0000000000000000  X25 0x0000000000000000  X26 0x0000000000000000  X27 0x0000000000000000
 X28 0x0000000000000000   FP 0x000000013EC6AA50   LR 0x00000001382F80F8  

  V0 0xAFAFAFAFAFAFAFAF AFAFAFAFAFAFAFAF   V1 0x6963702F66666666 6666666666666666
  V2 0x697363732F312C31 406567646972622D   V3 0x0000000000000000 0000000000000000
  V4 0x0000000000000400 0000000000000000   V5 0x4010040140100401 4010040140100401
  V6 0x0004000000000000 0004000000000000   V7 0x0000000000000000 0000000000000000
  V8 0x0000000000000000 0000000000000000   V9 0x0000000000000000 0000000000000000
 V10 0x0000000000000000 0000000000000000  V11 0x0000000000000000 0000000000000000
 V12 0x0000000000000000 0000000000000000  V13 0x0000000000000000 0000000000000000
 V14 0x0000000000000000 0000000000000000  V15 0x0000000000000000 0000000000000000
 V16 0x0000000000000000 0000000000000000  V17 0x0000000000000000 0000000000000000
 V18 0x0000000000000000 0000000000000000  V19 0x0000000000000000 0000000000000000
 V20 0x0000000000000000 0000000000000000  V21 0x0000000000000000 0000000000000000
 V22 0x0000000000000000 0000000000000000  V23 0x0000000000000000 0000000000000000
 V24 0x0000000000000000 0000000000000000  V25 0x0000000000000000 0000000000000000
 V26 0x0000000000000000 0000000000000000  V27 0x0000000000000000 0000000000000000
 V28 0x0000000000000000 0000000000000000  V29 0x0000000000000000 0000000000000000
 V30 0x0000000000000000 0000000000000000  V31 0x0000000000000000 0000000000000000

  SP 0x000000013EC6AA50  ELR 0x00000001380F7400  SPSR 0x60000205  FPSR 0x00000000
 ESR 0x8600000E          FAR 0x00000001380F7400

 ESR : EC 0x21  IL 0x1  ISS 0x0000000E

Instruction abort: Permission fault, second level

Stack dump:
  000013EC6A950: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  000013EC6A970: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  000013EC6A990: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  000013EC6A9B0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  000013EC6A9D0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  000013EC6A9F0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  000013EC6AA10: 0000000000000000 0000000000000000 00000001380F7400 0000000060000205
  000013EC6AA30: 0000000000000000 000000008600000E 00000001380F7400 0000000400000800
> 000013EC6AA50: 000000013EC6AB50 00000001382F8198 0000000138367370 000000013A6EEA98
  000013EC6AA70: 0000000138CDB698 000FD00000000000 00000001381F5018 0000000000000000
  000013EC6AA90: 0000000000000000 00000001382F3D18 0000000000001000 0000000000000000
  000013EC6AAB0: 000000013BFF0018 000000013A4BA518 000000013A6D2F98 0000000000000000
  000013EC6AAD0: 0000000000000000 0000000000000000 00000001382F4000 00000000000CBAE0
  000013EC6AAF0: 0000000200000001 0000000000000000 000000013A6D2AC0 11D295625B1B31A1
  000013EC6AB10: 3B7269C9A0003F8E 0000000000000000 00000001382F3F98 000000003EC6AB58
  000013EC6AB30: 000000013EC6AB60 800000000000000E 000000013EC6AB80 0000000000000000
ASSERT [ArmCpuDxe] .../ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c(265): ((BOOLEAN)(0==1))
-----------------

The "shim.efi" binary is not built with the required section alignment, but that's not a problem, it only elicits a warning, and that's it. "shim.efi" still proceeds to load "grubaa64.efi".

However, "grubaa64.efi" blows up.

I experience the same with my "RHEL for ARM 7.3" guest.

In my "openSUSE Tumbleweed" guest, "shim.efi" isn't actually used; there the "opensuse" UEFI boot option refers to grub directly. There I even catch the message "Welcome to GRUB!", but then it crashes too:

---------------
[Bds]Booting opensuse
FSOpen: Open '\EFI\opensuse\grubaa64.efi' Success
[Bds] DevicePath expand: HD(1,GPT,3E62269B-A0A1-4F3C-B404-081796D9CDB4,0x800,0x4E000)/\EFI\opensuse\grubaa64.efi -> PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)/Scsi(0x1,0x0)/HD(1,GPT,3E62269B-A0A1-4F3C-B404-081796D9CDB4,0x800,0x4E000)/\EFI\opensuse\grubaa64.efi
[Security] 3rd party image[0] can be loaded after EndOfDxe: PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)/Scsi(0x1,0x0)/HD(1,GPT,3E62269B-A0A1-4F3C-B404-081796D9CDB4,0x800,0x4E000)/\EFI\opensuse\grubaa64.efi.
InstallProtocolInterface: [EfiLoadedImageProtocol] 13A69FD40
Loading driver at 0x00138391000 EntryPoint=0x00138391400
Loading driver at 0x00138391000 EntryPoint=0x00138391400 
InstallProtocolInterface: [EfiLoadedImageDevicePathProtocol] 138CD4E98
ProtectUefiImageCommon - 0x3A69FD40
  - 0x0000000138391000 - 0x000000000002E600
!!!!!!!!  ProtectUefiImageCommon - Section Alignment(0x200) is incorrect  !!!!!!!!
Welcome to GRUB!

^M

Synchronous Exception at 0x00000000FFFF1F3C

  X0 0x00000000FFFF3720   X1 0x00000000FFFF1F3C   X2 0x000000000000000D   X3 0x000000013839FA30
  X4 0x00000000FFFF1F3C   X5 0x00000000FFFF0800   X6 0x00000000FFFF1000   X7 0x0000000000000000
  X8 0x00000000FFFF2120   X9 0x000000000000001F  X10 0x0000000000000000  X11 0x000000013EC6A880
 X12 0x0098989800989898  X13 0x0098989800989898  X14 0x0000000000000001  X15 0x0000000000000003
 X16 0x000000013EC6ABD0  X17 0x0000000000000000  X18 0x0000000000000000  X19 0x00000000FFFF3720
 X20 0x0000000138399000  X21 0x0000000138399000  X22 0x0000000000000000  X23 0x0000000000000000
 X24 0x0000000000000000  X25 0x0000000000000000  X26 0x0000000000000000  X27 0x0000000000000000
 X28 0x0000000000000000   FP 0x000000013EC6AB60   LR 0x00000001383980CC  

  V0 0xAFAFAFAFAFAFAFAF AFAFAFAFAFAFAFAF   V1 0x6963702F66666666 6666666666666666
  V2 0x697363732F312C31 406567646972622D   V3 0x0000000000000000 0000000000000000
  V4 0x0000000000000400 0000000000000000   V5 0x4010040140100401 4010040140100401
  V6 0x0004000000000000 0004000000000000   V7 0x0000000000000000 0000000000000000
  V8 0x0000000000000000 0000000000000000   V9 0x0000000000000000 0000000000000000
 V10 0x0000000000000000 0000000000000000  V11 0x0000000000000000 0000000000000000
 V12 0x0000000000000000 0000000000000000  V13 0x0000000000000000 0000000000000000
 V14 0x0000000000000000 0000000000000000  V15 0x0000000000000000 0000000000000000
 V16 0x0000000000000000 0000000000000000  V17 0x0000000000000000 0000000000000000
 V18 0x0000000000000000 0000000000000000  V19 0x0000000000000000 0000000000000000
 V20 0x0000000000000000 0000000000000000  V21 0x0000000000000000 0000000000000000
 V22 0x0000000000000000 0000000000000000  V23 0x0000000000000000 0000000000000000
 V24 0x0000000000000000 0000000000000000  V25 0x0000000000000000 0000000000000000
 V26 0x0000000000000000 0000000000000000  V27 0x0000000000000000 0000000000000000
 V28 0x0000000000000000 0000000000000000  V29 0x0000000000000000 0000000000000000
 V30 0x0000000000000000 0000000000000000  V31 0x0000000000000000 0000000000000000

  SP 0x000000013EC6AB60  ELR 0x00000000FFFF1F3C  SPSR 0x60000205  FPSR 0x00000000
 ESR 0x8600000D          FAR 0x00000000FFFF1F3C

 ESR : EC 0x21  IL 0x1  ISS 0x0000000D

Instruction abort: Permission fault, first level

Stack dump:
  000013EC6AA60: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  000013EC6AA80: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  000013EC6AAA0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  000013EC6AAC0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  000013EC6AAE0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  000013EC6AB00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  000013EC6AB20: 0000000000000000 0000000000000000 00000000FFFF1F3C 0000000060000205
  000013EC6AB40: 0000000000000000 000000008600000D 00000000FFFF1F3C 0000000000000000
> 000013EC6AB60: 000000013EC6AB80 0000000138399650 00000001383AAA18 0000000000000000
  000013EC6AB80: 000000013EC6ABD0 000000013EC71E50 000000013A732218 000000013A747718
  000013EC6ABA0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
  000013EC6ABC0: 0000000000000000 000000013A747718 000000013EC6AC50 000000013BA5C62C
  000013EC6ABE0: 000000013EC6AC00 0000000138CD4060 0000000138CD4068 000000013A6EE018
  000013EC6AC00: 000000013EC6AC30 000000013BA577B4 0000000000000000 0000000000000000
  000013EC6AC20: 000000013ECA83D8 0000000000000126 000000013EC6AC50 000000013A69FD18
  000013EC6AC40: 000000013A6EE018 0000000000000000 000000013EC6ACF0 000000013BA4AF88
ASSERT [ArmCpuDxe] .../ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c(265): ((BOOLEAN)(0==1))
---------------

If I revert the last patch in the series (leaving ArmVirtQemu's PCD at the default 0), then all three guests boot fine.

This reminds me of the case when Star introduced the non-executable stack for DXE, we turned it on in OVMF, and that killed the grub version shipped with an older Debian release:

https://www.mail-archive.com/edk2-devel@lists.01.org/msg02022.html

So ultimately we had to make the PCD dynamic, and make it default to "off" (see commit ab081a50e565, "OvmfPkg: PlatformPei: take no-exec DXE settings from the QEMU command line", 2015-09-15).

Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4 0/7] MdeModulePkg/DxeCore: increased memory protection
Posted by Ard Biesheuvel 7 years, 1 month ago
On 28 February 2017 at 10:46, Laszlo Ersek <lersek@redhat.com> wrote:
> On 02/27/17 15:38, Ard Biesheuvel wrote:
>> Hello all,
>>
>> First of all, thanks for the reviews and regression testing. However, I did
>> not add the tested-by tags nor some of the R-b's, given the changes in this v4.
>>
>> This series implements a memory protection policy that removes all executable
>> permissions from writable memory regions, which greatly enhances security.
>> It is based on Jiewen's recent work, which is a step in the right direction,
>> but still leaves most of memory exploitable due to the default R+W+X
>> permissions.
>>
>> The idea is that the implementation of the CPU arch protocol goes over the
>> memory map and removes exec permissions from all regions that are not already
>> marked as 'code. This requires some preparatory work to ensure that the DxeCore
>> itself is covered by a BootServicesCode region, not a BootServicesData region.
>> Exec permissions are re-granted selectively, when the PE/COFF loader allocates
>> the space for it. Combined with Jiewen's code/data split, this removes all
>> RWX mapped regions.
>>
>> Changes since v3:
>> - mandate that the same policy applies to EfiConventionalMemory regions and
>>   EfiBootServicesData regions: they are unlikely to differ in practice, and
>>   dealing with that corner case greatly complicates the implementation, given
>>   the way DxeCore allocates memory for itself in the implementation of the page
>>   and pool allocation routines.
>> - apply the EfiConventionalMemory policy to untested RAM regions in the GCD
>>   memory space map: without this, we may still have a large region of RAM that
>>   is exploitable, and it also removes the need to apply memory protections in
>>   PromoteMemoryResource (), which is very difficult to achieve without a major
>>   restructuring of the code due to the way locking is implemented here.
>> - add missing ApplyMemoryProtectionPolicy() call to CoreAddMemoryDescriptor()
>> - use CoreAcquireLockOrFail() on gMemoryLock for CoreAllocatePoolPages (#4)
>> - incorporate feedback from Liming (#2, #6)
>> - add patch to enable the NX memory protection policy for ArmVirtPkg (#7)
>>
>> Changes since v2:
>> - added patch to make EBC use EfiBootServicesCode pool allocations for thunks
>> - redefine PCD according to Jiewen's feedback, including default value
>> - use sorted memory map and merge adjacent entries with the same policy, to
>>   prevent unnecessary page table splitting
>> - ignore policy when executing in SMM
>> - refactor the logic for managing permission attributes of pool allocations
>> - added some R-b's
>>
>> Changes since v1:
>> - allocate code pages for PE/COFF images in PeiCore, so that DxeCore pages have
>>   the expected memory type (as suggested by Jiewen)
>> - add patch to inhibit page table updates while syncing the GCD memory space
>>   map with the page tables
>> - add PCD to set memory protection policy, which allows the policy for reserved
>>   and ACPI/NVS memory to be configured separately
>> - move attribute manipulation into DxeCore page allocation code: this way, we
>>   should be able to solve the EBC case by allocating BootServicesCode pool
>>   memory explicitly.
>>
>> Series can be found here:
>> https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/log/?h=memprot-take2-v4
>>
>> Ard Biesheuvel (7):
>>   ArmPkg/CpuDxe: ignore attribute changes during SyncCacheConfig()
>>   MdeModulePkg/PeiCore: allocate BootServicesCode memory for PE/COFF
>>     images
>>   MdeModulePkg/EbcDxe: use EfiBootServicesCode memory for thunks
>>   MdeModulePkg/DxeCore: use separate lock for pool allocations
>>   MdeModulePkg: define PCD for DXE memory protection policy
>>   MdeModulePkg/DxeCore: implement memory protection policy
>>   ArmVirtPkg/ArmVirt.dsc.inc: enable NX memory protection for all
>>     platforms
>>
>>  ArmPkg/Drivers/CpuDxe/CpuDxe.c                     |   3 +
>>  ArmPkg/Drivers/CpuDxe/CpuDxe.h                     |   1 +
>>  ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c               |   4 +
>>  ArmVirtPkg/ArmVirt.dsc.inc                         |   6 +
>>  MdeModulePkg/Core/Dxe/DxeMain.h                    |  24 ++
>>  MdeModulePkg/Core/Dxe/DxeMain.inf                  |   1 +
>>  MdeModulePkg/Core/Dxe/Mem/Page.c                   |   7 +
>>  MdeModulePkg/Core/Dxe/Mem/Pool.c                   |  65 +++-
>>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c      | 371 +++++++++++++++++++-
>>  MdeModulePkg/Core/Pei/Image/Image.c                |  23 +-
>>  MdeModulePkg/MdeModulePkg.dec                      |  32 ++
>>  MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c |   2 +-
>>  MdeModulePkg/Universal/EbcDxe/EbcInt.c             |  23 ++
>>  MdeModulePkg/Universal/EbcDxe/EbcInt.h             |  14 +
>>  MdeModulePkg/Universal/EbcDxe/Ia32/EbcSupport.c    |   2 +-
>>  MdeModulePkg/Universal/EbcDxe/Ipf/EbcSupport.c     |   2 +-
>>  MdeModulePkg/Universal/EbcDxe/X64/EbcSupport.c     |   2 +-
>>  17 files changed, 558 insertions(+), 24 deletions(-)
>>
>
> I regression-tested this series for x86 / OVMF as under v3, with the zero PCD default, and experienced no issues.
>
> However, v4 breaks booting Fedora 24 on my Mustang (aarch64/KVM):
>
> -----------------
> [Bds]Booting Fedora
> FSOpen: Open '\EFI\fedora\shim.efi' Success
> [Bds] DevicePath expand: HD(1,MBR,0xDB4976D3,0x800,0x64000)/\EFI\fedora\shim.efi -> PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)/Scsi(0x1,0x0)/HD(1,MBR,0xDB4976D3,0x800,0x64000)/\EFI\fedora\shim.efi
> [Security] 3rd party image[0] can be loaded after EndOfDxe: PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)/Scsi(0x1,0x0)/HD(1,MBR,0xDB4976D3,0x800,0x64000)/\EFI\fedora\shim.efi.
> InstallProtocolInterface: [EfiLoadedImageProtocol] 13A6D2AC0
> Loading driver at 0x001382F4000 EntryPoint=0x001382F4148
> Loading driver at 0x001382F4000 EntryPoint=0x001382F4148
> InstallProtocolInterface: [EfiLoadedImageDevicePathProtocol] 138CDBD98
> ProtectUefiImageCommon - 0x3A6D2AC0
>   - 0x00000001382F4000 - 0x00000000000CBAE0
> !!!!!!!!  ProtectUefiImageCommon - Section Alignment(0x20) is incorrect  !!!!!!!!
> FSOpen: Open '\EFI\fedora\grubaa64.efi' Success
>
>
> Synchronous Exception at 0x00000001380F7400
>
>   X0 0x000000013A6EEA98   X1 0x000000013BFF0018   X2 0x00000001380F7400   X3 0x00000000000FD000
>   X4 0x0000000000000000   X5 0x0000000000000000   X6 0x0000000138362AF4   X7 0x0000000000000000
>   X8 0x000000013C01F548   X9 0x0000000200000000  X10 0x00000001380F6000  X11 0x00000001382F3FFF
>  X12 0x0000000000000000  X13 0x0000000000000008  X14 0x0000000000000000  X15 0x0000000000000000
>  X16 0x000000013EC6ABD0  X17 0x0000000000000000  X18 0x0000000000000000  X19 0x0000000138CDB698
>  X20 0x000000013A746E18  X21 0x0000000000000000  X22 0x0000000000000000  X23 0x0000000000000000
>  X24 0x0000000000000000  X25 0x0000000000000000  X26 0x0000000000000000  X27 0x0000000000000000
>  X28 0x0000000000000000   FP 0x000000013EC6AA50   LR 0x00000001382F80F8
>
>   V0 0xAFAFAFAFAFAFAFAF AFAFAFAFAFAFAFAF   V1 0x6963702F66666666 6666666666666666
>   V2 0x697363732F312C31 406567646972622D   V3 0x0000000000000000 0000000000000000
>   V4 0x0000000000000400 0000000000000000   V5 0x4010040140100401 4010040140100401
>   V6 0x0004000000000000 0004000000000000   V7 0x0000000000000000 0000000000000000
>   V8 0x0000000000000000 0000000000000000   V9 0x0000000000000000 0000000000000000
>  V10 0x0000000000000000 0000000000000000  V11 0x0000000000000000 0000000000000000
>  V12 0x0000000000000000 0000000000000000  V13 0x0000000000000000 0000000000000000
>  V14 0x0000000000000000 0000000000000000  V15 0x0000000000000000 0000000000000000
>  V16 0x0000000000000000 0000000000000000  V17 0x0000000000000000 0000000000000000
>  V18 0x0000000000000000 0000000000000000  V19 0x0000000000000000 0000000000000000
>  V20 0x0000000000000000 0000000000000000  V21 0x0000000000000000 0000000000000000
>  V22 0x0000000000000000 0000000000000000  V23 0x0000000000000000 0000000000000000
>  V24 0x0000000000000000 0000000000000000  V25 0x0000000000000000 0000000000000000
>  V26 0x0000000000000000 0000000000000000  V27 0x0000000000000000 0000000000000000
>  V28 0x0000000000000000 0000000000000000  V29 0x0000000000000000 0000000000000000
>  V30 0x0000000000000000 0000000000000000  V31 0x0000000000000000 0000000000000000
>
>   SP 0x000000013EC6AA50  ELR 0x00000001380F7400  SPSR 0x60000205  FPSR 0x00000000
>  ESR 0x8600000E          FAR 0x00000001380F7400
>
>  ESR : EC 0x21  IL 0x1  ISS 0x0000000E
>
> Instruction abort: Permission fault, second level
>

Hmm, that is disappointing. This is probably due to GRUB's modular
nature, which means it allocates memory and loads executable code into
it, under the assumption that memory is always executable in UEFI.

The short term fix is to remove the NX bit from LoaderData regions,
but in the mean time, I will work with Leif to get this fixed properly
(assuming there is a proper way to fix this)

Thanks a lot for taking the time to test this,
Ard.



> Stack dump:
>   000013EC6A950: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013EC6A970: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013EC6A990: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013EC6A9B0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013EC6A9D0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013EC6A9F0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013EC6AA10: 0000000000000000 0000000000000000 00000001380F7400 0000000060000205
>   000013EC6AA30: 0000000000000000 000000008600000E 00000001380F7400 0000000400000800
>> 000013EC6AA50: 000000013EC6AB50 00000001382F8198 0000000138367370 000000013A6EEA98
>   000013EC6AA70: 0000000138CDB698 000FD00000000000 00000001381F5018 0000000000000000
>   000013EC6AA90: 0000000000000000 00000001382F3D18 0000000000001000 0000000000000000
>   000013EC6AAB0: 000000013BFF0018 000000013A4BA518 000000013A6D2F98 0000000000000000
>   000013EC6AAD0: 0000000000000000 0000000000000000 00000001382F4000 00000000000CBAE0
>   000013EC6AAF0: 0000000200000001 0000000000000000 000000013A6D2AC0 11D295625B1B31A1
>   000013EC6AB10: 3B7269C9A0003F8E 0000000000000000 00000001382F3F98 000000003EC6AB58
>   000013EC6AB30: 000000013EC6AB60 800000000000000E 000000013EC6AB80 0000000000000000
> ASSERT [ArmCpuDxe] .../ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c(265): ((BOOLEAN)(0==1))
> -----------------
>
> The "shim.efi" binary is not built with the required section alignment, but that's not a problem, it only elicits a warning, and that's it. "shim.efi" still proceeds to load "grubaa64.efi".
>
> However, "grubaa64.efi" blows up.
>
> I experience the same with my "RHEL for ARM 7.3" guest.
>
> In my "openSUSE Tumbleweed" guest, "shim.efi" isn't actually used; there the "opensuse" UEFI boot option refers to grub directly. There I even catch the message "Welcome to GRUB!", but then it crashes too:
>
> ---------------
> [Bds]Booting opensuse
> FSOpen: Open '\EFI\opensuse\grubaa64.efi' Success
> [Bds] DevicePath expand: HD(1,GPT,3E62269B-A0A1-4F3C-B404-081796D9CDB4,0x800,0x4E000)/\EFI\opensuse\grubaa64.efi -> PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)/Scsi(0x1,0x0)/HD(1,GPT,3E62269B-A0A1-4F3C-B404-081796D9CDB4,0x800,0x4E000)/\EFI\opensuse\grubaa64.efi
> [Security] 3rd party image[0] can be loaded after EndOfDxe: PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)/Scsi(0x1,0x0)/HD(1,GPT,3E62269B-A0A1-4F3C-B404-081796D9CDB4,0x800,0x4E000)/\EFI\opensuse\grubaa64.efi.
> InstallProtocolInterface: [EfiLoadedImageProtocol] 13A69FD40
> Loading driver at 0x00138391000 EntryPoint=0x00138391400
> Loading driver at 0x00138391000 EntryPoint=0x00138391400
> InstallProtocolInterface: [EfiLoadedImageDevicePathProtocol] 138CD4E98
> ProtectUefiImageCommon - 0x3A69FD40
>   - 0x0000000138391000 - 0x000000000002E600
> !!!!!!!!  ProtectUefiImageCommon - Section Alignment(0x200) is incorrect  !!!!!!!!
> Welcome to GRUB!
>
> ^M
>
> Synchronous Exception at 0x00000000FFFF1F3C
>
>   X0 0x00000000FFFF3720   X1 0x00000000FFFF1F3C   X2 0x000000000000000D   X3 0x000000013839FA30
>   X4 0x00000000FFFF1F3C   X5 0x00000000FFFF0800   X6 0x00000000FFFF1000   X7 0x0000000000000000
>   X8 0x00000000FFFF2120   X9 0x000000000000001F  X10 0x0000000000000000  X11 0x000000013EC6A880
>  X12 0x0098989800989898  X13 0x0098989800989898  X14 0x0000000000000001  X15 0x0000000000000003
>  X16 0x000000013EC6ABD0  X17 0x0000000000000000  X18 0x0000000000000000  X19 0x00000000FFFF3720
>  X20 0x0000000138399000  X21 0x0000000138399000  X22 0x0000000000000000  X23 0x0000000000000000
>  X24 0x0000000000000000  X25 0x0000000000000000  X26 0x0000000000000000  X27 0x0000000000000000
>  X28 0x0000000000000000   FP 0x000000013EC6AB60   LR 0x00000001383980CC
>
>   V0 0xAFAFAFAFAFAFAFAF AFAFAFAFAFAFAFAF   V1 0x6963702F66666666 6666666666666666
>   V2 0x697363732F312C31 406567646972622D   V3 0x0000000000000000 0000000000000000
>   V4 0x0000000000000400 0000000000000000   V5 0x4010040140100401 4010040140100401
>   V6 0x0004000000000000 0004000000000000   V7 0x0000000000000000 0000000000000000
>   V8 0x0000000000000000 0000000000000000   V9 0x0000000000000000 0000000000000000
>  V10 0x0000000000000000 0000000000000000  V11 0x0000000000000000 0000000000000000
>  V12 0x0000000000000000 0000000000000000  V13 0x0000000000000000 0000000000000000
>  V14 0x0000000000000000 0000000000000000  V15 0x0000000000000000 0000000000000000
>  V16 0x0000000000000000 0000000000000000  V17 0x0000000000000000 0000000000000000
>  V18 0x0000000000000000 0000000000000000  V19 0x0000000000000000 0000000000000000
>  V20 0x0000000000000000 0000000000000000  V21 0x0000000000000000 0000000000000000
>  V22 0x0000000000000000 0000000000000000  V23 0x0000000000000000 0000000000000000
>  V24 0x0000000000000000 0000000000000000  V25 0x0000000000000000 0000000000000000
>  V26 0x0000000000000000 0000000000000000  V27 0x0000000000000000 0000000000000000
>  V28 0x0000000000000000 0000000000000000  V29 0x0000000000000000 0000000000000000
>  V30 0x0000000000000000 0000000000000000  V31 0x0000000000000000 0000000000000000
>
>   SP 0x000000013EC6AB60  ELR 0x00000000FFFF1F3C  SPSR 0x60000205  FPSR 0x00000000
>  ESR 0x8600000D          FAR 0x00000000FFFF1F3C
>
>  ESR : EC 0x21  IL 0x1  ISS 0x0000000D
>
> Instruction abort: Permission fault, first level
>
> Stack dump:
>   000013EC6AA60: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013EC6AA80: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013EC6AAA0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013EC6AAC0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013EC6AAE0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013EC6AB00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013EC6AB20: 0000000000000000 0000000000000000 00000000FFFF1F3C 0000000060000205
>   000013EC6AB40: 0000000000000000 000000008600000D 00000000FFFF1F3C 0000000000000000
>> 000013EC6AB60: 000000013EC6AB80 0000000138399650 00000001383AAA18 0000000000000000
>   000013EC6AB80: 000000013EC6ABD0 000000013EC71E50 000000013A732218 000000013A747718
>   000013EC6ABA0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>   000013EC6ABC0: 0000000000000000 000000013A747718 000000013EC6AC50 000000013BA5C62C
>   000013EC6ABE0: 000000013EC6AC00 0000000138CD4060 0000000138CD4068 000000013A6EE018
>   000013EC6AC00: 000000013EC6AC30 000000013BA577B4 0000000000000000 0000000000000000
>   000013EC6AC20: 000000013ECA83D8 0000000000000126 000000013EC6AC50 000000013A69FD18
>   000013EC6AC40: 000000013A6EE018 0000000000000000 000000013EC6ACF0 000000013BA4AF88
> ASSERT [ArmCpuDxe] .../ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c(265): ((BOOLEAN)(0==1))
> ---------------
>
> If I revert the last patch in the series (leaving ArmVirtQemu's PCD at the default 0), then all three guests boot fine.
>
> This reminds me of the case when Star introduced the non-executable stack for DXE, we turned it on in OVMF, and that killed the grub version shipped with an older Debian release:
>
> https://www.mail-archive.com/edk2-devel@lists.01.org/msg02022.html
>
> So ultimately we had to make the PCD dynamic, and make it default to "off" (see commit ab081a50e565, "OvmfPkg: PlatformPei: take no-exec DXE settings from the QEMU command line", 2015-09-15).
>
> Thanks
> Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4 0/7] MdeModulePkg/DxeCore: increased memory protection
Posted by Ard Biesheuvel 7 years, 1 month ago
On 28 February 2017 at 10:52, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 28 February 2017 at 10:46, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 02/27/17 15:38, Ard Biesheuvel wrote:
>>> Hello all,
>>>
>>> First of all, thanks for the reviews and regression testing. However, I did
>>> not add the tested-by tags nor some of the R-b's, given the changes in this v4.
>>>
>>> This series implements a memory protection policy that removes all executable
>>> permissions from writable memory regions, which greatly enhances security.
>>> It is based on Jiewen's recent work, which is a step in the right direction,
>>> but still leaves most of memory exploitable due to the default R+W+X
>>> permissions.
>>>
>>> The idea is that the implementation of the CPU arch protocol goes over the
>>> memory map and removes exec permissions from all regions that are not already
>>> marked as 'code. This requires some preparatory work to ensure that the DxeCore
>>> itself is covered by a BootServicesCode region, not a BootServicesData region.
>>> Exec permissions are re-granted selectively, when the PE/COFF loader allocates
>>> the space for it. Combined with Jiewen's code/data split, this removes all
>>> RWX mapped regions.
>>>
>>> Changes since v3:
>>> - mandate that the same policy applies to EfiConventionalMemory regions and
>>>   EfiBootServicesData regions: they are unlikely to differ in practice, and
>>>   dealing with that corner case greatly complicates the implementation, given
>>>   the way DxeCore allocates memory for itself in the implementation of the page
>>>   and pool allocation routines.
>>> - apply the EfiConventionalMemory policy to untested RAM regions in the GCD
>>>   memory space map: without this, we may still have a large region of RAM that
>>>   is exploitable, and it also removes the need to apply memory protections in
>>>   PromoteMemoryResource (), which is very difficult to achieve without a major
>>>   restructuring of the code due to the way locking is implemented here.
>>> - add missing ApplyMemoryProtectionPolicy() call to CoreAddMemoryDescriptor()
>>> - use CoreAcquireLockOrFail() on gMemoryLock for CoreAllocatePoolPages (#4)
>>> - incorporate feedback from Liming (#2, #6)
>>> - add patch to enable the NX memory protection policy for ArmVirtPkg (#7)
>>>
>>> Changes since v2:
>>> - added patch to make EBC use EfiBootServicesCode pool allocations for thunks
>>> - redefine PCD according to Jiewen's feedback, including default value
>>> - use sorted memory map and merge adjacent entries with the same policy, to
>>>   prevent unnecessary page table splitting
>>> - ignore policy when executing in SMM
>>> - refactor the logic for managing permission attributes of pool allocations
>>> - added some R-b's
>>>
>>> Changes since v1:
>>> - allocate code pages for PE/COFF images in PeiCore, so that DxeCore pages have
>>>   the expected memory type (as suggested by Jiewen)
>>> - add patch to inhibit page table updates while syncing the GCD memory space
>>>   map with the page tables
>>> - add PCD to set memory protection policy, which allows the policy for reserved
>>>   and ACPI/NVS memory to be configured separately
>>> - move attribute manipulation into DxeCore page allocation code: this way, we
>>>   should be able to solve the EBC case by allocating BootServicesCode pool
>>>   memory explicitly.
>>>
>>> Series can be found here:
>>> https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/log/?h=memprot-take2-v4
>>>
>>> Ard Biesheuvel (7):
>>>   ArmPkg/CpuDxe: ignore attribute changes during SyncCacheConfig()
>>>   MdeModulePkg/PeiCore: allocate BootServicesCode memory for PE/COFF
>>>     images
>>>   MdeModulePkg/EbcDxe: use EfiBootServicesCode memory for thunks
>>>   MdeModulePkg/DxeCore: use separate lock for pool allocations
>>>   MdeModulePkg: define PCD for DXE memory protection policy
>>>   MdeModulePkg/DxeCore: implement memory protection policy
>>>   ArmVirtPkg/ArmVirt.dsc.inc: enable NX memory protection for all
>>>     platforms
>>>
>>>  ArmPkg/Drivers/CpuDxe/CpuDxe.c                     |   3 +
>>>  ArmPkg/Drivers/CpuDxe/CpuDxe.h                     |   1 +
>>>  ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c               |   4 +
>>>  ArmVirtPkg/ArmVirt.dsc.inc                         |   6 +
>>>  MdeModulePkg/Core/Dxe/DxeMain.h                    |  24 ++
>>>  MdeModulePkg/Core/Dxe/DxeMain.inf                  |   1 +
>>>  MdeModulePkg/Core/Dxe/Mem/Page.c                   |   7 +
>>>  MdeModulePkg/Core/Dxe/Mem/Pool.c                   |  65 +++-
>>>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c      | 371 +++++++++++++++++++-
>>>  MdeModulePkg/Core/Pei/Image/Image.c                |  23 +-
>>>  MdeModulePkg/MdeModulePkg.dec                      |  32 ++
>>>  MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c |   2 +-
>>>  MdeModulePkg/Universal/EbcDxe/EbcInt.c             |  23 ++
>>>  MdeModulePkg/Universal/EbcDxe/EbcInt.h             |  14 +
>>>  MdeModulePkg/Universal/EbcDxe/Ia32/EbcSupport.c    |   2 +-
>>>  MdeModulePkg/Universal/EbcDxe/Ipf/EbcSupport.c     |   2 +-
>>>  MdeModulePkg/Universal/EbcDxe/X64/EbcSupport.c     |   2 +-
>>>  17 files changed, 558 insertions(+), 24 deletions(-)
>>>
>>
>> I regression-tested this series for x86 / OVMF as under v3, with the zero PCD default, and experienced no issues.
>>
>> However, v4 breaks booting Fedora 24 on my Mustang (aarch64/KVM):
>>
>> -----------------
>> [Bds]Booting Fedora
>> FSOpen: Open '\EFI\fedora\shim.efi' Success
>> [Bds] DevicePath expand: HD(1,MBR,0xDB4976D3,0x800,0x64000)/\EFI\fedora\shim.efi -> PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)/Scsi(0x1,0x0)/HD(1,MBR,0xDB4976D3,0x800,0x64000)/\EFI\fedora\shim.efi
>> [Security] 3rd party image[0] can be loaded after EndOfDxe: PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)/Scsi(0x1,0x0)/HD(1,MBR,0xDB4976D3,0x800,0x64000)/\EFI\fedora\shim.efi.
>> InstallProtocolInterface: [EfiLoadedImageProtocol] 13A6D2AC0
>> Loading driver at 0x001382F4000 EntryPoint=0x001382F4148
>> Loading driver at 0x001382F4000 EntryPoint=0x001382F4148
>> InstallProtocolInterface: [EfiLoadedImageDevicePathProtocol] 138CDBD98
>> ProtectUefiImageCommon - 0x3A6D2AC0
>>   - 0x00000001382F4000 - 0x00000000000CBAE0
>> !!!!!!!!  ProtectUefiImageCommon - Section Alignment(0x20) is incorrect  !!!!!!!!
>> FSOpen: Open '\EFI\fedora\grubaa64.efi' Success
>>
>>
>> Synchronous Exception at 0x00000001380F7400
>>
>>   X0 0x000000013A6EEA98   X1 0x000000013BFF0018   X2 0x00000001380F7400   X3 0x00000000000FD000
>>   X4 0x0000000000000000   X5 0x0000000000000000   X6 0x0000000138362AF4   X7 0x0000000000000000
>>   X8 0x000000013C01F548   X9 0x0000000200000000  X10 0x00000001380F6000  X11 0x00000001382F3FFF
>>  X12 0x0000000000000000  X13 0x0000000000000008  X14 0x0000000000000000  X15 0x0000000000000000
>>  X16 0x000000013EC6ABD0  X17 0x0000000000000000  X18 0x0000000000000000  X19 0x0000000138CDB698
>>  X20 0x000000013A746E18  X21 0x0000000000000000  X22 0x0000000000000000  X23 0x0000000000000000
>>  X24 0x0000000000000000  X25 0x0000000000000000  X26 0x0000000000000000  X27 0x0000000000000000
>>  X28 0x0000000000000000   FP 0x000000013EC6AA50   LR 0x00000001382F80F8
>>
>>   V0 0xAFAFAFAFAFAFAFAF AFAFAFAFAFAFAFAF   V1 0x6963702F66666666 6666666666666666
>>   V2 0x697363732F312C31 406567646972622D   V3 0x0000000000000000 0000000000000000
>>   V4 0x0000000000000400 0000000000000000   V5 0x4010040140100401 4010040140100401
>>   V6 0x0004000000000000 0004000000000000   V7 0x0000000000000000 0000000000000000
>>   V8 0x0000000000000000 0000000000000000   V9 0x0000000000000000 0000000000000000
>>  V10 0x0000000000000000 0000000000000000  V11 0x0000000000000000 0000000000000000
>>  V12 0x0000000000000000 0000000000000000  V13 0x0000000000000000 0000000000000000
>>  V14 0x0000000000000000 0000000000000000  V15 0x0000000000000000 0000000000000000
>>  V16 0x0000000000000000 0000000000000000  V17 0x0000000000000000 0000000000000000
>>  V18 0x0000000000000000 0000000000000000  V19 0x0000000000000000 0000000000000000
>>  V20 0x0000000000000000 0000000000000000  V21 0x0000000000000000 0000000000000000
>>  V22 0x0000000000000000 0000000000000000  V23 0x0000000000000000 0000000000000000
>>  V24 0x0000000000000000 0000000000000000  V25 0x0000000000000000 0000000000000000
>>  V26 0x0000000000000000 0000000000000000  V27 0x0000000000000000 0000000000000000
>>  V28 0x0000000000000000 0000000000000000  V29 0x0000000000000000 0000000000000000
>>  V30 0x0000000000000000 0000000000000000  V31 0x0000000000000000 0000000000000000
>>
>>   SP 0x000000013EC6AA50  ELR 0x00000001380F7400  SPSR 0x60000205  FPSR 0x00000000
>>  ESR 0x8600000E          FAR 0x00000001380F7400
>>
>>  ESR : EC 0x21  IL 0x1  ISS 0x0000000E
>>
>> Instruction abort: Permission fault, second level
>>
>
> Hmm, that is disappointing. This is probably due to GRUB's modular
> nature, which means it allocates memory and loads executable code into
> it, under the assumption that memory is always executable in UEFI.
>
> The short term fix is to remove the NX bit from LoaderData regions,
> but in the mean time, I will work with Leif to get this fixed properly
> (assuming there is a proper way to fix this)
>

Care to have a quick go at using 0xC000000000007FD1 instead? (if you
are not already doing so)

Thanks,
Ard.


>> Stack dump:
>>   000013EC6A950: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>   000013EC6A970: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>   000013EC6A990: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>   000013EC6A9B0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>   000013EC6A9D0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>   000013EC6A9F0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>   000013EC6AA10: 0000000000000000 0000000000000000 00000001380F7400 0000000060000205
>>   000013EC6AA30: 0000000000000000 000000008600000E 00000001380F7400 0000000400000800
>>> 000013EC6AA50: 000000013EC6AB50 00000001382F8198 0000000138367370 000000013A6EEA98
>>   000013EC6AA70: 0000000138CDB698 000FD00000000000 00000001381F5018 0000000000000000
>>   000013EC6AA90: 0000000000000000 00000001382F3D18 0000000000001000 0000000000000000
>>   000013EC6AAB0: 000000013BFF0018 000000013A4BA518 000000013A6D2F98 0000000000000000
>>   000013EC6AAD0: 0000000000000000 0000000000000000 00000001382F4000 00000000000CBAE0
>>   000013EC6AAF0: 0000000200000001 0000000000000000 000000013A6D2AC0 11D295625B1B31A1
>>   000013EC6AB10: 3B7269C9A0003F8E 0000000000000000 00000001382F3F98 000000003EC6AB58
>>   000013EC6AB30: 000000013EC6AB60 800000000000000E 000000013EC6AB80 0000000000000000
>> ASSERT [ArmCpuDxe] .../ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c(265): ((BOOLEAN)(0==1))
>> -----------------
>>
>> The "shim.efi" binary is not built with the required section alignment, but that's not a problem, it only elicits a warning, and that's it. "shim.efi" still proceeds to load "grubaa64.efi".
>>
>> However, "grubaa64.efi" blows up.
>>
>> I experience the same with my "RHEL for ARM 7.3" guest.
>>
>> In my "openSUSE Tumbleweed" guest, "shim.efi" isn't actually used; there the "opensuse" UEFI boot option refers to grub directly. There I even catch the message "Welcome to GRUB!", but then it crashes too:
>>
>> ---------------
>> [Bds]Booting opensuse
>> FSOpen: Open '\EFI\opensuse\grubaa64.efi' Success
>> [Bds] DevicePath expand: HD(1,GPT,3E62269B-A0A1-4F3C-B404-081796D9CDB4,0x800,0x4E000)/\EFI\opensuse\grubaa64.efi -> PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)/Scsi(0x1,0x0)/HD(1,GPT,3E62269B-A0A1-4F3C-B404-081796D9CDB4,0x800,0x4E000)/\EFI\opensuse\grubaa64.efi
>> [Security] 3rd party image[0] can be loaded after EndOfDxe: PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)/Scsi(0x1,0x0)/HD(1,GPT,3E62269B-A0A1-4F3C-B404-081796D9CDB4,0x800,0x4E000)/\EFI\opensuse\grubaa64.efi.
>> InstallProtocolInterface: [EfiLoadedImageProtocol] 13A69FD40
>> Loading driver at 0x00138391000 EntryPoint=0x00138391400
>> Loading driver at 0x00138391000 EntryPoint=0x00138391400
>> InstallProtocolInterface: [EfiLoadedImageDevicePathProtocol] 138CD4E98
>> ProtectUefiImageCommon - 0x3A69FD40
>>   - 0x0000000138391000 - 0x000000000002E600
>> !!!!!!!!  ProtectUefiImageCommon - Section Alignment(0x200) is incorrect  !!!!!!!!
>> Welcome to GRUB!
>>
>> ^M
>>
>> Synchronous Exception at 0x00000000FFFF1F3C
>>
>>   X0 0x00000000FFFF3720   X1 0x00000000FFFF1F3C   X2 0x000000000000000D   X3 0x000000013839FA30
>>   X4 0x00000000FFFF1F3C   X5 0x00000000FFFF0800   X6 0x00000000FFFF1000   X7 0x0000000000000000
>>   X8 0x00000000FFFF2120   X9 0x000000000000001F  X10 0x0000000000000000  X11 0x000000013EC6A880
>>  X12 0x0098989800989898  X13 0x0098989800989898  X14 0x0000000000000001  X15 0x0000000000000003
>>  X16 0x000000013EC6ABD0  X17 0x0000000000000000  X18 0x0000000000000000  X19 0x00000000FFFF3720
>>  X20 0x0000000138399000  X21 0x0000000138399000  X22 0x0000000000000000  X23 0x0000000000000000
>>  X24 0x0000000000000000  X25 0x0000000000000000  X26 0x0000000000000000  X27 0x0000000000000000
>>  X28 0x0000000000000000   FP 0x000000013EC6AB60   LR 0x00000001383980CC
>>
>>   V0 0xAFAFAFAFAFAFAFAF AFAFAFAFAFAFAFAF   V1 0x6963702F66666666 6666666666666666
>>   V2 0x697363732F312C31 406567646972622D   V3 0x0000000000000000 0000000000000000
>>   V4 0x0000000000000400 0000000000000000   V5 0x4010040140100401 4010040140100401
>>   V6 0x0004000000000000 0004000000000000   V7 0x0000000000000000 0000000000000000
>>   V8 0x0000000000000000 0000000000000000   V9 0x0000000000000000 0000000000000000
>>  V10 0x0000000000000000 0000000000000000  V11 0x0000000000000000 0000000000000000
>>  V12 0x0000000000000000 0000000000000000  V13 0x0000000000000000 0000000000000000
>>  V14 0x0000000000000000 0000000000000000  V15 0x0000000000000000 0000000000000000
>>  V16 0x0000000000000000 0000000000000000  V17 0x0000000000000000 0000000000000000
>>  V18 0x0000000000000000 0000000000000000  V19 0x0000000000000000 0000000000000000
>>  V20 0x0000000000000000 0000000000000000  V21 0x0000000000000000 0000000000000000
>>  V22 0x0000000000000000 0000000000000000  V23 0x0000000000000000 0000000000000000
>>  V24 0x0000000000000000 0000000000000000  V25 0x0000000000000000 0000000000000000
>>  V26 0x0000000000000000 0000000000000000  V27 0x0000000000000000 0000000000000000
>>  V28 0x0000000000000000 0000000000000000  V29 0x0000000000000000 0000000000000000
>>  V30 0x0000000000000000 0000000000000000  V31 0x0000000000000000 0000000000000000
>>
>>   SP 0x000000013EC6AB60  ELR 0x00000000FFFF1F3C  SPSR 0x60000205  FPSR 0x00000000
>>  ESR 0x8600000D          FAR 0x00000000FFFF1F3C
>>
>>  ESR : EC 0x21  IL 0x1  ISS 0x0000000D
>>
>> Instruction abort: Permission fault, first level
>>
>> Stack dump:
>>   000013EC6AA60: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>   000013EC6AA80: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>   000013EC6AAA0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>   000013EC6AAC0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>   000013EC6AAE0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>   000013EC6AB00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>   000013EC6AB20: 0000000000000000 0000000000000000 00000000FFFF1F3C 0000000060000205
>>   000013EC6AB40: 0000000000000000 000000008600000D 00000000FFFF1F3C 0000000000000000
>>> 000013EC6AB60: 000000013EC6AB80 0000000138399650 00000001383AAA18 0000000000000000
>>   000013EC6AB80: 000000013EC6ABD0 000000013EC71E50 000000013A732218 000000013A747718
>>   000013EC6ABA0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>   000013EC6ABC0: 0000000000000000 000000013A747718 000000013EC6AC50 000000013BA5C62C
>>   000013EC6ABE0: 000000013EC6AC00 0000000138CD4060 0000000138CD4068 000000013A6EE018
>>   000013EC6AC00: 000000013EC6AC30 000000013BA577B4 0000000000000000 0000000000000000
>>   000013EC6AC20: 000000013ECA83D8 0000000000000126 000000013EC6AC50 000000013A69FD18
>>   000013EC6AC40: 000000013A6EE018 0000000000000000 000000013EC6ACF0 000000013BA4AF88
>> ASSERT [ArmCpuDxe] .../ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c(265): ((BOOLEAN)(0==1))
>> ---------------
>>
>> If I revert the last patch in the series (leaving ArmVirtQemu's PCD at the default 0), then all three guests boot fine.
>>
>> This reminds me of the case when Star introduced the non-executable stack for DXE, we turned it on in OVMF, and that killed the grub version shipped with an older Debian release:
>>
>> https://www.mail-archive.com/edk2-devel@lists.01.org/msg02022.html
>>
>> So ultimately we had to make the PCD dynamic, and make it default to "off" (see commit ab081a50e565, "OvmfPkg: PlatformPei: take no-exec DXE settings from the QEMU command line", 2015-09-15).
>>
>> Thanks
>> Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4 0/7] MdeModulePkg/DxeCore: increased memory protection
Posted by Ard Biesheuvel 7 years, 1 month ago
On 28 February 2017 at 10:59, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 28 February 2017 at 10:52, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 28 February 2017 at 10:46, Laszlo Ersek <lersek@redhat.com> wrote:
>>> On 02/27/17 15:38, Ard Biesheuvel wrote:
>>>> Hello all,
>>>>
>>>> First of all, thanks for the reviews and regression testing. However, I did
>>>> not add the tested-by tags nor some of the R-b's, given the changes in this v4.
>>>>
>>>> This series implements a memory protection policy that removes all executable
>>>> permissions from writable memory regions, which greatly enhances security.
>>>> It is based on Jiewen's recent work, which is a step in the right direction,
>>>> but still leaves most of memory exploitable due to the default R+W+X
>>>> permissions.
>>>>
>>>> The idea is that the implementation of the CPU arch protocol goes over the
>>>> memory map and removes exec permissions from all regions that are not already
>>>> marked as 'code. This requires some preparatory work to ensure that the DxeCore
>>>> itself is covered by a BootServicesCode region, not a BootServicesData region.
>>>> Exec permissions are re-granted selectively, when the PE/COFF loader allocates
>>>> the space for it. Combined with Jiewen's code/data split, this removes all
>>>> RWX mapped regions.
>>>>
>>>> Changes since v3:
>>>> - mandate that the same policy applies to EfiConventionalMemory regions and
>>>>   EfiBootServicesData regions: they are unlikely to differ in practice, and
>>>>   dealing with that corner case greatly complicates the implementation, given
>>>>   the way DxeCore allocates memory for itself in the implementation of the page
>>>>   and pool allocation routines.
>>>> - apply the EfiConventionalMemory policy to untested RAM regions in the GCD
>>>>   memory space map: without this, we may still have a large region of RAM that
>>>>   is exploitable, and it also removes the need to apply memory protections in
>>>>   PromoteMemoryResource (), which is very difficult to achieve without a major
>>>>   restructuring of the code due to the way locking is implemented here.
>>>> - add missing ApplyMemoryProtectionPolicy() call to CoreAddMemoryDescriptor()
>>>> - use CoreAcquireLockOrFail() on gMemoryLock for CoreAllocatePoolPages (#4)
>>>> - incorporate feedback from Liming (#2, #6)
>>>> - add patch to enable the NX memory protection policy for ArmVirtPkg (#7)
>>>>
>>>> Changes since v2:
>>>> - added patch to make EBC use EfiBootServicesCode pool allocations for thunks
>>>> - redefine PCD according to Jiewen's feedback, including default value
>>>> - use sorted memory map and merge adjacent entries with the same policy, to
>>>>   prevent unnecessary page table splitting
>>>> - ignore policy when executing in SMM
>>>> - refactor the logic for managing permission attributes of pool allocations
>>>> - added some R-b's
>>>>
>>>> Changes since v1:
>>>> - allocate code pages for PE/COFF images in PeiCore, so that DxeCore pages have
>>>>   the expected memory type (as suggested by Jiewen)
>>>> - add patch to inhibit page table updates while syncing the GCD memory space
>>>>   map with the page tables
>>>> - add PCD to set memory protection policy, which allows the policy for reserved
>>>>   and ACPI/NVS memory to be configured separately
>>>> - move attribute manipulation into DxeCore page allocation code: this way, we
>>>>   should be able to solve the EBC case by allocating BootServicesCode pool
>>>>   memory explicitly.
>>>>
>>>> Series can be found here:
>>>> https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/log/?h=memprot-take2-v4
>>>>
>>>> Ard Biesheuvel (7):
>>>>   ArmPkg/CpuDxe: ignore attribute changes during SyncCacheConfig()
>>>>   MdeModulePkg/PeiCore: allocate BootServicesCode memory for PE/COFF
>>>>     images
>>>>   MdeModulePkg/EbcDxe: use EfiBootServicesCode memory for thunks
>>>>   MdeModulePkg/DxeCore: use separate lock for pool allocations
>>>>   MdeModulePkg: define PCD for DXE memory protection policy
>>>>   MdeModulePkg/DxeCore: implement memory protection policy
>>>>   ArmVirtPkg/ArmVirt.dsc.inc: enable NX memory protection for all
>>>>     platforms
>>>>
>>>>  ArmPkg/Drivers/CpuDxe/CpuDxe.c                     |   3 +
>>>>  ArmPkg/Drivers/CpuDxe/CpuDxe.h                     |   1 +
>>>>  ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c               |   4 +
>>>>  ArmVirtPkg/ArmVirt.dsc.inc                         |   6 +
>>>>  MdeModulePkg/Core/Dxe/DxeMain.h                    |  24 ++
>>>>  MdeModulePkg/Core/Dxe/DxeMain.inf                  |   1 +
>>>>  MdeModulePkg/Core/Dxe/Mem/Page.c                   |   7 +
>>>>  MdeModulePkg/Core/Dxe/Mem/Pool.c                   |  65 +++-
>>>>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c      | 371 +++++++++++++++++++-
>>>>  MdeModulePkg/Core/Pei/Image/Image.c                |  23 +-
>>>>  MdeModulePkg/MdeModulePkg.dec                      |  32 ++
>>>>  MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c |   2 +-
>>>>  MdeModulePkg/Universal/EbcDxe/EbcInt.c             |  23 ++
>>>>  MdeModulePkg/Universal/EbcDxe/EbcInt.h             |  14 +
>>>>  MdeModulePkg/Universal/EbcDxe/Ia32/EbcSupport.c    |   2 +-
>>>>  MdeModulePkg/Universal/EbcDxe/Ipf/EbcSupport.c     |   2 +-
>>>>  MdeModulePkg/Universal/EbcDxe/X64/EbcSupport.c     |   2 +-
>>>>  17 files changed, 558 insertions(+), 24 deletions(-)
>>>>
>>>
>>> I regression-tested this series for x86 / OVMF as under v3, with the zero PCD default, and experienced no issues.
>>>
>>> However, v4 breaks booting Fedora 24 on my Mustang (aarch64/KVM):
>>>
>>> -----------------
>>> [Bds]Booting Fedora
>>> FSOpen: Open '\EFI\fedora\shim.efi' Success
>>> [Bds] DevicePath expand: HD(1,MBR,0xDB4976D3,0x800,0x64000)/\EFI\fedora\shim.efi -> PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)/Scsi(0x1,0x0)/HD(1,MBR,0xDB4976D3,0x800,0x64000)/\EFI\fedora\shim.efi
>>> [Security] 3rd party image[0] can be loaded after EndOfDxe: PciRoot(0x0)/Pci(0x1,0x1)/Pci(0x0,0x0)/Scsi(0x1,0x0)/HD(1,MBR,0xDB4976D3,0x800,0x64000)/\EFI\fedora\shim.efi.
>>> InstallProtocolInterface: [EfiLoadedImageProtocol] 13A6D2AC0
>>> Loading driver at 0x001382F4000 EntryPoint=0x001382F4148
>>> Loading driver at 0x001382F4000 EntryPoint=0x001382F4148
>>> InstallProtocolInterface: [EfiLoadedImageDevicePathProtocol] 138CDBD98
>>> ProtectUefiImageCommon - 0x3A6D2AC0
>>>   - 0x00000001382F4000 - 0x00000000000CBAE0
>>> !!!!!!!!  ProtectUefiImageCommon - Section Alignment(0x20) is incorrect  !!!!!!!!
>>> FSOpen: Open '\EFI\fedora\grubaa64.efi' Success
>>>
>>>
>>> Synchronous Exception at 0x00000001380F7400
>>>
>>>   X0 0x000000013A6EEA98   X1 0x000000013BFF0018   X2 0x00000001380F7400   X3 0x00000000000FD000
>>>   X4 0x0000000000000000   X5 0x0000000000000000   X6 0x0000000138362AF4   X7 0x0000000000000000
>>>   X8 0x000000013C01F548   X9 0x0000000200000000  X10 0x00000001380F6000  X11 0x00000001382F3FFF
>>>  X12 0x0000000000000000  X13 0x0000000000000008  X14 0x0000000000000000  X15 0x0000000000000000
>>>  X16 0x000000013EC6ABD0  X17 0x0000000000000000  X18 0x0000000000000000  X19 0x0000000138CDB698
>>>  X20 0x000000013A746E18  X21 0x0000000000000000  X22 0x0000000000000000  X23 0x0000000000000000
>>>  X24 0x0000000000000000  X25 0x0000000000000000  X26 0x0000000000000000  X27 0x0000000000000000
>>>  X28 0x0000000000000000   FP 0x000000013EC6AA50   LR 0x00000001382F80F8
>>>
>>>   V0 0xAFAFAFAFAFAFAFAF AFAFAFAFAFAFAFAF   V1 0x6963702F66666666 6666666666666666
>>>   V2 0x697363732F312C31 406567646972622D   V3 0x0000000000000000 0000000000000000
>>>   V4 0x0000000000000400 0000000000000000   V5 0x4010040140100401 4010040140100401
>>>   V6 0x0004000000000000 0004000000000000   V7 0x0000000000000000 0000000000000000
>>>   V8 0x0000000000000000 0000000000000000   V9 0x0000000000000000 0000000000000000
>>>  V10 0x0000000000000000 0000000000000000  V11 0x0000000000000000 0000000000000000
>>>  V12 0x0000000000000000 0000000000000000  V13 0x0000000000000000 0000000000000000
>>>  V14 0x0000000000000000 0000000000000000  V15 0x0000000000000000 0000000000000000
>>>  V16 0x0000000000000000 0000000000000000  V17 0x0000000000000000 0000000000000000
>>>  V18 0x0000000000000000 0000000000000000  V19 0x0000000000000000 0000000000000000
>>>  V20 0x0000000000000000 0000000000000000  V21 0x0000000000000000 0000000000000000
>>>  V22 0x0000000000000000 0000000000000000  V23 0x0000000000000000 0000000000000000
>>>  V24 0x0000000000000000 0000000000000000  V25 0x0000000000000000 0000000000000000
>>>  V26 0x0000000000000000 0000000000000000  V27 0x0000000000000000 0000000000000000
>>>  V28 0x0000000000000000 0000000000000000  V29 0x0000000000000000 0000000000000000
>>>  V30 0x0000000000000000 0000000000000000  V31 0x0000000000000000 0000000000000000
>>>
>>>   SP 0x000000013EC6AA50  ELR 0x00000001380F7400  SPSR 0x60000205  FPSR 0x00000000
>>>  ESR 0x8600000E          FAR 0x00000001380F7400
>>>
>>>  ESR : EC 0x21  IL 0x1  ISS 0x0000000E
>>>
>>> Instruction abort: Permission fault, second level
>>>
>>
>> Hmm, that is disappointing. This is probably due to GRUB's modular
>> nature, which means it allocates memory and loads executable code into
>> it, under the assumption that memory is always executable in UEFI.
>>
>> The short term fix is to remove the NX bit from LoaderData regions,
>> but in the mean time, I will work with Leif to get this fixed properly
>> (assuming there is a proper way to fix this)
>>
>
> Care to have a quick go at using 0xC000000000007FD1 instead? (if you
> are not already doing so)
>

Adding my own data point: running CelloBoard with
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy set to
0xC000000000007FD5 happily boots straight into the kernel, but crashes
when booting via GRUB. Changing the value to 0xC000000000007FD1 gets
things working again.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4 0/7] MdeModulePkg/DxeCore: increased memory protection
Posted by Laszlo Ersek 7 years, 1 month ago
On 02/28/17 11:59, Ard Biesheuvel wrote:
> On 28 February 2017 at 10:52, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 28 February 2017 at 10:46, Laszlo Ersek <lersek@redhat.com> wrote:

>>> I regression-tested this series for x86 / OVMF as under v3, with the zero PCD default, and experienced no issues.
>>>
>>> However, v4 breaks booting Fedora 24 on my Mustang (aarch64/KVM):

>> Hmm, that is disappointing. This is probably due to GRUB's modular
>> nature, which means it allocates memory and loads executable code into
>> it, under the assumption that memory is always executable in UEFI.
>>
>> The short term fix is to remove the NX bit from LoaderData regions,
>> but in the mean time, I will work with Leif to get this fixed properly
>> (assuming there is a proper way to fix this)
>>
> 
> Care to have a quick go at using 0xC000000000007FD1 instead? (if you
> are not already doing so)


With the following patch on top:

> commit ef6be33275e45045a15201a15a2be26e6fbabcaa
> Author: Laszlo Ersek <lersek@redhat.com>
> Date:   Wed Mar 1 00:06:37 2017 +0100
>
>     ArmVirtPkg: remove the NX bit from LoaderData regions
>
>     msgid <CAKv+Gu8V4o0-s9jhQSM5hFaaC6yppdC001MiuBX830WrXi_VKQ@mail.gmail.com>
>
>     Suggested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>     Contributed-under: TianoCore Contribution Agreement 1.0
>     Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index 23b601a199ed..4d3ae5d0bc80 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -386,7 +386,7 @@ [PcdsFixedAtBuild.AARCH64]
>    # Enable NX memory protection for all non-code regions, including OEM and OS
>    # reserved ones.
>    #
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD5
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
>
>  [Components.common]
>    #

all three guests mentioned previously boot okay.

(I also made sure that the "applying strict permissions..." messages showed up in the firmware log for each.)

Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4 0/7] MdeModulePkg/DxeCore: increased memory protection
Posted by Michael Zimmermann 7 years, 1 month ago
I fail to get this working on my target. I've enabled the following
Pcd's like in ArmVirt:
gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x3
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE

but apparently, DxeCore removes the executable permission from it's own code.
after the BL instruction of the call to gCpu->SetMemoryAttributes I
get an instruction permission fault:

InitializeDxeNxMemoryProtectionPolicy: applying strict permissions to
active memory regions
SetUefiImageMemoryAttributes - 0x0000000080200000 - 0x0000000008C00000
(0x0000000000004000)
SetUefiImageMemoryAttributes - 0x0000000089000000 - 0x0000000004A00000
(0x0000000000004000)
SetUefiImageMemoryAttributes - 0x000000008EC00000 - 0x0000000000400000
(0x0000000000004000)
SetUefiImageMemoryAttributes - 0x000000008F700000 - 0x0000000000700000
(0x0000000000004000)
SetUefiImageMemoryAttributes - 0x000000008FF00000 - 0x000000006E095000
(0x0000000000004000)
SetUefiImageMemoryAttributes - 0x00000000FDFB9000 - 0x0000000000047000
(0x0000000000004000)
SetUefiImageMemoryAttributes - 0x00000000FE400000 - 0x0000000001C00000
(0x0000000000004000)

Prefetch Abort Exception PC at 0xFEEA630E  CPSR 0x20000033 nzCveaifT_svc
Build/LittleKernelPkg/DEBUG_GCC5/ARM/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/DxeCore.dll
loaded at 0xFEEA4000 (PE/COFF offset) 0x230E (ELF or Mach-O offset) 0x130E
0xBF00       IT     EQ
  R0 0x00000000   R1 0x01C00000   R2 0x00000000   R3 0x00000000
  R4 0x00000000   R5 0x00026000   R6 0x00000000   R7 0xFE000214
  R8 0x80000000   R9 0xFE400000  R10 0xFFFEF000  R11 0x00000004
 R12 0x00000002   SP 0xFFFFEBA0   LR 0xFDF98B4D   PC 0xFEEA630E
DFSR 0x00000000 DFAR 0x00000000 IFSR 0x0000000D IFAR 0xFEEA630E
 Instruction Permission fault on Section at 0xFEEA630E

ASSERT [ArmCpuDxe]
ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c(268):
((BOOLEAN)(0==1))

----

so did I miss anything?

Thanks
Michael

On Wed, Mar 1, 2017 at 12:46 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> On 02/28/17 11:59, Ard Biesheuvel wrote:
>> On 28 February 2017 at 10:52, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>>> On 28 February 2017 at 10:46, Laszlo Ersek <lersek@redhat.com> wrote:
>
>>>> I regression-tested this series for x86 / OVMF as under v3, with the zero PCD default, and experienced no issues.
>>>>
>>>> However, v4 breaks booting Fedora 24 on my Mustang (aarch64/KVM):
>
>>> Hmm, that is disappointing. This is probably due to GRUB's modular
>>> nature, which means it allocates memory and loads executable code into
>>> it, under the assumption that memory is always executable in UEFI.
>>>
>>> The short term fix is to remove the NX bit from LoaderData regions,
>>> but in the mean time, I will work with Leif to get this fixed properly
>>> (assuming there is a proper way to fix this)
>>>
>>
>> Care to have a quick go at using 0xC000000000007FD1 instead? (if you
>> are not already doing so)
>
>
> With the following patch on top:
>
>> commit ef6be33275e45045a15201a15a2be26e6fbabcaa
>> Author: Laszlo Ersek <lersek@redhat.com>
>> Date:   Wed Mar 1 00:06:37 2017 +0100
>>
>>     ArmVirtPkg: remove the NX bit from LoaderData regions
>>
>>     msgid <CAKv+Gu8V4o0-s9jhQSM5hFaaC6yppdC001MiuBX830WrXi_VKQ@mail.gmail.com>
>>
>>     Suggested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>     Contributed-under: TianoCore Contribution Agreement 1.0
>>     Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>
>> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
>> index 23b601a199ed..4d3ae5d0bc80 100644
>> --- a/ArmVirtPkg/ArmVirt.dsc.inc
>> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
>> @@ -386,7 +386,7 @@ [PcdsFixedAtBuild.AARCH64]
>>    # Enable NX memory protection for all non-code regions, including OEM and OS
>>    # reserved ones.
>>    #
>> -  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD5
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
>>
>>  [Components.common]
>>    #
>
> all three guests mentioned previously boot okay.
>
> (I also made sure that the "applying strict permissions..." messages showed up in the firmware log for each.)
>
> Thanks
> Laszlo
> _______________________________________________
> 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 v4 0/7] MdeModulePkg/DxeCore: increased memory protection
Posted by Ard Biesheuvel 7 years, 1 month ago
On 13 March 2017 at 09:43, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote:
> I fail to get this working on my target. I've enabled the following
> Pcd's like in ArmVirt:
> gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x3
> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
> gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
>
> but apparently, DxeCore removes the executable permission from it's own code.
> after the BL instruction of the call to gCpu->SetMemoryAttributes I
> get an instruction permission fault:
>
> InitializeDxeNxMemoryProtectionPolicy: applying strict permissions to
> active memory regions
> SetUefiImageMemoryAttributes - 0x0000000080200000 - 0x0000000008C00000
> (0x0000000000004000)
> SetUefiImageMemoryAttributes - 0x0000000089000000 - 0x0000000004A00000
> (0x0000000000004000)
> SetUefiImageMemoryAttributes - 0x000000008EC00000 - 0x0000000000400000
> (0x0000000000004000)
> SetUefiImageMemoryAttributes - 0x000000008F700000 - 0x0000000000700000
> (0x0000000000004000)
> SetUefiImageMemoryAttributes - 0x000000008FF00000 - 0x000000006E095000
> (0x0000000000004000)
> SetUefiImageMemoryAttributes - 0x00000000FDFB9000 - 0x0000000000047000
> (0x0000000000004000)
> SetUefiImageMemoryAttributes - 0x00000000FE400000 - 0x0000000001C00000
> (0x0000000000004000)
>
> Prefetch Abort Exception PC at 0xFEEA630E  CPSR 0x20000033 nzCveaifT_svc
> Build/LittleKernelPkg/DEBUG_GCC5/ARM/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/DxeCore.dll
> loaded at 0xFEEA4000 (PE/COFF offset) 0x230E (ELF or Mach-O offset) 0x130E
> 0xBF00       IT     EQ
>   R0 0x00000000   R1 0x01C00000   R2 0x00000000   R3 0x00000000
>   R4 0x00000000   R5 0x00026000   R6 0x00000000   R7 0xFE000214
>   R8 0x80000000   R9 0xFE400000  R10 0xFFFEF000  R11 0x00000004
>  R12 0x00000002   SP 0xFFFFEBA0   LR 0xFDF98B4D   PC 0xFEEA630E
> DFSR 0x00000000 DFAR 0x00000000 IFSR 0x0000000D IFAR 0xFEEA630E
>  Instruction Permission fault on Section at 0xFEEA630E
>
> ASSERT [ArmCpuDxe]
> ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c(268):
> ((BOOLEAN)(0==1))
>
> ----
>
> so did I miss anything?
>

You are using PrePi, right?
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4 0/7] MdeModulePkg/DxeCore: increased memory protection
Posted by Michael Zimmermann 7 years, 1 month ago
> You are using PrePi, right?
yes. Isn't that supported yet? afaik ArmVirtXen uses PrePi too.

On Mon, Mar 13, 2017 at 9:50 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On 13 March 2017 at 09:43, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote:
>> I fail to get this working on my target. I've enabled the following
>> Pcd's like in ArmVirt:
>> gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x3
>> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
>> gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
>>
>> but apparently, DxeCore removes the executable permission from it's own code.
>> after the BL instruction of the call to gCpu->SetMemoryAttributes I
>> get an instruction permission fault:
>>
>> InitializeDxeNxMemoryProtectionPolicy: applying strict permissions to
>> active memory regions
>> SetUefiImageMemoryAttributes - 0x0000000080200000 - 0x0000000008C00000
>> (0x0000000000004000)
>> SetUefiImageMemoryAttributes - 0x0000000089000000 - 0x0000000004A00000
>> (0x0000000000004000)
>> SetUefiImageMemoryAttributes - 0x000000008EC00000 - 0x0000000000400000
>> (0x0000000000004000)
>> SetUefiImageMemoryAttributes - 0x000000008F700000 - 0x0000000000700000
>> (0x0000000000004000)
>> SetUefiImageMemoryAttributes - 0x000000008FF00000 - 0x000000006E095000
>> (0x0000000000004000)
>> SetUefiImageMemoryAttributes - 0x00000000FDFB9000 - 0x0000000000047000
>> (0x0000000000004000)
>> SetUefiImageMemoryAttributes - 0x00000000FE400000 - 0x0000000001C00000
>> (0x0000000000004000)
>>
>> Prefetch Abort Exception PC at 0xFEEA630E  CPSR 0x20000033 nzCveaifT_svc
>> Build/LittleKernelPkg/DEBUG_GCC5/ARM/MdeModulePkg/Core/Dxe/DxeMain/DEBUG/DxeCore.dll
>> loaded at 0xFEEA4000 (PE/COFF offset) 0x230E (ELF or Mach-O offset) 0x130E
>> 0xBF00       IT     EQ
>>   R0 0x00000000   R1 0x01C00000   R2 0x00000000   R3 0x00000000
>>   R4 0x00000000   R5 0x00026000   R6 0x00000000   R7 0xFE000214
>>   R8 0x80000000   R9 0xFE400000  R10 0xFFFEF000  R11 0x00000004
>>  R12 0x00000002   SP 0xFFFFEBA0   LR 0xFDF98B4D   PC 0xFEEA630E
>> DFSR 0x00000000 DFAR 0x00000000 IFSR 0x0000000D IFAR 0xFEEA630E
>>  Instruction Permission fault on Section at 0xFEEA630E
>>
>> ASSERT [ArmCpuDxe]
>> ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c(268):
>> ((BOOLEAN)(0==1))
>>
>> ----
>>
>> so did I miss anything?
>>
>
> You are using PrePi, right?
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel