On Sat, 9 Mar 2024 at 20:06, Oliver Smith-Denny
<o...@linux.microsoft.com> wrote:
>
> Currently, there are multiple issues when page or pool guards are
> allocated for runtime memory regions that are aligned to
> non-EFI_PAGE_SIZE alignments. Multiple other issues have been fixed
> for these same systems (notably ARM64 which has a 64k runtime page
> allocation granularity) recently. The heap guard system is only built
> to support 4k guard pages and 4k alignment.
>
> Today, the address returned to a caller of AllocatePages will not be
> aligned correctly to the runtime page allocation granularity, because
> the heap guard system does not take non-4k alignment requirements into
> consideration.
>
> However, even with this bug fixed, the Memory Allocation Table cannot
> be produced and an OS with a larger than 4k page granularity will not
> have aligned memory regions because the guard pages are reported as
> part of the same memory allocation. So what would have been, on an
> ARM64 system, a 64k runtime memory allocation is actually a 72k
> memory allocation as tracked by the Page.c code because the guard
> pages are tracked as part of the same allocation. This is a core
> function of the current heap guard architecture.
>
> This could also be fixed with rearchitecting the heap guard system to
> respect alignment requirements and shift the guard pages inside of the
> outer rounded allocation or by having guard pages be the runtime
> granularity. Both of these approaches have issues. In the former case,
> we break UEFI spec 2.10 section 2.3.6 for AARCH64, which states that
> each 64k page for runtime memory regions may not have mixed memory
> attributes, which pushing the guard pages inside would create. In the
> latter case, an immense amount of memory is wasted to support such
> large guard pages, and with pool guard many systems could not support
> an additional 128k allocation for all runtime memory.
>
> The simpler and safer solution is to disallow page and pool guards for
> runtime memory allocations for systems that have a runtime granularity
> greater than the EFI_PAGE_SIZE (4k). The usefulness of such guards is
> limited, as OSes do not map guard pages today, so there is only boot
> time protection of these ranges. This also prevents other bugs from
> being exposed by using guards for regions that have a non-4k alignment
> requirement, as again, multiple have cropped up because the heap guard
> system was not built to support it.
>
> This patch adds both a static assert to ensure that either the runtime
> granularity is the EFI_PAGE_SIZE or that the PCD bits are not set to
> enable heap guard for runtime memory regions. It also adds a check in
> the page and pool allocation system to ensure that at runtime we are
> not allocating a runtime region and attempt to guard it (the PCDs are
> close to being removed in favor of dynamic heap guard configurations).
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4674
> Github PR: https://github.com/tianocore/edk2/pull/5382
>
> Cc: Leif Lindholm <quic_llind...@quicinc.com>
> Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>
> Cc: Sami Mujawar <sami.muja...@arm.com>
> Cc: Liming Gao <gaolim...@byosoft.com.cn>
>
> Signed-off-by: Oliver Smith-Denny <o...@linux.microsoft.com>
> ---
> MdeModulePkg/MdeModulePkg.dec | 10 ++++++++++
> MdeModulePkg/Core/Dxe/Mem/HeapGuard.h | 14 ++++++++++++++
> MdeModulePkg/Core/Dxe/Mem/Page.c | 11 +++++++++++
> MdeModulePkg/Core/Dxe/Mem/Pool.c | 11 +++++++++++
> 4 files changed, 46 insertions(+)
>
Reviewed-by: Ard Biesheuvel <a...@kernel.org>
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index a2cd83345f5b..a82dedc070df 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -1027,6 +1027,11 @@ [PcdsFixedAtBuild]
> # free pages for all of them. The page allocation for the type related to
> # cleared bits keeps the same as ususal.
> #
> + # The heap guard system only supports guarding EfiRuntimeServicesCode,
> EfiRuntimeServicesData,
> + # EfiReservedMemoryType, and EfiACPIMemoryNVS memory types for systems
> that have
> + # RUNTIME_PAGE_ALLOCATION_GRANULARITY == EFI_PAGE_SIZE. This is to
> preserve alignment requirements
> + # without extending the page guard size to very large granularities.
> + #
> # This PCD is only valid if BIT0 and/or BIT2 are set in
> PcdHeapGuardPropertyMask.
> #
> # Below is bit mask for this PCD: (Order is same as UEFI spec)<BR>
> @@ -1058,6 +1063,11 @@ [PcdsFixedAtBuild]
> # if there's enough free memory for all of them. The pool allocation for
> the
> # type related to cleared bits keeps the same as ususal.
> #
> + # The heap guard system only supports guarding EfiRuntimeServicesCode,
> EfiRuntimeServicesData,
> + # EfiReservedMemoryType, and EfiACPIMemoryNVS memory types for systems
> that have
> + # RUNTIME_PAGE_ALLOCATION_GRANULARITY == EFI_PAGE_SIZE. This is to
> preserve alignment requirements
> + # without extending the page guard size to very large granularities.
> + #
> # This PCD is only valid if BIT1 and/or BIT3 are set in
> PcdHeapGuardPropertyMask.
> #
> # Below is bit mask for this PCD: (Order is same as UEFI spec)<BR>
> diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.h
> b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.h
> index 24b4206c0e02..578e85746585 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.h
> +++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.h
> @@ -469,4 +469,18 @@ PromoteGuardedFreePages (
>
> extern BOOLEAN mOnGuarding;
>
> +//
> +// The heap guard system does not support non-EFI_PAGE_SIZE alignments.
> +// Architectures that require larger RUNTIME_PAGE_ALLOCATION_GRANULARITY
> +// cannot have EfiRuntimeServicesCode, EfiRuntimeServicesData,
> EfiReservedMemoryType,
> +// and EfiACPIMemoryNVS guarded. OSes do not map guard pages anyway, so this
> is a
> +// minimal loss. Not guarding prevents alignment mismatches
> +//
> +STATIC_ASSERT (
> + RUNTIME_PAGE_ALLOCATION_GRANULARITY == EFI_PAGE_SIZE ||
> + (((FixedPcdGet64 (PcdHeapGuardPageType) & 0x461) == 0) &&
> + ((FixedPcdGet64 (PcdHeapGuardPoolType) & 0x461) == 0)),
> + "Unsupported Heap Guard configuration on system with greater than
> EFI_PAGE_SIZE RUNTIME_PAGE_ALLOCATION_GRANULARITY"
> + );
> +
> #endif
> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c
> b/MdeModulePkg/Core/Dxe/Mem/Page.c
> index cd201d36a389..26584648c236 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> @@ -1411,6 +1411,17 @@ CoreInternalAllocatePages (
> Alignment = RUNTIME_PAGE_ALLOCATION_GRANULARITY;
> }
>
> + //
> + // The heap guard system does not support non-EFI_PAGE_SIZE alignments.
> + // Architectures that require larger RUNTIME_PAGE_ALLOCATION_GRANULARITY
> + // will have the runtime memory regions unguarded. OSes do not
> + // map guard pages anyway, so this is a minimal loss. Not guarding prevents
> + // alignment mismatches
> + //
> + if (Alignment != EFI_PAGE_SIZE) {
> + NeedGuard = FALSE;
> + }
> +
> if (Type == AllocateAddress) {
> if ((*Memory & (Alignment - 1)) != 0) {
> return EFI_NOT_FOUND;
> diff --git a/MdeModulePkg/Core/Dxe/Mem/Pool.c
> b/MdeModulePkg/Core/Dxe/Mem/Pool.c
> index ccfce8c5f959..72293e6dfe40 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/Pool.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/Pool.c
> @@ -380,6 +380,17 @@ CoreAllocatePoolI (
> Granularity = DEFAULT_PAGE_ALLOCATION_GRANULARITY;
> }
>
> + //
> + // The heap guard system does not support non-EFI_PAGE_SIZE alignments.
> + // Architectures that require larger RUNTIME_PAGE_ALLOCATION_GRANULARITY
> + // will have the runtime memory regions unguarded. OSes do not
> + // map guard pages anyway, so this is a minimal loss. Not guarding prevents
> + // alignment mismatches
> + //
> + if (Granularity != EFI_PAGE_SIZE) {
> + NeedGuard = FALSE;
> + }
> +
> //
> // Adjust the size by the pool header & tail overhead
> //
> --
> 2.40.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116628): https://edk2.groups.io/g/devel/message/116628
Mute This Topic: https://groups.io/mt/104832607/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
