On 2023-03-16 13:50, Ard Biesheuvel wrote:
On Thu, 16 Mar 2023 at 14:33, Leif Lindholm <quic_llind...@quicinc.com> wrote:

On Mon, Mar 13, 2023 at 18:16:59 +0100, Ard Biesheuvel wrote:
Deal with DRAM memory potentially being mapped with non-executable
permissions, by mapping the DXE core code sections explicitly before
launch.

Could you add a note about why LoadPeCoffImage/LoadDxeCoreFromFfsFile
are made private?


Actually, they shouldn't - they are used elsewhere too.

I confused myself into thinking that LoadPeCoffImage() should be made
private as it now has 'special' behavior that only applies to DxeCore,
but in fact, the whole purpose in life of PrePi is to load DxeCore and
run it, so that was kind of implied anyway.

So I intend to simply drop those changes.

Works for me :)
With that:
Reviewed-by: Leif Lindholm <quic_llind...@quicinc.com>



Signed-off-by: Ard Biesheuvel <a...@kernel.org>
---
  EmbeddedPkg/Include/Library/PrePiLib.h          | 16 ------
  EmbeddedPkg/Library/PrePiLib/Arm/RemapDxeCore.c | 51 ++++++++++++++++++++
  EmbeddedPkg/Library/PrePiLib/PrePi.h            | 13 +++++
  EmbeddedPkg/Library/PrePiLib/PrePiLib.c         |  4 ++
  EmbeddedPkg/Library/PrePiLib/PrePiLib.inf       | 12 +++++
  EmbeddedPkg/Library/PrePiLib/X86/RemapDxeCore.c | 23 +++++++++
  6 files changed, 103 insertions(+), 16 deletions(-)

diff --git a/EmbeddedPkg/Include/Library/PrePiLib.h 
b/EmbeddedPkg/Include/Library/PrePiLib.h
index 93a9115eac2d..14f2bbc38dae 100644
--- a/EmbeddedPkg/Include/Library/PrePiLib.h
+++ b/EmbeddedPkg/Include/Library/PrePiLib.h
@@ -758,22 +758,6 @@ AllocateAlignedPages (
    IN UINTN  Alignment
    );

-EFI_STATUS
-EFIAPI
-LoadPeCoffImage (
-  IN  VOID                  *PeCoffImage,
-  OUT EFI_PHYSICAL_ADDRESS  *ImageAddress,
-  OUT UINT64                *ImageSize,
-  OUT EFI_PHYSICAL_ADDRESS  *EntryPoint
-  );
-
-EFI_STATUS
-EFIAPI
-LoadDxeCoreFromFfsFile (
-  IN EFI_PEI_FILE_HANDLE  FileHandle,
-  IN UINTN                StackSize
-  );
-
  EFI_STATUS
  EFIAPI
  LoadDxeCoreFromFv (
diff --git a/EmbeddedPkg/Library/PrePiLib/Arm/RemapDxeCore.c 
b/EmbeddedPkg/Library/PrePiLib/Arm/RemapDxeCore.c
new file mode 100644
index 000000000000..40d4ed9d77bd
--- /dev/null
+++ b/EmbeddedPkg/Library/PrePiLib/Arm/RemapDxeCore.c
@@ -0,0 +1,51 @@
+/** @file
+  Copyright (c) 2023, Google LLC. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "PrePi.h"
+
+#include <Library/ArmMmuLib.h>
+
+/**
+  Remap the code section of the DXE core with the read-only and executable
+  permissions.
+
+  @param  ImageContext    The image context describing the loaded PE/COFF image
+
+**/
+VOID
+EFIAPI
+RemapDxeCore (
+  IN  CONST PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext
+  )
+{
+  EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION  Hdr;
+  EFI_IMAGE_SECTION_HEADER             *Section;
+  UINTN                                Index;
+
+  Hdr.Union = (EFI_IMAGE_OPTIONAL_HEADER_UNION *)((UINT8 
*)ImageContext->Handle +
+                                                  
ImageContext->PeCoffHeaderOffset);
+  ASSERT (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE);
+
+  Section = (EFI_IMAGE_SECTION_HEADER *)((UINT8 *)Hdr.Union + sizeof (UINT32) +
+                                         sizeof (EFI_IMAGE_FILE_HEADER) +
+                                         
Hdr.Pe32->FileHeader.SizeOfOptionalHeader
+                                         );
+
+  for (Index = 0; Index < Hdr.Pe32->FileHeader.NumberOfSections; Index++) {
+    if ((Section[Index].Characteristics & EFI_IMAGE_SCN_CNT_CODE) != 0) {
+      ArmSetMemoryRegionReadOnly (
+        (UINTN)(ImageContext->ImageAddress + Section[Index].VirtualAddress),
+        Section[Index].Misc.VirtualSize
+        );
+
+      ArmClearMemoryRegionNoExec (
+        (UINTN)(ImageContext->ImageAddress + Section[Index].VirtualAddress),
+        Section[Index].Misc.VirtualSize
+        );
+    }
+  }
+}
diff --git a/EmbeddedPkg/Library/PrePiLib/PrePi.h 
b/EmbeddedPkg/Library/PrePiLib/PrePi.h
index a00c946512f4..a0f8837d1d37 100644
--- a/EmbeddedPkg/Library/PrePiLib/PrePi.h
+++ b/EmbeddedPkg/Library/PrePiLib/PrePi.h
@@ -37,4 +37,17 @@
  #define GET_GUID_HOB_DATA(GuidHob)       ((VOID *) (((UINT8 *) 
&((GuidHob)->Name)) + sizeof (EFI_GUID)))
  #define GET_GUID_HOB_DATA_SIZE(GuidHob)  (((GuidHob)->Header).HobLength - 
