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

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(-)

-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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
>

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.