[PATCH 0/2] DxeCore: Fix double free pages on LoadImage failure path

[PATCH 0/2] DxeCore: Fix double free pages on LoadImage failure path

[Star Zeng]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=667
REF: https://lists.01.org/pipermail/edk2-devel/2017-August/013112.html

Star Zeng (2):
  MdeModulePkg DxeCore: Fix double free pages on LoadImage failure path
  MdeModulePkg DxeCore: Enhance "ConvertPages: Incompatible memory
    types"

 MdeModulePkg/Core/Dxe/Image/Image.c | 2 ++
 MdeModulePkg/Core/Dxe/Mem/Page.c    | 9 +++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

-- 
2.7.0.windows.1

[PATCH 1/2] MdeModulePkg DxeCore: Fix double free pages on LoadImage failure path

[Star Zeng]

https://bugzilla.tianocore.org/show_bug.cgi?id=667
reported there is double free pages on LoadImage failure path.

CoreLoadPeImage()
...
  return EFI_SUCCESS;

Done:

  //
  // Free memory.
  //
  if (DstBufAlocated) {
    CoreFreePages (Image->ImageContext.ImageAddress, Image->NumberOfPages);
  }
...

CoreUnloadAndCloseImage()
...
  if ((Image->ImageBasePage != 0) && FreePage) {
    CoreFreePages (Image->ImageBasePage, Image->NumberOfPages);
  }
...

This patch is to follow the suggestion at
https://lists.01.org/pipermail/edk2-devel/2017-August/013112.html
to set Image->ImageContext.ImageAddress and Image->ImageBasePage to 0
after the free in CoreLoadPeImage().

Cc: Liming Gao <liming.gao@intel.com>
Cc: Andrew Fish <afish@apple.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Core/Dxe/Image/Image.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
index 03e979a60409..4e22aa6dc7e3 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -788,6 +788,8 @@ Done:
 
   if (DstBufAlocated) {
     CoreFreePages (Image->ImageContext.ImageAddress, Image->NumberOfPages);
+    Image->ImageContext.ImageAddress = 0;
+    Image->ImageBasePage = 0;
   }
 
   if (Image->ImageContext.FixupData != NULL) {
-- 
2.7.0.windows.1

[PATCH 2/2] MdeModulePkg DxeCore: Enhance "ConvertPages: Incompatible memory types"

[Star Zeng]

When double free pages by FreePages() or allocate allocated pages by
AllocatePages() with AllocateAddress type, the code will print debug
message "ConvertPages: Incompatible memory types", but the debug
message is not very obvious for the error paths by FreePages() or
AllocatePages().

Refer https://lists.01.org/pipermail/edk2-devel/2017-August/013075.html
for the discussion.

This patch is to enhance the debug message for the error paths by
FreePages() or AllocatePages.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Andrew Fish <afish@apple.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Core/Dxe/Mem/Page.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index 3b3b9a8131a2..a142c79ee2ca 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -1,7 +1,7 @@
 /** @file
   UEFI Memory page management functions.
 
-Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.<BR>
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -788,7 +788,12 @@ CoreConvertPagesEx (
       // Debug code - verify conversion is allowed
       //
       if (!(NewType == EfiConventionalMemory ? 1 : 0) ^ (Entry->Type == EfiConventionalMemory ? 1 : 0)) {
-        DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "ConvertPages: Incompatible memory types\n"));
+        DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "ConvertPages: Incompatible memory types, "));
+        if (Entry->Type == EfiConventionalMemory) {
+          DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "the pages to free have been freed\n"));
+        } else {
+          DEBUG ((DEBUG_ERROR | DEBUG_PAGE, "the pages to allocate have been allocated\n"));
+        }
         return EFI_NOT_FOUND;
       }
 
-- 
2.7.0.windows.1

Re: [PATCH 0/2] DxeCore: Fix double free pages on LoadImage failure path

[Gao, Liming]

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: Friday, August 11, 2017 11:35 AM
>To: edk2-devel@lists.01.org
>Cc: Zeng, Star <star.zeng@intel.com>
>Subject: [edk2] [PATCH 0/2] DxeCore: Fix double free pages on LoadImage
>failure path
>
>REF: https://bugzilla.tianocore.org/show_bug.cgi?id=667
>REF: https://lists.01.org/pipermail/edk2-devel/2017-August/013112.html
>
>Star Zeng (2):
>  MdeModulePkg DxeCore: Fix double free pages on LoadImage failure path
>  MdeModulePkg DxeCore: Enhance "ConvertPages: Incompatible memory
>    types"
>
> MdeModulePkg/Core/Dxe/Image/Image.c | 2 ++
> MdeModulePkg/Core/Dxe/Mem/Page.c    | 9 +++++++--
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
>--
>2.7.0.windows.1
>
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel