public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: edk2-devel@lists.01.org, afish@apple.com,
	leif.lindholm@linaro.org, michael.d.kinney@intel.com,
	liming.gao@intel.com, jiewen.yao@intel.com
Cc: lersek@redhat.com, feng.tian@intel.com, star.zeng@intel.com,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [PATCH v4 0/7] MdeModulePkg/DxeCore: increased memory protection
Date: Mon, 27 Feb 2017 14:38:04 +0000	[thread overview]
Message-ID: <1488206291-25768-1-git-send-email-ard.biesheuvel@linaro.org> (raw)

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



             reply	other threads:[~2017-02-27 14:38 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-27 14:38 Ard Biesheuvel [this message]
2017-02-27 14:38 ` [PATCH v4 1/7] ArmPkg/CpuDxe: ignore attribute changes during SyncCacheConfig() Ard Biesheuvel
2017-02-27 15:32   ` Leif Lindholm
2017-02-27 15:33     ` Ard Biesheuvel
2017-02-27 15:38       ` Leif Lindholm
2017-02-27 15:39         ` Ard Biesheuvel
2017-02-27 15:41           ` Leif Lindholm
2017-02-27 14:38 ` [PATCH v4 2/7] MdeModulePkg/PeiCore: allocate BootServicesCode memory for PE/COFF images Ard Biesheuvel
2017-02-28  5:42   ` Gao, Liming
2017-02-27 14:38 ` [PATCH v4 3/7] MdeModulePkg/EbcDxe: use EfiBootServicesCode memory for thunks Ard Biesheuvel
2017-02-27 14:38 ` [PATCH v4 4/7] MdeModulePkg/DxeCore: use separate lock for pool allocations Ard Biesheuvel
2017-02-28  9:32   ` Gao, Liming
2017-02-27 14:38 ` [PATCH v4 5/7] MdeModulePkg: define PCD for DXE memory protection policy Ard Biesheuvel
2017-02-27 14:38 ` [PATCH v4 6/7] MdeModulePkg/DxeCore: implement " Ard Biesheuvel
2017-02-28  9:33   ` Gao, Liming
2017-02-27 14:38 ` [PATCH v4 7/7] ArmVirtPkg/ArmVirt.dsc.inc: enable NX memory protection for all platforms Ard Biesheuvel
2017-02-28  5:48 ` [PATCH v4 0/7] MdeModulePkg/DxeCore: increased memory protection Yao, Jiewen
2017-02-28 14:59   ` Ard Biesheuvel
2017-02-28 10:46 ` Laszlo Ersek
2017-02-28 10:52   ` Ard Biesheuvel
2017-02-28 10:59     ` Ard Biesheuvel
2017-02-28 11:47       ` Ard Biesheuvel
2017-02-28 23:46       ` Laszlo Ersek
2017-03-13  8:43         ` Michael Zimmermann
2017-03-13  8:50           ` Ard Biesheuvel
2017-03-13  8:53             ` Michael Zimmermann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1488206291-25768-1-git-send-email-ard.biesheuvel@linaro.org \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox