1. Patchew
  2. EDK2
  3. View series

[edk2] [PATCH] MdeModulePkg PiSmmCoreMemoryAllocLib: Fix a FreePool() assertion issue

Star Zeng posted 1 patch 6 years, 8 months ago
Failed in applying to current master (apply log)
.../MemoryAllocationLib.c                          | 32 +++++++++++++++++++---
1 file changed, 28 insertions(+), 4 deletions(-)
[edk2] [PATCH] MdeModulePkg PiSmmCoreMemoryAllocLib: Fix a FreePool() assertion issue
Posted by Star Zeng 6 years, 8 months ago 
When PiSmmCore links against PeiDxeDebugLibReportStatusCode, the code
flow below will cause a FreePool() assertion issue.

PiSmmCoreMemoryAllocationLibConstructor() ->
SmmInitializeMemoryServices() ->
DEBUG ((DEBUG_INFO, "SmmAddMemoryRegion\n")) in SmmAddMemoryRegion() ->
DebugPrint() -> REPORT_STATUS_CODE_EX() -> ReportStatusCodeEx() ->
AllocatePool()/FreePool(PiSmmCoreMemoryAllocLib) ->
ASSERT() at Head = CR (Buffer, POOL_HEAD, Data, POOL_HEAD_SIGNATURE)
  in CoreFreePoolI() of DxeCore Pool.c

It is because at the point of FreePool() in the code flow above,
mSmmCoreMemoryAllocLibSmramRanges/mSmmCoreMemoryAllocLibSmramRangeCount
are not been initialized yet, the FreePool() will be directed to
gBS->FreePool(), that is wrong.

This patch is to temporarily use BootServicesData to hold the
SmramRanges data before calling SmmInitializeMemoryServices().

Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 .../MemoryAllocationLib.c                          | 32 +++++++++++++++++++---
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/MemoryAllocationLib.c b/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/MemoryAllocationLib.c
index 96cb275cc9d7..4216a12d18f5 100644
--- a/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/MemoryAllocationLib.c
+++ b/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/MemoryAllocationLib.c
@@ -1068,20 +1068,44 @@ PiSmmCoreMemoryAllocationLibConstructor (
   IN EFI_SYSTEM_TABLE  *SystemTable
   )
 {
+  EFI_STATUS             Status;
   SMM_CORE_PRIVATE_DATA  *SmmCorePrivate;
   UINTN                  Size;
+  VOID                   *BootServicesData;
 
   SmmCorePrivate = (SMM_CORE_PRIVATE_DATA *)ImageHandle;
+
   //
-  // Initialize memory service using free SMRAM
+  // The FreePool()/FreePages() will need use SmramRanges data to know whether
+  // the buffer to free is in SMRAM range or not. And there may be FreePool()/
+  // FreePages() indrectly during calling SmmInitializeMemoryServices(), but
+  // no SMRAM could be allocated before calling SmmInitializeMemoryServices(),
+  // so temporarily use BootServicesData to hold the SmramRanges data.
   //
-  SmmInitializeMemoryServices (SmmCorePrivate->SmramRangeCount, SmmCorePrivate->SmramRanges);
-
   mSmmCoreMemoryAllocLibSmramRangeCount = SmmCorePrivate->SmramRangeCount;
   Size = mSmmCoreMemoryAllocLibSmramRangeCount * sizeof (EFI_SMRAM_DESCRIPTOR);
-  mSmmCoreMemoryAllocLibSmramRanges = (EFI_SMRAM_DESCRIPTOR *) AllocatePool (Size);
+  Status = gBS->AllocatePool (EfiBootServicesData, Size, (VOID **) &mSmmCoreMemoryAllocLibSmramRanges);
+  ASSERT_EFI_ERROR (Status);
   ASSERT (mSmmCoreMemoryAllocLibSmramRanges != NULL);
   CopyMem (mSmmCoreMemoryAllocLibSmramRanges, SmmCorePrivate->SmramRanges, Size);
 
+  //
+  // Initialize memory service using free SMRAM
+  //
+  SmmInitializeMemoryServices (SmmCorePrivate->SmramRangeCount, SmmCorePrivate->SmramRanges);
+
+  //
+  // Move the SmramRanges data from BootServicesData to SMRAM.
+  //
+  BootServicesData = mSmmCoreMemoryAllocLibSmramRanges;
+  mSmmCoreMemoryAllocLibSmramRanges = (EFI_SMRAM_DESCRIPTOR *) AllocateCopyPool (Size, (VOID *) BootServicesData);
+  ASSERT (mSmmCoreMemoryAllocLibSmramRanges != NULL);
+
+  //
+  // Free the temporarily used BootServicesData.
+  //
+  Status = gBS->FreePool (BootServicesData);
+  ASSERT_EFI_ERROR (Status);
+
   return EFI_SUCCESS;
 }
-- 
2.7.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] MdeModulePkg PiSmmCoreMemoryAllocLib: Fix a FreePool() assertion issue
Posted by Gao, Liming 6 years, 8 months ago 
Reviewed-by: Liming Gao <liming.gao@intel.com>

>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Star
>Zeng
>Sent: Monday, July 31, 2017 3:47 PM
>To: edk2-devel@lists.01.org
>Cc: Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming
><liming.gao@intel.com>; Zeng, Star <star.zeng@intel.com>
>Subject: [edk2] [PATCH] MdeModulePkg PiSmmCoreMemoryAllocLib: Fix a
>FreePool() assertion issue
>
>When PiSmmCore links against PeiDxeDebugLibReportStatusCode, the code
>flow below will cause a FreePool() assertion issue.
>
>PiSmmCoreMemoryAllocationLibConstructor() ->
>SmmInitializeMemoryServices() ->
>DEBUG ((DEBUG_INFO, "SmmAddMemoryRegion\n")) in
>SmmAddMemoryRegion() ->
>DebugPrint() -> REPORT_STATUS_CODE_EX() -> ReportStatusCodeEx() ->
>AllocatePool()/FreePool(PiSmmCoreMemoryAllocLib) ->
>ASSERT() at Head = CR (Buffer, POOL_HEAD, Data, POOL_HEAD_SIGNATURE)
>  in CoreFreePoolI() of DxeCore Pool.c
>
>It is because at the point of FreePool() in the code flow above,
>mSmmCoreMemoryAllocLibSmramRanges/mSmmCoreMemoryAllocLibSmra
>mRangeCount
>are not been initialized yet, the FreePool() will be directed to
>gBS->FreePool(), that is wrong.
>
>This patch is to temporarily use BootServicesData to hold the
>SmramRanges data before calling SmmInitializeMemoryServices().
>
>Cc: Liming Gao <liming.gao@intel.com>
>Cc: Jiewen Yao <jiewen.yao@intel.com>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Star Zeng <star.zeng@intel.com>
>---
> .../MemoryAllocationLib.c                          | 32 +++++++++++++++++++---
> 1 file changed, 28 insertions(+), 4 deletions(-)
>
>diff --git
>a/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/MemoryAllocati
>onLib.c
>b/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/MemoryAllocati
>onLib.c
>index 96cb275cc9d7..4216a12d18f5 100644
>---
>a/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/MemoryAllocati
>onLib.c
>+++
>b/MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib/MemoryAllocati
>onLib.c
>@@ -1068,20 +1068,44 @@ PiSmmCoreMemoryAllocationLibConstructor (
>   IN EFI_SYSTEM_TABLE  *SystemTable
>   )
> {
>+  EFI_STATUS             Status;
>   SMM_CORE_PRIVATE_DATA  *SmmCorePrivate;
>   UINTN                  Size;
>+  VOID                   *BootServicesData;
>
>   SmmCorePrivate = (SMM_CORE_PRIVATE_DATA *)ImageHandle;
>+
>   //
>-  // Initialize memory service using free SMRAM
>+  // The FreePool()/FreePages() will need use SmramRanges data to know
>whether
>+  // the buffer to free is in SMRAM range or not. And there may be
>FreePool()/
>+  // FreePages() indrectly during calling SmmInitializeMemoryServices(), but
>+  // no SMRAM could be allocated before calling
>SmmInitializeMemoryServices(),
>+  // so temporarily use BootServicesData to hold the SmramRanges data.
>   //
>-  SmmInitializeMemoryServices (SmmCorePrivate->SmramRangeCount,
>SmmCorePrivate->SmramRanges);
>-
>   mSmmCoreMemoryAllocLibSmramRangeCount = SmmCorePrivate-
>>SmramRangeCount;
>   Size = mSmmCoreMemoryAllocLibSmramRangeCount * sizeof
>(EFI_SMRAM_DESCRIPTOR);
>-  mSmmCoreMemoryAllocLibSmramRanges = (EFI_SMRAM_DESCRIPTOR *)
>AllocatePool (Size);
>+  Status = gBS->AllocatePool (EfiBootServicesData, Size, (VOID **)
>&mSmmCoreMemoryAllocLibSmramRanges);
>+  ASSERT_EFI_ERROR (Status);
>   ASSERT (mSmmCoreMemoryAllocLibSmramRanges != NULL);
>   CopyMem (mSmmCoreMemoryAllocLibSmramRanges, SmmCorePrivate-
>>SmramRanges, Size);
>
>+  //
>+  // Initialize memory service using free SMRAM
>+  //
>+  SmmInitializeMemoryServices (SmmCorePrivate->SmramRangeCount,
>SmmCorePrivate->SmramRanges);
>+
>+  //
>+  // Move the SmramRanges data from BootServicesData to SMRAM.
>+  //
>+  BootServicesData = mSmmCoreMemoryAllocLibSmramRanges;
>+  mSmmCoreMemoryAllocLibSmramRanges = (EFI_SMRAM_DESCRIPTOR *)
>AllocateCopyPool (Size, (VOID *) BootServicesData);
>+  ASSERT (mSmmCoreMemoryAllocLibSmramRanges != NULL);
>+
>+  //
>+  // Free the temporarily used BootServicesData.
>+  //
>+  Status = gBS->FreePool (BootServicesData);
>+  ASSERT_EFI_ERROR (Status);
>+
>   return EFI_SUCCESS;
> }
>--
>2.7.0.windows.1
>
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.