[PATCH] MdeModulePkg/Core: fix mem alloc issues in heap guard

[PATCH] MdeModulePkg/Core: fix mem alloc issues in heap guard

[Jian J Wang]

There're two ASSERT issues which will be triggered by boot loader of
Windows 10.

The first is caused by allocating memory in heap guard during another
memory allocation, which is not allowed in DXE core. Avoiding reentry
of memory allocation has been considered in heap guard feature. But
there's a hole in the code of function FindGuardedMemoryMap(). The fix
is adding AllocMapUnit parameter in the condition of while(), which
will prevent memory allocation from happenning during Guard page
check operation.

The second is caused by the core trying to allocate page 0 with Guard
page, which will cause the start address rolling back to the end of
supported system address. According to the requirement of heap guard,
the fix is just simply skipping the free memory at page 0 and let
the core continue searching free memory after it.

Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>

---
 MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
index 19245049c2..ac043b5d9b 100644
--- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
+++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
@@ -225,8 +225,8 @@ FindGuardedMemoryMap (
   //
   // Adjust current map table depth according to the address to access
   //
-  while (mMapLevel < GUARDED_HEAP_MAP_TABLE_DEPTH
-         &&
+  while (AllocMapUnit &&
+         mMapLevel < GUARDED_HEAP_MAP_TABLE_DEPTH &&
          RShiftU64 (
            Address,
            mLevelShift[GUARDED_HEAP_MAP_TABLE_DEPTH - mMapLevel - 1]
@@ -904,6 +904,10 @@ AdjustMemoryS (
   }
 
   Target = Start + Size - SizeRequested;
+  ASSERT (Target >= Start);
+  if (Target == 0) {
+    return 0;
+  }
 
   if (!IsGuardPage (Start + Size)) {
     // No Guard at tail to share. One more page is needed.
-- 
2.16.2.windows.1

Re: [PATCH] MdeModulePkg/Core: fix mem alloc issues in heap guard

[Yao, Jiewen]

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: Wang, Jian J
> Sent: Tuesday, March 13, 2018 10:35 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH] MdeModulePkg/Core: fix mem alloc issues in heap guard
> 
> There're two ASSERT issues which will be triggered by boot loader of
> Windows 10.
> 
> The first is caused by allocating memory in heap guard during another
> memory allocation, which is not allowed in DXE core. Avoiding reentry
> of memory allocation has been considered in heap guard feature. But
> there's a hole in the code of function FindGuardedMemoryMap(). The fix
> is adding AllocMapUnit parameter in the condition of while(), which
> will prevent memory allocation from happenning during Guard page
> check operation.
> 
> The second is caused by the core trying to allocate page 0 with Guard
> page, which will cause the start address rolling back to the end of
> supported system address. According to the requirement of heap guard,
> the fix is just simply skipping the free memory at page 0 and let
> the core continue searching free memory after it.
> 
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> 
> ---
>  MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> index 19245049c2..ac043b5d9b 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> @@ -225,8 +225,8 @@ FindGuardedMemoryMap (
>    //
>    // Adjust current map table depth according to the address to access
>    //
> -  while (mMapLevel < GUARDED_HEAP_MAP_TABLE_DEPTH
> -         &&
> +  while (AllocMapUnit &&
> +         mMapLevel < GUARDED_HEAP_MAP_TABLE_DEPTH &&
>           RShiftU64 (
>             Address,
>             mLevelShift[GUARDED_HEAP_MAP_TABLE_DEPTH - mMapLevel -
> 1]
> @@ -904,6 +904,10 @@ AdjustMemoryS (
>    }
> 
>    Target = Start + Size - SizeRequested;
> +  ASSERT (Target >= Start);
> +  if (Target == 0) {
> +    return 0;
> +  }
> 
>    if (!IsGuardPage (Start + Size)) {
>      // No Guard at tail to share. One more page is needed.
> --
> 2.16.2.windows.1