* [PATCH v2] MdeModulePkg/Gcd: Check memory allocation when initializing memory
[not found] <cover.1603813842.git.jbrasen@nvidia.com>
@ 2020-10-27 15:52 ` Jeff Brasen
2020-10-28 6:06 ` 回复: [edk2-devel] " gaoliming
0 siblings, 1 reply; 2+ messages in thread
From: Jeff Brasen @ 2020-10-27 15:52 UTC (permalink / raw)
To: devel; +Cc: dandan.bi, gaoliming, lersek, Jeff Brasen
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 2d8c076f7113..4a22ee96b758 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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* 回复: [edk2-devel] [PATCH v2] MdeModulePkg/Gcd: Check memory allocation when initializing memory
2020-10-27 15:52 ` [PATCH v2] MdeModulePkg/Gcd: Check memory allocation when initializing memory Jeff Brasen
@ 2020-10-28 6:06 ` gaoliming
0 siblings, 0 replies; 2+ messages in thread
From: gaoliming @ 2020-10-28 6:06 UTC (permalink / raw)
To: devel, jbrasen; +Cc: dandan.bi, lersek
Jeff:
The logic is correct. It handles the memory allocated by creating memory
allocation hob instead of allocating page service.
I add minor comment on below.
> -----邮件原件-----
> 发件人: bounce+27952+66661+4905953+8761045@groups.io
> <bounce+27952+66661+4905953+8761045@groups.io> 代表 Jeff Brasen
> 发送时间: 2020年10月27日 23:53
> 收件人: devel@edk2.groups.io
> 抄送: dandan.bi@intel.com; gaoliming@byosoft.com.cn; lersek@redhat.com;
> Jeff Brasen <jbrasen@nvidia.com>
> 主题: [edk2-devel] [PATCH v2] MdeModulePkg/Gcd: Check memory
> allocation when initializing memory
>
> 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 2d8c076f7113..4a22ee96b758 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;
> +
Please move local variable declaration in the begin of the function.
> + 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;
> +
Please move local variable declaration in the begin of the function.
With this change, Liming Gao <gaoliming@byosoft.com.cn>
Thanks
Liming
> + 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
>
>
>
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-10-28 6:06 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1603813842.git.jbrasen@nvidia.com>
2020-10-27 15:52 ` [PATCH v2] MdeModulePkg/Gcd: Check memory allocation when initializing memory Jeff Brasen
2020-10-28 6:06 ` 回复: [edk2-devel] " gaoliming
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox