public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
To: edk2-devel@lists.01.org
Cc: michael.d.kinney@intel.com, liming.gao@intel.com,
	jiewen.yao@intel.com, achin.gupta@arm.com,
	leif.lindholm@linaro.org, ard.biesheuvel@linaro.org,
	Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Subject: [PATCH v1 15/18] ArmPkg: Extra action to update permissions for S-ELO MM Image.
Date: Fri,  6 Apr 2018 15:42:20 +0100	[thread overview]
Message-ID: <20180406144223.10931-16-supreeth.venkatesh@arm.com> (raw)
In-Reply-To: <20180406144223.10931-1-supreeth.venkatesh@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: Achin Gupta <achin.gupta@arm.com>
Signed-off-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
---
 .../DebugPeCoffExtraActionLib.c                    | 185 +++++++++++++++++++--
 .../DebugPeCoffExtraActionLib.inf                  |   7 +
 2 files changed, 181 insertions(+), 11 deletions(-)

diff --git a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.c
index f298e58cdf..c87aaf05c7 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,23 +234,29 @@ PeCoffLoaderRelocateImageExtraAction (
   CHAR8 Temp[512];
 #endif
 
+  if (PcdGetBool(PcdStandaloneMmEnable) == TRUE)
+  {
+     UpdatePeCoffPermissions (ImageContext, ArmClearMemoryRegionNoExec,
+                              ArmSetMemoryRegionReadOnly);
+  }
+
   if (ImageContext->PdbPointer) {
 #ifdef __CC_ARM
 #if (__ARMCC_VERSION < 500000)
     // Print out the command for the RVD debugger to load symbols for this image
-    DEBUG ((EFI_D_LOAD | EFI_D_INFO, "load /a /ni /np %a &0x%p\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
+    DEBUG ((DEBUG_LOAD | DEBUG_INFO, "load /a /ni /np %a &0x%p\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
 #else
     // Print out the command for the DS-5 to load symbols for this image
-    DEBUG ((EFI_D_LOAD | EFI_D_INFO, "add-symbol-file %a 0x%p\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
+    DEBUG ((DEBUG_LOAD | DEBUG_INFO, "add-symbol-file %a 0x%p\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
 #endif
 #elif __GNUC__
     // This may not work correctly if you generate PE/COFF directlyas then the Offset would not be required
-    DEBUG ((EFI_D_LOAD | EFI_D_INFO, "add-symbol-file %a 0x%p\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
+    DEBUG ((DEBUG_LOAD | DEBUG_INFO, "add-symbol-file %a 0x%p\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
 #else
-    DEBUG ((EFI_D_LOAD | EFI_D_INFO, "Loading driver at 0x%11p EntryPoint=0x%11p\n", (VOID *)(UINTN) ImageContext->ImageAddress, FUNCTION_ENTRY_POINT (ImageContext->EntryPoint)));
+    DEBUG ((DEBUG_LOAD | DEBUG_INFO, "Loading driver at 0x%11p EntryPoint=0x%11p\n", (VOID *)(UINTN) ImageContext->ImageAddress, FUNCTION_ENTRY_POINT (ImageContext->EntryPoint)));
 #endif
   } else {
-    DEBUG ((EFI_D_LOAD | EFI_D_INFO, "Loading driver at 0x%11p EntryPoint=0x%11p\n", (VOID *)(UINTN) ImageContext->ImageAddress, FUNCTION_ENTRY_POINT (ImageContext->EntryPoint)));
+    DEBUG ((DEBUG_LOAD | DEBUG_INFO, "Loading driver at 0x%11p EntryPoint=0x%11p\n", (VOID *)(UINTN) ImageContext->ImageAddress, FUNCTION_ENTRY_POINT (ImageContext->EntryPoint)));
   }
 }
 
@@ -125,17 +282,23 @@ 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
-    DEBUG ((EFI_D_ERROR, "unload symbols_only %a\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp))));
+    DEBUG ((DEBUG_ERROR, "unload symbols_only %a\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp))));
 #elif __GNUC__
     // This may not work correctly if you generate PE/COFF directlyas then the Offset would not be required
-    DEBUG ((EFI_D_ERROR, "remove-symbol-file %a 0x%08x\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
+    DEBUG ((DEBUG_ERROR, "remove-symbol-file %a 0x%08x\n", DeCygwinPathIfNeeded (ImageContext->PdbPointer, Temp, sizeof (Temp)), (UINTN)(ImageContext->ImageAddress + ImageContext->SizeOfHeaders)));
 #else
-    DEBUG ((EFI_D_ERROR, "Unloading %a\n", ImageContext->PdbPointer));
+    DEBUG ((DEBUG_ERROR, "Unloading %a\n", ImageContext->PdbPointer));
 #endif
   } else {
-    DEBUG ((EFI_D_ERROR, "Unloading driver at 0x%11p\n", (VOID *)(UINTN) ImageContext->ImageAddress));
+    DEBUG ((DEBUG_ERROR, "Unloading driver at 0x%11p\n", (VOID *)(UINTN) ImageContext->ImageAddress));
   }
 }
diff --git a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
index c1f717e5bd..38bf3993ae 100644
--- a/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
+++ b/ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
@@ -33,7 +33,14 @@
   DebugPeCoffExtraActionLib.c
 
 [Packages]
+  ArmPkg/ArmPkg.dec
   MdePkg/MdePkg.dec
+  StandaloneMmPkg/StandaloneMmPkg.dec
+
+[FeaturePcd]
+  gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable
 
 [LibraryClasses]
+  ArmMmuLib
   DebugLib
+  PcdLib
-- 
2.16.2



  parent reply	other threads:[~2018-04-06 14:43 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-06 14:42 [PATCH v1 00/18] *** Standalone Management Mode Core Interface for AARCH64 Platforms *** Supreeth Venkatesh
2018-04-06 14:42 ` [PATCH v1 01/18] ArmPkg: Add PCDs needed for MM communication driver Supreeth Venkatesh
2018-04-11 14:43   ` Achin Gupta
     [not found]     ` <AM4PR0802MB23063743A3B2F5A552BE320580870@AM4PR0802MB2306.eurprd08.prod.outlook.com>
2018-05-04 23:13       ` Supreeth Venkatesh
2018-05-04 23:17     ` Supreeth Venkatesh
2018-04-06 14:42 ` [PATCH v1 02/18] ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver Supreeth Venkatesh
2018-04-11 14:00   ` Achin Gupta
2018-05-04 23:18     ` Supreeth Venkatesh
2018-04-06 14:42 ` [PATCH v1 03/18] ArmPkg/Include: Add MM interface SVC return codes Supreeth Venkatesh
2018-04-11 14:38   ` Achin Gupta
2018-05-04 23:19     ` Supreeth Venkatesh
2018-04-06 14:42 ` [PATCH v1 04/18] ArmPkg/ArmMmuLib: Add MMU Library suitable for use in S-EL0 Supreeth Venkatesh
2018-04-11 19:21   ` Achin Gupta
2018-05-04 23:19     ` Supreeth Venkatesh
2018-04-06 14:42 ` [PATCH v1 05/18] ArmPkg/ArmMmuLib: Add MMU library inf file " Supreeth Venkatesh
2018-04-11 19:24   ` Achin Gupta
2018-05-04 23:19     ` Supreeth Venkatesh
2018-04-06 14:42 ` [PATCH v1 06/18] StandaloneMmPkg: Add an AArch64 specific entry point library Supreeth Venkatesh
2018-04-16 14:04   ` Achin Gupta
2018-05-04 23:20     ` Supreeth Venkatesh
2018-04-06 14:42 ` [PATCH v1 07/18] StandaloneMmPkg/FvLib: Add a common FV Library for management mode Supreeth Venkatesh
2018-04-16 14:44   ` Achin Gupta
2018-05-04 23:21     ` Supreeth Venkatesh
2018-04-06 14:42 ` [PATCH v1 08/18] StandaloneMmPkg/MemLib: AARCH64 Specific instance of memory check library Supreeth Venkatesh
2018-04-16 15:12   ` Achin Gupta
2018-04-16 22:30     ` Yao, Jiewen
2018-04-25 10:35       ` Achin Gupta
2018-04-26 13:02         ` Yao, Jiewen
2018-05-04 23:21     ` Supreeth Venkatesh
2018-04-06 14:42 ` [PATCH v1 09/18] StandaloneMmPkg/MemoryAllocationLib: Add MM memory allocation library Supreeth Venkatesh
2018-04-25 14:33   ` Achin Gupta
2018-04-26 13:05     ` Yao, Jiewen
2018-05-04 23:23       ` Supreeth Venkatesh
2018-05-04 23:21     ` Supreeth Venkatesh
2018-04-06 14:42 ` [PATCH v1 10/18] StandaloneMmPkg/HobLib: Add AARCH64 Specific HOB Library for management mode Supreeth Venkatesh
2018-04-25 14:50   ` Achin Gupta
2018-04-26 13:04     ` Yao, Jiewen
2018-05-04 23:22       ` Supreeth Venkatesh
2018-05-04 23:25     ` Supreeth Venkatesh
2018-04-06 14:42 ` [PATCH v1 11/18] StandaloneMmPkg: MM driver entry point library Supreeth Venkatesh
2018-04-30 14:29   ` Achin Gupta
2018-05-04 23:24     ` Supreeth Venkatesh
2018-04-06 14:42 ` [PATCH v1 12/18] StandaloneMmPkg/CpuMm: Add CPU driver suitable for ARM Platforms Supreeth Venkatesh
2018-04-18 22:09   ` Daniil Egranov
2018-05-04 23:25     ` Supreeth Venkatesh
2018-04-30 15:50   ` Achin Gupta
2018-05-04 23:24     ` Supreeth Venkatesh
2018-04-06 14:42 ` [PATCH v1 13/18] StandaloneMmPkg/Core: Implementation of Standalone MM Core Module Supreeth Venkatesh
2018-04-30 19:19   ` Achin Gupta
2018-04-30 19:28     ` Ard Biesheuvel
2018-04-30 20:17       ` Achin Gupta
2018-05-01  8:18       ` Laszlo Ersek
2018-05-04 23:28     ` Supreeth Venkatesh
2018-04-06 14:42 ` [PATCH v1 14/18] StandaloneMmPkg: Describe the declaration, definition and fdf files Supreeth Venkatesh
2018-04-18 19:50   ` Daniil Egranov
2018-05-04 23:29     ` Supreeth Venkatesh
2018-04-30 19:32   ` Achin Gupta
2018-05-04 23:28     ` Supreeth Venkatesh
2018-04-06 14:42 ` Supreeth Venkatesh [this message]
2018-04-30 19:49   ` [PATCH v1 15/18] ArmPkg: Extra action to update permissions for S-ELO MM Image Achin Gupta
2018-05-04 23:30     ` Supreeth Venkatesh
2018-04-06 14:42 ` [PATCH v1 16/18] BaseTools/AutoGen: Update header file for MM modules Supreeth Venkatesh
2018-04-30 19:52   ` Achin Gupta
2018-05-04 23:30     ` Supreeth Venkatesh
2018-04-06 14:42 ` [PATCH v1 17/18] StandaloneMmPkg: Add application to test MM communication protocol Supreeth Venkatesh
2018-04-30 20:02   ` Achin Gupta
2018-05-04 23:31     ` Supreeth Venkatesh
2018-04-06 14:42 ` [PATCH v1 18/18] StandaloneMmPkg: Add handler to handle event received from Normal World Supreeth Venkatesh
2018-04-08  6:01 ` [PATCH v1 00/18] *** Standalone Management Mode Core Interface for AARCH64 Platforms *** Yao, Jiewen
2018-05-04 23:15   ` Supreeth Venkatesh

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=20180406144223.10931-16-supreeth.venkatesh@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