From: Sughosh Ganu <sughosh.ganu@arm.com>
To: edk2-devel@lists.01.org
Cc: Leif Lindholm <leif.lindholm@linaro.com>,
Achin Gupta <achin.gupta@arm.com>,
Sughosh Ganu <sughosh.ganu@arm.com>
Subject: [PATCH v2 7/7] ArmPkg: Extra action to update permissions for S-ELO MM Image
Date: Fri, 20 Jul 2018 18:08:20 +0530 [thread overview]
Message-ID: <1532090300-5250-8-git-send-email-sughosh.ganu@arm.com> (raw)
In-Reply-To: <1532090300-5250-1-git-send-email-sughosh.ganu@arm.com>
From: Achin Gupta <achin.gupta@arm.com>
The Standalone MM drivers runs in S-EL0 in AArch64 on ARM Standard
Platforms and is deployed during SEC phase. The memory allocated to
the Standalone MM drivers should be marked as RO+X.
During PE/COFF Image section parsing, this patch implements extra
action "UpdatePeCoffPermissions" to request the privileged firmware in
EL3 to update the permissions.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sughosh Ganu <sughosh.ganu@arm.com>
---
| 7 +
| 171 +++++++++++++++++++-
2 files changed, 176 insertions(+), 2 deletions(-)
--git a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
index c1f717e5bda1..38bf3993ae99 100644
--- a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
+++ b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
@@ -33,7 +33,14 @@ [Sources.common]
DebugPeCoffExtraActionLib.c
[Packages]
+ ArmPkg/ArmPkg.dec
MdePkg/MdePkg.dec
+ StandaloneMmPkg/StandaloneMmPkg.dec
+
+[FeaturePcd]
+ gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable
[LibraryClasses]
+ ArmMmuLib
DebugLib
+ PcdLib
--git a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
index f298e58cdfca..8e621de4a87a 100644
--- a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
+++ b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
@@ -15,14 +15,165 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/
#include <PiDxe.h>
-#include <Library/PeCoffLib.h>
+#include <Library/ArmMmuLib.h>
#include <Library/BaseLib.h>
-#include <Library/DebugLib.h>
#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PeCoffLib.h>
#include <Library/PeCoffExtraActionLib.h>
#include <Library/PrintLib.h>
+typedef RETURN_STATUS (*REGION_PERMISSION_UPDATE_FUNC) (
+ IN EFI_PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length
+ );
+
+STATIC
+RETURN_STATUS
+UpdatePeCoffPermissions (
+ IN CONST PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext,
+ IN REGION_PERMISSION_UPDATE_FUNC NoExecUpdater,
+ IN REGION_PERMISSION_UPDATE_FUNC ReadOnlyUpdater
+ )
+{
+ RETURN_STATUS Status;
+ EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr;
+ EFI_IMAGE_OPTIONAL_HEADER_UNION HdrData;
+ UINTN Size;
+ UINTN ReadSize;
+ UINT32 SectionHeaderOffset;
+ UINTN NumberOfSections;
+ UINTN Index;
+ EFI_IMAGE_SECTION_HEADER SectionHeader;
+ PE_COFF_LOADER_IMAGE_CONTEXT TmpContext;
+ EFI_PHYSICAL_ADDRESS Base;
+
+ //
+ // We need to copy ImageContext since PeCoffLoaderGetImageInfo ()
+ // will mangle the ImageAddress field
+ //
+ CopyMem (&TmpContext, ImageContext, sizeof (TmpContext));
+
+ if (TmpContext.PeCoffHeaderOffset == 0) {
+ Status = PeCoffLoaderGetImageInfo (&TmpContext);
+ if (RETURN_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR,
+ "%a: PeCoffLoaderGetImageInfo () failed (Status = %r)\n",
+ __FUNCTION__, Status));
+ return Status;
+ }
+ }
+
+ if (TmpContext.IsTeImage &&
+ TmpContext.ImageAddress == ImageContext->ImageAddress) {
+ DEBUG ((DEBUG_INFO, "%a: ignoring XIP TE image at 0x%lx\n", __FUNCTION__,
+ ImageContext->ImageAddress));
+ return RETURN_SUCCESS;
+ }
+
+ if (TmpContext.SectionAlignment < EFI_PAGE_SIZE) {
+ //
+ // The sections need to be at least 4 KB aligned, since that is the
+ // granularity at which we can tighten permissions. So just clear the
+ // noexec permissions on the entire region.
+ //
+ if (!TmpContext.IsTeImage) {
+ DEBUG ((DEBUG_WARN,
+ "%a: non-TE Image at 0x%lx has SectionAlignment < 4 KB (%lu)\n",
+ __FUNCTION__, ImageContext->ImageAddress, TmpContext.SectionAlignment));
+ }
+ Base = ImageContext->ImageAddress & ~(EFI_PAGE_SIZE - 1);
+ Size = ImageContext->ImageAddress - Base + ImageContext->ImageSize;
+ return NoExecUpdater (Base, ALIGN_VALUE (Size, EFI_PAGE_SIZE));
+ }
+
+ //
+ // Read the PE/COFF Header. For PE32 (32-bit) this will read in too much
+ // data, but that should not hurt anything. Hdr.Pe32->OptionalHeader.Magic
+ // determines if this is a PE32 or PE32+ image. The magic is in the same
+ // location in both images.
+ //
+ Hdr.Union = &HdrData;
+ Size = sizeof (EFI_IMAGE_OPTIONAL_HEADER_UNION);
+ ReadSize = Size;
+ Status = TmpContext.ImageRead (TmpContext.Handle,
+ TmpContext.PeCoffHeaderOffset, &Size, Hdr.Pe32);
+ if (RETURN_ERROR (Status) || (Size != ReadSize)) {
+ DEBUG ((DEBUG_ERROR,
+ "%a: TmpContext.ImageRead () failed (Status = %r)\n",
+ __FUNCTION__, Status));
+ return Status;
+ }
+
+ ASSERT (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE);
+
+ SectionHeaderOffset = TmpContext.PeCoffHeaderOffset + sizeof (UINT32) +
+ sizeof (EFI_IMAGE_FILE_HEADER);
+ NumberOfSections = (UINTN)(Hdr.Pe32->FileHeader.NumberOfSections);
+
+ switch (Hdr.Pe32->OptionalHeader.Magic) {
+ case EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC:
+ SectionHeaderOffset += Hdr.Pe32->FileHeader.SizeOfOptionalHeader;
+ break;
+ case EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC:
+ SectionHeaderOffset += Hdr.Pe32Plus->FileHeader.SizeOfOptionalHeader;
+ break;
+ default:
+ ASSERT (FALSE);
+ }
+
+ //
+ // Iterate over the sections
+ //
+ for (Index = 0; Index < NumberOfSections; Index++) {
+ //
+ // Read section header from file
+ //
+ Size = sizeof (EFI_IMAGE_SECTION_HEADER);
+ ReadSize = Size;
+ Status = TmpContext.ImageRead (TmpContext.Handle, SectionHeaderOffset,
+ &Size, &SectionHeader);
+ if (RETURN_ERROR (Status) || (Size != ReadSize)) {
+ DEBUG ((DEBUG_ERROR,
+ "%a: TmpContext.ImageRead () failed (Status = %r)\n",
+ __FUNCTION__, Status));
+ return Status;
+ }
+
+ Base = TmpContext.ImageAddress + SectionHeader.VirtualAddress;
+
+ if ((SectionHeader.Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE) == 0) {
+
+ if ((SectionHeader.Characteristics & EFI_IMAGE_SCN_MEM_WRITE) == 0 &&
+ TmpContext.ImageType != EFI_IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER) {
+
+ DEBUG ((DEBUG_INFO,
+ "%a: Mapping section %d of image at 0x%lx with RO-XN permissions and size 0x%x\n",
+ __FUNCTION__, Index, Base, SectionHeader.Misc.VirtualSize));
+ ReadOnlyUpdater (Base, SectionHeader.Misc.VirtualSize);
+ } else {
+ DEBUG ((DEBUG_WARN,
+ "%a: Mapping section %d of image at 0x%lx with RW-XN permissions and size 0x%x\n",
+ __FUNCTION__, Index, Base, SectionHeader.Misc.VirtualSize));
+ }
+ } else {
+ DEBUG ((DEBUG_INFO,
+ "%a: Mapping section %d of image at 0x%lx with RO-XN permissions and size 0x%x\n",
+ __FUNCTION__, Index, Base, SectionHeader.Misc.VirtualSize));
+ ReadOnlyUpdater (Base, SectionHeader.Misc.VirtualSize);
+
+ DEBUG ((DEBUG_INFO,
+ "%a: Mapping section %d of image at 0x%lx with RO-X permissions and size 0x%x\n",
+ __FUNCTION__, Index, Base, SectionHeader.Misc.VirtualSize));
+ NoExecUpdater (Base, SectionHeader.Misc.VirtualSize);
+ }
+
+ SectionHeaderOffset += sizeof (EFI_IMAGE_SECTION_HEADER);
+ }
+ return RETURN_SUCCESS;
+}
/**
If the build is done on cygwin the paths are cygpaths.
@@ -83,6 +234,14 @@ PeCoffLoaderRelocateImageExtraAction (
CHAR8 Temp[512];
#endif
+ if (PcdGetBool(PcdStandaloneMmEnable) == TRUE) {
+ UpdatePeCoffPermissions (
+ ImageContext,
+ ArmClearMemoryRegionNoExec,
+ ArmSetMemoryRegionReadOnly
+ );
+ }
+
if (ImageContext->PdbPointer) {
#ifdef __CC_ARM
#if (__ARMCC_VERSION < 500000)
@@ -125,6 +284,14 @@ PeCoffLoaderUnloadImageExtraAction (
CHAR8 Temp[512];
#endif
+ if (PcdGetBool(PcdStandaloneMmEnable) == TRUE) {
+ UpdatePeCoffPermissions (
+ ImageContext,
+ ArmSetMemoryRegionNoExec,
+ ArmClearMemoryRegionReadOnly
+ );
+ }
+
if (ImageContext->PdbPointer) {
#ifdef __CC_ARM
// Print out the command for the RVD debugger to load symbols for this image
--
2.7.4
next prev parent reply other threads:[~2018-07-20 12:38 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-20 12:38 [PATCH v2 0/7] ArmPkg related changes for StandaloneMM package Sughosh Ganu
2018-07-20 12:38 ` [PATCH v2 1/7] ArmPkg: Add PCDs needed for MM communication driver Sughosh Ganu
2018-07-20 12:38 ` [PATCH v2 2/7] ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver Sughosh Ganu
2018-07-20 12:38 ` [PATCH v2 3/7] ArmPkg/Include: Fix the SPM version SVC ID Sughosh Ganu
2018-07-20 12:38 ` [PATCH v2 4/7] ArmPkg/Include: Add MM interface SVC return codes Sughosh Ganu
2018-07-20 12:38 ` [PATCH v2 5/7] ArmPkg/ArmMmuLib: Add MMU Library suitable for use in S-EL0 Sughosh Ganu
2018-07-20 12:38 ` [PATCH v2 6/7] ArmPkg/ArmMmuLib: Add MMU library inf file " Sughosh Ganu
2018-07-20 12:38 ` Sughosh Ganu [this message]
2018-07-21 11:06 ` [PATCH v2 7/7] ArmPkg: Extra action to update permissions for S-ELO MM Image Ard Biesheuvel
2018-07-23 17:33 ` Supreeth Venkatesh
[not found] ` <CAHxXkBfPzG5Z+oWmQeV_zKu7a0nOEW54jmPd1uJT_XkUZAmb-g@mail.gmail.com>
2018-08-21 6:50 ` Sughosh Ganu
2018-08-24 14:55 ` Ard Biesheuvel
2018-08-28 14:30 ` Achin Gupta
2018-10-24 8:22 ` Achin Gupta
2018-10-24 11:05 ` Ard Biesheuvel
2018-11-09 13:27 ` Achin Gupta
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1532090300-5250-8-git-send-email-sughosh.ganu@arm.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox