[edk2-devel][PATCH v3 0/3] Fix Runtime Granularity Issues

[edk2-devel][PATCH v3 0/3] Fix Runtime Granularity Issues

[Oliver Smith-Denny]

This patch series is the third version of
MdeModulePkg: DxeCore: Don't Guard Large Runtime Granularity Allocations​.
The subject line has been updated because this went from a one commit
patch with no cover letter to a multi-commit patch.

The commit messages cover the vast amount of detail here, but this
patchset fixes three issues:
- a UEFI spec violation for which memory types require runtime page
allocation granularity alignment
- An incompatibility of the heap guard system to guard these regions
that require runtime page allocation granularities greater than
the EFI_PAGE_SIZE.
- A CodeQL error that fails CI when updating the Page.c code

v3:
- edit comments for readability

v2:
- Add commit to fix UEFI spec violation
- Add commit to fix newly flagged CodeQL error
- Update guard commit message, comments, and static assert to use
the correct types

Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>

Oliver Smith-Denny (3):
  MdeModulePkg: DxeCore: Fix CodeQL Error in FreePages
  MdeModulePkg: DxeCore: Correct Runtime Granularity Memory Type
  MdeModulePkg: DxeCore: Do Not Apply Guards to Unsupported Types

 MdeModulePkg/MdeModulePkg.dec                 | 10 +++++++++
 MdeModulePkg/Core/Dxe/Mem/HeapGuard.h         | 14 +++++++++++++
 MdeModulePkg/Core/Dxe/Mem/Page.c              | 22 +++++++++++++++++---
 MdeModulePkg/Core/Dxe/Mem/Pool.c              | 15 +++++++++++--
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c |  4 ++--
 MdeModulePkg/Core/Pei/Memory/MemoryServices.c |  2 +-
 6 files changed, 59 insertions(+), 8 deletions(-)

-- 
2.40.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116559): https://edk2.groups.io/g/devel/message/116559
Mute This Topic: https://groups.io/mt/104832605/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel][PATCH v3 1/3] MdeModulePkg: DxeCore: Fix CodeQL Error in FreePages

[Oliver Smith-Denny]

CodeQL flags the Free Pages logic for not ensuring that
Entry is non-null before using it. Add a check for this
and appropriately bail out if we hit this case.

Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>
---
 MdeModulePkg/Core/Dxe/Mem/Page.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index 3205732ede17..dd558696ba59 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -1658,9 +1658,14 @@ CoreInternalFreePages (
     goto Done;
   }
 
+  if (Entry == NULL) {
+    ASSERT (Entry != NULL);
+    Status = EFI_NOT_FOUND;
+    goto Done;
+  }
+
   Alignment = DEFAULT_PAGE_ALLOCATION_GRANULARITY;
 
-  ASSERT (Entry != NULL);
   if ((Entry->Type == EfiACPIReclaimMemory) ||
       (Entry->Type == EfiACPIMemoryNVS) ||
       (Entry->Type == EfiRuntimeServicesCode) ||
-- 
2.40.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116558): https://edk2.groups.io/g/devel/message/116558
Mute This Topic: https://groups.io/mt/104832604/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel][PATCH v3 2/3] MdeModulePkg: DxeCore: Correct Runtime Granularity Memory Type

[Oliver Smith-Denny]

Per the UEFI spec 2.10, section 2.3.6 (for the AARCH64 arch,
other architectures in section two confirm the same) the
memory types that need runtime page allocation granularity
are EfiReservedMemoryType, EfiACPIMemoryNVS, EfiRuntimeServicesCode,
and EfiRuntimeServicesData. However, legacy code was setting
runtime page allocation granularity for EfiACPIReclaimMemory
and not EfiReservedMemoryType. This patch fixes that error.

Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>

Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>
Suggested-by: Ard Biesheuvel <ardb+tianocore@kernel.org>
---
 MdeModulePkg/Core/Dxe/Mem/Page.c              | 4 ++--
 MdeModulePkg/Core/Dxe/Mem/Pool.c              | 4 ++--
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 4 ++--
 MdeModulePkg/Core/Pei/Memory/MemoryServices.c | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index dd558696ba59..cd201d36a389 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -1403,7 +1403,7 @@ CoreInternalAllocatePages (
 
   Alignment = DEFAULT_PAGE_ALLOCATION_GRANULARITY;
 
-  if ((MemoryType == EfiACPIReclaimMemory) ||
+  if ((MemoryType == EfiReservedMemoryType) ||
       (MemoryType == EfiACPIMemoryNVS) ||
       (MemoryType == EfiRuntimeServicesCode) ||
       (MemoryType == EfiRuntimeServicesData))
@@ -1666,7 +1666,7 @@ CoreInternalFreePages (
 
   Alignment = DEFAULT_PAGE_ALLOCATION_GRANULARITY;
 
-  if ((Entry->Type == EfiACPIReclaimMemory) ||
+  if ((Entry->Type == EfiReservedMemoryType) ||
       (Entry->Type == EfiACPIMemoryNVS) ||
       (Entry->Type == EfiRuntimeServicesCode) ||
       (Entry->Type == EfiRuntimeServicesData))
diff --git a/MdeModulePkg/Core/Dxe/Mem/Pool.c b/MdeModulePkg/Core/Dxe/Mem/Pool.c
index 716dd045f9fd..ccfce8c5f959 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Pool.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Pool.c
@@ -370,7 +370,7 @@ CoreAllocatePoolI (
 
   ASSERT_LOCKED (&mPoolMemoryLock);
 
-  if ((PoolType == EfiACPIReclaimMemory) ||
+  if ((PoolType == EfiReservedMemoryType) ||
       (PoolType == EfiACPIMemoryNVS) ||
       (PoolType == EfiRuntimeServicesCode) ||
       (PoolType == EfiRuntimeServicesData))
@@ -753,7 +753,7 @@ CoreFreePoolI (
   Pool->Used -= Size;
   DEBUG ((DEBUG_POOL, "FreePool: %p (len %lx) %,ld\n", Head->Data, (UINT64)(Head->Size - POOL_OVERHEAD), (UINT64)Pool->Used));
 
-  if ((Head->Type == EfiACPIReclaimMemory) ||
+  if ((Head->Type == EfiReservedMemoryType) ||
       (Head->Type == EfiACPIMemoryNVS) ||
       (Head->Type == EfiRuntimeServicesCode) ||
       (Head->Type == EfiRuntimeServicesData))
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 977239d08afc..eb243a0137b1 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -300,18 +300,18 @@ IsMemoryProtectionSectionAligned (
   switch (MemoryType) {
     case EfiRuntimeServicesCode:
     case EfiACPIMemoryNVS:
+    case EfiReservedMemoryType:
       PageAlignment = RUNTIME_PAGE_ALLOCATION_GRANULARITY;
       break;
     case EfiRuntimeServicesData:
-    case EfiACPIReclaimMemory:
       ASSERT (FALSE);
       PageAlignment = RUNTIME_PAGE_ALLOCATION_GRANULARITY;
       break;
     case EfiBootServicesCode:
     case EfiLoaderCode:
-    case EfiReservedMemoryType:
       PageAlignment = EFI_PAGE_SIZE;
       break;
+    case EfiACPIReclaimMemory:
     default:
       ASSERT (FALSE);
       PageAlignment = EFI_PAGE_SIZE;
diff --git a/MdeModulePkg/Core/Pei/Memory/MemoryServices.c b/MdeModulePkg/Core/Pei/Memory/MemoryServices.c
index a30925da39ee..52f37c960e46 100644
--- a/MdeModulePkg/Core/Pei/Memory/MemoryServices.c
+++ b/MdeModulePkg/Core/Pei/Memory/MemoryServices.c
@@ -584,7 +584,7 @@ PeiAllocatePages (
   }
 
   if ((RUNTIME_PAGE_ALLOCATION_GRANULARITY > DEFAULT_PAGE_ALLOCATION_GRANULARITY) &&
-      ((MemoryType == EfiACPIReclaimMemory) ||
+      ((MemoryType == EfiReservedMemoryType) ||
        (MemoryType == EfiACPIMemoryNVS) ||
        (MemoryType == EfiRuntimeServicesCode) ||
        (MemoryType == EfiRuntimeServicesData)))
-- 
2.40.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116560): https://edk2.groups.io/g/devel/message/116560
Mute This Topic: https://groups.io/mt/104832606/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel][PATCH v3 3/3] MdeModulePkg: DxeCore: Do Not Apply Guards to Unsupported Types

[Oliver Smith-Denny]

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_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>

Signed-off-by: Oliver Smith-Denny <osde@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(+)

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 (#116561): https://edk2.groups.io/g/devel/message/116561
Mute This Topic: https://groups.io/mt/104832607/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel][PATCH v3 3/3] MdeModulePkg: DxeCore: Do Not Apply Guards to Unsupported Types

[Ard Biesheuvel]

On Sat, 9 Mar 2024 at 20:06, Oliver Smith-Denny
<osde@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_llindhol@quicinc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>
> Signed-off-by: Oliver Smith-Denny <osde@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 <ardb@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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

回复: [edk2-devel][PATCH v3 0/3] Fix Runtime Granularity Issues

[gaoliming via groups.io]

For this patch set, I have no comments. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: Oliver Smith-Denny <osde@linux.microsoft.com>
> 发送时间: 2024年3月10日 3:06
> 收件人: devel@edk2.groups.io
> 抄送: Leif Lindholm <quic_llindhol@quicinc.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Sami Mujawar <sami.mujawar@arm.com>;
> Liming Gao <gaoliming@byosoft.com.cn>
> 主题: [edk2-devel][PATCH v3 0/3] Fix Runtime Granularity Issues
> 
> This patch series is the third version of
> MdeModulePkg: DxeCore: Don't Guard Large Runtime Granularity Allocations​.
> The subject line has been updated because this went from a one commit
> patch with no cover letter to a multi-commit patch.
> 
> The commit messages cover the vast amount of detail here, but this
> patchset fixes three issues:
> - a UEFI spec violation for which memory types require runtime page
> allocation granularity alignment
> - An incompatibility of the heap guard system to guard these regions
> that require runtime page allocation granularities greater than
> the EFI_PAGE_SIZE.
> - A CodeQL error that fails CI when updating the Page.c code
> 
> v3:
> - edit comments for readability
> 
> v2:
> - Add commit to fix UEFI spec violation
> - Add commit to fix newly flagged CodeQL error
> - Update guard commit message, comments, and static assert to use
> the correct types
> 
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> 
> Oliver Smith-Denny (3):
>   MdeModulePkg: DxeCore: Fix CodeQL Error in FreePages
>   MdeModulePkg: DxeCore: Correct Runtime Granularity Memory Type
>   MdeModulePkg: DxeCore: Do Not Apply Guards to Unsupported Types
> 
>  MdeModulePkg/MdeModulePkg.dec                 | 10 +++++++++
>  MdeModulePkg/Core/Dxe/Mem/HeapGuard.h         | 14
> +++++++++++++
>  MdeModulePkg/Core/Dxe/Mem/Page.c              | 22
> +++++++++++++++++---
>  MdeModulePkg/Core/Dxe/Mem/Pool.c              | 15
> +++++++++++--
>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c |  4 ++--
>  MdeModulePkg/Core/Pei/Memory/MemoryServices.c |  2 +-
>  6 files changed, 59 insertions(+), 8 deletions(-)
> 
> --
> 2.40.1





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116762): https://edk2.groups.io/g/devel/message/116762
Mute This Topic: https://groups.io/mt/104927832/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel][PATCH v3 0/3] Fix Runtime Granularity Issues

[Ard Biesheuvel]

On Thu, 14 Mar 2024 at 15:52, gaoliming via groups.io
<gaoliming=byosoft.com.cn@groups.io> wrote:
>
> For this patch set, I have no comments. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
>

Merged as #5471



> > -----邮件原件-----
> > 发件人: Oliver Smith-Denny <osde@linux.microsoft.com>
> > 发送时间: 2024年3月10日 3:06
> > 收件人: devel@edk2.groups.io
> > 抄送: Leif Lindholm <quic_llindhol@quicinc.com>; Ard Biesheuvel
> > <ardb+tianocore@kernel.org>; Sami Mujawar <sami.mujawar@arm.com>;
> > Liming Gao <gaoliming@byosoft.com.cn>
> > 主题: [edk2-devel][PATCH v3 0/3] Fix Runtime Granularity Issues
> >
> > This patch series is the third version of
> > MdeModulePkg: DxeCore: Don't Guard Large Runtime Granularity Allocations.
> > The subject line has been updated because this went from a one commit
> > patch with no cover letter to a multi-commit patch.
> >
> > The commit messages cover the vast amount of detail here, but this
> > patchset fixes three issues:
> > - a UEFI spec violation for which memory types require runtime page
> > allocation granularity alignment
> > - An incompatibility of the heap guard system to guard these regions
> > that require runtime page allocation granularities greater than
> > the EFI_PAGE_SIZE.
> > - A CodeQL error that fails CI when updating the Page.c code
> >
> > v3:
> > - edit comments for readability
> >
> > v2:
> > - Add commit to fix UEFI spec violation
> > - Add commit to fix newly flagged CodeQL error
> > - Update guard commit message, comments, and static assert to use
> > the correct types
> >
> > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > Cc: Sami Mujawar <sami.mujawar@arm.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> >
> > Oliver Smith-Denny (3):
> >   MdeModulePkg: DxeCore: Fix CodeQL Error in FreePages
> >   MdeModulePkg: DxeCore: Correct Runtime Granularity Memory Type
> >   MdeModulePkg: DxeCore: Do Not Apply Guards to Unsupported Types
> >
> >  MdeModulePkg/MdeModulePkg.dec                 | 10 +++++++++
> >  MdeModulePkg/Core/Dxe/Mem/HeapGuard.h         | 14
> > +++++++++++++
> >  MdeModulePkg/Core/Dxe/Mem/Page.c              | 22
> > +++++++++++++++++---
> >  MdeModulePkg/Core/Dxe/Mem/Pool.c              | 15
> > +++++++++++--
> >  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c |  4 ++--
> >  MdeModulePkg/Core/Pei/Memory/MemoryServices.c |  2 +-
> >  6 files changed, 59 insertions(+), 8 deletions(-)
> >
> > --
> > 2.40.1
>
>
>
>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116772): https://edk2.groups.io/g/devel/message/116772
Mute This Topic: https://groups.io/mt/104930045/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel][PATCH v3 0/3] Fix Runtime Granularity Issues

[Oliver Smith-Denny]

Hi Ray and Mike,

Can you help get this patchset reviewed and merged? It contains a fix
for a UEFI spec violation and it has been sitting on the mailing list
for a month. Unfortunately Liming has not reviewed after weekly
friendly pings and he is the only main level MdeModulePkg
maintainer. Liming, perhaps you could use a co-maintainer from the
community to help balance the load? I totally understand that being
a maintainer is a large burden, but I also have several patchsets up
that need reviews and merging for over a month.

Thanks,
Oliver

On 3/9/2024 11:06 AM, Oliver Smith-Denny wrote:
> This patch series is the third version of
> MdeModulePkg: DxeCore: Don't Guard Large Runtime Granularity Allocations​.
> The subject line has been updated because this went from a one commit
> patch with no cover letter to a multi-commit patch.
> 
> The commit messages cover the vast amount of detail here, but this
> patchset fixes three issues:
> - a UEFI spec violation for which memory types require runtime page
> allocation granularity alignment
> - An incompatibility of the heap guard system to guard these regions
> that require runtime page allocation granularities greater than
> the EFI_PAGE_SIZE.
> - A CodeQL error that fails CI when updating the Page.c code
> 
> v3:
> - edit comments for readability
> 
> v2:
> - Add commit to fix UEFI spec violation
> - Add commit to fix newly flagged CodeQL error
> - Update guard commit message, comments, and static assert to use
> the correct types
> 
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> 
> Oliver Smith-Denny (3):
>    MdeModulePkg: DxeCore: Fix CodeQL Error in FreePages
>    MdeModulePkg: DxeCore: Correct Runtime Granularity Memory Type
>    MdeModulePkg: DxeCore: Do Not Apply Guards to Unsupported Types
> 
>   MdeModulePkg/MdeModulePkg.dec                 | 10 +++++++++
>   MdeModulePkg/Core/Dxe/Mem/HeapGuard.h         | 14 +++++++++++++
>   MdeModulePkg/Core/Dxe/Mem/Page.c              | 22 +++++++++++++++++---
>   MdeModulePkg/Core/Dxe/Mem/Pool.c              | 15 +++++++++++--
>   MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c |  4 ++--
>   MdeModulePkg/Core/Pei/Memory/MemoryServices.c |  2 +-
>   6 files changed, 59 insertions(+), 8 deletions(-)
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116719): https://edk2.groups.io/g/devel/message/116719
Mute This Topic: https://groups.io/mt/104832605/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-