[v1] MdeModulePkg/Gcd: Check memory allocation when initializing memory

[edk2-devel] [PATCH] MdeModulePkg/Gcd: Check memory allocation when initializing memory

Posted by Jeff Brasen 3 years, 6 months ago

CoreInitializeMemoryServices was not checking for any existing memory
allocation created in the HOB producer phase. If there are memory
allocations outside of the region covered by the HOB List then Gcd could
select that region for memory which can result in the memory allocation
to not be handled and memory overwrites.

Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
---
 MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 60 +++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
index 2d8c076f71..4a22ee96b7 100644
--- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
+++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
@@ -2097,6 +2097,62 @@ CalculateTotalMemoryBinSizeNeeded (
   return TotalSize;

 }

 

+/**

+   Find the largest region in the specified region that is not covered by an existing memory allocation

+

+   @param BaseAddress   On input start of the region to check.

+                        On output start of the largest free region.

+   @param Length        On input size of region to check.

+                        On output size of the largest free region.

+   @param MemoryHob     Hob pointer for the first memory allocation pointer to check

+**/

+VOID

+FindLargestFreeRegion (

+    IN OUT EFI_PHYSICAL_ADDRESS  *BaseAddress,

+    IN OUT UINT64                *Length,

+    IN EFI_HOB_MEMORY_ALLOCATION *MemoryHob

+    )

+{

+  EFI_PHYSICAL_ADDRESS TopAddress;

+

+  TopAddress = *BaseAddress + *Length;

+  while (MemoryHob != NULL) {

+    EFI_PHYSICAL_ADDRESS AllocatedTop;

+

+    AllocatedTop = MemoryHob->AllocDescriptor.MemoryBaseAddress + MemoryHob->AllocDescriptor.MemoryLength;

+

+    if ((MemoryHob->AllocDescriptor.MemoryBaseAddress >= *BaseAddress) &&

+        (AllocatedTop <= TopAddress)) {

+      EFI_PHYSICAL_ADDRESS LowerBase;

+      UINT64               LowerSize;

+      EFI_PHYSICAL_ADDRESS UpperBase;

+      UINT64               UpperSize;

+

+      LowerBase = *BaseAddress;

+      LowerSize = MemoryHob->AllocDescriptor.MemoryBaseAddress - *BaseAddress;

+      UpperBase = AllocatedTop;

+      UpperSize = TopAddress - AllocatedTop;

+

+      if (LowerSize != 0) {

+        FindLargestFreeRegion (&LowerBase, &LowerSize, (EFI_HOB_MEMORY_ALLOCATION *) GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, GET_NEXT_HOB (MemoryHob)));

+      }

+      if (UpperSize != 0) {

+        FindLargestFreeRegion (&UpperBase, &UpperSize, (EFI_HOB_MEMORY_ALLOCATION *) GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, GET_NEXT_HOB (MemoryHob)));

+      }

+

+      if (UpperSize >= LowerSize) {

+        *Length = UpperSize;

+        *BaseAddress = UpperBase;

+      } else {

+        *Length = LowerSize;

+        *BaseAddress = LowerBase;

+      }

+      return;

+    }

+    MemoryHob = GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, GET_NEXT_HOB (MemoryHob));

+  }

+}

