Re: [PATCH v3 0/6] RFC: increased memory protection

[Laszlo Ersek <lersek@redhat.com>]

On 02/26/17 19:29, Ard Biesheuvel wrote:
> Hello all,
> 
> This is a proof of concept implementation 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 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
> 
> Note that to test this properly, the default value of 0 should be changed
> to 0x7FD5, which applies non-exec permissions to everything except Efi*Code
> regions.
> 
> Ard Biesheuvel (6):
>   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
> 
>  ArmPkg/Drivers/CpuDxe/CpuDxe.c                     |   3 +
>  ArmPkg/Drivers/CpuDxe/CpuDxe.h                     |   1 +
>  ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c               |   4 +
>  MdeModulePkg/Core/Dxe/DxeMain.h                    |  24 ++
>  MdeModulePkg/Core/Dxe/DxeMain.inf                  |   1 +
>  MdeModulePkg/Core/Dxe/Mem/Page.c                   |   4 +
>  MdeModulePkg/Core/Dxe/Mem/Pool.c                   |  60 +++-
>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c      | 306 +++++++++++++++++++-
>  MdeModulePkg/Core/Pei/Image/Image.c                |  10 +-
>  MdeModulePkg/MdeModulePkg.dec                      |  31 ++
>  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 +-
>  16 files changed, 471 insertions(+), 18 deletions(-)
> 

with the default 0 value for the PCD:

Regression-tested-by: Laszlo Ersek <lersek@redhat.com>

For testing I more or less used
<https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt>,
plus booted a few guests on aarch64/KVM with this (Fedora 24, RHEL-7.3,
openSUSE Tumbleweed).

Thanks
Laszlo