[PATCH] EmbeddedPkg/PrePiLib: allocate code pages for DxeCore

[PATCH] EmbeddedPkg/PrePiLib: allocate code pages for DxeCore

[Ard Biesheuvel]

The recently introduced memory protection features inadvertently broke
the boot on all PrePi platforms, because the changes to explicitly use
EfiBootServicesCode for loading the DxeCore PE/COFF image need to be
applied in a different way for PrePi. So add a simple helper function
that sets the type of an allocation to EfiBootServicesCode, and invoke
it to allocate the space for DxeCore.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 EmbeddedPkg/Library/PrePiLib/PrePiLib.c | 34 +++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/EmbeddedPkg/Library/PrePiLib/PrePiLib.c b/EmbeddedPkg/Library/PrePiLib/PrePiLib.c
index 9a1ef344df6e..bba8e7384edc 100644
--- a/EmbeddedPkg/Library/PrePiLib/PrePiLib.c
+++ b/EmbeddedPkg/Library/PrePiLib/PrePiLib.c
@@ -28,6 +28,38 @@ SecWinNtPeiLoadFile (
   IN  EFI_PHYSICAL_ADDRESS    *EntryPoint
   );
 
+STATIC
+VOID*
+EFIAPI
+AllocateCodePages (
+  IN  UINTN     Pages
+  )
+{
+  VOID                    *Alloc;
+  EFI_PEI_HOB_POINTERS    Hob;
+
+  Alloc = AllocatePages (Pages);
+  if (Alloc == NULL) {
+    return NULL;
+  }
+
+  // find the HOB we just created, and change the type to EfiBootServicesCode
+  Hob.Raw = GetFirstHob (EFI_HOB_TYPE_MEMORY_ALLOCATION);
+  while (Hob.Raw != NULL) {
+    if (Hob.MemoryAllocation->AllocDescriptor.MemoryBaseAddress == (UINTN)Alloc) {
+      Hob.MemoryAllocation->AllocDescriptor.MemoryType = EfiBootServicesCode;
+      return Alloc;
+    }
+    Hob.Raw = GET_NEXT_HOB (Hob);
+    Hob.Raw = GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, Hob.Raw);
+  }
+
+  ASSERT (FALSE);
+
+  FreePages (Alloc, Pages);
+  return NULL;
+}
+
 
 EFI_STATUS
 EFIAPI
@@ -54,7 +86,7 @@ LoadPeCoffImage (
   //
   // Allocate Memory for the image
   //
-  Buffer = AllocatePages (EFI_SIZE_TO_PAGES((UINT32)ImageContext.ImageSize));
+  Buffer = AllocateCodePages (EFI_SIZE_TO_PAGES((UINT32)ImageContext.ImageSize));
   ASSERT (Buffer != 0);
 
 
-- 
2.7.4

Re: [PATCH] EmbeddedPkg/PrePiLib: allocate code pages for DxeCore

[Michael Zimmermann]

I had to include <Library/MemoryAllocationLib.h> because I was getting
this error:
EmbeddedPkg/Library/PrePiLib/PrePiLib.c: In function ‘AllocateCodePages’:
EmbeddedPkg/Library/PrePiLib/PrePiLib.c:59:3: error: implicit
declaration of function ‘FreePages’
[-Werror=implicit-function-declaration]
FreePages (Alloc, Pages);

Other than that it works great(you can take this as a Tested-By).
Sry if it's a stupid question because I didn't look into all of the
protection code yet, but wouldn't this mark everything as
Code(including Data)?

Thanks
Michael

On Mon, Mar 13, 2017 at 10:28 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> The recently introduced memory protection features inadvertently broke
> the boot on all PrePi platforms, because the changes to explicitly use
> EfiBootServicesCode for loading the DxeCore PE/COFF image need to be
> applied in a different way for PrePi. So add a simple helper function
> that sets the type of an allocation to EfiBootServicesCode, and invoke
> it to allocate the space for DxeCore.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  EmbeddedPkg/Library/PrePiLib/PrePiLib.c | 34 +++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/EmbeddedPkg/Library/PrePiLib/PrePiLib.c b/EmbeddedPkg/Library/PrePiLib/PrePiLib.c
> index 9a1ef344df6e..bba8e7384edc 100644
> --- a/EmbeddedPkg/Library/PrePiLib/PrePiLib.c
> +++ b/EmbeddedPkg/Library/PrePiLib/PrePiLib.c
> @@ -28,6 +28,38 @@ SecWinNtPeiLoadFile (
>    IN  EFI_PHYSICAL_ADDRESS    *EntryPoint
>    );
>
> +STATIC
> +VOID*
> +EFIAPI
> +AllocateCodePages (
> +  IN  UINTN     Pages
> +  )
> +{
> +  VOID                    *Alloc;
> +  EFI_PEI_HOB_POINTERS    Hob;
> +
> +  Alloc = AllocatePages (Pages);
> +  if (Alloc == NULL) {
> +    return NULL;
> +  }
> +
> +  // find the HOB we just created, and change the type to EfiBootServicesCode
> +  Hob.Raw = GetFirstHob (EFI_HOB_TYPE_MEMORY_ALLOCATION);
> +  while (Hob.Raw != NULL) {
> +    if (Hob.MemoryAllocation->AllocDescriptor.MemoryBaseAddress == (UINTN)Alloc) {
> +      Hob.MemoryAllocation->AllocDescriptor.MemoryType = EfiBootServicesCode;
> +      return Alloc;
> +    }
> +    Hob.Raw = GET_NEXT_HOB (Hob);
> +    Hob.Raw = GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, Hob.Raw);
> +  }
> +
> +  ASSERT (FALSE);
> +
> +  FreePages (Alloc, Pages);
> +  return NULL;
> +}
> +
>
>  EFI_STATUS
>  EFIAPI
> @@ -54,7 +86,7 @@ LoadPeCoffImage (
>    //
>    // Allocate Memory for the image
>    //
> -  Buffer = AllocatePages (EFI_SIZE_TO_PAGES((UINT32)ImageContext.ImageSize));
> +  Buffer = AllocateCodePages (EFI_SIZE_TO_PAGES((UINT32)ImageContext.ImageSize));
>    ASSERT (Buffer != 0);
>
>
> --
> 2.7.4
>

Re: [PATCH] EmbeddedPkg/PrePiLib: allocate code pages for DxeCore

[Ard Biesheuvel]

On 13 March 2017 at 09:55, Michael Zimmermann <sigmaepsilon92@gmail.com> wrote:
> I had to include <Library/MemoryAllocationLib.h> because I was getting
> this error:
> EmbeddedPkg/Library/PrePiLib/PrePiLib.c: In function ‘AllocateCodePages’:
> EmbeddedPkg/Library/PrePiLib/PrePiLib.c:59:3: error: implicit
> declaration of function ‘FreePages’
> [-Werror=implicit-function-declaration]
> FreePages (Alloc, Pages);
>
> Other than that it works great(you can take this as a Tested-By).

Thanks!

> Sry if it's a stupid question because I didn't look into all of the
> protection code yet, but wouldn't this mark everything as
> Code(including Data)?

It will mark the entire DxeCore PE/COFF image read-write-execute.
Later on, if the sections are sufficiently aligned, it will remove
write permissions from the PE/COFF text section and remove executable
permissions from the PE/COFF data section. If the sections are not
sufficiently aligned, it will leave the entire region as RWX.