sizeof (EFI_HOB_GUID_TYPE))

+/**
+  Remap the code section of the DXE core with the read-only and executable
+  permissions.
+
+  @param  ImageContext    The image context describing the loaded PE/COFF image
+
+**/
+VOID
+EFIAPI
+RemapDxeCore (
+  IN  CONST PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext
+  );
+
  #endif
diff --git a/EmbeddedPkg/Library/PrePiLib/PrePiLib.c 
b/EmbeddedPkg/Library/PrePiLib/PrePiLib.c
index 3cf866dab248..188ad5c518a8 100644
--- a/EmbeddedPkg/Library/PrePiLib/PrePiLib.c
+++ b/EmbeddedPkg/Library/PrePiLib/PrePiLib.c
@@ -54,6 +54,7 @@ AllocateCodePages (
    return NULL;
  }

+STATIC
  EFI_STATUS
  EFIAPI
  LoadPeCoffImage (
@@ -105,6 +106,8 @@ LoadPeCoffImage (
    //
    InvalidateInstructionCacheRange ((VOID *)(UINTN)*ImageAddress, 
(UINTN)*ImageSize);

+  RemapDxeCore (&ImageContext);
+
    return Status;
  }

@@ -114,6 +117,7 @@ VOID
    IN  VOID *HobStart
    );

+STATIC
  EFI_STATUS
  EFIAPI
  LoadDxeCoreFromFfsFile (
diff --git a/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf 
b/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
index 090bfe888f52..2df5928c51d5 100644
--- a/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
+++ b/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
@@ -31,11 +31,20 @@ [Sources.common]
    FwVol.c
    PrePiLib.c

+[Sources.X64, Sources.IA32]
+  X86/RemapDxeCore.c
+
+[Sources.AARCH64, Sources.ARM]
+  Arm/RemapDxeCore.c
+
  [Packages]
    MdePkg/MdePkg.dec
    EmbeddedPkg/EmbeddedPkg.dec
    MdeModulePkg/MdeModulePkg.dec

+[Packages.ARM, Packages.AARCH64]
+  ArmPkg/ArmPkg.dec
+
  [LibraryClasses]
    BaseLib
    DebugLib
@@ -50,6 +59,9 @@ [LibraryClasses]
    PerformanceLib
    HobLib

+[LibraryClasses.ARM, LibraryClasses.AARCH64]
+  ArmMmuLib
+
  [Guids]
    gEfiMemoryTypeInformationGuid

diff --git a/EmbeddedPkg/Library/PrePiLib/X86/RemapDxeCore.c 
b/EmbeddedPkg/Library/PrePiLib/X86/RemapDxeCore.c
new file mode 100644
index 000000000000..1899c050fdec
--- /dev/null
+++ b/EmbeddedPkg/Library/PrePiLib/X86/RemapDxeCore.c
@@ -0,0 +1,23 @@
+/** @file
+  Copyright (c) 2023, Google LLC. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "PrePi.h"
+
+/**
+  Remap the code section of the DXE core with the read-only and executable
+  permissions.
+
+  @param  ImageContext    The image context describing the loaded PE/COFF image
+
+**/
+VOID
+EFIAPI
+RemapDxeCore (
+  IN  CONST PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext
+  )
+{
+}
--
2.39.2









-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101276): https://edk2.groups.io/g/devel/message/101276
Mute This Topic: https://groups.io/mt/97586028/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to