+

 /**

   External function. Initializes memory services based on the memory

   descriptor HOBs.  This function is responsible for priming the memory

@@ -2235,6 +2291,7 @@ CoreInitializeMemoryServices (
     Attributes  = PhitResourceHob->ResourceAttribute;

     BaseAddress = PageAlignAddress (PhitHob->EfiMemoryTop);

     Length      = PageAlignLength  (ResourceHob->PhysicalStart + ResourceHob->ResourceLength - BaseAddress);

+    FindLargestFreeRegion (&BaseAddress, &Length, (EFI_HOB_MEMORY_ALLOCATION *)GetFirstHob (EFI_HOB_TYPE_MEMORY_ALLOCATION));

     if (Length < MinimalMemorySizeNeeded) {

       //

       // If that range is not large enough to intialize the DXE Core, then

@@ -2242,6 +2299,7 @@ CoreInitializeMemoryServices (
       //

       BaseAddress = PageAlignAddress (PhitHob->EfiFreeMemoryBottom);

       Length      = PageAlignLength  (PhitHob->EfiFreeMemoryTop - BaseAddress);

+      //This region is required to have no memory allocation inside it, skip check for entries in HOB List

       if (Length < MinimalMemorySizeNeeded) {

         //

         // If that range is not large enough to intialize the DXE Core, then

@@ -2249,6 +2307,7 @@ CoreInitializeMemoryServices (
         //

         BaseAddress = PageAlignAddress (ResourceHob->PhysicalStart);

         Length      = PageAlignLength  ((UINT64)((UINTN)*HobStart - BaseAddress));

+        FindLargestFreeRegion (&BaseAddress, &Length, (EFI_HOB_MEMORY_ALLOCATION *)GetFirstHob (EFI_HOB_TYPE_MEMORY_ALLOCATION));

       }

     }

     break;

@@ -2312,6 +2371,7 @@ CoreInitializeMemoryServices (
       //

       TestedMemoryBaseAddress = PageAlignAddress (ResourceHob->PhysicalStart);

       TestedMemoryLength      = PageAlignLength  (ResourceHob->PhysicalStart + ResourceHob->ResourceLength - TestedMemoryBaseAddress);

+      FindLargestFreeRegion (&TestedMemoryBaseAddress, &TestedMemoryLength, (EFI_HOB_MEMORY_ALLOCATION *)GetFirstHob (EFI_HOB_TYPE_MEMORY_ALLOCATION));

       if (TestedMemoryLength < MinimalMemorySizeNeeded) {

         continue;

       }

-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#66572): https://edk2.groups.io/g/devel/message/66572
Mute This Topic: https://groups.io/mt/77762677/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] MdeModulePkg/Gcd: Check memory allocation when initializing memory

Posted by Laszlo Ersek 3 years, 6 months ago

Hi Jeff,

On 10/24/20 00:10, Jeff Brasen wrote:
> CoreInitializeMemoryServices was not checking for any existing memory
> allocation created in the HOB producer phase. If there are memory
> allocations outside of the region covered by the HOB List then Gcd could
> select that region for memory which can result in the memory allocation
> to not be handled and memory overwrites.
> 
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> ---
>  MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 60 +++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)

can you please run "BaseTools/Scripts/SetupGit.py" in your edk2 clone?

Thanks,
Laszlo

> 
> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> index 2d8c076f71..4a22ee96b7 100644
> --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
> @@ -2097,6 +2097,62 @@ CalculateTotalMemoryBinSizeNeeded (
>    return TotalSize;
> 
>  }
> 
>  
> 
> +/**
> 
> +   Find the largest region in the specified region that is not covered by an existing memory allocation
> 
> +
> 
> +   @param BaseAddress   On input start of the region to check.
> 
> +                        On output start of the largest free region.
> 
> +   @param Length        On input size of region to check.
> 
> +                        On output size of the largest free region.
> 
> +   @param MemoryHob     Hob pointer for the first memory allocation pointer to check
> 
> +**/
> 
> +VOID
> 
> +FindLargestFreeRegion (
> 
> +    IN OUT EFI_PHYSICAL_ADDRESS  *BaseAddress,
> 
> +    IN OUT UINT64                *Length,
> 
> +    IN EFI_HOB_MEMORY_ALLOCATION *MemoryHob
> 
> +    )
> 
> +{
> 
> +  EFI_PHYSICAL_ADDRESS TopAddress;
> 
> +
> 
> +  TopAddress = *BaseAddress + *Length;
> 
> +  while (MemoryHob != NULL) {
> 
> +    EFI_PHYSICAL_ADDRESS AllocatedTop;
> 
> +
> 
> +    AllocatedTop = MemoryHob->AllocDescriptor.MemoryBaseAddress + MemoryHob->AllocDescriptor.MemoryLength;
> 
> +
> 
> +    if ((MemoryHob->AllocDescriptor.MemoryBaseAddress >= *BaseAddress) &&
> 
> +        (AllocatedTop <= TopAddress)) {
> 
> +      EFI_PHYSICAL_ADDRESS LowerBase;
> 
> +      UINT64               LowerSize;
> 
> +      EFI_PHYSICAL_ADDRESS UpperBase;
> 
> +      UINT64               UpperSize;
> 
> +
> 
> +      LowerBase = *BaseAddress;
> 
> +      LowerSize = MemoryHob->AllocDescriptor.MemoryBaseAddress - *BaseAddress;
> 
> +      UpperBase = AllocatedTop;
> 
> +      UpperSize = TopAddress - AllocatedTop;
> 
> +
> 
> +      if (LowerSize != 0) {
> 
> +        FindLargestFreeRegion (&LowerBase, &LowerSize, (EFI_HOB_MEMORY_ALLOCATION *) GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, GET_NEXT_HOB (MemoryHob)));
> 
> +      }
> 
> +      if (UpperSize != 0) {
> 
> +        FindLargestFreeRegion (&UpperBase, &UpperSize, (EFI_HOB_MEMORY_ALLOCATION *) GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, GET_NEXT_HOB (MemoryHob)));
> 
> +      }
> 
> +
> 
> +      if (UpperSize >= LowerSize) {
> 
> +        *Length = UpperSize;
> 
> +        *BaseAddress = UpperBase;
> 
> +      } else {
> 
> +        *Length = LowerSize;
> 
> +        *BaseAddress = LowerBase;
> 
> +      }
> 
> +      return;
> 
> +    }
> 
> +    MemoryHob = GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, GET_NEXT_HOB (MemoryHob));
> 
> +  }
> 
> +}
> 
> +
> 
>  /**
> 
>    External function. Initializes memory services based on the memory
> 
>    descriptor HOBs.  This function is responsible for priming the memory
> 
> @@ -2235,6 +2291,7 @@ CoreInitializeMemoryServices (
>      Attributes  = PhitResourceHob->ResourceAttribute;
> 
>      BaseAddress = PageAlignAddress (PhitHob->EfiMemoryTop);
> 
>      Length      = PageAlignLength  (ResourceHob->PhysicalStart + ResourceHob->ResourceLength - BaseAddress);
> 
> +    FindLargestFreeRegion (&BaseAddress, &Length, (EFI_HOB_MEMORY_ALLOCATION *)GetFirstHob (EFI_HOB_TYPE_MEMORY_ALLOCATION));
> 
>      if (Length < MinimalMemorySizeNeeded) {
> 
>        //
> 
>        // If that range is not large enough to intialize the DXE Core, then
> 
> @@ -2242,6 +2299,7 @@ CoreInitializeMemoryServices (
>        //
> 
>        BaseAddress = PageAlignAddress (PhitHob->EfiFreeMemoryBottom);
> 
>        Length      = PageAlignLength  (PhitHob->EfiFreeMemoryTop - BaseAddress);
> 
> +      //This region is required to have no memory allocation inside it, skip check for entries in HOB List
> 
>        if (Length < MinimalMemorySizeNeeded) {
> 
>          //
> 
>          // If that range is not large enough to intialize the DXE Core, then
> 
> @@ -2249,6 +2307,7 @@ CoreInitializeMemoryServices (
>          //
> 
>          BaseAddress = PageAlignAddress (ResourceHob->PhysicalStart);
> 
>          Length      = PageAlignLength  ((UINT64)((UINTN)*HobStart - BaseAddress));
> 
> +        FindLargestFreeRegion (&BaseAddress, &Length, (EFI_HOB_MEMORY_ALLOCATION *)GetFirstHob (EFI_HOB_TYPE_MEMORY_ALLOCATION));
> 
>        }
> 
>      }
> 
>      break;
> 
> @@ -2312,6 +2371,7 @@ CoreInitializeMemoryServices (
>        //
> 
>        TestedMemoryBaseAddress = PageAlignAddress (ResourceHob->PhysicalStart);
> 
>        TestedMemoryLength      = PageAlignLength  (ResourceHob->PhysicalStart + ResourceHob->ResourceLength - TestedMemoryBaseAddress);
> 
> +      FindLargestFreeRegion (&TestedMemoryBaseAddress, &TestedMemoryLength, (EFI_HOB_MEMORY_ALLOCATION *)GetFirstHob (EFI_HOB_TYPE_MEMORY_ALLOCATION));
> 
>        if (TestedMemoryLength < MinimalMemorySizeNeeded) {
> 
>          continue;
> 
>        }
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#66649): https://edk2.groups.io/g/devel/message/66649
Mute This Topic: https://groups.io/mt/77762677/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-