Deal with DRAM memory potentially being mapped with non-executable
permissions, by mapping the DXE core code sections explicitly before
launch.
Signed-off-by: Ard Biesheuvel <ardb@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 (#101127): https://edk2.groups.io/g/devel/message/101127
Mute This Topic: https://groups.io/mt/97586028/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-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?
/
Leif
> Signed-off-by: Ard Biesheuvel <ardb@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 (#101270): https://edk2.groups.io/g/devel/message/101270
Mute This Topic: https://groups.io/mt/97586028/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On Thu, 16 Mar 2023 at 14:33, Leif Lindholm <quic_llindhol@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.
>
> > Signed-off-by: Ard Biesheuvel <ardb@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 (#101272): https://edk2.groups.io/g/devel/message/101272
Mute This Topic: https://groups.io/mt/97586028/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 2023-03-16 13:50, Ard Biesheuvel wrote:
> On Thu, 16 Mar 2023 at 14:33, Leif Lindholm <quic_llindhol@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_llindhol@quicinc.com>
>
>>
>>> Signed-off-by: Ard Biesheuvel <ardb@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.