* [PATCH V3 1/4] UefiCpuPkg/CpuDxe: Add memory attribute setting.
2017-02-09 7:20 [PATCH V3 0/4] DXE Memory Protection Jiewen Yao
@ 2017-02-09 7:20 ` Jiewen Yao
2017-02-09 7:20 ` [PATCH V3 2/4] ArmPkg/CpuDxe: Correct EFI_MEMORY_RO usage Jiewen Yao
` (4 subsequent siblings)
5 siblings, 0 replies; 27+ messages in thread
From: Jiewen Yao @ 2017-02-09 7:20 UTC (permalink / raw)
To: edk2-devel; +Cc: Jeff Fan, Michael Kinney
Add memory attribute setting in CpuArch protocol.
Previous SetMemoryAttributes() API only supports cache attribute setting.
This patch updated SetMemoryAttributes() API to support memory attribute
setting by updating CPU page table.
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
UefiCpuPkg/CpuDxe/CpuDxe.c | 141 ++--
UefiCpuPkg/CpuDxe/CpuDxe.inf | 5 +-
UefiCpuPkg/CpuDxe/CpuPageTable.c | 779 ++++++++++++++++++++
UefiCpuPkg/CpuDxe/CpuPageTable.h | 113 +++
4 files changed, 977 insertions(+), 61 deletions(-)
diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
index f6d0a67..3f3ddad 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.c
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
@@ -1,7 +1,7 @@
/** @file
CPU DXE Module to produce CPU ARCH Protocol.
- Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -14,6 +14,10 @@
#include "CpuDxe.h"
#include "CpuMp.h"
+#include "CpuPageTable.h"
+
+#define CACHE_ATTRIBUTE_MASK (EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE | EFI_MEMORY_WP)
+#define MEMORY_ATTRIBUTE_MASK (EFI_MEMORY_RP | EFI_MEMORY_XP | EFI_MEMORY_RO)
//
// Global Variables
@@ -368,10 +372,9 @@ CpuSetMemoryAttributes (
EFI_STATUS MpStatus;
EFI_MP_SERVICES_PROTOCOL *MpService;
MTRR_SETTINGS MtrrSettings;
-
- if (!IsMtrrSupported ()) {
- return EFI_UNSUPPORTED;
- }
+ UINT64 CacheAttributes;
+ UINT64 MemoryAttributes;
+ MTRR_MEMORY_CACHE_TYPE CurrentCacheType;
//
// If this function is called because GCD SetMemorySpaceAttributes () is called
@@ -384,69 +387,87 @@ CpuSetMemoryAttributes (
return EFI_SUCCESS;
}
- switch (Attributes) {
- case EFI_MEMORY_UC:
- CacheType = CacheUncacheable;
- break;
- case EFI_MEMORY_WC:
- CacheType = CacheWriteCombining;
- break;
+ CacheAttributes = Attributes & CACHE_ATTRIBUTE_MASK;
+ MemoryAttributes = Attributes & MEMORY_ATTRIBUTE_MASK;
- case EFI_MEMORY_WT:
- CacheType = CacheWriteThrough;
- break;
+ if (Attributes != (CacheAttributes | MemoryAttributes)) {
+ return EFI_INVALID_PARAMETER;
+ }
- case EFI_MEMORY_WP:
- CacheType = CacheWriteProtected;
- break;
+ if (CacheAttributes != 0) {
+ if (!IsMtrrSupported ()) {
+ return EFI_UNSUPPORTED;
+ }
- case EFI_MEMORY_WB:
- CacheType = CacheWriteBack;
- break;
+ switch (CacheAttributes) {
+ case EFI_MEMORY_UC:
+ CacheType = CacheUncacheable;
+ break;
- case EFI_MEMORY_UCE:
- case EFI_MEMORY_RP:
- case EFI_MEMORY_XP:
- case EFI_MEMORY_RUNTIME:
- return EFI_UNSUPPORTED;
+ case EFI_MEMORY_WC:
+ CacheType = CacheWriteCombining;
+ break;
- default:
- return EFI_INVALID_PARAMETER;
- }
- //
- // call MTRR libary function
- //
- Status = MtrrSetMemoryAttribute (
- BaseAddress,
- Length,
- CacheType
- );
+ case EFI_MEMORY_WT:
+ CacheType = CacheWriteThrough;
+ break;
- if (!RETURN_ERROR (Status)) {
- MpStatus = gBS->LocateProtocol (
- &gEfiMpServiceProtocolGuid,
- NULL,
- (VOID **)&MpService
- );
- //
- // Synchronize the update with all APs
- //
- if (!EFI_ERROR (MpStatus)) {
- MtrrGetAllMtrrs (&MtrrSettings);
- MpStatus = MpService->StartupAllAPs (
- MpService, // This
- SetMtrrsFromBuffer, // Procedure
- FALSE, // SingleThread
- NULL, // WaitEvent
- 0, // TimeoutInMicrosecsond
- &MtrrSettings, // ProcedureArgument
- NULL // FailedCpuList
- );
- ASSERT (MpStatus == EFI_SUCCESS || MpStatus == EFI_NOT_STARTED);
+ case EFI_MEMORY_WP:
+ CacheType = CacheWriteProtected;
+ break;
+
+ case EFI_MEMORY_WB:
+ CacheType = CacheWriteBack;
+ break;
+
+ default:
+ return EFI_INVALID_PARAMETER;
+ }
+ CurrentCacheType = MtrrGetMemoryAttribute(BaseAddress);
+ if (CurrentCacheType != CacheType) {
+ //
+ // call MTRR libary function
+ //
+ Status = MtrrSetMemoryAttribute (
+ BaseAddress,
+ Length,
+ CacheType
+ );
+
+ if (!RETURN_ERROR (Status)) {
+ MpStatus = gBS->LocateProtocol (
+ &gEfiMpServiceProtocolGuid,
+ NULL,
+ (VOID **)&MpService
+ );
+ //
+ // Synchronize the update with all APs
+ //
+ if (!EFI_ERROR (MpStatus)) {
+ MtrrGetAllMtrrs (&MtrrSettings);
+ MpStatus = MpService->StartupAllAPs (
+ MpService, // This
+ SetMtrrsFromBuffer, // Procedure
+ FALSE, // SingleThread
+ NULL, // WaitEvent
+ 0, // TimeoutInMicrosecsond
+ &MtrrSettings, // ProcedureArgument
+ NULL // FailedCpuList
+ );
+ ASSERT (MpStatus == EFI_SUCCESS || MpStatus == EFI_NOT_STARTED);
+ }
+ }
+ if (EFI_ERROR(Status)) {
+ return Status;
+ }
}
}
- return (EFI_STATUS) Status;
+
+ //
+ // Set memory attribute by page table
+ //
+ return AssignMemoryPageAttributes (NULL, BaseAddress, Length, MemoryAttributes, AllocatePages);
}
/**
@@ -888,6 +909,8 @@ InitializeCpu (
{
EFI_STATUS Status;
EFI_EVENT IdleLoopEvent;
+
+ InitializePageTableLib();
InitializeFloatingPointUnits ();
diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.inf b/UefiCpuPkg/CpuDxe/CpuDxe.inf
index bf389bb..f61b2c9 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.inf
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.inf
@@ -1,7 +1,7 @@
## @file
# CPU driver installs CPU Architecture Protocol and CPU MP protocol.
#
-# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR>
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
# which accompanies this distribution. The full text of the license may be found at
@@ -19,7 +19,6 @@
FILE_GUID = 1A1E4886-9517-440e-9FDE-3BE44CEE2136
MODULE_TYPE = DXE_DRIVER
VERSION_STRING = 1.0
-
ENTRY_POINT = InitializeCpu
[Packages]
@@ -52,6 +51,8 @@
CpuGdt.h
CpuMp.c
CpuMp.h
+ CpuPageTable.h
+ CpuPageTable.c
[Sources.IA32]
Ia32/CpuAsm.asm
diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c
new file mode 100644
index 0000000..c9c6fcb
--- /dev/null
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
@@ -0,0 +1,779 @@
+/** @file
+ Page table management support.
+
+ Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Base.h>
+#include <Uefi.h>
+#include <Library/BaseLib.h>
+#include <Library/CpuLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/DebugLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/MpService.h>
+#include "CpuPageTable.h"
+
+///
+/// Page Table Entry
+///
+#define IA32_PG_P BIT0
+#define IA32_PG_RW BIT1
+#define IA32_PG_U BIT2
+#define IA32_PG_WT BIT3
+#define IA32_PG_CD BIT4
+#define IA32_PG_A BIT5
+#define IA32_PG_D BIT6
+#define IA32_PG_PS BIT7
+#define IA32_PG_PAT_2M BIT12
+#define IA32_PG_PAT_4K IA32_PG_PS
+#define IA32_PG_PMNT BIT62
+#define IA32_PG_NX BIT63
+
+#define PAGE_ATTRIBUTE_BITS (IA32_PG_D | IA32_PG_A | IA32_PG_U | IA32_PG_RW | IA32_PG_P)
+//
+// Bits 1, 2, 5, 6 are reserved in the IA32 PAE PDPTE
+// X64 PAE PDPTE does not have such restriction
+//
+#define IA32_PAE_PDPTE_ATTRIBUTE_BITS (IA32_PG_P)
+
+#define PAGE_PROGATE_BITS (IA32_PG_NX | PAGE_ATTRIBUTE_BITS)
+
+#define PAGING_4K_MASK 0xFFF
+#define PAGING_2M_MASK 0x1FFFFF
+#define PAGING_1G_MASK 0x3FFFFFFF
+
+#define PAGING_PAE_INDEX_MASK 0x1FF
+
+#define PAGING_4K_ADDRESS_MASK_64 0x000FFFFFFFFFF000ull
+#define PAGING_2M_ADDRESS_MASK_64 0x000FFFFFFFE00000ull
+#define PAGING_1G_ADDRESS_MASK_64 0x000FFFFFC0000000ull
+
+typedef enum {
+ PageNone,
+ Page4K,
+ Page2M,
+ Page1G,
+} PAGE_ATTRIBUTE;
+
+typedef struct {
+ PAGE_ATTRIBUTE Attribute;
+ UINT64 Length;
+ UINT64 AddressMask;
+} PAGE_ATTRIBUTE_TABLE;
+
+typedef enum {
+ PageActionAssign,
+ PageActionSet,
+ PageActionClear,
+} PAGE_ACTION;
+
+PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = {
+ {Page4K, SIZE_4KB, PAGING_4K_ADDRESS_MASK_64},
+ {Page2M, SIZE_2MB, PAGING_2M_ADDRESS_MASK_64},
+ {Page1G, SIZE_1GB, PAGING_1G_ADDRESS_MASK_64},
+};
+
+/**
+ Enable write protection function for AP.
+
+ @param[in,out] Buffer The pointer to private data buffer.
+**/
+VOID
+EFIAPI
+SyncCpuEnableWriteProtection (
+ IN OUT VOID *Buffer
+ )
+{
+ AsmWriteCr0 (AsmReadCr0 () | BIT16);
+}
+
+/**
+ CpuFlushTlb function for AP.
+
+ @param[in,out] Buffer The pointer to private data buffer.
+**/
+VOID
+EFIAPI
+SyncCpuFlushTlb (
+ IN OUT VOID *Buffer
+ )
+{
+ CpuFlushTlb();
+}
+
+/**
+ Sync memory page attributes for AP.
+
+ @param[in] Procedure A pointer to the function to be run on enabled APs of
+ the system.
+**/
+VOID
+SyncMemoryPageAttributesAp (
+ IN EFI_AP_PROCEDURE Procedure
+ )
+{
+ EFI_STATUS Status;
+ EFI_MP_SERVICES_PROTOCOL *MpService;
+
+ Status = gBS->LocateProtocol (
+ &gEfiMpServiceProtocolGuid,
+ NULL,
+ (VOID **)&MpService
+ );
+ //
+ // Synchronize the update with all APs
+ //
+ if (!EFI_ERROR (Status)) {
+ Status = MpService->StartupAllAPs (
+ MpService, // This
+ Procedure, // Procedure
+ FALSE, // SingleThread
+ NULL, // WaitEvent
+ 0, // TimeoutInMicrosecsond
+ NULL, // ProcedureArgument
+ NULL // FailedCpuList
+ );
+ ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_STARTED);
+ }
+}
+
+/**
+ Return current paging context.
+
+ @param[in,out] PagingContext The paging context.
+**/
+VOID
+GetCurrentPagingContext (
+ IN OUT PAGE_TABLE_LIB_PAGING_CONTEXT *PagingContext
+ )
+{
+ UINT32 RegEax;
+ UINT32 RegEdx;
+
+ ZeroMem(PagingContext, sizeof(*PagingContext));
+ if (sizeof(UINTN) == sizeof(UINT64)) {
+ PagingContext->MachineType = IMAGE_FILE_MACHINE_X64;
+ } else {
+ PagingContext->MachineType = IMAGE_FILE_MACHINE_I386;
+ }
+ if ((AsmReadCr0 () & BIT31) != 0) {
+ PagingContext->ContextData.X64.PageTableBase = (AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64);
+ if ((AsmReadCr0 () & BIT16) == 0) {
+ AsmWriteCr0 (AsmReadCr0 () | BIT16);
+ SyncMemoryPageAttributesAp (SyncCpuEnableWriteProtection);
+ }
+ } else {
+ PagingContext->ContextData.X64.PageTableBase = 0;
+ }
+
+ if ((AsmReadCr4 () & BIT4) != 0) {
+ PagingContext->ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PSE;
+ }
+ if ((AsmReadCr4 () & BIT5) != 0) {
+ PagingContext->ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PAE;
+ }
+ if ((AsmReadCr0 () & BIT16) != 0) {
+ PagingContext->ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_WP_ENABLE;
+ }
+
+ AsmCpuid (0x80000000, &RegEax, NULL, NULL, NULL);
+ if (RegEax > 0x80000000) {
+ AsmCpuid (0x80000001, NULL, NULL, NULL, &RegEdx);
+ if ((RegEdx & BIT20) != 0) {
+ // XD supported
+ if ((AsmReadMsr64 (0x000001A0) & BIT34) == 0) {
+ // XD enabled
+ if ((AsmReadMsr64 (0xC0000080) & BIT11) != 0) {
+ // XD activated
+ PagingContext->ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_XD_ACTIVATED;
+ }
+ }
+ }
+ if ((RegEdx & BIT26) != 0) {
+ PagingContext->ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PAGE_1G_SUPPORT;
+ }
+ }
+}
+
+/**
+ Return length according to page attributes.
+
+ @param[in] PageAttributes The page attribute of the page entry.
+
+ @return The length of page entry.
+**/
+UINTN
+PageAttributeToLength (
+ IN PAGE_ATTRIBUTE PageAttribute
+ )
+{
+ UINTN Index;
+ for (Index = 0; Index < sizeof(mPageAttributeTable)/sizeof(mPageAttributeTable[0]); Index++) {
+ if (PageAttribute == mPageAttributeTable[Index].Attribute) {
+ return (UINTN)mPageAttributeTable[Index].Length;
+ }
+ }
+ return 0;
+}
+
+/**
+ Return address mask according to page attributes.
+
+ @param[in] PageAttributes The page attribute of the page entry.
+
+ @return The address mask of page entry.
+**/
+UINTN
+PageAttributeToMask (
+ IN PAGE_ATTRIBUTE PageAttribute
+ )
+{
+ UINTN Index;
+ for (Index = 0; Index < sizeof(mPageAttributeTable)/sizeof(mPageAttributeTable[0]); Index++) {
+ if (PageAttribute == mPageAttributeTable[Index].Attribute) {
+ return (UINTN)mPageAttributeTable[Index].AddressMask;
+ }
+ }
+ return 0;
+}
+
+/**
+ Return page table entry to match the address.
+
+ @param[in] PagingContext The paging context.
+ @param[in] Address The address to be checked.
+ @param[out] PageAttributes The page attribute of the page entry.
+
+ @return The page entry.
+**/
+VOID *
+GetPageTableEntry (
+ IN PAGE_TABLE_LIB_PAGING_CONTEXT *PagingContext,
+ IN PHYSICAL_ADDRESS Address,
+ OUT PAGE_ATTRIBUTE *PageAttribute
+ )
+{
+ UINTN Index1;
+ UINTN Index2;
+ UINTN Index3;
+ UINTN Index4;
+ UINT64 *L1PageTable;
+ UINT64 *L2PageTable;
+ UINT64 *L3PageTable;
+ UINT64 *L4PageTable;
+
+ ASSERT (PagingContext != NULL);
+
+ Index4 = ((UINTN)RShiftU64 (Address, 39)) & PAGING_PAE_INDEX_MASK;
+ Index3 = ((UINTN)Address >> 30) & PAGING_PAE_INDEX_MASK;
+ Index2 = ((UINTN)Address >> 21) & PAGING_PAE_INDEX_MASK;
+ Index1 = ((UINTN)Address >> 12) & PAGING_PAE_INDEX_MASK;
+
+ if (PagingContext->MachineType == IMAGE_FILE_MACHINE_X64) {
+ L4PageTable = (UINT64 *)(UINTN)PagingContext->ContextData.X64.PageTableBase;
+ if (L4PageTable[Index4] == 0) {
+ *PageAttribute = PageNone;
+ return NULL;
+ }
+
+ L3PageTable = (UINT64 *)(UINTN)(L4PageTable[Index4] & PAGING_4K_ADDRESS_MASK_64);
+ } else {
+ ASSERT((PagingContext->ContextData.Ia32.Attributes & PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PAE) != 0);
+ L3PageTable = (UINT64 *)(UINTN)PagingContext->ContextData.Ia32.PageTableBase;
+ }
+ if (L3PageTable[Index3] == 0) {
+ *PageAttribute = PageNone;
+ return NULL;
+ }
+ if ((L3PageTable[Index3] & IA32_PG_PS) != 0) {
+ // 1G
+ *PageAttribute = Page1G;
+ return &L3PageTable[Index3];
+ }
+
+ L2PageTable = (UINT64 *)(UINTN)(L3PageTable[Index3] & PAGING_4K_ADDRESS_MASK_64);
+ if (L2PageTable[Index2] == 0) {
+ *PageAttribute = PageNone;
+ return NULL;
+ }
+ if ((L2PageTable[Index2] & IA32_PG_PS) != 0) {
+ // 2M
+ *PageAttribute = Page2M;
+ return &L2PageTable[Index2];
+ }
+
+ // 4k
+ L1PageTable = (UINT64 *)(UINTN)(L2PageTable[Index2] & PAGING_4K_ADDRESS_MASK_64);
+ if ((L1PageTable[Index1] == 0) && (Address != 0)) {
+ *PageAttribute = PageNone;
+ return NULL;
+ }
+ *PageAttribute = Page4K;
+ return &L1PageTable[Index1];
+}
+
+/**
+ Return memory attributes of page entry.
+
+ @param[in] PageEntry The page entry.
+
+ @return Memory attributes of page entry.
+**/
+UINT64
+GetAttributesFromPageEntry (
+ IN UINT64 *PageEntry
+ )
+{
+ UINT64 Attributes;
+ Attributes = 0;
+ if ((*PageEntry & IA32_PG_P) == 0) {
+ Attributes |= EFI_MEMORY_RP;
+ }
+ if ((*PageEntry & IA32_PG_RW) == 0) {
+ Attributes |= EFI_MEMORY_RO;
+ }
+ if ((*PageEntry & IA32_PG_NX) != 0) {
+ Attributes |= EFI_MEMORY_XP;
+ }
+ return Attributes;
+}
+
+/**
+ Modify memory attributes of page entry.
+
+ @param[in] PagingContext The paging context.
+ @param[in] PageEntry The page entry.
+ @param[in] Attributes The bit mask of attributes to modify for the memory region.
+ @param[in] PageAction The page action.
+ @param[out] IsModified TRUE means page table modified. FALSE means page table not modified.
+**/
+VOID
+ConvertPageEntryAttribute (
+ IN PAGE_TABLE_LIB_PAGING_CONTEXT *PagingContext,
+ IN UINT64 *PageEntry,
+ IN UINT64 Attributes,
+ IN PAGE_ACTION PageAction,
+ OUT BOOLEAN *IsModified
+ )
+{
+ UINT64 CurrentPageEntry;
+ UINT64 NewPageEntry;
+
+ CurrentPageEntry = *PageEntry;
+ NewPageEntry = CurrentPageEntry;
+ if ((Attributes & EFI_MEMORY_RP) != 0) {
+ switch (PageAction) {
+ case PageActionAssign:
+ case PageActionSet:
+ NewPageEntry &= ~(UINT64)IA32_PG_P;
+ break;
+ case PageActionClear:
+ NewPageEntry |= IA32_PG_P;
+ break;
+ }
+ } else {
+ switch (PageAction) {
+ case PageActionAssign:
+ NewPageEntry |= IA32_PG_P;
+ break;
+ case PageActionSet:
+ case PageActionClear:
+ break;
+ }
+ }
+ if ((Attributes & EFI_MEMORY_RO) != 0) {
+ switch (PageAction) {
+ case PageActionAssign:
+ case PageActionSet:
+ NewPageEntry &= ~(UINT64)IA32_PG_RW;
+ break;
+ case PageActionClear:
+ NewPageEntry |= IA32_PG_RW;
+ break;
+ }
+ } else {
+ switch (PageAction) {
+ case PageActionAssign:
+ NewPageEntry |= IA32_PG_RW;
+ break;
+ case PageActionSet:
+ case PageActionClear:
+ break;
+ }
+ }
+ if ((PagingContext->ContextData.Ia32.Attributes & PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_XD_ACTIVATED) != 0) {
+ if ((Attributes & EFI_MEMORY_XP) != 0) {
+ switch (PageAction) {
+ case PageActionAssign:
+ case PageActionSet:
+ NewPageEntry |= IA32_PG_NX;
+ break;
+ case PageActionClear:
+ NewPageEntry &= ~IA32_PG_NX;
+ break;
+ }
+ } else {
+ switch (PageAction) {
+ case PageActionAssign:
+ NewPageEntry &= ~IA32_PG_NX;
+ break;
+ case PageActionSet:
+ case PageActionClear:
+ break;
+ }
+ }
+ }
+ *PageEntry = NewPageEntry;
+ if (CurrentPageEntry != NewPageEntry) {
+ *IsModified = TRUE;
+ DEBUG ((DEBUG_INFO, "ConvertPageEntryAttribute 0x%lx", CurrentPageEntry));
+ DEBUG ((DEBUG_INFO, "->0x%lx\n", NewPageEntry));
+ } else {
+ *IsModified = FALSE;
+ }
+}
+
+/**
+ This function returns if there is need to split page entry.
+
+ @param[in] BaseAddress The base address to be checked.
+ @param[in] Length The length to be checked.
+ @param[in] PageEntry The page entry to be checked.
+ @param[in] PageAttribute The page attribute of the page entry.
+
+ @retval SplitAttributes on if there is need to split page entry.
+**/
+PAGE_ATTRIBUTE
+NeedSplitPage (
+ IN PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 *PageEntry,
+ IN PAGE_ATTRIBUTE PageAttribute
+ )
+{
+ UINT64 PageEntryLength;
+
+ PageEntryLength = PageAttributeToLength (PageAttribute);
+
+ if (((BaseAddress & (PageEntryLength - 1)) == 0) && (Length >= PageEntryLength)) {
+ return PageNone;
+ }
+
+ if (((BaseAddress & PAGING_2M_MASK) != 0) || (Length < SIZE_2MB)) {
+ return Page4K;
+ }
+
+ return Page2M;
+}
+
+/**
+ This function splits one page entry to small page entries.
+
+ @param[in] PageEntry The page entry to be splitted.
+ @param[in] PageAttribute The page attribute of the page entry.
+ @param[in] SplitAttribute How to split the page entry.
+ @param[in] AllocatePagesFunc If page split is needed, this function is used to allocate more pages.
+
+ @retval RETURN_SUCCESS The page entry is splitted.
+ @retval RETURN_UNSUPPORTED The page entry does not support to be splitted.
+ @retval RETURN_OUT_OF_RESOURCES No resource to split page entry.
+**/
+RETURN_STATUS
+SplitPage (
+ IN UINT64 *PageEntry,
+ IN PAGE_ATTRIBUTE PageAttribute,
+ IN PAGE_ATTRIBUTE SplitAttribute,
+ IN PAGE_TABLE_LIB_ALLOCATE_PAGES AllocatePagesFunc
+ )
+{
+ UINT64 BaseAddress;
+ UINT64 *NewPageEntry;
+ UINTN Index;
+
+ ASSERT (PageAttribute == Page2M || PageAttribute == Page1G);
+
+ ASSERT (AllocatePagesFunc != NULL);
+
+ if (PageAttribute == Page2M) {
+ //
+ // Split 2M to 4K
+ //
+ ASSERT (SplitAttribute == Page4K);
+ if (SplitAttribute == Page4K) {
+ NewPageEntry = AllocatePagesFunc (1);
+ DEBUG ((DEBUG_INFO, "Split - 0x%x\n", NewPageEntry));
+ if (NewPageEntry == NULL) {
+ return RETURN_OUT_OF_RESOURCES;
+ }
+ BaseAddress = *PageEntry & PAGING_2M_ADDRESS_MASK_64;
+ for (Index = 0; Index < SIZE_4KB / sizeof(UINT64); Index++) {
+ NewPageEntry[Index] = BaseAddress + SIZE_4KB * Index + ((*PageEntry) & PAGE_PROGATE_BITS);
+ }
+ (*PageEntry) = (UINT64)(UINTN)NewPageEntry + ((*PageEntry) & PAGE_PROGATE_BITS);
+ return RETURN_SUCCESS;
+ } else {
+ return RETURN_UNSUPPORTED;
+ }
+ } else if (PageAttribute == Page1G) {
+ //
+ // Split 1G to 2M
+ // No need support 1G->4K directly, we should use 1G->2M, then 2M->4K to get more compact page table.
+ //
+ ASSERT (SplitAttribute == Page2M || SplitAttribute == Page4K);
+ if ((SplitAttribute == Page2M || SplitAttribute == Page4K)) {
+ NewPageEntry = AllocatePagesFunc (1);
+ DEBUG ((DEBUG_INFO, "Split - 0x%x\n", NewPageEntry));
+ if (NewPageEntry == NULL) {
+ return RETURN_OUT_OF_RESOURCES;
+ }
+ BaseAddress = *PageEntry & PAGING_1G_ADDRESS_MASK_64;
+ for (Index = 0; Index < SIZE_4KB / sizeof(UINT64); Index++) {
+ NewPageEntry[Index] = BaseAddress + SIZE_2MB * Index + IA32_PG_PS + ((*PageEntry) & PAGE_PROGATE_BITS);
+ }
+ (*PageEntry) = (UINT64)(UINTN)NewPageEntry + ((*PageEntry) & PAGE_PROGATE_BITS);
+ return RETURN_SUCCESS;
+ } else {
+ return RETURN_UNSUPPORTED;
+ }
+ } else {
+ return RETURN_UNSUPPORTED;
+ }
+}
+
+/**
+ This function modifies the page attributes for the memory region specified by BaseAddress and
+ Length from their current attributes to the attributes specified by Attributes.
+
+ Caller should make sure BaseAddress and Length is at page boundary.
+
+ @param[in] PagingContext The paging context. NULL means get page table from current CPU context.
+ @param[in] BaseAddress The physical address that is the start address of a memory region.
+ @param[in] Length The size in bytes of the memory region.
+ @param[in] Attributes The bit mask of attributes to modify for the memory region.
+ @param[in] PageAction The page action.
+ @param[in] AllocatePagesFunc If page split is needed, this function is used to allocate more pages.
+ NULL mean page split is unsupported.
+ @param[out] IsSplitted TRUE means page table splitted. FALSE means page table not splitted.
+ @param[out] IsModified TRUE means page table modified. FALSE means page table not modified.
+
+ @retval RETURN_SUCCESS The attributes were modified for the memory region.
+ @retval RETURN_ACCESS_DENIED The attributes for the memory resource range specified by
+ BaseAddress and Length cannot be modified.
+ @retval RETURN_INVALID_PARAMETER Length is zero.
+ Attributes specified an illegal combination of attributes that
+ cannot be set together.
+ @retval RETURN_OUT_OF_RESOURCES There are not enough system resources to modify the attributes of
+ the memory resource range.
+ @retval RETURN_UNSUPPORTED The processor does not support one or more bytes of the memory
+ resource range specified by BaseAddress and Length.
+ The bit mask of attributes is not support for the memory resource
+ range specified by BaseAddress and Length.
+**/
+RETURN_STATUS
+ConvertMemoryPageAttributes (
+ IN PAGE_TABLE_LIB_PAGING_CONTEXT *PagingContext OPTIONAL,
+ IN PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes,
+ IN PAGE_ACTION PageAction,
+ IN PAGE_TABLE_LIB_ALLOCATE_PAGES AllocatePagesFunc OPTIONAL,
+ OUT BOOLEAN *IsSplitted, OPTIONAL
+ OUT BOOLEAN *IsModified OPTIONAL
+ )
+{
+ PAGE_TABLE_LIB_PAGING_CONTEXT CurrentPagingContext;
+ UINT64 *PageEntry;
+ PAGE_ATTRIBUTE PageAttribute;
+ UINTN PageEntryLength;
+ PAGE_ATTRIBUTE SplitAttribute;
+ RETURN_STATUS Status;
+ BOOLEAN IsEntryModified;
+
+ if ((BaseAddress & (SIZE_4KB - 1)) != 0) {
+ DEBUG ((DEBUG_ERROR, "BaseAddress(0x%lx) is not aligned!\n", BaseAddress));
+ return EFI_UNSUPPORTED;
+ }
+ if ((Length & (SIZE_4KB - 1)) != 0) {
+ DEBUG ((DEBUG_ERROR, "Length(0x%lx) is not aligned!\n", Length));
+ return EFI_UNSUPPORTED;
+ }
+ if (Length == 0) {
+ DEBUG ((DEBUG_ERROR, "Length is 0!\n"));
+ return RETURN_INVALID_PARAMETER;
+ }
+
+ if ((Attributes & ~(EFI_MEMORY_RP | EFI_MEMORY_RO | EFI_MEMORY_XP)) != 0) {
+ DEBUG ((DEBUG_ERROR, "Attributes(0x%lx) has unsupported bit\n", Attributes));
+ return EFI_UNSUPPORTED;
+ }
+
+ if (PagingContext == NULL) {
+ GetCurrentPagingContext (&CurrentPagingContext);
+ } else {
+ CopyMem (&CurrentPagingContext, PagingContext, sizeof(CurrentPagingContext));
+ }
+ switch(CurrentPagingContext.MachineType) {
+ case IMAGE_FILE_MACHINE_I386:
+ if (CurrentPagingContext.ContextData.Ia32.PageTableBase == 0) {
+ DEBUG ((DEBUG_ERROR, "PageTable is 0!\n"));
+ if (Attributes == 0) {
+ return EFI_SUCCESS;
+ } else {
+ return EFI_UNSUPPORTED;
+ }
+ }
+ if ((CurrentPagingContext.ContextData.Ia32.Attributes & PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PAE) == 0) {
+ DEBUG ((DEBUG_ERROR, "Non-PAE Paging!\n"));
+ return EFI_UNSUPPORTED;
+ }
+ break;
+ case IMAGE_FILE_MACHINE_X64:
+ ASSERT (CurrentPagingContext.ContextData.X64.PageTableBase != 0);
+ break;
+ default:
+ ASSERT(FALSE);
+ return EFI_UNSUPPORTED;
+ break;
+ }
+
+// DEBUG ((DEBUG_ERROR, "ConvertMemoryPageAttributes(%x) - %016lx, %016lx, %02lx\n", IsSet, BaseAddress, Length, Attributes));
+
+ if (IsSplitted != NULL) {
+ *IsSplitted = FALSE;
+ }
+ if (IsModified != NULL) {
+ *IsModified = FALSE;
+ }
+
+ //
+ // Below logic is to check 2M/4K page to make sure we donot waist memory.
+ //
+ while (Length != 0) {
+ PageEntry = GetPageTableEntry (&CurrentPagingContext, BaseAddress, &PageAttribute);
+ if (PageEntry == NULL) {
+ return RETURN_UNSUPPORTED;
+ }
+ PageEntryLength = PageAttributeToLength (PageAttribute);
+ SplitAttribute = NeedSplitPage (BaseAddress, Length, PageEntry, PageAttribute);
+ if (SplitAttribute == PageNone) {
+ ConvertPageEntryAttribute (&CurrentPagingContext, PageEntry, Attributes, PageAction, &IsEntryModified);
+ if (IsEntryModified) {
+ if (IsModified != NULL) {
+ *IsModified = TRUE;
+ }
+ }
+ //
+ // Convert success, move to next
+ //
+ BaseAddress += PageEntryLength;
+ Length -= PageEntryLength;
+ } else {
+ if (AllocatePagesFunc == NULL) {
+ return RETURN_UNSUPPORTED;
+ }
+ Status = SplitPage (PageEntry, PageAttribute, SplitAttribute, AllocatePagesFunc);
+ if (RETURN_ERROR (Status)) {
+ return RETURN_UNSUPPORTED;
+ }
+ if (IsSplitted != NULL) {
+ *IsSplitted = TRUE;
+ }
+ if (IsModified != NULL) {
+ *IsModified = TRUE;
+ }
+ //
+ // Just split current page
+ // Convert success in next around
+ //
+ }
+ }
+
+ return RETURN_SUCCESS;
+}
+
+/**
+ This function assigns the page attributes for the memory region specified by BaseAddress and
+ Length from their current attributes to the attributes specified by Attributes.
+
+ Caller should make sure BaseAddress and Length is at page boundary.
+
+ Caller need guarentee the TPL <= TPL_NOTIFY, if there is split page request.
+
+ @param[in] PagingContext The paging context. NULL means get page table from current CPU context.
+ @param[in] BaseAddress The physical address that is the start address of a memory region.
+ @param[in] Length The size in bytes of the memory region.
+ @param[in] Attributes The bit mask of attributes to set for the memory region.
+ @param[in] AllocatePagesFunc If page split is needed, this function is used to allocate more pages.
+ NULL mean page split is unsupported.
+
+ @retval RETURN_SUCCESS The attributes were cleared for the memory region.
+ @retval RETURN_ACCESS_DENIED The attributes for the memory resource range specified by
+ BaseAddress and Length cannot be modified.
+ @retval RETURN_INVALID_PARAMETER Length is zero.
+ Attributes specified an illegal combination of attributes that
+ cannot be set together.
+ @retval RETURN_OUT_OF_RESOURCES There are not enough system resources to modify the attributes of
+ the memory resource range.
+ @retval RETURN_UNSUPPORTED The processor does not support one or more bytes of the memory
+ resource range specified by BaseAddress and Length.
+ The bit mask of attributes is not support for the memory resource
+ range specified by BaseAddress and Length.
+**/
+RETURN_STATUS
+EFIAPI
+AssignMemoryPageAttributes (
+ IN PAGE_TABLE_LIB_PAGING_CONTEXT *PagingContext OPTIONAL,
+ IN PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes,
+ IN PAGE_TABLE_LIB_ALLOCATE_PAGES AllocatePagesFunc OPTIONAL
+ )
+{
+ RETURN_STATUS Status;
+ BOOLEAN IsModified;
+ BOOLEAN IsSplitted;
+
+// DEBUG((DEBUG_INFO, "AssignMemoryPageAttributes: 0x%lx - 0x%lx (0x%lx)\n", BaseAddress, Length, Attributes));
+ Status = ConvertMemoryPageAttributes (PagingContext, BaseAddress, Length, Attributes, PageActionAssign, AllocatePagesFunc, &IsSplitted, &IsModified);
+ if (!EFI_ERROR(Status)) {
+ if ((PagingContext == NULL) && IsModified) {
+ //
+ // Flush TLB as last step
+ //
+ CpuFlushTlb();
+ SyncMemoryPageAttributesAp (SyncCpuFlushTlb);
+ }
+ }
+
+ return Status;
+}
+
+/**
+ Initialize the Page Table lib.
+**/
+VOID
+InitializePageTableLib (
+ VOID
+ )
+{
+ PAGE_TABLE_LIB_PAGING_CONTEXT CurrentPagingContext;
+
+ GetCurrentPagingContext (&CurrentPagingContext);
+ DEBUG ((DEBUG_INFO, "CurrentPagingContext:\n", CurrentPagingContext.MachineType));
+ DEBUG ((DEBUG_INFO, " MachineType - 0x%x\n", CurrentPagingContext.MachineType));
+ DEBUG ((DEBUG_INFO, " PageTableBase - 0x%x\n", CurrentPagingContext.ContextData.X64.PageTableBase));
+ DEBUG ((DEBUG_INFO, " Attributes - 0x%x\n", CurrentPagingContext.ContextData.X64.Attributes));
+
+ return ;
+}
+
diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.h b/UefiCpuPkg/CpuDxe/CpuPageTable.h
new file mode 100644
index 0000000..eaff595
--- /dev/null
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.h
@@ -0,0 +1,113 @@
+/** @file
+ Page table management header file.
+
+ Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef _PAGE_TABLE_LIB_H_
+#define _PAGE_TABLE_LIB_H_
+
+#include <IndustryStandard/PeImage.h>
+
+#define PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PSE BIT0
+#define PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PAE BIT1
+#define PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PAGE_1G_SUPPORT BIT2
+#define PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_WP_ENABLE BIT30
+#define PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_XD_ACTIVATED BIT31
+// Other bits are reserved for future use
+typedef struct {
+ UINT32 PageTableBase;
+ UINT32 Reserved;
+ UINT32 Attributes;
+} PAGE_TABLE_LIB_PAGING_CONTEXT_IA32;
+
+typedef struct {
+ UINT64 PageTableBase;
+ UINT32 Attributes;
+} PAGE_TABLE_LIB_PAGING_CONTEXT_X64;
+
+typedef union {
+ PAGE_TABLE_LIB_PAGING_CONTEXT_IA32 Ia32;
+ PAGE_TABLE_LIB_PAGING_CONTEXT_X64 X64;
+} PAGE_TABLE_LIB_PAGING_CONTEXT_DATA;
+
+typedef struct {
+ //
+ // PE32+ Machine type for EFI images
+ //
+ // #define IMAGE_FILE_MACHINE_I386 0x014c
+ // #define IMAGE_FILE_MACHINE_X64 0x8664
+ //
+ UINT16 MachineType;
+ PAGE_TABLE_LIB_PAGING_CONTEXT_DATA ContextData;
+} PAGE_TABLE_LIB_PAGING_CONTEXT;
+
+/**
+ Allocates one or more 4KB pages for page table.
+
+ @param Pages The number of 4 KB pages to allocate.
+
+ @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+typedef
+VOID *
+(EFIAPI *PAGE_TABLE_LIB_ALLOCATE_PAGES) (
+ IN UINTN Pages
+ );
+
+/**
+ This function assigns the page attributes for the memory region specified by BaseAddress and
+ Length from their current attributes to the attributes specified by Attributes.
+
+ Caller should make sure BaseAddress and Length is at page boundary.
+
+ Caller need guarentee the TPL <= TPL_NOTIFY, if there is split page request.
+
+ @param PagingContext The paging context. NULL means get page table from current CPU context.
+ @param BaseAddress The physical address that is the start address of a memory region.
+ @param Length The size in bytes of the memory region.
+ @param Attributes The bit mask of attributes to set for the memory region.
+ @param AllocatePagesFunc If page split is needed, this function is used to allocate more pages.
+ NULL mean page split is unsupported.
+
+ @retval RETURN_SUCCESS The attributes were cleared for the memory region.
+ @retval RETURN_ACCESS_DENIED The attributes for the memory resource range specified by
+ BaseAddress and Length cannot be modified.
+ @retval RETURN_INVALID_PARAMETER Length is zero.
+ Attributes specified an illegal combination of attributes that
+ cannot be set together.
+ @retval RETURN_OUT_OF_RESOURCES There are not enough system resources to modify the attributes of
+ the memory resource range.
+ @retval RETURN_UNSUPPORTED The processor does not support one or more bytes of the memory
+ resource range specified by BaseAddress and Length.
+ The bit mask of attributes is not support for the memory resource
+ range specified by BaseAddress and Length.
+**/
+RETURN_STATUS
+EFIAPI
+AssignMemoryPageAttributes (
+ IN PAGE_TABLE_LIB_PAGING_CONTEXT *PagingContext OPTIONAL,
+ IN PHYSICAL_ADDRESS BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes,
+ IN PAGE_TABLE_LIB_ALLOCATE_PAGES AllocatePagesFunc OPTIONAL
+ );
+
+/**
+ Initialize the Page Table lib.
+**/
+VOID
+InitializePageTableLib (
+ VOID
+ );
+
+#endif
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH V3 2/4] ArmPkg/CpuDxe: Correct EFI_MEMORY_RO usage
2017-02-09 7:20 [PATCH V3 0/4] DXE Memory Protection Jiewen Yao
2017-02-09 7:20 ` [PATCH V3 1/4] UefiCpuPkg/CpuDxe: Add memory attribute setting Jiewen Yao
@ 2017-02-09 7:20 ` Jiewen Yao
2017-02-09 7:20 ` [PATCH V3 3/4] MdeModulePkg/dec: add PcdImageProtectionPolicy Jiewen Yao
` (3 subsequent siblings)
5 siblings, 0 replies; 27+ messages in thread
From: Jiewen Yao @ 2017-02-09 7:20 UTC (permalink / raw)
To: edk2-devel; +Cc: Leif Lindholm, Ard Biesheuvel
Current Arm CpuDxe driver uses EFI_MEMORY_WP for write protection,
according to UEFI spec, we should use EFI_MEMORY_RO for write protection.
The EFI_MEMORY_WP is the cache attribute instead of memory attribute.
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 3 ++-
ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 14 ++++++--------
ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c | 5 +++--
ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 3 ++-
4 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
index d8bb419..15d5a81 100644
--- a/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c
@@ -3,6 +3,7 @@
Copyright (c) 2009, Hewlett-Packard Company. All rights reserved.<BR>
Portions copyright (c) 2010, Apple Inc. All rights reserved.<BR>
Portions copyright (c) 2011-2013, ARM Ltd. All rights reserved.<BR>
+Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -224,7 +225,7 @@ EfiAttributeToArmAttribute (
ArmAttributes |= TT_AF;
// Determine protection attributes
- if (EfiAttributes & EFI_MEMORY_WP) {
+ if (EfiAttributes & EFI_MEMORY_RO) {
ArmAttributes |= TT_AP_RO_RO;
}
diff --git a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
index 14fc22d..6dcfba6 100644
--- a/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
+++ b/ArmPkg/Drivers/CpuDxe/Arm/Mmu.c
@@ -3,6 +3,7 @@
Copyright (c) 2009, Hewlett-Packard Company. All rights reserved.<BR>
Portions copyright (c) 2010, Apple Inc. All rights reserved.<BR>
Portions copyright (c) 2013, ARM Ltd. All rights reserved.<BR>
+Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -62,7 +63,7 @@ SectionToGcdAttributes (
// determine protection attributes
switch(SectionAttributes & TT_DESCRIPTOR_SECTION_AP_MASK) {
case TT_DESCRIPTOR_SECTION_AP_NO_NO: // no read, no write
- //*GcdAttributes |= EFI_MEMORY_WP | EFI_MEMORY_RP;
+ //*GcdAttributes |= EFI_MEMORY_RO | EFI_MEMORY_RP;
break;
case TT_DESCRIPTOR_SECTION_AP_RW_NO:
@@ -73,7 +74,7 @@ SectionToGcdAttributes (
// read only cases map to write-protect
case TT_DESCRIPTOR_SECTION_AP_RO_NO:
case TT_DESCRIPTOR_SECTION_AP_RO_RO:
- *GcdAttributes |= EFI_MEMORY_WP;
+ *GcdAttributes |= EFI_MEMORY_RO;
break;
default:
@@ -126,7 +127,7 @@ PageToGcdAttributes (
// determine protection attributes
switch(PageAttributes & TT_DESCRIPTOR_PAGE_AP_MASK) {
case TT_DESCRIPTOR_PAGE_AP_NO_NO: // no read, no write
- //*GcdAttributes |= EFI_MEMORY_WP | EFI_MEMORY_RP;
+ //*GcdAttributes |= EFI_MEMORY_RO | EFI_MEMORY_RP;
break;
case TT_DESCRIPTOR_PAGE_AP_RW_NO:
@@ -137,7 +138,7 @@ PageToGcdAttributes (
// read only cases map to write-protect
case TT_DESCRIPTOR_PAGE_AP_RO_NO:
case TT_DESCRIPTOR_PAGE_AP_RO_RO:
- *GcdAttributes |= EFI_MEMORY_WP;
+ *GcdAttributes |= EFI_MEMORY_RO;
break;
default:
@@ -730,9 +731,6 @@ EfiAttributeToArmAttribute (
ArmAttributes = TT_DESCRIPTOR_SECTION_CACHE_POLICY_WRITE_BACK_ALLOC; // TEX [2:0] = 001, C=1, B=1
break;
- case EFI_MEMORY_WP:
- case EFI_MEMORY_XP:
- case EFI_MEMORY_RP:
case EFI_MEMORY_UCE:
default:
// Cannot be implemented UEFI definition unclear for ARM
@@ -743,7 +741,7 @@ EfiAttributeToArmAttribute (
}
// Determine protection attributes
- if (EfiAttributes & EFI_MEMORY_WP) {
+ if (EfiAttributes & EFI_MEMORY_RO) {
ArmAttributes |= TT_DESCRIPTOR_SECTION_AP_RO_RO;
} else {
ArmAttributes |= TT_DESCRIPTOR_SECTION_AP_RW_RW;
diff --git a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
index 723604d..54d9b01 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
+++ b/ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c
@@ -1,6 +1,7 @@
/** @file
*
* Copyright (c) 2013, ARM Limited. All rights reserved.
+* Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
*
* This program and the accompanying materials
* are licensed and made available under the terms and conditions of the BSD License
@@ -236,7 +237,7 @@ CpuConvertPagesToUncachedVirtualAddress (
// be the PCI address. Code should always use the CPU address, and we will or in VirtualMask
// to that address.
//
- Status = SetMemoryAttributes (Address, Length, EFI_MEMORY_WP, 0);
+ Status = SetMemoryAttributes (Address, Length, EFI_MEMORY_RO, 0);
if (!EFI_ERROR (Status)) {
Status = SetMemoryAttributes (Address | VirtualMask, Length, EFI_MEMORY_UC, VirtualMask);
}
@@ -264,7 +265,7 @@ CpuReconvertPages (
//
// Unmap the aliased Address
//
- Status = SetMemoryAttributes (Address | VirtualMask, Length, EFI_MEMORY_WP, 0);
+ Status = SetMemoryAttributes (Address | VirtualMask, Length, EFI_MEMORY_RO, 0);
if (!EFI_ERROR (Status)) {
//
// Restore atttributes
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index 540069a..6aa970b 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -3,6 +3,7 @@
*
* Copyright (c) 2011-2014, ARM Limited. All rights reserved.
* Copyright (c) 2016, Linaro Limited. All rights reserved.
+* Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
*
* This program and the accompanying materials
* are licensed and made available under the terms and conditions of the BSD License
@@ -89,7 +90,7 @@ PageAttributeToGcdAttribute (
// Determine protection attributes
if (((PageAttributes & TT_AP_MASK) == TT_AP_NO_RO) || ((PageAttributes & TT_AP_MASK) == TT_AP_RO_RO)) {
// Read only cases map to write-protect
- GcdAttributes |= EFI_MEMORY_WP;
+ GcdAttributes |= EFI_MEMORY_RO;
}
// Process eXecute Never attribute
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH V3 3/4] MdeModulePkg/dec: add PcdImageProtectionPolicy.
2017-02-09 7:20 [PATCH V3 0/4] DXE Memory Protection Jiewen Yao
2017-02-09 7:20 ` [PATCH V3 1/4] UefiCpuPkg/CpuDxe: Add memory attribute setting Jiewen Yao
2017-02-09 7:20 ` [PATCH V3 2/4] ArmPkg/CpuDxe: Correct EFI_MEMORY_RO usage Jiewen Yao
@ 2017-02-09 7:20 ` Jiewen Yao
2017-02-09 7:20 ` [PATCH V3 4/4] MdeModulePkg/DxeCore: Add UEFI image protection Jiewen Yao
` (2 subsequent siblings)
5 siblings, 0 replies; 27+ messages in thread
From: Jiewen Yao @ 2017-02-09 7:20 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng, Feng Tian, Michael Kinney
Add PCD for image protection policy.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/MdeModulePkg.dec | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 273cd7e..ab0490f 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1087,6 +1087,16 @@
# @Prompt Memory profile driver path.
gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfileDriverPath|{0x0}|VOID*|0x00001043
+ ## Set image protection policy. The policy is bitwise.
+ # If a bit is set, the image will be protected by DxeCore if it is aligned.
+ # The code section becomes read-only, and the data section becomes non-executable.
+ # If a bit is clear, the image will not be protected.<BR><BR>
+ # BIT0 - Image from unknown device. <BR>
+ # BIT1 - Image from firmware volume.<BR>
+ # @Prompt Set image protection policy.
+ # @ValidRange 0x80000002 | 0x00000000 - 0x0000001F
+ gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x00000002|UINT32|0x00001047
+
## PCI Serial Device Info. It is an array of Device, Function, and Power Management
# information that describes the path that contains zero or more PCI to PCI briges
# followed by a PCI serial device. Each array entry is 4-bytes in length. The
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH V3 4/4] MdeModulePkg/DxeCore: Add UEFI image protection.
2017-02-09 7:20 [PATCH V3 0/4] DXE Memory Protection Jiewen Yao
` (2 preceding siblings ...)
2017-02-09 7:20 ` [PATCH V3 3/4] MdeModulePkg/dec: add PcdImageProtectionPolicy Jiewen Yao
@ 2017-02-09 7:20 ` Jiewen Yao
2017-02-09 7:43 ` [PATCH V3 0/4] DXE Memory Protection Yao, Jiewen
2017-02-09 14:23 ` Yao, Jiewen
5 siblings, 0 replies; 27+ messages in thread
From: Jiewen Yao @ 2017-02-09 7:20 UTC (permalink / raw)
To: edk2-devel; +Cc: Star Zeng, Feng Tian, Michael Kinney
If the UEFI image is page aligned, the image code section is set to read
only and the image data section is set to non-executable.
1) This policy is applied for all UEFI image including boot service driver,
runtime driver or application.
2) This policy is applied only if the UEFI image meets the page alignment
requirement.
3) This policy is applied only if the Source UEFI image matches the
PcdImageProtectionPolicy definition.
4) This policy is not applied to the non-PE image region.
The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
the image. If the CpuArch protocol is not installed yet, the DxeCore
enqueues the protection request. Once the CpuArch is installed, the
DxeCore dequeues the protection request and applies policy.
Once the image is unloaded, the protection is removed automatically.
NOTE: It is per-requisite that code section and data section
should not be not merged. That is same criteria for SMM/runtime driver.
We are not able to detect during BIOS boot, because
we can only get LINK warning below:
"LINK : warning LNK4254: section '.data' (C0000040) merged into
'.text' (60000020) with different attributes"
But final attribute in PE code section is same.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/Core/Dxe/DxeMain.h | 53 ++
MdeModulePkg/Core/Dxe/DxeMain.inf | 5 +-
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +-
MdeModulePkg/Core/Dxe/Image/Image.c | 7 +-
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 735 ++++++++++++++++++++
MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 24 +-
6 files changed, 801 insertions(+), 26 deletions(-)
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
index ae35fbb..67b5a5a 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -267,6 +267,26 @@ typedef struct {
#define LOADED_IMAGE_PRIVATE_DATA_FROM_THIS(a) \
CR(a, LOADED_IMAGE_PRIVATE_DATA, Info, LOADED_IMAGE_PRIVATE_DATA_SIGNATURE)
+#define IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE SIGNATURE_32 ('I','P','R','C')
+
+typedef struct {
+ UINT32 Signature;
+ LIST_ENTRY Link;
+ EFI_PHYSICAL_ADDRESS CodeSegmentBase;
+ UINT64 CodeSegmentSize;
+} IMAGE_PROPERTIES_RECORD_CODE_SECTION;
+
+#define IMAGE_PROPERTIES_RECORD_SIGNATURE SIGNATURE_32 ('I','P','R','D')
+
+typedef struct {
+ UINT32 Signature;
+ LIST_ENTRY Link;
+ EFI_PHYSICAL_ADDRESS ImageBase;
+ UINT64 ImageSize;
+ UINTN CodeSegmentCount;
+ LIST_ENTRY CodeSegmentList;
+} IMAGE_PROPERTIES_RECORD;
+
//
// DXE Core Global Variables
//
@@ -2859,6 +2879,15 @@ CoreInitializeMemoryAttributesTable (
);
/**
+ Initialize Memory Protection support.
+**/
+VOID
+EFIAPI
+CoreInitializeMemoryProtection (
+ VOID
+ );
+
+/**
Install MemoryAttributesTable on memory allocation.
@param[in] MemoryType EFI memory type.
@@ -2888,4 +2917,28 @@ RemoveImageRecord (
IN EFI_RUNTIME_IMAGE_ENTRY *RuntimeImage
);
+/**
+ Protect UEFI image.
+
+ @param[in] LoadedImage The loaded image protocol
+ @param[in] LoadedImageDevicePath The loaded image device path protocol
+**/
+VOID
+ProtectUefiImage (
+ IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
+ IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
+ );
+
+/**
+ Unprotect UEFI image.
+
+ @param[in] LoadedImage The loaded image protocol
+ @param[in] LoadedImageDevicePath The loaded image device path protocol
+**/
+VOID
+UnprotectUefiImage (
+ IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
+ IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
+ );
+
#endif
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 13a2381..371e91c 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -3,7 +3,7 @@
#
# It provides an implementation of DXE Core that is compliant with DXE CIS.
#
-# Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
# which accompanies this distribution. The full text of the license may be found at
@@ -42,6 +42,7 @@
Misc/InstallConfigurationTable.c
Misc/PropertiesTable.c
Misc/MemoryAttributesTable.c
+ Misc/MemoryProtection.c
Library/Library.c
Hand/DriverSupport.c
Hand/Notify.c
@@ -159,6 +160,7 @@
gEfiHiiPackageListProtocolGuid ## SOMETIMES_PRODUCES
gEfiEbcProtocolGuid ## SOMETIMES_CONSUMES
gEfiSmmBase2ProtocolGuid ## SOMETIMES_CONSUMES
+ gEfiBlockIoProtocolGuid ## SOMETIMES_CONSUMES
# Arch Protocols
gEfiBdsArchProtocolGuid ## CONSUMES
@@ -188,6 +190,7 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfilePropertyMask ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfileDriverPath ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy ## CONSUMES
# [Hob]
# RESOURCE_DESCRIPTOR ## CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
index 21cd61a..b133e5e 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
+++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
@@ -1,7 +1,7 @@
/** @file
DXE Core Main Entry Point
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -398,6 +398,7 @@ DxeMain (
CoreInitializePropertiesTable ();
CoreInitializeMemoryAttributesTable ();
+ CoreInitializeMemoryProtection ();
//
// Get persisted vector hand-off info from GUIDeed HOB again due to HobStart may be updated,
diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
index 4a8a16d..652da8b 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -1,7 +1,7 @@
/** @file
Core image handling services to load and unload PeImage.
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -203,6 +203,8 @@ CoreInitializeImageServices (
);
}
+ ProtectUefiImage (&Image->Info, Image->LoadedImageDevicePath);
+
return Status;
}
@@ -862,6 +864,8 @@ CoreUnloadAndCloseImage (
UnregisterMemoryProfileImage (Image);
}
+ UnprotectUefiImage (&Image->Info, Image->LoadedImageDevicePath);
+
if (Image->Ebc != NULL) {
//
// If EBC protocol exists we must perform cleanups for this image.
@@ -1341,6 +1345,7 @@ CoreLoadImageCommon (
goto Done;
}
}
+ ProtectUefiImage (&Image->Info, Image->LoadedImageDevicePath);
//
// Success. Return the image handle
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
new file mode 100644
index 0000000..72b5d95
--- /dev/null
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -0,0 +1,735 @@
+/** @file
+ UEFI Memory Protection support.
+
+ If the UEFI image is page aligned, the image code section is set to read only
+ and the image data section is set to non-executable.
+
+ 1) This policy is applied for all UEFI image including boot service driver,
+ runtime driver or application.
+ 2) This policy is applied only if the UEFI image meets the page alignment
+ requirement.
+ 3) This policy is applied only if the Source UEFI image matches the
+ PcdImageProtectionPolicy definition.
+ 4) This policy is not applied to the non-PE image region.
+
+ The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
+ the image. If the CpuArch protocol is not installed yet, the DxeCore
+ enqueues the protection request. Once the CpuArch is installed, the
+ DxeCore dequeues the protection request and applies policy.
+
+ Once the image is unloaded, the protection is removed automatically.
+
+Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD License
+which accompanies this distribution. The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <PiDxe.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/DxeServicesTableLib.h>
+#include <Library/DebugLib.h>
+#include <Library/UefiLib.h>
+
+#include <Guid/EventGroup.h>
+#include <Guid/MemoryAttributesTable.h>
+#include <Guid/PropertiesTable.h>
+
+#include <Protocol/FirmwareVolume2.h>
+#include <Protocol/BlockIo.h>
+#include <Protocol/SimpleFileSystem.h>
+
+#include "DxeMain.h"
+
+#define CACHE_ATTRIBUTE_MASK (EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE | EFI_MEMORY_WP)
+#define MEMORY_ATTRIBUTE_MASK (EFI_MEMORY_RP | EFI_MEMORY_XP | EFI_MEMORY_RO)
+
+//
+// Image type definitions
+//
+#define IMAGE_UNKNOWN 0x00000001
+#define IMAGE_FROM_FV 0x00000002
+
+//
+// Protection policy bit definition
+//
+#define DO_NOT_PROTECT 0x00000000
+#define PROTECT_IF_ALIGNED_ELSE_ALLOW 0x00000001
+
+UINT32 mImageProtectionPolicy;
+
+/**
+ Sort code section in image record, based upon CodeSegmentBase from low to high.
+
+ @param ImageRecord image record to be sorted
+**/
+VOID
+SortImageRecordCodeSection (
+ IN IMAGE_PROPERTIES_RECORD *ImageRecord
+ );
+
+/**
+ Check if code section in image record is valid.
+
+ @param ImageRecord image record to be checked
+
+ @retval TRUE image record is valid
+ @retval FALSE image record is invalid
+**/
+BOOLEAN
+IsImageRecordCodeSectionValid (
+ IN IMAGE_PROPERTIES_RECORD *ImageRecord
+ );
+
+/**
+ Get the image type.
+
+ @param[in] File This is a pointer to the device path of the file that is
+ being dispatched.
+
+ @return UINT32 Image Type
+**/
+UINT32
+GetImageType (
+ IN CONST EFI_DEVICE_PATH_PROTOCOL *File
+ )
+{
+ EFI_STATUS Status;
+ EFI_HANDLE DeviceHandle;
+ EFI_DEVICE_PATH_PROTOCOL *TempDevicePath;
+
+ if (File == NULL) {
+ return IMAGE_UNKNOWN;
+ }
+
+ //
+ // First check to see if File is from a Firmware Volume
+ //
+ DeviceHandle = NULL;
+ TempDevicePath = (EFI_DEVICE_PATH_PROTOCOL *) File;
+ Status = gBS->LocateDevicePath (
+ &gEfiFirmwareVolume2ProtocolGuid,
+ &TempDevicePath,
+ &DeviceHandle
+ );
+ if (!EFI_ERROR (Status)) {
+ Status = gBS->OpenProtocol (
+ DeviceHandle,
+ &gEfiFirmwareVolume2ProtocolGuid,
+ NULL,
+ NULL,
+ NULL,
+ EFI_OPEN_PROTOCOL_TEST_PROTOCOL
+ );
+ if (!EFI_ERROR (Status)) {
+ return IMAGE_FROM_FV;
+ }
+ }
+ return IMAGE_UNKNOWN;
+}
+
+/**
+ Get UEFI image protection policy based upon image type.
+
+ @param[in] ImageType The UEFI image type
+
+ @return UEFI image protection policy
+**/
+UINT32
+GetProtectionPolicyFromImageType (
+ IN UINT32 ImageType
+ )
+{
+ if ((ImageType & mImageProtectionPolicy) == 0) {
+ return DO_NOT_PROTECT;
+ } else {
+ return PROTECT_IF_ALIGNED_ELSE_ALLOW;
+ }
+}
+
+/**
+ Get UEFI image protection policy based upon loaded image device path.
+
+ @param[in] LoadedImage The loaded image protocol
+ @param[in] LoadedImageDevicePath The loaded image device path protocol
+
+ @return UEFI image protection policy
+**/
+UINT32
+GetUefiImageProtectionPolicy (
+ IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
+ IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
+ )
+{
+ BOOLEAN InSmm;
+ UINT32 ImageType;
+ UINT32 ProtectionPolicy;
+
+ //
+ // Check SMM
+ //
+ InSmm = FALSE;
+ if (gSmmBase2 != NULL) {
+ gSmmBase2->InSmm (gSmmBase2, &InSmm);
+ }
+ if (InSmm) {
+ return FALSE;
+ }
+
+ //
+ // Check DevicePath
+ //
+ if (LoadedImage == gDxeCoreLoadedImage) {
+ ImageType = IMAGE_FROM_FV;
+ } else {
+ ImageType = GetImageType (LoadedImageDevicePath);
+ }
+ ProtectionPolicy = GetProtectionPolicyFromImageType (ImageType);
+ return ProtectionPolicy;
+}
+
+
+/**
+ Set UEFI image memory attributes.
+
+ @param[in] BaseAddress Specified start address
+ @param[in] Length Specified length
+ @param[in] Attributes Specified attributes
+**/
+VOID
+SetUefiImageMemoryAttributes (
+ IN UINT64 BaseAddress,
+ IN UINT64 Length,
+ IN UINT64 Attributes
+ )
+{
+ EFI_STATUS Status;
+ EFI_GCD_MEMORY_SPACE_DESCRIPTOR Descriptor;
+ UINT64 FinalAttributes;
+
+ Status = CoreGetMemorySpaceDescriptor(BaseAddress, &Descriptor);
+ ASSERT_EFI_ERROR(Status);
+
+ FinalAttributes = (Descriptor.Attributes & CACHE_ATTRIBUTE_MASK) | (Attributes & MEMORY_ATTRIBUTE_MASK);
+
+ DEBUG ((DEBUG_INFO, "SetUefiImageMemoryAttributes - 0x%016lx - 0x%016lx (0x%016lx)\n", BaseAddress, Length, FinalAttributes));
+
+ ASSERT(gCpu != NULL);
+ gCpu->SetMemoryAttributes (gCpu, BaseAddress, Length, FinalAttributes);
+}
+
+/**
+ Set UEFI image protection attributes.
+
+ @param[in] ImageRecord A UEFI image record
+ @param[in] Protect TRUE: Protect the UEFI image.
+ FALSE: Unprotect the UEFI image.
+**/
+VOID
+SetUefiImageProtectionAttributes (
+ IN IMAGE_PROPERTIES_RECORD *ImageRecord,
+ IN BOOLEAN Protect
+ )
+{
+ IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
+ LIST_ENTRY *ImageRecordCodeSectionLink;
+ LIST_ENTRY *ImageRecordCodeSectionEndLink;
+ LIST_ENTRY *ImageRecordCodeSectionList;
+ UINT64 CurrentBase;
+ UINT64 ImageEnd;
+ UINT64 Attribute;
+
+ ImageRecordCodeSectionList = &ImageRecord->CodeSegmentList;
+
+ CurrentBase = ImageRecord->ImageBase;
+ ImageEnd = ImageRecord->ImageBase + ImageRecord->ImageSize;
+
+ ImageRecordCodeSectionLink = ImageRecordCodeSectionList->ForwardLink;
+ ImageRecordCodeSectionEndLink = ImageRecordCodeSectionList;
+ while (ImageRecordCodeSectionLink != ImageRecordCodeSectionEndLink) {
+ ImageRecordCodeSection = CR (
+ ImageRecordCodeSectionLink,
+ IMAGE_PROPERTIES_RECORD_CODE_SECTION,
+ Link,
+ IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE
+ );
+ ImageRecordCodeSectionLink = ImageRecordCodeSectionLink->ForwardLink;
+
+ ASSERT (CurrentBase <= ImageRecordCodeSection->CodeSegmentBase);
+ if (CurrentBase < ImageRecordCodeSection->CodeSegmentBase) {
+ //
+ // DATA
+ //
+ if (Protect) {
+ Attribute = EFI_MEMORY_XP;
+ } else {
+ Attribute = 0;
+ }
+ SetUefiImageMemoryAttributes (
+ CurrentBase,
+ ImageRecordCodeSection->CodeSegmentBase - CurrentBase,
+ Attribute
+ );
+ }
+ //
+ // CODE
+ //
+ if (Protect) {
+ Attribute = EFI_MEMORY_RO;
+ } else {
+ Attribute = 0;
+ }
+ SetUefiImageMemoryAttributes (
+ ImageRecordCodeSection->CodeSegmentBase,
+ ImageRecordCodeSection->CodeSegmentSize,
+ Attribute
+ );
+ CurrentBase = ImageRecordCodeSection->CodeSegmentBase + ImageRecordCodeSection->CodeSegmentSize;
+ }
+ //
+ // Last DATA
+ //
+ ASSERT (CurrentBase <= ImageEnd);
+ if (CurrentBase < ImageEnd) {
+ //
+ // DATA
+ //
+ if (Protect) {
+ Attribute = EFI_MEMORY_XP;
+ } else {
+ Attribute = 0;
+ }
+ SetUefiImageMemoryAttributes (
+ CurrentBase,
+ ImageEnd - CurrentBase,
+ Attribute
+ );
+ }
+ return ;
+}
+
+/**
+ Return if the PE image section is aligned.
+
+ @param[in] SectionAlignment PE/COFF section alignment
+ @param[in] MemoryType PE/COFF image memory type
+
+ @retval TRUE The PE image section is aligned.
+ @retval FALSE The PE image section is not aligned.
+**/
+BOOLEAN
+IsMemoryProtectionSectionAligned (
+ IN UINT32 SectionAlignment,
+ IN EFI_MEMORY_TYPE MemoryType
+ )
+{
+ UINT32 PageAlignment;
+
+ switch (MemoryType) {
+ case EfiRuntimeServicesCode:
+ case EfiACPIMemoryNVS:
+ PageAlignment = EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT;
+ break;
+ case EfiRuntimeServicesData:
+ case EfiACPIReclaimMemory:
+ ASSERT (FALSE);
+ PageAlignment = EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT;
+ break;
+ case EfiBootServicesCode:
+ case EfiLoaderCode:
+ case EfiReservedMemoryType:
+ PageAlignment = EFI_PAGE_SIZE;
+ break;
+ default:
+ ASSERT (FALSE);
+ PageAlignment = EFI_PAGE_SIZE;
+ break;
+ }
+
+ if ((SectionAlignment & (PageAlignment - 1)) != 0) {
+ return FALSE;
+ } else {
+ return TRUE;
+ }
+}
+
+/**
+ Free Image record.
+
+ @param[in] ImageRecord A UEFI image record
+**/
+VOID
+FreeImageRecord (
+ IN IMAGE_PROPERTIES_RECORD *ImageRecord
+ )
+{
+ LIST_ENTRY *CodeSegmentListHead;
+ IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
+
+ CodeSegmentListHead = &ImageRecord->CodeSegmentList;
+ while (!IsListEmpty (CodeSegmentListHead)) {
+ ImageRecordCodeSection = CR (
+ CodeSegmentListHead->ForwardLink,
+ IMAGE_PROPERTIES_RECORD_CODE_SECTION,
+ Link,
+ IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE
+ );
+ RemoveEntryList (&ImageRecordCodeSection->Link);
+ FreePool (ImageRecordCodeSection);
+ }
+
+ if (ImageRecord->Link.ForwardLink != NULL) {
+ RemoveEntryList (&ImageRecord->Link);
+ }
+ FreePool (ImageRecord);
+}
+
+/**
+ Protect or unprotect UEFI image common function.
+
+ @param[in] LoadedImage The loaded image protocol
+ @param[in] LoadedImageDevicePath The loaded image device path protocol
+ @param[in] Protect TRUE: Protect the UEFI image.
+ FALSE: Unprotect the UEFI image.
+**/
+VOID
+ProtectUefiImageCommon (
+ IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
+ IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath,
+ IN BOOLEAN Protect
+ )
+{
+ VOID *ImageAddress;
+ EFI_IMAGE_DOS_HEADER *DosHdr;
+ UINT32 PeCoffHeaderOffset;
+ UINT32 SectionAlignment;
+ EFI_IMAGE_SECTION_HEADER *Section;
+ EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr;
+ UINT8 *Name;
+ UINTN Index;
+ IMAGE_PROPERTIES_RECORD *ImageRecord;
+ CHAR8 *PdbPointer;
+ IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
+ UINT16 Magic;
+ BOOLEAN IsAligned;
+ UINT32 ProtectionPolicy;
+
+ DEBUG ((DEBUG_INFO, "ProtectUefiImageCommon - 0x%x\n", LoadedImage));
+ DEBUG ((DEBUG_INFO, " - 0x%016lx - 0x%016lx\n", (EFI_PHYSICAL_ADDRESS)(UINTN)LoadedImage->ImageBase, LoadedImage->ImageSize));
+
+ if (gCpu == NULL) {
+ return ;
+ }
+
+ ProtectionPolicy = GetUefiImageProtectionPolicy (LoadedImage, LoadedImageDevicePath);
+ switch (ProtectionPolicy) {
+ case DO_NOT_PROTECT:
+ return ;
+ case PROTECT_IF_ALIGNED_ELSE_ALLOW:
+ break;
+ default:
+ ASSERT(FALSE);
+ return ;
+ }
+
+ ImageRecord = AllocateZeroPool (sizeof(*ImageRecord));
+ if (ImageRecord == NULL) {
+ return ;
+ }
+ ImageRecord->Signature = IMAGE_PROPERTIES_RECORD_SIGNATURE;
+
+ //
+ // Step 1: record whole region
+ //
+ ImageRecord->ImageBase = (EFI_PHYSICAL_ADDRESS)(UINTN)LoadedImage->ImageBase;
+ ImageRecord->ImageSize = LoadedImage->ImageSize;
+
+ ImageAddress = LoadedImage->ImageBase;
+
+ PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN) ImageAddress);
+ if (PdbPointer != NULL) {
+ DEBUG ((DEBUG_VERBOSE, " Image - %a\n", PdbPointer));
+ }
+
+ //
+ // Check PE/COFF image
+ //
+ DosHdr = (EFI_IMAGE_DOS_HEADER *) (UINTN) ImageAddress;
+ PeCoffHeaderOffset = 0;
+ if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) {
+ PeCoffHeaderOffset = DosHdr->e_lfanew;
+ }
+
+ Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINT8 *) (UINTN) ImageAddress + PeCoffHeaderOffset);
+ if (Hdr.Pe32->Signature != EFI_IMAGE_NT_SIGNATURE) {
+ DEBUG ((DEBUG_VERBOSE, "Hdr.Pe32->Signature invalid - 0x%x\n", Hdr.Pe32->Signature));
+ // It might be image in SMM.
+ goto Finish;
+ }
+
+ //
+ // Get SectionAlignment
+ //
+ if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ //
+ // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
+ // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
+ // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
+ // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
+ //
+ Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
+ } else {
+ //
+ // Get the magic value from the PE/COFF Optional Header
+ //
+ Magic = Hdr.Pe32->OptionalHeader.Magic;
+ }
+ if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
+ } else {
+ SectionAlignment = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
+ }
+
+ IsAligned = IsMemoryProtectionSectionAligned (SectionAlignment, LoadedImage->ImageCodeType);
+ if (!IsAligned) {
+ DEBUG ((DEBUG_VERBOSE, "!!!!!!!! ProtectUefiImageCommon - Section Alignment(0x%x) is incorrect !!!!!!!!\n",
+ SectionAlignment));
+ PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN) ImageAddress);
+ if (PdbPointer != NULL) {
+ DEBUG ((DEBUG_VERBOSE, "!!!!!!!! Image - %a !!!!!!!!\n", PdbPointer));
+ }
+ goto Finish;
+ }
+
+ Section = (EFI_IMAGE_SECTION_HEADER *) (
+ (UINT8 *) (UINTN) ImageAddress +
+ PeCoffHeaderOffset +
+ sizeof(UINT32) +
+ sizeof(EFI_IMAGE_FILE_HEADER) +
+ Hdr.Pe32->FileHeader.SizeOfOptionalHeader
+ );
+ ImageRecord->CodeSegmentCount = 0;
+ InitializeListHead (&ImageRecord->CodeSegmentList);
+ for (Index = 0; Index < Hdr.Pe32->FileHeader.NumberOfSections; Index++) {
+ Name = Section[Index].Name;
+ DEBUG ((
+ DEBUG_VERBOSE,
+ " Section - '%c%c%c%c%c%c%c%c'\n",
+ Name[0],
+ Name[1],
+ Name[2],
+ Name[3],
+ Name[4],
+ Name[5],
+ Name[6],
+ Name[7]
+ ));
+
+ if ((Section[Index].Characteristics & EFI_IMAGE_SCN_CNT_CODE) != 0) {
+ DEBUG ((DEBUG_VERBOSE, " VirtualSize - 0x%08x\n", Section[Index].Misc.VirtualSize));
+ DEBUG ((DEBUG_VERBOSE, " VirtualAddress - 0x%08x\n", Section[Index].VirtualAddress));
+ DEBUG ((DEBUG_VERBOSE, " SizeOfRawData - 0x%08x\n", Section[Index].SizeOfRawData));
+ DEBUG ((DEBUG_VERBOSE, " PointerToRawData - 0x%08x\n", Section[Index].PointerToRawData));
+ DEBUG ((DEBUG_VERBOSE, " PointerToRelocations - 0x%08x\n", Section[Index].PointerToRelocations));
+ DEBUG ((DEBUG_VERBOSE, " PointerToLinenumbers - 0x%08x\n", Section[Index].PointerToLinenumbers));
+ DEBUG ((DEBUG_VERBOSE, " NumberOfRelocations - 0x%08x\n", Section[Index].NumberOfRelocations));
+ DEBUG ((DEBUG_VERBOSE, " NumberOfLinenumbers - 0x%08x\n", Section[Index].NumberOfLinenumbers));
+ DEBUG ((DEBUG_VERBOSE, " Characteristics - 0x%08x\n", Section[Index].Characteristics));
+
+ //
+ // Step 2: record code section
+ //
+ ImageRecordCodeSection = AllocatePool (sizeof(*ImageRecordCodeSection));
+ if (ImageRecordCodeSection == NULL) {
+ return ;
+ }
+ ImageRecordCodeSection->Signature = IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE;
+
+ ImageRecordCodeSection->CodeSegmentBase = (UINTN)ImageAddress + Section[Index].VirtualAddress;
+ ImageRecordCodeSection->CodeSegmentSize = ALIGN_VALUE(Section[Index].SizeOfRawData, SectionAlignment);
+
+ DEBUG ((DEBUG_VERBOSE, "ImageCode: 0x%016lx - 0x%016lx\n", ImageRecordCodeSection->CodeSegmentBase, ImageRecordCodeSection->CodeSegmentSize));
+
+ InsertTailList (&ImageRecord->CodeSegmentList, &ImageRecordCodeSection->Link);
+ ImageRecord->CodeSegmentCount++;
+ }
+ }
+
+ if (ImageRecord->CodeSegmentCount == 0) {
+ DEBUG ((DEBUG_ERROR, "!!!!!!!! ProtectUefiImageCommon - CodeSegmentCount is 0 !!!!!!!!\n"));
+ PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN) ImageAddress);
+ if (PdbPointer != NULL) {
+ DEBUG ((DEBUG_ERROR, "!!!!!!!! Image - %a !!!!!!!!\n", PdbPointer));
+ }
+ goto Finish;
+ }
+
+ //
+ // Final
+ //
+ SortImageRecordCodeSection (ImageRecord);
+ //
+ // Check overlap all section in ImageBase/Size
+ //
+ if (!IsImageRecordCodeSectionValid (ImageRecord)) {
+ DEBUG ((DEBUG_ERROR, "IsImageRecordCodeSectionValid - FAIL\n"));
+ goto Finish;
+ }
+
+ //
+ // CPU ARCH present. Update memory attribute directly.
+ //
+ SetUefiImageProtectionAttributes (ImageRecord, Protect);
+
+ //
+ // Clean up
+ //
+ FreeImageRecord (ImageRecord);
+
+Finish:
+ return ;
+}
+
+/**
+ Protect UEFI image.
+
+ @param[in] LoadedImage The loaded image protocol
+ @param[in] LoadedImageDevicePath The loaded image device path protocol
+**/
+VOID
+ProtectUefiImage (
+ IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
+ IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
+ )
+{
+ if (PcdGet32(PcdImageProtectionPolicy) != 0) {
+ ProtectUefiImageCommon (LoadedImage, LoadedImageDevicePath, TRUE);
+ }
+}
+
+/**
+ Unprotect UEFI image.
+
+ @param[in] LoadedImage The loaded image protocol
+ @param[in] LoadedImageDevicePath The loaded image device path protocol
+**/
+VOID
+UnprotectUefiImage (
+ IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
+ IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
+ )
+{
+ if (PcdGet32(PcdImageProtectionPolicy) != 0) {
+ ProtectUefiImageCommon (LoadedImage, LoadedImageDevicePath, FALSE);
+ }
+}
+
+/**
+ A notification for CPU_ARCH protocol.
+
+ @param[in] Event Event whose notification function is being invoked.
+ @param[in] Context Pointer to the notification function's context,
+ which is implementation-dependent.
+
+**/
+VOID
+EFIAPI
+MemoryProtectionCpuArchProtocolNotify (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ EFI_STATUS Status;
+ EFI_LOADED_IMAGE_PROTOCOL *LoadedImage;
+ EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath;
+ UINTN NoHandles;
+ EFI_HANDLE *HandleBuffer;
+ UINTN Index;
+
+ DEBUG ((DEBUG_INFO, "MemoryProtectionCpuArchProtocolNotify:\n"));
+ Status = CoreLocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&gCpu);
+ if (EFI_ERROR (Status)) {
+ return;
+ }
+
+ Status = gBS->LocateHandleBuffer (
+ ByProtocol,
+ &gEfiLoadedImageProtocolGuid,
+ NULL,
+ &NoHandles,
+ &HandleBuffer
+ );
+ if (EFI_ERROR (Status) && (NoHandles == 0)) {
+ return ;
+ }
+
+ for (Index = 0; Index < NoHandles; Index++) {
+ Status = gBS->HandleProtocol (
+ HandleBuffer[Index],
+ &gEfiLoadedImageProtocolGuid,
+ (VOID **)&LoadedImage
+ );
+ if (EFI_ERROR(Status)) {
+ continue;
+ }
+ Status = gBS->HandleProtocol (
+ HandleBuffer[Index],
+ &gEfiLoadedImageDevicePathProtocolGuid,
+ (VOID **)&LoadedImageDevicePath
+ );
+ if (EFI_ERROR(Status)) {
+ LoadedImageDevicePath = NULL;
+ }
+
+ ProtectUefiImage (LoadedImage, LoadedImageDevicePath);
+ }
+
+ CoreCloseEvent (Event);
+ return;
+}
+
+/**
+ Initialize Memory Protection support.
+**/
+VOID
+EFIAPI
+CoreInitializeMemoryProtection (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ EFI_EVENT Event;
+ VOID *Registration;
+
+ mImageProtectionPolicy = PcdGet32(PcdImageProtectionPolicy);
+
+ if (mImageProtectionPolicy != 0) {
+ Status = CoreCreateEvent (
+ EVT_NOTIFY_SIGNAL,
+ TPL_CALLBACK,
+ MemoryProtectionCpuArchProtocolNotify,
+ NULL,
+ &Event
+ );
+ ASSERT_EFI_ERROR(Status);
+
+ //
+ // Register for protocol notifactions on this event
+ //
+ Status = CoreRegisterProtocolNotify (
+ &gEfiCpuArchProtocolGuid,
+ Event,
+ &Registration
+ );
+ ASSERT_EFI_ERROR(Status);
+ }
+ return ;
+}
diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
index 7ecad89..5ea20db 100644
--- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
@@ -1,7 +1,7 @@
/** @file
UEFI PropertiesTable support
-Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -36,26 +36,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#define PREVIOUS_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \
((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) - (Size)))
-#define IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE SIGNATURE_32 ('I','P','R','C')
-
-typedef struct {
- UINT32 Signature;
- LIST_ENTRY Link;
- EFI_PHYSICAL_ADDRESS CodeSegmentBase;
- UINT64 CodeSegmentSize;
-} IMAGE_PROPERTIES_RECORD_CODE_SECTION;
-
-#define IMAGE_PROPERTIES_RECORD_SIGNATURE SIGNATURE_32 ('I','P','R','D')
-
-typedef struct {
- UINT32 Signature;
- LIST_ENTRY Link;
- EFI_PHYSICAL_ADDRESS ImageBase;
- UINT64 ImageSize;
- UINTN CodeSegmentCount;
- LIST_ENTRY CodeSegmentList;
-} IMAGE_PROPERTIES_RECORD;
-
#define IMAGE_PROPERTIES_PRIVATE_DATA_SIGNATURE SIGNATURE_32 ('I','P','P','D')
typedef struct {
@@ -864,7 +844,6 @@ SwapImageRecordCodeSection (
@param ImageRecord image record to be sorted
**/
-STATIC
VOID
SortImageRecordCodeSection (
IN IMAGE_PROPERTIES_RECORD *ImageRecord
@@ -915,7 +894,6 @@ SortImageRecordCodeSection (
@retval TRUE image record is valid
@retval FALSE image record is invalid
**/
-STATIC
BOOLEAN
IsImageRecordCodeSectionValid (
IN IMAGE_PROPERTIES_RECORD *ImageRecord
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-09 7:20 [PATCH V3 0/4] DXE Memory Protection Jiewen Yao
` (3 preceding siblings ...)
2017-02-09 7:20 ` [PATCH V3 4/4] MdeModulePkg/DxeCore: Add UEFI image protection Jiewen Yao
@ 2017-02-09 7:43 ` Yao, Jiewen
2017-02-09 8:49 ` Ard Biesheuvel
2017-02-09 14:23 ` Yao, Jiewen
5 siblings, 1 reply; 27+ messages in thread
From: Yao, Jiewen @ 2017-02-09 7:43 UTC (permalink / raw)
To: Yao, Jiewen, edk2-devel@lists.01.org
Cc: Tian, Feng, Ard Biesheuvel, Leif Lindholm, Kinney, Michael D,
Fan, Jeff, Zeng, Star
Hi Lindholm/Ard
This version 3 contains both of your feedback before.
If you can do me a favor to evaluated the impact to ARM, that will be great.
Thank you
Yao Jiewen
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jiewen
> Yao
> Sent: Wednesday, February 8, 2017 11:20 PM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng,
> Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH V3 0/4] DXE Memory Protection
>
> ==== V3 ====
> 1) Add PCD for policy control (feedback from Ard Biesheuvel)
> (Discussed with Mike Kinney)
> + # BIT0 - Image from unknown device. <BR>
> + # BIT1 - Image from firmware volume.<BR>
> + # @Prompt Set image protection policy.
> + # @ValidRange 0x80000002 | 0x00000000 - 0x0000001F
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x00000002|UIN
> T32|0x00001047
>
> 2) Remove unused function in CpuDxe.(feedback from Liming Gao)
> 3) Add commit log on link option assumption (feedback from Feng Tian)
>
> ==== V2 ====
> 1) Clean up ArmPkg, (feedback from Leif Lindholm)
>
> ==== V1 ====
> This series patch provides capability to protect PE/COFF image
> in DXE memory.
> If the UEFI image is page aligned, the image code section is set to read
> only and the image data section is set to non-executable.
>
> The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
> the image.
>
> Tested platform: NT32/Quark IA32/OVMF IA32/OVMF IA32X64/Intel internal X64/
> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
>
> Untested platform: ARM/AARCH64.
> Can ARM/AARCH64 owner help to take a look and try the ARM platform?
>
>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
>
> Jiewen Yao (4):
> UefiCpuPkg/CpuDxe: Add memory attribute setting.
> ArmPkg/CpuDxe: Correct EFI_MEMORY_RO usage
> MdeModulePkg/dec: add PcdImageProtectionPolicy.
> MdeModulePkg/DxeCore: Add UEFI image protection.
>
> ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 3 +-
> ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 14 +-
> ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c | 5 +-
> ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 3 +-
> MdeModulePkg/Core/Dxe/DxeMain.h | 53 ++
> MdeModulePkg/Core/Dxe/DxeMain.inf | 5 +-
> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +-
> MdeModulePkg/Core/Dxe/Image/Image.c | 7 +-
> MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 735
> ++++++++++++++++++
> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 24 +-
> MdeModulePkg/MdeModulePkg.dec | 10 +
> UefiCpuPkg/CpuDxe/CpuDxe.c | 141 ++--
> UefiCpuPkg/CpuDxe/CpuDxe.inf | 5 +-
> UefiCpuPkg/CpuDxe/CpuPageTable.c | 779
> ++++++++++++++++++++
> UefiCpuPkg/CpuDxe/CpuPageTable.h | 113 +++
> 15 files changed, 1801 insertions(+), 99 deletions(-)
> create mode 100644 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.c
> create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.h
>
> --
> 2.7.4.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-09 7:43 ` [PATCH V3 0/4] DXE Memory Protection Yao, Jiewen
@ 2017-02-09 8:49 ` Ard Biesheuvel
2017-02-09 9:09 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-02-09 8:49 UTC (permalink / raw)
To: Yao, Jiewen
Cc: edk2-devel@lists.01.org, Tian, Feng, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
On 9 February 2017 at 07:43, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> Hi Lindholm/Ard
> This version 3 contains both of your feedback before.
>
> If you can do me a favor to evaluated the impact to ARM, that will be great.
>
I will take a look right away.
> Thank you
> Yao Jiewen
>
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jiewen
>> Yao
>> Sent: Wednesday, February 8, 2017 11:20 PM
>> To: edk2-devel@lists.01.org
>> Cc: Tian, Feng <feng.tian@intel.com>; Ard Biesheuvel
>> <ard.biesheuvel@linaro.org>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney,
>> Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng,
>> Star <star.zeng@intel.com>
>> Subject: [edk2] [PATCH V3 0/4] DXE Memory Protection
>>
>> ==== V3 ====
>> 1) Add PCD for policy control (feedback from Ard Biesheuvel)
>> (Discussed with Mike Kinney)
>> + # BIT0 - Image from unknown device. <BR>
>> + # BIT1 - Image from firmware volume.<BR>
>> + # @Prompt Set image protection policy.
>> + # @ValidRange 0x80000002 | 0x00000000 - 0x0000001F
>> +
>> gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x00000002|UIN
>> T32|0x00001047
>>
>> 2) Remove unused function in CpuDxe.(feedback from Liming Gao)
>> 3) Add commit log on link option assumption (feedback from Feng Tian)
>>
>> ==== V2 ====
>> 1) Clean up ArmPkg, (feedback from Leif Lindholm)
>>
>> ==== V1 ====
>> This series patch provides capability to protect PE/COFF image
>> in DXE memory.
>> If the UEFI image is page aligned, the image code section is set to read
>> only and the image data section is set to non-executable.
>>
>> The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
>> the image.
>>
>> Tested platform: NT32/Quark IA32/OVMF IA32/OVMF IA32X64/Intel internal X64/
>> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
>>
>> Untested platform: ARM/AARCH64.
>> Can ARM/AARCH64 owner help to take a look and try the ARM platform?
>>
>>
>> Cc: Jeff Fan <jeff.fan@intel.com>
>> Cc: Michael Kinney <michael.d.kinney@intel.com>
>> Cc: Leif Lindholm <leif.lindholm@linaro.org>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Star Zeng <star.zeng@intel.com>
>> Cc: Feng Tian <feng.tian@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
>>
>> Jiewen Yao (4):
>> UefiCpuPkg/CpuDxe: Add memory attribute setting.
>> ArmPkg/CpuDxe: Correct EFI_MEMORY_RO usage
>> MdeModulePkg/dec: add PcdImageProtectionPolicy.
>> MdeModulePkg/DxeCore: Add UEFI image protection.
>>
>> ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 3 +-
>> ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 14 +-
>> ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c | 5 +-
>> ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 3 +-
>> MdeModulePkg/Core/Dxe/DxeMain.h | 53 ++
>> MdeModulePkg/Core/Dxe/DxeMain.inf | 5 +-
>> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +-
>> MdeModulePkg/Core/Dxe/Image/Image.c | 7 +-
>> MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 735
>> ++++++++++++++++++
>> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 24 +-
>> MdeModulePkg/MdeModulePkg.dec | 10 +
>> UefiCpuPkg/CpuDxe/CpuDxe.c | 141 ++--
>> UefiCpuPkg/CpuDxe/CpuDxe.inf | 5 +-
>> UefiCpuPkg/CpuDxe/CpuPageTable.c | 779
>> ++++++++++++++++++++
>> UefiCpuPkg/CpuDxe/CpuPageTable.h | 113 +++
>> 15 files changed, 1801 insertions(+), 99 deletions(-)
>> create mode 100644 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
>> create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.c
>> create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.h
>>
>> --
>> 2.7.4.windows.1
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-09 8:49 ` Ard Biesheuvel
@ 2017-02-09 9:09 ` Ard Biesheuvel
2017-02-09 9:22 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-02-09 9:09 UTC (permalink / raw)
To: Yao, Jiewen
Cc: edk2-devel@lists.01.org, Tian, Feng, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
On 9 February 2017 at 08:49, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 9 February 2017 at 07:43, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>> Hi Lindholm/Ard
>> This version 3 contains both of your feedback before.
>>
>> If you can do me a favor to evaluated the impact to ARM, that will be great.
>>
>
> I will take a look right away.
>
I am hitting the following assert as soon as ArmCpuDxe is loaded:
Loading driver at 0x0005BE15000 EntryPoint=0x0005BE16048 ArmCpuDxe.efi
InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 5B142398
ProtectUefiImageCommon - 0x5B142140
- 0x000000005BE15000 - 0x0000000000010000
InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 5BE23008
InstallProtocolInterface: AD651C7D-3C22-4DBF-92E8-38A7CDAE87B2 5BE230B0
MemoryProtectionCpuArchProtocolNotify:
ProtectUefiImageCommon - 0x5EF02400
- 0x000000005EEC4000 - 0x0000000000042000
SetUefiImageMemoryAttributes - 0x000000005EEC4000 - 0x0000000000001000
(0x0000000000004000)
EfiAttributeToArmAttribute: 0x4000 attributes is not supported.
ASSERT [ArmCpuDxe]
/home/ard/build/uefi-next/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c(220): 0
The reason appears to be that the GCD memory descriptor retrieved in
SetUefiImageMemoryAttributes() for this region has Attributes == 0
I am digging further ...
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-09 9:09 ` Ard Biesheuvel
@ 2017-02-09 9:22 ` Ard Biesheuvel
2017-02-09 13:19 ` Yao, Jiewen
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-02-09 9:22 UTC (permalink / raw)
To: Yao, Jiewen
Cc: edk2-devel@lists.01.org, Tian, Feng, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
On 9 February 2017 at 09:09, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 9 February 2017 at 08:49, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 9 February 2017 at 07:43, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>>> Hi Lindholm/Ard
>>> This version 3 contains both of your feedback before.
>>>
>>> If you can do me a favor to evaluated the impact to ARM, that will be great.
>>>
>>
>> I will take a look right away.
>>
>
> I am hitting the following assert as soon as ArmCpuDxe is loaded:
>
> Loading driver at 0x0005BE15000 EntryPoint=0x0005BE16048 ArmCpuDxe.efi
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 5B142398
> ProtectUefiImageCommon - 0x5B142140
> - 0x000000005BE15000 - 0x0000000000010000
> InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 5BE23008
> InstallProtocolInterface: AD651C7D-3C22-4DBF-92E8-38A7CDAE87B2 5BE230B0
> MemoryProtectionCpuArchProtocolNotify:
> ProtectUefiImageCommon - 0x5EF02400
> - 0x000000005EEC4000 - 0x0000000000042000
> SetUefiImageMemoryAttributes - 0x000000005EEC4000 - 0x0000000000001000
> (0x0000000000004000)
> EfiAttributeToArmAttribute: 0x4000 attributes is not supported.
> ASSERT [ArmCpuDxe]
> /home/ard/build/uefi-next/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c(220): 0
>
> The reason appears to be that the GCD memory descriptor retrieved in
> SetUefiImageMemoryAttributes() for this region has Attributes == 0
>
> I am digging further ...
Attached is the output of DEBUG_GCD
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-09 9:22 ` Ard Biesheuvel
@ 2017-02-09 13:19 ` Yao, Jiewen
2017-02-09 13:51 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Yao, Jiewen @ 2017-02-09 13:19 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Tian, Feng, edk2-devel@lists.01.org, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
Thank you Ard. I check the ARM code again.
It seems the ARM CPU driver is similar as X86 CPU driver - it installs CpuArch protocol, then SyncCacheConfig.
========================
Status = gBS->InstallMultipleProtocolInterfaces (
&mCpuHandle,
&gEfiCpuArchProtocolGuid, &mCpu,
&gVirtualUncachedPagesProtocolGuid, &gVirtualUncachedPages,
NULL
);
//
// Make sure GCD and MMU settings match. This API calls gDS->SetMemorySpaceAttributes ()
// and that calls EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes, so this code needs to go
// after the protocol is installed
//
SyncCacheConfig (&mCpu);
========================
In my update, the DxeCore registers a CPU ARCH callback function to setup PE code/data section attribute there.
At that time, the GCD attribute is not ready. As such, the cache attribute is all 0.
After SyncCacheConfig(), the rest attribute setting will carry expected GCD update.
Here is my thought:
1) How about I set default PcdImageProtectionPolicy to be 0 for ARM in DEC file? As such the ARM platform is not impacted.
2) At same time, someone from ARM can help to enhance the CPU driver to make it work.
In my opinion, ASSERT() here is not the best idea. According to PI spec V2, 12.3, EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes(), the EFI_UNSUPPORTED is expected.
Just for your reference, below is the OVMF log for X86 CPU:
It sets page attribute only before GCD sync, then set both page attribute and cache attributes after GCD sync.
============================
Loading driver at 0x00007605000 EntryPoint=0x000076066E9 CpuDxe.efi
InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 70D9410
ProtectUefiImageCommon - 0x70D91A8
- 0x0000000007605000 - 0x000000000000E000
CurrentPagingContext:
MachineType - 0x14C
PageTableBase - 0x76A9000
Attributes - 0xC0000002
InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 7611128
MemoryProtectionCpuArchProtocolNotify: ImageRecordCount - 0xA
SetUefiImageMemoryAttributes - 0x00000000076CE000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x76CE023->0x80000000076CE023
SetUefiImageMemoryAttributes - 0x00000000076CF000 - 0x0000000000016000 (0x0000000000020000)
ConvertPageEntryAttribute 0x76CF023->0x76CF021
ConvertPageEntryAttribute 0x76D0023->0x76D0021
ConvertPageEntryAttribute 0x76D1023->0x76D1021
ConvertPageEntryAttribute 0x76D2023->0x76D2021
ConvertPageEntryAttribute 0x76D3023->0x76D3021
ConvertPageEntryAttribute 0x76D4023->0x76D4021
ConvertPageEntryAttribute 0x76D5023->0x76D5021
ConvertPageEntryAttribute 0x76D6023->0x76D6021
ConvertPageEntryAttribute 0x76D7023->0x76D7021
ConvertPageEntryAttribute 0x76D8023->0x76D8021
ConvertPageEntryAttribute 0x76D9023->0x76D9021
ConvertPageEntryAttribute 0x76DA023->0x76DA021
ConvertPageEntryAttribute 0x76DB023->0x76DB021
ConvertPageEntryAttribute 0x76DC023->0x76DC021
ConvertPageEntryAttribute 0x76DD023->0x76DD021
ConvertPageEntryAttribute 0x76DE023->0x76DE021
ConvertPageEntryAttribute 0x76DF023->0x76DF021
ConvertPageEntryAttribute 0x76E0023->0x76E0021
ConvertPageEntryAttribute 0x76E1023->0x76E1021
ConvertPageEntryAttribute 0x76E2003->0x76E2001
ConvertPageEntryAttribute 0x76E3023->0x76E3021
ConvertPageEntryAttribute 0x76E4023->0x76E4021
SetUefiImageMemoryAttributes - 0x00000000076E5000 - 0x000000000000B000 (0x0000000000004000)
ConvertPageEntryAttribute 0x76E5023->0x80000000076E5023
ConvertPageEntryAttribute 0x76E6023->0x80000000076E6023
ConvertPageEntryAttribute 0x76E7023->0x80000000076E7023
ConvertPageEntryAttribute 0x76E8023->0x80000000076E8023
ConvertPageEntryAttribute 0x76E9023->0x80000000076E9023
ConvertPageEntryAttribute 0x76EA023->0x80000000076EA023
ConvertPageEntryAttribute 0x76EB063->0x80000000076EB063
ConvertPageEntryAttribute 0x76EC063->0x80000000076EC063
ConvertPageEntryAttribute 0x76ED063->0x80000000076ED063
ConvertPageEntryAttribute 0x76EE003->0x80000000076EE003
ConvertPageEntryAttribute 0x76EF003->0x80000000076EF003
SetUefiImageMemoryAttributes - 0x0000000007637000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7637063->0x8000000007637063
SetUefiImageMemoryAttributes - 0x0000000007638000 - 0x0000000000006000 (0x0000000000020000)
ConvertPageEntryAttribute 0x7638063->0x7638061
ConvertPageEntryAttribute 0x7639063->0x7639061
ConvertPageEntryAttribute 0x763A063->0x763A061
ConvertPageEntryAttribute 0x763B063->0x763B061
ConvertPageEntryAttribute 0x763C063->0x763C061
ConvertPageEntryAttribute 0x763D063->0x763D061
SetUefiImageMemoryAttributes - 0x000000000763E000 - 0x0000000000005000 (0x0000000000004000)
ConvertPageEntryAttribute 0x763E063->0x800000000763E063
ConvertPageEntryAttribute 0x763F063->0x800000000763F063
ConvertPageEntryAttribute 0x7640063->0x8000000007640063
ConvertPageEntryAttribute 0x7641063->0x8000000007641063
ConvertPageEntryAttribute 0x7642063->0x8000000007642063
SetUefiImageMemoryAttributes - 0x000000000762E000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x762E063->0x800000000762E063
SetUefiImageMemoryAttributes - 0x000000000762F000 - 0x0000000000004000 (0x0000000000020000)
ConvertPageEntryAttribute 0x762F063->0x762F061
ConvertPageEntryAttribute 0x7630063->0x7630061
ConvertPageEntryAttribute 0x7631063->0x7631061
ConvertPageEntryAttribute 0x7632063->0x7632061
SetUefiImageMemoryAttributes - 0x0000000007633000 - 0x0000000000004000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7633063->0x8000000007633063
ConvertPageEntryAttribute 0x7634063->0x8000000007634063
ConvertPageEntryAttribute 0x7635063->0x8000000007635063
ConvertPageEntryAttribute 0x7636063->0x8000000007636063
SetUefiImageMemoryAttributes - 0x000000000766C000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x766C063->0x800000000766C063
SetUefiImageMemoryAttributes - 0x000000000766D000 - 0x0000000000002000 (0x0000000000020000)
ConvertPageEntryAttribute 0x766D063->0x766D061
ConvertPageEntryAttribute 0x766E063->0x766E061
SetUefiImageMemoryAttributes - 0x000000000766F000 - 0x0000000000004000 (0x0000000000004000)
ConvertPageEntryAttribute 0x766F063->0x800000000766F063
ConvertPageEntryAttribute 0x7670063->0x8000000007670063
ConvertPageEntryAttribute 0x7671063->0x8000000007671063
ConvertPageEntryAttribute 0x7672063->0x8000000007672063
SetUefiImageMemoryAttributes - 0x0000000007666000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7666063->0x8000000007666063
SetUefiImageMemoryAttributes - 0x0000000007667000 - 0x0000000000002000 (0x0000000000020000)
ConvertPageEntryAttribute 0x7667063->0x7667061
ConvertPageEntryAttribute 0x7668063->0x7668061
SetUefiImageMemoryAttributes - 0x0000000007669000 - 0x0000000000003000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7669063->0x8000000007669063
ConvertPageEntryAttribute 0x766A063->0x800000000766A063
ConvertPageEntryAttribute 0x766B063->0x800000000766B063
SetUefiImageMemoryAttributes - 0x0000000007627000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7627063->0x8000000007627063
SetUefiImageMemoryAttributes - 0x0000000007628000 - 0x0000000000002000 (0x0000000000020000)
ConvertPageEntryAttribute 0x7628063->0x7628061
ConvertPageEntryAttribute 0x7629063->0x7629061
SetUefiImageMemoryAttributes - 0x000000000762A000 - 0x0000000000004000 (0x0000000000004000)
ConvertPageEntryAttribute 0x762A063->0x800000000762A063
ConvertPageEntryAttribute 0x762B063->0x800000000762B063
ConvertPageEntryAttribute 0x762C063->0x800000000762C063
ConvertPageEntryAttribute 0x762D063->0x800000000762D063
SetUefiImageMemoryAttributes - 0x000000000761F000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x761F063->0x800000000761F063
SetUefiImageMemoryAttributes - 0x0000000007620000 - 0x0000000000004000 (0x0000000000020000)
ConvertPageEntryAttribute 0x7620063->0x7620061
ConvertPageEntryAttribute 0x7621063->0x7621061
ConvertPageEntryAttribute 0x7622063->0x7622061
ConvertPageEntryAttribute 0x7623063->0x7623061
SetUefiImageMemoryAttributes - 0x0000000007624000 - 0x0000000000003000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7624063->0x8000000007624063
ConvertPageEntryAttribute 0x7625063->0x8000000007625063
ConvertPageEntryAttribute 0x7626063->0x8000000007626063
SetUefiImageMemoryAttributes - 0x0000000007619000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7619063->0x8000000007619063
SetUefiImageMemoryAttributes - 0x000000000761A000 - 0x0000000000002000 (0x0000000000020000)
ConvertPageEntryAttribute 0x761A063->0x761A061
ConvertPageEntryAttribute 0x761B063->0x761B061
SetUefiImageMemoryAttributes - 0x000000000761C000 - 0x0000000000003000 (0x0000000000004000)
ConvertPageEntryAttribute 0x761C063->0x800000000761C063
ConvertPageEntryAttribute 0x761D063->0x800000000761D063
ConvertPageEntryAttribute 0x761E063->0x800000000761E063
SetUefiImageMemoryAttributes - 0x0000000007613000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7613063->0x8000000007613063
SetUefiImageMemoryAttributes - 0x0000000007614000 - 0x0000000000002000 (0x0000000000020000)
ConvertPageEntryAttribute 0x7614063->0x7614061
ConvertPageEntryAttribute 0x7615063->0x7615061
SetUefiImageMemoryAttributes - 0x0000000007616000 - 0x0000000000003000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7616063->0x8000000007616063
ConvertPageEntryAttribute 0x7617063->0x8000000007617063
ConvertPageEntryAttribute 0x7618063->0x8000000007618063
SetUefiImageMemoryAttributes - 0x0000000007605000 - 0x0000000000001000 (0x0000000000004000)
ConvertPageEntryAttribute 0x7605063->0x8000000007605063
SetUefiImageMemoryAttributes - 0x0000000007606000 - 0x0000000000008000 (0x0000000000020000)
ConvertPageEntryAttribute 0x7606063->0x7606061
ConvertPageEntryAttribute 0x7607063->0x7607061
ConvertPageEntryAttribute 0x7608063->0x7608061
ConvertPageEntryAttribute 0x7609063->0x7609061
ConvertPageEntryAttribute 0x760A063->0x760A061
ConvertPageEntryAttribute 0x760B063->0x760B061
ConvertPageEntryAttribute 0x760C063->0x760C061
ConvertPageEntryAttribute 0x760D063->0x760D061
SetUefiImageMemoryAttributes - 0x000000000760E000 - 0x0000000000005000 (0x0000000000004000)
ConvertPageEntryAttribute 0x760E063->0x800000000760E063
ConvertPageEntryAttribute 0x760F063->0x800000000760F063
ConvertPageEntryAttribute 0x7610063->0x8000000007610063
ConvertPageEntryAttribute 0x7611063->0x8000000007611063
ConvertPageEntryAttribute 0x7612063->0x8000000007612063
Flushing GCD
Flushing GCD
Flushing GCD
Flushing GCD
Flushing GCD
Flushing GCD
Flushing GCD
Flushing GCD
Flushing GCD
Flushing GCD
Flushing GCD
AP Loop Mode is 1
Detect CPU count: 4
InstallProtocolInterface: 3FDDA605-A76E-4F46-AD29-12F4531B3D08 76111D4
Loading driver F6697AC4-A776-4EE1-B643-1FEFF2B615BB
InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 70D8CA8
Loading driver at 0x000075FF000 EntryPoint=0x000076000B5 IncompatiblePciDeviceSupportDxe.efi
InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 70D9010
ProtectUefiImageCommon - 0x70D8CA8
- 0x00000000075FF000 - 0x0000000000006000
SetUefiImageMemoryAttributes - 0x00000000075FF000 - 0x0000000000001000 (0x0000000000004008)
Split - 0x6CA7000
ConvertPageEntryAttribute 0x75FF063->0x80000000075FF063
SetUefiImageMemoryAttributes - 0x0000000007600000 - 0x0000000000002000 (0x0000000000020008)
ConvertPageEntryAttribute 0x7600063->0x7600061
ConvertPageEntryAttribute 0x7601063->0x7601061
SetUefiImageMemoryAttributes - 0x0000000007602000 - 0x0000000000003000 (0x0000000000004008)
ConvertPageEntryAttribute 0x7602063->0x8000000007602063
ConvertPageEntryAttribute 0x7603063->0x8000000007603063
ConvertPageEntryAttribute 0x7604063->0x8000000007604063
============================
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Thursday, February 9, 2017 1:22 AM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
On 9 February 2017 at 09:09, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> wrote:
> On 9 February 2017 at 08:49, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> wrote:
>> On 9 February 2017 at 07:43, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
>>> Hi Lindholm/Ard
>>> This version 3 contains both of your feedback before.
>>>
>>> If you can do me a favor to evaluated the impact to ARM, that will be great.
>>>
>>
>> I will take a look right away.
>>
>
> I am hitting the following assert as soon as ArmCpuDxe is loaded:
>
> Loading driver at 0x0005BE15000 EntryPoint=0x0005BE16048 ArmCpuDxe.efi
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 5B142398
> ProtectUefiImageCommon - 0x5B142140
> - 0x000000005BE15000 - 0x0000000000010000
> InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 5BE23008
> InstallProtocolInterface: AD651C7D-3C22-4DBF-92E8-38A7CDAE87B2 5BE230B0
> MemoryProtectionCpuArchProtocolNotify:
> ProtectUefiImageCommon - 0x5EF02400
> - 0x000000005EEC4000 - 0x0000000000042000
> SetUefiImageMemoryAttributes - 0x000000005EEC4000 - 0x0000000000001000
> (0x0000000000004000)
> EfiAttributeToArmAttribute: 0x4000 attributes is not supported.
> ASSERT [ArmCpuDxe]
> /home/ard/build/uefi-next/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c(220): 0
>
> The reason appears to be that the GCD memory descriptor retrieved in
> SetUefiImageMemoryAttributes() for this region has Attributes == 0
>
> I am digging further ...
Attached is the output of DEBUG_GCD
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-09 13:19 ` Yao, Jiewen
@ 2017-02-09 13:51 ` Ard Biesheuvel
2017-02-09 14:08 ` Yao, Jiewen
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-02-09 13:51 UTC (permalink / raw)
To: Yao, Jiewen
Cc: Tian, Feng, edk2-devel@lists.01.org, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
On 9 February 2017 at 13:19, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> Thank you Ard. I check the ARM code again.
>
>
>
> It seems the ARM CPU driver is similar as X86 CPU driver – it installs
> CpuArch protocol, then SyncCacheConfig.
>
> ========================
>
> Status = gBS->InstallMultipleProtocolInterfaces (
>
> &mCpuHandle,
>
> &gEfiCpuArchProtocolGuid, &mCpu,
>
> &gVirtualUncachedPagesProtocolGuid, &gVirtualUncachedPages,
>
> NULL
>
> );
>
>
>
> //
>
> // Make sure GCD and MMU settings match. This API calls
> gDS->SetMemorySpaceAttributes ()
>
> // and that calls EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes, so this code
> needs to go
>
> // after the protocol is installed
>
> //
>
> SyncCacheConfig (&mCpu);
>
> ========================
>
>
Thanks a lot for taking the time to look at this.
>
> In my update, the DxeCore registers a CPU ARCH callback function to setup PE
> code/data section attribute there.
>
> At that time, the GCD attribute is not ready. As such, the cache attribute
> is all 0.
>
>
>
> After SyncCacheConfig(), the rest attribute setting will carry expected GCD
> update.
>
So how does x86 deal with this situation if it follows the same basic approach?
>
>
> Here is my thought:
>
> 1) How about I set default PcdImageProtectionPolicy to be 0 for ARM in
> DEC file? As such the ARM platform is not impacted.
>
> 2) At same time, someone from ARM can help to enhance the CPU driver
> to make it work.
>
> In my opinion, ASSERT() here is not the best idea. According to PI spec V2,
> 12.3, EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes(), the EFI_UNSUPPORTED is
> expected.
>
>
>
>
>
>
>
> Just for your reference, below is the OVMF log for X86 CPU:
>
> It sets page attribute only before GCD sync, then set both page attribute
> and cache attributes after GCD sync.
>
> ============================
>
> Loading driver at 0x00007605000 EntryPoint=0x000076066E9 CpuDxe.efi
>
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 70D9410
>
> ProtectUefiImageCommon - 0x70D91A8
>
> - 0x0000000007605000 - 0x000000000000E000
>
> CurrentPagingContext:
>
> MachineType - 0x14C
>
> PageTableBase - 0x76A9000
>
> Attributes - 0xC0000002
>
> InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 7611128
>
> MemoryProtectionCpuArchProtocolNotify: ImageRecordCount - 0xA
>
> SetUefiImageMemoryAttributes - 0x00000000076CE000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x76CE023->0x80000000076CE023
>
> SetUefiImageMemoryAttributes - 0x00000000076CF000 - 0x0000000000016000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x76CF023->0x76CF021
>
> ConvertPageEntryAttribute 0x76D0023->0x76D0021
>
> ConvertPageEntryAttribute 0x76D1023->0x76D1021
>
> ConvertPageEntryAttribute 0x76D2023->0x76D2021
>
> ConvertPageEntryAttribute 0x76D3023->0x76D3021
>
> ConvertPageEntryAttribute 0x76D4023->0x76D4021
>
> ConvertPageEntryAttribute 0x76D5023->0x76D5021
>
> ConvertPageEntryAttribute 0x76D6023->0x76D6021
>
> ConvertPageEntryAttribute 0x76D7023->0x76D7021
>
> ConvertPageEntryAttribute 0x76D8023->0x76D8021
>
> ConvertPageEntryAttribute 0x76D9023->0x76D9021
>
> ConvertPageEntryAttribute 0x76DA023->0x76DA021
>
> ConvertPageEntryAttribute 0x76DB023->0x76DB021
>
> ConvertPageEntryAttribute 0x76DC023->0x76DC021
>
> ConvertPageEntryAttribute 0x76DD023->0x76DD021
>
> ConvertPageEntryAttribute 0x76DE023->0x76DE021
>
> ConvertPageEntryAttribute 0x76DF023->0x76DF021
>
> ConvertPageEntryAttribute 0x76E0023->0x76E0021
>
> ConvertPageEntryAttribute 0x76E1023->0x76E1021
>
> ConvertPageEntryAttribute 0x76E2003->0x76E2001
>
> ConvertPageEntryAttribute 0x76E3023->0x76E3021
>
> ConvertPageEntryAttribute 0x76E4023->0x76E4021
>
> SetUefiImageMemoryAttributes - 0x00000000076E5000 - 0x000000000000B000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x76E5023->0x80000000076E5023
>
> ConvertPageEntryAttribute 0x76E6023->0x80000000076E6023
>
> ConvertPageEntryAttribute 0x76E7023->0x80000000076E7023
>
> ConvertPageEntryAttribute 0x76E8023->0x80000000076E8023
>
> ConvertPageEntryAttribute 0x76E9023->0x80000000076E9023
>
> ConvertPageEntryAttribute 0x76EA023->0x80000000076EA023
>
> ConvertPageEntryAttribute 0x76EB063->0x80000000076EB063
>
> ConvertPageEntryAttribute 0x76EC063->0x80000000076EC063
>
> ConvertPageEntryAttribute 0x76ED063->0x80000000076ED063
>
> ConvertPageEntryAttribute 0x76EE003->0x80000000076EE003
>
> ConvertPageEntryAttribute 0x76EF003->0x80000000076EF003
>
> SetUefiImageMemoryAttributes - 0x0000000007637000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7637063->0x8000000007637063
>
> SetUefiImageMemoryAttributes - 0x0000000007638000 - 0x0000000000006000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7638063->0x7638061
>
> ConvertPageEntryAttribute 0x7639063->0x7639061
>
> ConvertPageEntryAttribute 0x763A063->0x763A061
>
> ConvertPageEntryAttribute 0x763B063->0x763B061
>
> ConvertPageEntryAttribute 0x763C063->0x763C061
>
> ConvertPageEntryAttribute 0x763D063->0x763D061
>
> SetUefiImageMemoryAttributes - 0x000000000763E000 - 0x0000000000005000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x763E063->0x800000000763E063
>
> ConvertPageEntryAttribute 0x763F063->0x800000000763F063
>
> ConvertPageEntryAttribute 0x7640063->0x8000000007640063
>
> ConvertPageEntryAttribute 0x7641063->0x8000000007641063
>
> ConvertPageEntryAttribute 0x7642063->0x8000000007642063
>
> SetUefiImageMemoryAttributes - 0x000000000762E000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x762E063->0x800000000762E063
>
> SetUefiImageMemoryAttributes - 0x000000000762F000 - 0x0000000000004000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x762F063->0x762F061
>
> ConvertPageEntryAttribute 0x7630063->0x7630061
>
> ConvertPageEntryAttribute 0x7631063->0x7631061
>
> ConvertPageEntryAttribute 0x7632063->0x7632061
>
> SetUefiImageMemoryAttributes - 0x0000000007633000 - 0x0000000000004000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7633063->0x8000000007633063
>
> ConvertPageEntryAttribute 0x7634063->0x8000000007634063
>
> ConvertPageEntryAttribute 0x7635063->0x8000000007635063
>
> ConvertPageEntryAttribute 0x7636063->0x8000000007636063
>
> SetUefiImageMemoryAttributes - 0x000000000766C000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x766C063->0x800000000766C063
>
> SetUefiImageMemoryAttributes - 0x000000000766D000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x766D063->0x766D061
>
> ConvertPageEntryAttribute 0x766E063->0x766E061
>
> SetUefiImageMemoryAttributes - 0x000000000766F000 - 0x0000000000004000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x766F063->0x800000000766F063
>
> ConvertPageEntryAttribute 0x7670063->0x8000000007670063
>
> ConvertPageEntryAttribute 0x7671063->0x8000000007671063
>
> ConvertPageEntryAttribute 0x7672063->0x8000000007672063
>
> SetUefiImageMemoryAttributes - 0x0000000007666000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7666063->0x8000000007666063
>
> SetUefiImageMemoryAttributes - 0x0000000007667000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7667063->0x7667061
>
> ConvertPageEntryAttribute 0x7668063->0x7668061
>
> SetUefiImageMemoryAttributes - 0x0000000007669000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7669063->0x8000000007669063
>
> ConvertPageEntryAttribute 0x766A063->0x800000000766A063
>
> ConvertPageEntryAttribute 0x766B063->0x800000000766B063
>
> SetUefiImageMemoryAttributes - 0x0000000007627000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7627063->0x8000000007627063
>
> SetUefiImageMemoryAttributes - 0x0000000007628000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7628063->0x7628061
>
> ConvertPageEntryAttribute 0x7629063->0x7629061
>
> SetUefiImageMemoryAttributes - 0x000000000762A000 - 0x0000000000004000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x762A063->0x800000000762A063
>
> ConvertPageEntryAttribute 0x762B063->0x800000000762B063
>
> ConvertPageEntryAttribute 0x762C063->0x800000000762C063
>
> ConvertPageEntryAttribute 0x762D063->0x800000000762D063
>
> SetUefiImageMemoryAttributes - 0x000000000761F000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x761F063->0x800000000761F063
>
> SetUefiImageMemoryAttributes - 0x0000000007620000 - 0x0000000000004000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7620063->0x7620061
>
> ConvertPageEntryAttribute 0x7621063->0x7621061
>
> ConvertPageEntryAttribute 0x7622063->0x7622061
>
> ConvertPageEntryAttribute 0x7623063->0x7623061
>
> SetUefiImageMemoryAttributes - 0x0000000007624000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7624063->0x8000000007624063
>
> ConvertPageEntryAttribute 0x7625063->0x8000000007625063
>
> ConvertPageEntryAttribute 0x7626063->0x8000000007626063
>
> SetUefiImageMemoryAttributes - 0x0000000007619000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7619063->0x8000000007619063
>
> SetUefiImageMemoryAttributes - 0x000000000761A000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x761A063->0x761A061
>
> ConvertPageEntryAttribute 0x761B063->0x761B061
>
> SetUefiImageMemoryAttributes - 0x000000000761C000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x761C063->0x800000000761C063
>
> ConvertPageEntryAttribute 0x761D063->0x800000000761D063
>
> ConvertPageEntryAttribute 0x761E063->0x800000000761E063
>
> SetUefiImageMemoryAttributes - 0x0000000007613000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7613063->0x8000000007613063
>
> SetUefiImageMemoryAttributes - 0x0000000007614000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7614063->0x7614061
>
> ConvertPageEntryAttribute 0x7615063->0x7615061
>
> SetUefiImageMemoryAttributes - 0x0000000007616000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7616063->0x8000000007616063
>
> ConvertPageEntryAttribute 0x7617063->0x8000000007617063
>
> ConvertPageEntryAttribute 0x7618063->0x8000000007618063
>
> SetUefiImageMemoryAttributes - 0x0000000007605000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7605063->0x8000000007605063
>
> SetUefiImageMemoryAttributes - 0x0000000007606000 - 0x0000000000008000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7606063->0x7606061
>
> ConvertPageEntryAttribute 0x7607063->0x7607061
>
> ConvertPageEntryAttribute 0x7608063->0x7608061
>
> ConvertPageEntryAttribute 0x7609063->0x7609061
>
> ConvertPageEntryAttribute 0x760A063->0x760A061
>
> ConvertPageEntryAttribute 0x760B063->0x760B061
>
> ConvertPageEntryAttribute 0x760C063->0x760C061
>
> ConvertPageEntryAttribute 0x760D063->0x760D061
>
> SetUefiImageMemoryAttributes - 0x000000000760E000 - 0x0000000000005000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x760E063->0x800000000760E063
>
> ConvertPageEntryAttribute 0x760F063->0x800000000760F063
>
> ConvertPageEntryAttribute 0x7610063->0x8000000007610063
>
> ConvertPageEntryAttribute 0x7611063->0x8000000007611063
>
> ConvertPageEntryAttribute 0x7612063->0x8000000007612063
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> AP Loop Mode is 1
>
> Detect CPU count: 4
>
> InstallProtocolInterface: 3FDDA605-A76E-4F46-AD29-12F4531B3D08 76111D4
>
> Loading driver F6697AC4-A776-4EE1-B643-1FEFF2B615BB
>
> InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 70D8CA8
>
> Loading driver at 0x000075FF000 EntryPoint=0x000076000B5
> IncompatiblePciDeviceSupportDxe.efi
>
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 70D9010
>
> ProtectUefiImageCommon - 0x70D8CA8
>
> - 0x00000000075FF000 - 0x0000000000006000
>
> SetUefiImageMemoryAttributes - 0x00000000075FF000 - 0x0000000000001000
> (0x0000000000004008)
>
> Split - 0x6CA7000
>
> ConvertPageEntryAttribute 0x75FF063->0x80000000075FF063
>
> SetUefiImageMemoryAttributes - 0x0000000007600000 - 0x0000000000002000
> (0x0000000000020008)
>
> ConvertPageEntryAttribute 0x7600063->0x7600061
>
> ConvertPageEntryAttribute 0x7601063->0x7601061
>
> SetUefiImageMemoryAttributes - 0x0000000007602000 - 0x0000000000003000
> (0x0000000000004008)
>
> ConvertPageEntryAttribute 0x7602063->0x8000000007602063
>
> ConvertPageEntryAttribute 0x7603063->0x8000000007603063
>
> ConvertPageEntryAttribute 0x7604063->0x8000000007604063
>
> ============================
>
>
>
>
>
>
>
>
>
>
>
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard
> Biesheuvel
> Sent: Thursday, February 9, 2017 1:22 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm
> <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
>
>
>
> On 9 February 2017 at 09:09, Ard Biesheuvel <ard.biesheuvel@linaro.org>
> wrote:
>
>> On 9 February 2017 at 08:49, Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> wrote:
>>> On 9 February 2017 at 07:43, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>>>> Hi Lindholm/Ard
>>>> This version 3 contains both of your feedback before.
>>>>
>>>> If you can do me a favor to evaluated the impact to ARM, that will be
>>>> great.
>>>>
>>>
>>> I will take a look right away.
>>>
>>
>> I am hitting the following assert as soon as ArmCpuDxe is loaded:
>>
>> Loading driver at 0x0005BE15000 EntryPoint=0x0005BE16048 ArmCpuDxe.efi
>> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 5B142398
>> ProtectUefiImageCommon - 0x5B142140
>> - 0x000000005BE15000 - 0x0000000000010000
>> InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 5BE23008
>> InstallProtocolInterface: AD651C7D-3C22-4DBF-92E8-38A7CDAE87B2 5BE230B0
>> MemoryProtectionCpuArchProtocolNotify:
>> ProtectUefiImageCommon - 0x5EF02400
>> - 0x000000005EEC4000 - 0x0000000000042000
>> SetUefiImageMemoryAttributes - 0x000000005EEC4000 - 0x0000000000001000
>> (0x0000000000004000)
>> EfiAttributeToArmAttribute: 0x4000 attributes is not supported.
>> ASSERT [ArmCpuDxe]
>> /home/ard/build/uefi-next/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c(220): 0
>>
>> The reason appears to be that the GCD memory descriptor retrieved in
>> SetUefiImageMemoryAttributes() for this region has Attributes == 0
>>
>> I am digging further ...
>
> Attached is the output of DEBUG_GCD
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-09 13:51 ` Ard Biesheuvel
@ 2017-02-09 14:08 ` Yao, Jiewen
2017-02-09 14:55 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Yao, Jiewen @ 2017-02-09 14:08 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Tian, Feng, edk2-devel@lists.01.org, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
For X86 CPU, the memory protection attribute goes to page table, the cache attribute goes to MTRR register.
Those are 2 difference resource, and can be set separately.
The high level pseudo code is below:
=======================
CacheAttribute = Attribute & CACHE_ATTRIBUTE_MASK
MemoryProtectionAttribute = Attribute & MEMORY_PROTECTION_ATTRIBUTE_MASK
If (CacheAttribute != 0) {
SetCacheAttribute(CacheAttribute)
}
SetMemoryProtectionAttribute(MemoryProtectionAttribute)
=======================
NOTE: we need compare CacheAttribute == 0, because the cache attribute is an individual mask. 0x1 means UN_CACHE, 0x8 means WRITE_BACK. 0 is meaningless.
We do not compare MemoryProtectionAttribute == 0, because 0 is a valid memory protection attribute, which means to disable protection.
Before GCD sync, the DxeCore does not know the cache attribute, so that it can only set memory attribute. The CPU driver only modifies page table based upon MemoryProtectionAttribute and keep cache attribute untouched.
Thank you
Yao Jiewen
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Thursday, February 9, 2017 5:52 AM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
On 9 February 2017 at 13:19, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
> Thank you Ard. I check the ARM code again.
>
>
>
> It seems the ARM CPU driver is similar as X86 CPU driver – it installs
> CpuArch protocol, then SyncCacheConfig.
>
> ========================
>
> Status = gBS->InstallMultipleProtocolInterfaces (
>
> &mCpuHandle,
>
> &gEfiCpuArchProtocolGuid, &mCpu,
>
> &gVirtualUncachedPagesProtocolGuid, &gVirtualUncachedPages,
>
> NULL
>
> );
>
>
>
> //
>
> // Make sure GCD and MMU settings match. This API calls
> gDS->SetMemorySpaceAttributes ()
>
> // and that calls EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes, so this code
> needs to go
>
> // after the protocol is installed
>
> //
>
> SyncCacheConfig (&mCpu);
>
> ========================
>
>
Thanks a lot for taking the time to look at this.
>
> In my update, the DxeCore registers a CPU ARCH callback function to setup PE
> code/data section attribute there.
>
> At that time, the GCD attribute is not ready. As such, the cache attribute
> is all 0.
>
>
>
> After SyncCacheConfig(), the rest attribute setting will carry expected GCD
> update.
>
So how does x86 deal with this situation if it follows the same basic approach?
>
>
> Here is my thought:
>
> 1) How about I set default PcdImageProtectionPolicy to be 0 for ARM in
> DEC file? As such the ARM platform is not impacted.
>
> 2) At same time, someone from ARM can help to enhance the CPU driver
> to make it work.
>
> In my opinion, ASSERT() here is not the best idea. According to PI spec V2,
> 12.3, EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes(), the EFI_UNSUPPORTED is
> expected.
>
>
>
>
>
>
>
> Just for your reference, below is the OVMF log for X86 CPU:
>
> It sets page attribute only before GCD sync, then set both page attribute
> and cache attributes after GCD sync.
>
> ============================
>
> Loading driver at 0x00007605000 EntryPoint=0x000076066E9 CpuDxe.efi
>
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 70D9410
>
> ProtectUefiImageCommon - 0x70D91A8
>
> - 0x0000000007605000 - 0x000000000000E000
>
> CurrentPagingContext:
>
> MachineType - 0x14C
>
> PageTableBase - 0x76A9000
>
> Attributes - 0xC0000002
>
> InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 7611128
>
> MemoryProtectionCpuArchProtocolNotify: ImageRecordCount - 0xA
>
> SetUefiImageMemoryAttributes - 0x00000000076CE000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x76CE023->0x80000000076CE023
>
> SetUefiImageMemoryAttributes - 0x00000000076CF000 - 0x0000000000016000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x76CF023->0x76CF021
>
> ConvertPageEntryAttribute 0x76D0023->0x76D0021
>
> ConvertPageEntryAttribute 0x76D1023->0x76D1021
>
> ConvertPageEntryAttribute 0x76D2023->0x76D2021
>
> ConvertPageEntryAttribute 0x76D3023->0x76D3021
>
> ConvertPageEntryAttribute 0x76D4023->0x76D4021
>
> ConvertPageEntryAttribute 0x76D5023->0x76D5021
>
> ConvertPageEntryAttribute 0x76D6023->0x76D6021
>
> ConvertPageEntryAttribute 0x76D7023->0x76D7021
>
> ConvertPageEntryAttribute 0x76D8023->0x76D8021
>
> ConvertPageEntryAttribute 0x76D9023->0x76D9021
>
> ConvertPageEntryAttribute 0x76DA023->0x76DA021
>
> ConvertPageEntryAttribute 0x76DB023->0x76DB021
>
> ConvertPageEntryAttribute 0x76DC023->0x76DC021
>
> ConvertPageEntryAttribute 0x76DD023->0x76DD021
>
> ConvertPageEntryAttribute 0x76DE023->0x76DE021
>
> ConvertPageEntryAttribute 0x76DF023->0x76DF021
>
> ConvertPageEntryAttribute 0x76E0023->0x76E0021
>
> ConvertPageEntryAttribute 0x76E1023->0x76E1021
>
> ConvertPageEntryAttribute 0x76E2003->0x76E2001
>
> ConvertPageEntryAttribute 0x76E3023->0x76E3021
>
> ConvertPageEntryAttribute 0x76E4023->0x76E4021
>
> SetUefiImageMemoryAttributes - 0x00000000076E5000 - 0x000000000000B000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x76E5023->0x80000000076E5023
>
> ConvertPageEntryAttribute 0x76E6023->0x80000000076E6023
>
> ConvertPageEntryAttribute 0x76E7023->0x80000000076E7023
>
> ConvertPageEntryAttribute 0x76E8023->0x80000000076E8023
>
> ConvertPageEntryAttribute 0x76E9023->0x80000000076E9023
>
> ConvertPageEntryAttribute 0x76EA023->0x80000000076EA023
>
> ConvertPageEntryAttribute 0x76EB063->0x80000000076EB063
>
> ConvertPageEntryAttribute 0x76EC063->0x80000000076EC063
>
> ConvertPageEntryAttribute 0x76ED063->0x80000000076ED063
>
> ConvertPageEntryAttribute 0x76EE003->0x80000000076EE003
>
> ConvertPageEntryAttribute 0x76EF003->0x80000000076EF003
>
> SetUefiImageMemoryAttributes - 0x0000000007637000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7637063->0x8000000007637063
>
> SetUefiImageMemoryAttributes - 0x0000000007638000 - 0x0000000000006000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7638063->0x7638061
>
> ConvertPageEntryAttribute 0x7639063->0x7639061
>
> ConvertPageEntryAttribute 0x763A063->0x763A061
>
> ConvertPageEntryAttribute 0x763B063->0x763B061
>
> ConvertPageEntryAttribute 0x763C063->0x763C061
>
> ConvertPageEntryAttribute 0x763D063->0x763D061
>
> SetUefiImageMemoryAttributes - 0x000000000763E000 - 0x0000000000005000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x763E063->0x800000000763E063
>
> ConvertPageEntryAttribute 0x763F063->0x800000000763F063
>
> ConvertPageEntryAttribute 0x7640063->0x8000000007640063
>
> ConvertPageEntryAttribute 0x7641063->0x8000000007641063
>
> ConvertPageEntryAttribute 0x7642063->0x8000000007642063
>
> SetUefiImageMemoryAttributes - 0x000000000762E000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x762E063->0x800000000762E063
>
> SetUefiImageMemoryAttributes - 0x000000000762F000 - 0x0000000000004000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x762F063->0x762F061
>
> ConvertPageEntryAttribute 0x7630063->0x7630061
>
> ConvertPageEntryAttribute 0x7631063->0x7631061
>
> ConvertPageEntryAttribute 0x7632063->0x7632061
>
> SetUefiImageMemoryAttributes - 0x0000000007633000 - 0x0000000000004000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7633063->0x8000000007633063
>
> ConvertPageEntryAttribute 0x7634063->0x8000000007634063
>
> ConvertPageEntryAttribute 0x7635063->0x8000000007635063
>
> ConvertPageEntryAttribute 0x7636063->0x8000000007636063
>
> SetUefiImageMemoryAttributes - 0x000000000766C000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x766C063->0x800000000766C063
>
> SetUefiImageMemoryAttributes - 0x000000000766D000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x766D063->0x766D061
>
> ConvertPageEntryAttribute 0x766E063->0x766E061
>
> SetUefiImageMemoryAttributes - 0x000000000766F000 - 0x0000000000004000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x766F063->0x800000000766F063
>
> ConvertPageEntryAttribute 0x7670063->0x8000000007670063
>
> ConvertPageEntryAttribute 0x7671063->0x8000000007671063
>
> ConvertPageEntryAttribute 0x7672063->0x8000000007672063
>
> SetUefiImageMemoryAttributes - 0x0000000007666000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7666063->0x8000000007666063
>
> SetUefiImageMemoryAttributes - 0x0000000007667000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7667063->0x7667061
>
> ConvertPageEntryAttribute 0x7668063->0x7668061
>
> SetUefiImageMemoryAttributes - 0x0000000007669000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7669063->0x8000000007669063
>
> ConvertPageEntryAttribute 0x766A063->0x800000000766A063
>
> ConvertPageEntryAttribute 0x766B063->0x800000000766B063
>
> SetUefiImageMemoryAttributes - 0x0000000007627000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7627063->0x8000000007627063
>
> SetUefiImageMemoryAttributes - 0x0000000007628000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7628063->0x7628061
>
> ConvertPageEntryAttribute 0x7629063->0x7629061
>
> SetUefiImageMemoryAttributes - 0x000000000762A000 - 0x0000000000004000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x762A063->0x800000000762A063
>
> ConvertPageEntryAttribute 0x762B063->0x800000000762B063
>
> ConvertPageEntryAttribute 0x762C063->0x800000000762C063
>
> ConvertPageEntryAttribute 0x762D063->0x800000000762D063
>
> SetUefiImageMemoryAttributes - 0x000000000761F000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x761F063->0x800000000761F063
>
> SetUefiImageMemoryAttributes - 0x0000000007620000 - 0x0000000000004000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7620063->0x7620061
>
> ConvertPageEntryAttribute 0x7621063->0x7621061
>
> ConvertPageEntryAttribute 0x7622063->0x7622061
>
> ConvertPageEntryAttribute 0x7623063->0x7623061
>
> SetUefiImageMemoryAttributes - 0x0000000007624000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7624063->0x8000000007624063
>
> ConvertPageEntryAttribute 0x7625063->0x8000000007625063
>
> ConvertPageEntryAttribute 0x7626063->0x8000000007626063
>
> SetUefiImageMemoryAttributes - 0x0000000007619000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7619063->0x8000000007619063
>
> SetUefiImageMemoryAttributes - 0x000000000761A000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x761A063->0x761A061
>
> ConvertPageEntryAttribute 0x761B063->0x761B061
>
> SetUefiImageMemoryAttributes - 0x000000000761C000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x761C063->0x800000000761C063
>
> ConvertPageEntryAttribute 0x761D063->0x800000000761D063
>
> ConvertPageEntryAttribute 0x761E063->0x800000000761E063
>
> SetUefiImageMemoryAttributes - 0x0000000007613000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7613063->0x8000000007613063
>
> SetUefiImageMemoryAttributes - 0x0000000007614000 - 0x0000000000002000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7614063->0x7614061
>
> ConvertPageEntryAttribute 0x7615063->0x7615061
>
> SetUefiImageMemoryAttributes - 0x0000000007616000 - 0x0000000000003000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7616063->0x8000000007616063
>
> ConvertPageEntryAttribute 0x7617063->0x8000000007617063
>
> ConvertPageEntryAttribute 0x7618063->0x8000000007618063
>
> SetUefiImageMemoryAttributes - 0x0000000007605000 - 0x0000000000001000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x7605063->0x8000000007605063
>
> SetUefiImageMemoryAttributes - 0x0000000007606000 - 0x0000000000008000
> (0x0000000000020000)
>
> ConvertPageEntryAttribute 0x7606063->0x7606061
>
> ConvertPageEntryAttribute 0x7607063->0x7607061
>
> ConvertPageEntryAttribute 0x7608063->0x7608061
>
> ConvertPageEntryAttribute 0x7609063->0x7609061
>
> ConvertPageEntryAttribute 0x760A063->0x760A061
>
> ConvertPageEntryAttribute 0x760B063->0x760B061
>
> ConvertPageEntryAttribute 0x760C063->0x760C061
>
> ConvertPageEntryAttribute 0x760D063->0x760D061
>
> SetUefiImageMemoryAttributes - 0x000000000760E000 - 0x0000000000005000
> (0x0000000000004000)
>
> ConvertPageEntryAttribute 0x760E063->0x800000000760E063
>
> ConvertPageEntryAttribute 0x760F063->0x800000000760F063
>
> ConvertPageEntryAttribute 0x7610063->0x8000000007610063
>
> ConvertPageEntryAttribute 0x7611063->0x8000000007611063
>
> ConvertPageEntryAttribute 0x7612063->0x8000000007612063
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> Flushing GCD
>
> AP Loop Mode is 1
>
> Detect CPU count: 4
>
> InstallProtocolInterface: 3FDDA605-A76E-4F46-AD29-12F4531B3D08 76111D4
>
> Loading driver F6697AC4-A776-4EE1-B643-1FEFF2B615BB
>
> InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 70D8CA8
>
> Loading driver at 0x000075FF000 EntryPoint=0x000076000B5
> IncompatiblePciDeviceSupportDxe.efi
>
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 70D9010
>
> ProtectUefiImageCommon - 0x70D8CA8
>
> - 0x00000000075FF000 - 0x0000000000006000
>
> SetUefiImageMemoryAttributes - 0x00000000075FF000 - 0x0000000000001000
> (0x0000000000004008)
>
> Split - 0x6CA7000
>
> ConvertPageEntryAttribute 0x75FF063->0x80000000075FF063
>
> SetUefiImageMemoryAttributes - 0x0000000007600000 - 0x0000000000002000
> (0x0000000000020008)
>
> ConvertPageEntryAttribute 0x7600063->0x7600061
>
> ConvertPageEntryAttribute 0x7601063->0x7601061
>
> SetUefiImageMemoryAttributes - 0x0000000007602000 - 0x0000000000003000
> (0x0000000000004008)
>
> ConvertPageEntryAttribute 0x7602063->0x8000000007602063
>
> ConvertPageEntryAttribute 0x7603063->0x8000000007603063
>
> ConvertPageEntryAttribute 0x7604063->0x8000000007604063
>
> ============================
>
>
>
>
>
>
>
>
>
>
>
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard
> Biesheuvel
> Sent: Thursday, February 9, 2017 1:22 AM
> To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Leif Lindholm
> <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>;
> Fan, Jeff <jeff.fan@intel.com<mailto:jeff.fan@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
>
>
>
> On 9 February 2017 at 09:09, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>
> wrote:
>
>> On 9 February 2017 at 08:49, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>
>> wrote:
>>> On 9 February 2017 at 07:43, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
>>>> Hi Lindholm/Ard
>>>> This version 3 contains both of your feedback before.
>>>>
>>>> If you can do me a favor to evaluated the impact to ARM, that will be
>>>> great.
>>>>
>>>
>>> I will take a look right away.
>>>
>>
>> I am hitting the following assert as soon as ArmCpuDxe is loaded:
>>
>> Loading driver at 0x0005BE15000 EntryPoint=0x0005BE16048 ArmCpuDxe.efi
>> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 5B142398
>> ProtectUefiImageCommon - 0x5B142140
>> - 0x000000005BE15000 - 0x0000000000010000
>> InstallProtocolInterface: 26BACCB1-6F42-11D4-BCE7-0080C73C8881 5BE23008
>> InstallProtocolInterface: AD651C7D-3C22-4DBF-92E8-38A7CDAE87B2 5BE230B0
>> MemoryProtectionCpuArchProtocolNotify:
>> ProtectUefiImageCommon - 0x5EF02400
>> - 0x000000005EEC4000 - 0x0000000000042000
>> SetUefiImageMemoryAttributes - 0x000000005EEC4000 - 0x0000000000001000
>> (0x0000000000004000)
>> EfiAttributeToArmAttribute: 0x4000 attributes is not supported.
>> ASSERT [ArmCpuDxe]
>> /home/ard/build/uefi-next/ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c(220): 0
>>
>> The reason appears to be that the GCD memory descriptor retrieved in
>> SetUefiImageMemoryAttributes() for this region has Attributes == 0
>>
>> I am digging further ...
>
> Attached is the output of DEBUG_GCD
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-09 14:08 ` Yao, Jiewen
@ 2017-02-09 14:55 ` Ard Biesheuvel
2017-02-09 15:27 ` Yao, Jiewen
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-02-09 14:55 UTC (permalink / raw)
To: Yao, Jiewen
Cc: Tian, Feng, edk2-devel@lists.01.org, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
On 9 February 2017 at 14:08, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> For X86 CPU, the memory protection attribute goes to page table, the cache
> attribute goes to MTRR register.
>
> Those are 2 difference resource, and can be set separately.
>
>
>
> The high level pseudo code is below:
>
> =======================
>
> CacheAttribute = Attribute & CACHE_ATTRIBUTE_MASK
>
> MemoryProtectionAttribute = Attribute & MEMORY_PROTECTION_ATTRIBUTE_MASK
>
> If (CacheAttribute != 0) {
>
> SetCacheAttribute(CacheAttribute)
>
> }
>
> SetMemoryProtectionAttribute(MemoryProtectionAttribute)
>
> =======================
>
>
>
> NOTE: we need compare CacheAttribute == 0, because the cache attribute is an
> individual mask. 0x1 means UN_CACHE, 0x8 means WRITE_BACK. 0 is meaningless.
>
> We do not compare MemoryProtectionAttribute == 0, because 0 is a valid
> memory protection attribute, which means to disable protection.
>
>
>
> Before GCD sync, the DxeCore does not know the cache attribute, so that it
> can only set memory attribute. The CPU driver only modifies page table based
> upon MemoryProtectionAttribute and keep cache attribute untouched.
>
OK, I think we should be able to work around this, although it is not
pretty. I will send it out as a separate patch.
I do have one other question though: would it be possible to round up
the end of the image to page size in this code? (in
SetUefiImageProtectionAttributes()) Otherwise, we may end up calling
SetMemoryAttributes() with a length that is not page aligned, which
hits an EFI_UNSUPPORTED on ARM. Given that the PE/COFF loader always
allocates full pages, we know the space after the image is not used,
so it is possible (and even more secure!) to clear the exec bit on it.
"""
//
// Last DATA
//
ASSERT (CurrentBase <= ImageEnd);
if (CurrentBase < ImageEnd) {
//
// DATA
//
if (Protect) {
Attribute = EFI_MEMORY_XP;
} else {
Attribute = 0;
}
SetUefiImageMemoryAttributes (
CurrentBase,
ImageEnd - CurrentBase,
Attribute
);
}
"""
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-09 14:55 ` Ard Biesheuvel
@ 2017-02-09 15:27 ` Yao, Jiewen
2017-02-09 15:28 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Yao, Jiewen @ 2017-02-09 15:27 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Tian, Feng, edk2-devel@lists.01.org, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
1) That is great. I appreciate your quick response and help.
I will drop my patch for ARM 2/4, and wait for yours.
2) For ImageEnd alignment issue, I agree with you.
I plan to round up with:
ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize, SectionAlignment);
before SetUefiImageProtectionAttributes (ImageRecord, Protect);
I will update it at V4.
Thank you
Yao Jiewen
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Thursday, February 9, 2017 6:56 AM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
On 9 February 2017 at 14:08, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
> For X86 CPU, the memory protection attribute goes to page table, the cache
> attribute goes to MTRR register.
>
> Those are 2 difference resource, and can be set separately.
>
>
>
> The high level pseudo code is below:
>
> =======================
>
> CacheAttribute = Attribute & CACHE_ATTRIBUTE_MASK
>
> MemoryProtectionAttribute = Attribute & MEMORY_PROTECTION_ATTRIBUTE_MASK
>
> If (CacheAttribute != 0) {
>
> SetCacheAttribute(CacheAttribute)
>
> }
>
> SetMemoryProtectionAttribute(MemoryProtectionAttribute)
>
> =======================
>
>
>
> NOTE: we need compare CacheAttribute == 0, because the cache attribute is an
> individual mask. 0x1 means UN_CACHE, 0x8 means WRITE_BACK. 0 is meaningless.
>
> We do not compare MemoryProtectionAttribute == 0, because 0 is a valid
> memory protection attribute, which means to disable protection.
>
>
>
> Before GCD sync, the DxeCore does not know the cache attribute, so that it
> can only set memory attribute. The CPU driver only modifies page table based
> upon MemoryProtectionAttribute and keep cache attribute untouched.
>
OK, I think we should be able to work around this, although it is not
pretty. I will send it out as a separate patch.
I do have one other question though: would it be possible to round up
the end of the image to page size in this code? (in
SetUefiImageProtectionAttributes()) Otherwise, we may end up calling
SetMemoryAttributes() with a length that is not page aligned, which
hits an EFI_UNSUPPORTED on ARM. Given that the PE/COFF loader always
allocates full pages, we know the space after the image is not used,
so it is possible (and even more secure!) to clear the exec bit on it.
"""
//
// Last DATA
//
ASSERT (CurrentBase <= ImageEnd);
if (CurrentBase < ImageEnd) {
//
// DATA
//
if (Protect) {
Attribute = EFI_MEMORY_XP;
} else {
Attribute = 0;
}
SetUefiImageMemoryAttributes (
CurrentBase,
ImageEnd - CurrentBase,
Attribute
);
}
"""
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-09 15:27 ` Yao, Jiewen
@ 2017-02-09 15:28 ` Ard Biesheuvel
2017-02-09 16:21 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-02-09 15:28 UTC (permalink / raw)
To: Yao, Jiewen
Cc: Tian, Feng, edk2-devel@lists.01.org, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
On 9 February 2017 at 15:27, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> 1) That is great. I appreciate your quick response and help.
>
> I will drop my patch for ARM 2/4, and wait for yours.
>
OK
>
>
> 2) For ImageEnd alignment issue, I agree with you.
>
> I plan to round up with:
>
> ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
> SectionAlignment);
>
> before SetUefiImageProtectionAttributes (ImageRecord, Protect);
>
Great, that should fix my issue!
Thanks,
Ard.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-09 15:28 ` Ard Biesheuvel
@ 2017-02-09 16:21 ` Ard Biesheuvel
2017-02-09 16:29 ` Yao, Jiewen
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-02-09 16:21 UTC (permalink / raw)
To: Yao, Jiewen
Cc: Tian, Feng, edk2-devel@lists.01.org, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
On 9 February 2017 at 15:28, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 9 February 2017 at 15:27, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>> 1) That is great. I appreciate your quick response and help.
>>
>> I will drop my patch for ARM 2/4, and wait for yours.
>>
>
> OK
>
>>
>>
>> 2) For ImageEnd alignment issue, I agree with you.
>>
>> I plan to round up with:
>>
>> ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
>> SectionAlignment);
>>
>> before SetUefiImageProtectionAttributes (ImageRecord, Protect);
>>
>
> Great, that should fix my issue!
>
Actually, does that still work correctly with 64 KB section alignment?
I don't think the PE/COFF loader rounds up the allocation to section
alignment, does it?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-09 16:21 ` Ard Biesheuvel
@ 2017-02-09 16:29 ` Yao, Jiewen
2017-02-09 16:30 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Yao, Jiewen @ 2017-02-09 16:29 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Tian, Feng, edk2-devel@lists.01.org, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
Very good point.
Can ARCH64 set 4K paging for 64K aligned runtime memory?
If yes, how about we use
“ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize, EFI_PAGE_SIZE);”
Thank you
Yao Jiewen
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Thursday, February 9, 2017 8:21 AM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
On 9 February 2017 at 15:28, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> wrote:
> On 9 February 2017 at 15:27, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
>> 1) That is great. I appreciate your quick response and help.
>>
>> I will drop my patch for ARM 2/4, and wait for yours.
>>
>
> OK
>
>>
>>
>> 2) For ImageEnd alignment issue, I agree with you.
>>
>> I plan to round up with:
>>
>> ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
>> SectionAlignment);
>>
>> before SetUefiImageProtectionAttributes (ImageRecord, Protect);
>>
>
> Great, that should fix my issue!
>
Actually, does that still work correctly with 64 KB section alignment?
I don't think the PE/COFF loader rounds up the allocation to section
alignment, does it?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-09 16:29 ` Yao, Jiewen
@ 2017-02-09 16:30 ` Ard Biesheuvel
2017-02-09 16:48 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-02-09 16:30 UTC (permalink / raw)
To: Yao, Jiewen
Cc: Tian, Feng, edk2-devel@lists.01.org, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
On 9 February 2017 at 16:29, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> Very good point.
>
> Can ARCH64 set 4K paging for 64K aligned runtime memory?
>
UEFI always uses 4 KB, but the OS may use 64 KB, so to create the
virtual address map it needs the runtime regions to be 64 KB aligned.
>
>
> If yes, how about we use
>
> “ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
> EFI_PAGE_SIZE);”
>
Perfect, thanks.
>
>
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Thursday, February 9, 2017 8:21 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm
> <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
>
>
>
> On 9 February 2017 at 15:28, Ard Biesheuvel <ard.biesheuvel@linaro.org>
> wrote:
>
>
>> On 9 February 2017 at 15:27, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>>> 1) That is great. I appreciate your quick response and help.
>>>
>>> I will drop my patch for ARM 2/4, and wait for yours.
>>>
>>
>> OK
>>
>>>
>>>
>>> 2) For ImageEnd alignment issue, I agree with you.
>>>
>>> I plan to round up with:
>>>
>>> ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
>>> SectionAlignment);
>>>
>>> before SetUefiImageProtectionAttributes (ImageRecord, Protect);
>>>
>>
>> Great, that should fix my issue!
>>
>
> Actually, does that still work correctly with 64 KB section alignment?
> I don't think the PE/COFF loader rounds up the allocation to section
> alignment, does it?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-09 16:30 ` Ard Biesheuvel
@ 2017-02-09 16:48 ` Ard Biesheuvel
2017-02-10 2:26 ` Yao, Jiewen
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-02-09 16:48 UTC (permalink / raw)
To: Yao, Jiewen
Cc: Tian, Feng, edk2-devel@lists.01.org, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
On 9 February 2017 at 16:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 9 February 2017 at 16:29, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>> Very good point.
>>
>> Can ARCH64 set 4K paging for 64K aligned runtime memory?
>>
>
> UEFI always uses 4 KB, but the OS may use 64 KB, so to create the
> virtual address map it needs the runtime regions to be 64 KB aligned.
>
>>
>>
>> If yes, how about we use
>>
>> “ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
>> EFI_PAGE_SIZE);”
>>
>
Another question: did you try SetVirtualAddressMap()? It looks like we
need to lift read-only permissions to allow the runtime PE/COFF
relocation to apply the fixups
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-09 16:48 ` Ard Biesheuvel
@ 2017-02-10 2:26 ` Yao, Jiewen
2017-02-10 6:34 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Yao, Jiewen @ 2017-02-10 2:26 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Tian, Feng, edk2-devel@lists.01.org, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
Very good question.
1) Yes, I did test UEFI OS boot, which is mentioned in V1 summary:
======
Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
======
2) Star helps double confirm that OS already takes over the control of page table on SetVirtualAddressMap().
See below log on UEFI Win10.
======
DXEIPL CR3 0x88140000
RUNTIMEDXE CR3 0x1AB000
======
Thank you
Yao Jiewen
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Thursday, February 9, 2017 8:48 AM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
On 9 February 2017 at 16:30, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> wrote:
> On 9 February 2017 at 16:29, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
>> Very good point.
>>
>> Can ARCH64 set 4K paging for 64K aligned runtime memory?
>>
>
> UEFI always uses 4 KB, but the OS may use 64 KB, so to create the
> virtual address map it needs the runtime regions to be 64 KB aligned.
>
>>
>>
>> If yes, how about we use
>>
>> “ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
>> EFI_PAGE_SIZE);”
>>
>
Another question: did you try SetVirtualAddressMap()? It looks like we
need to lift read-only permissions to allow the runtime PE/COFF
relocation to apply the fixups
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-10 2:26 ` Yao, Jiewen
@ 2017-02-10 6:34 ` Ard Biesheuvel
2017-02-10 6:41 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-02-10 6:34 UTC (permalink / raw)
To: Yao, Jiewen
Cc: Tian, Feng, edk2-devel@lists.01.org, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
> On 10 Feb 2017, at 02:26, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>
> Very good question.
>
> 1) Yes, I did test UEFI OS boot, which is mentioned in V1 summary:
> ======
> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
> ======
>
> 2) Star helps double confirm that OS already takes over the control of page table on SetVirtualAddressMap().
> See below log on UEFI Win10.
> ======
> DXEIPL CR3 0x88140000
> RUNTIMEDXE CR3 0x1AB000
> ======
>
Not on AArch64/ARM linux, and the spec does not mandate it, so we need to deal with this imo
> Thank you
> Yao Jiewen
>
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
> Sent: Thursday, February 9, 2017 8:48 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
>
> On 9 February 2017 at 16:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > On 9 February 2017 at 16:29, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> >> Very good point.
> >>
> >> Can ARCH64 set 4K paging for 64K aligned runtime memory?
> >>
> >
> > UEFI always uses 4 KB, but the OS may use 64 KB, so to create the
> > virtual address map it needs the runtime regions to be 64 KB aligned.
> >
> >>
> >>
> >> If yes, how about we use
> >>
> >> “ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
> >> EFI_PAGE_SIZE);”
> >>
> >
>
> Another question: did you try SetVirtualAddressMap()? It looks like we
> need to lift read-only permissions to allow the runtime PE/COFF
> relocation to apply the fixups
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-10 6:34 ` Ard Biesheuvel
@ 2017-02-10 6:41 ` Ard Biesheuvel
2017-02-10 11:32 ` Yao, Jiewen
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-02-10 6:41 UTC (permalink / raw)
To: Yao, Jiewen
Cc: Tian, Feng, edk2-devel@lists.01.org, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
> On 10 Feb 2017, at 06:34, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>
>
>> On 10 Feb 2017, at 02:26, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>>
>> Very good question.
>>
>> 1) Yes, I did test UEFI OS boot, which is mentioned in V1 summary:
>> ======
>> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
>> ======
>>
>> 2) Star helps double confirm that OS already takes over the control of page table on SetVirtualAddressMap().
>> See below log on UEFI Win10.
>> ======
>> DXEIPL CR3 0x88140000
>> RUNTIMEDXE CR3 0x1AB000
>> ======
>>
>
> Not on AArch64/ARM linux, and the spec does not mandate it, so we need to deal with this imo
>
I think we should probably undo the protections for runtime drivers in EBS()
>> Thank you
>> Yao Jiewen
>>
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
>> Sent: Thursday, February 9, 2017 8:48 AM
>> To: Yao, Jiewen <jiewen.yao@intel.com>
>> Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
>> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
>>
>> On 9 February 2017 at 16:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> > On 9 February 2017 at 16:29, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>> >> Very good point.
>> >>
>> >> Can ARCH64 set 4K paging for 64K aligned runtime memory?
>> >>
>> >
>> > UEFI always uses 4 KB, but the OS may use 64 KB, so to create the
>> > virtual address map it needs the runtime regions to be 64 KB aligned.
>> >
>> >>
>> >>
>> >> If yes, how about we use
>> >>
>> >> “ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
>> >> EFI_PAGE_SIZE);”
>> >>
>> >
>>
>> Another question: did you try SetVirtualAddressMap()? It looks like we
>> need to lift read-only permissions to allow the runtime PE/COFF
>> relocation to apply the fixups
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-10 6:41 ` Ard Biesheuvel
@ 2017-02-10 11:32 ` Yao, Jiewen
2017-02-10 11:42 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Yao, Jiewen @ 2017-02-10 11:32 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Tian, Feng, edk2-devel@lists.01.org, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
Good to learn that ARM/ARCH64 behavior. That is interesting.
Yes, if that is the case, we need figure out a way to make it work.
IMHO, “Undo the protection” directly is also risky.
Maybe the protection is used or setup by OS purposely before EBS (WoW. ☺). Unprotecting them in BIOS might break the OS expectation.
Would you please provide more info on ARM Linux? When the OSLoader or OS takes over the page table? After EBS?
Thank you
Yao Jiewen
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Thursday, February 9, 2017 10:42 PM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
> On 10 Feb 2017, at 06:34, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> wrote:
>
>
>
>> On 10 Feb 2017, at 02:26, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
>>
>> Very good question.
>>
>> 1) Yes, I did test UEFI OS boot, which is mentioned in V1 summary:
>> ======
>> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
>> ======
>>
>> 2) Star helps double confirm that OS already takes over the control of page table on SetVirtualAddressMap().
>> See below log on UEFI Win10.
>> ======
>> DXEIPL CR3 0x88140000
>> RUNTIMEDXE CR3 0x1AB000
>> ======
>>
>
> Not on AArch64/ARM linux, and the spec does not mandate it, so we need to deal with this imo
>
I think we should probably undo the protections for runtime drivers in EBS()
>> Thank you
>> Yao Jiewen
>>
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
>> Sent: Thursday, February 9, 2017 8:48 AM
>> To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
>> Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Leif Lindholm <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Fan, Jeff <jeff.fan@intel.com<mailto:jeff.fan@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
>> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
>>
>> On 9 February 2017 at 16:30, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> wrote:
>> > On 9 February 2017 at 16:29, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
>> >> Very good point.
>> >>
>> >> Can ARCH64 set 4K paging for 64K aligned runtime memory?
>> >>
>> >
>> > UEFI always uses 4 KB, but the OS may use 64 KB, so to create the
>> > virtual address map it needs the runtime regions to be 64 KB aligned.
>> >
>> >>
>> >>
>> >> If yes, how about we use
>> >>
>> >> “ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
>> >> EFI_PAGE_SIZE);”
>> >>
>> >
>>
>> Another question: did you try SetVirtualAddressMap()? It looks like we
>> need to lift read-only permissions to allow the runtime PE/COFF
>> relocation to apply the fixups
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
>> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-10 11:32 ` Yao, Jiewen
@ 2017-02-10 11:42 ` Ard Biesheuvel
2017-02-10 12:59 ` Yao, Jiewen
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2017-02-10 11:42 UTC (permalink / raw)
To: Yao, Jiewen
Cc: Tian, Feng, edk2-devel@lists.01.org, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
> On 10 Feb 2017, at 11:32, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>
> Good to learn that ARM/ARCH64 behavior. That is interesting.
>
> Yes, if that is the case, we need figure out a way to make it work.
>
> IMHO, “Undo the protection” directly is also risky.
> Maybe the protection is used or setup by OS purposely before EBS (WoW. J). Unprotecting them in BIOS might break the OS expectation.
>
>
> Would you please provide more info on ARM Linux? When the OSLoader or OS takes over the page table? After EBS?
On ARM, we call SetVirtualAddressMap directly after EBS, in the OS loader. The caches are disabled much later. It is the OS itself that reenables the MMU, and install the UEFI virtual mapping as per-process mapping, which is only live when a runtime services call is in progress.
Therefore, we use a low virtual mapping for runtime services, which may conflict with the 1:1 mapping, so we never map any uefi regions 1:1 under the os.
This implies that the virtual mapping we install is not yet live when we install it, and so the easiest way to do that is to install it immediately after ebs, when the firmware's 1:1 mapping is still live.
> Thank you
> Yao Jiewen
>
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
> Sent: Thursday, February 9, 2017 10:42 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
>
>
>
> > On 10 Feb 2017, at 06:34, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> >
> >
> >> On 10 Feb 2017, at 02:26, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> >>
> >> Very good question.
> >>
> >> 1) Yes, I did test UEFI OS boot, which is mentioned in V1 summary:
> >> ======
> >> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
> >> ======
> >>
> >> 2) Star helps double confirm that OS already takes over the control of page table on SetVirtualAddressMap().
> >> See below log on UEFI Win10.
> >> ======
> >> DXEIPL CR3 0x88140000
> >> RUNTIMEDXE CR3 0x1AB000
> >> ======
> >>
> >
> > Not on AArch64/ARM linux, and the spec does not mandate it, so we need to deal with this imo
> >
>
> I think we should probably undo the protections for runtime drivers in EBS()
>
>
> >> Thank you
> >> Yao Jiewen
> >>
> >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
> >> Sent: Thursday, February 9, 2017 8:48 AM
> >> To: Yao, Jiewen <jiewen.yao@intel.com>
> >> Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
> >> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
> >>
> >> On 9 February 2017 at 16:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> > On 9 February 2017 at 16:29, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> >> >> Very good point.
> >> >>
> >> >> Can ARCH64 set 4K paging for 64K aligned runtime memory?
> >> >>
> >> >
> >> > UEFI always uses 4 KB, but the OS may use 64 KB, so to create the
> >> > virtual address map it needs the runtime regions to be 64 KB aligned.
> >> >
> >> >>
> >> >>
> >> >> If yes, how about we use
> >> >>
> >> >> “ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
> >> >> EFI_PAGE_SIZE);”
> >> >>
> >> >
> >>
> >> Another question: did you try SetVirtualAddressMap()? It looks like we
> >> need to lift read-only permissions to allow the runtime PE/COFF
> >> relocation to apply the fixups
> >> _______________________________________________
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-10 11:42 ` Ard Biesheuvel
@ 2017-02-10 12:59 ` Yao, Jiewen
2017-02-10 14:16 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Yao, Jiewen @ 2017-02-10 12:59 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Tian, Feng, edk2-devel@lists.01.org, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
Thanks for the info.
Mike/Vincent also mentioned that FW does not own page tables after ExitBootServices(), so the OS would have to relax NX protection of RT code pages across SVA.
Or delay setting NX protections on RT code pages until after SVA.
I agree with you that we can mark RT region to be RW.
Here is the pseudo code I plan to put to CoreExitBootServices().
=======================
VOID
MemoryprotectionExitBootServicesCallback (
VOID
)
{
EFI_RUNTIME_IMAGE_ENTRY *RuntimeImage;
LIST_ENTRY *Link;
//
// We need remove the RT protection, because RT relocation need write code segment
// at SetVirtualAddressMap(). We cannot assume OS/Loader has taken over page table at that time.
//
// Firmware does not own page tables after ExitBootServices(), so the OS would
// have to relax protection of RT code pages across SetVirtualAddressMap(), or
// delay setting protections on RT code pages until after SetVirtualAddressMap().
// OS may set protection on RT based upon EFI_MEMORY_ATTRIBUTES_TABLE later.
//
if (mImageProtectionPolicy != 0) {
for (Link = gRuntime->ImageHead.ForwardLink; Link != &gRuntime->ImageHead; Link = Link->ForwardLink) {
RuntimeImage = BASE_CR (Link, EFI_RUNTIME_IMAGE_ENTRY, Link);
SetUefiImageMemoryAttributes ((UINT64)(UINTN)RuntimeImage->ImageBase, ALIGN_VALUE(RuntimeImage->ImageSize, EFI_PAGE_SIZE), 0);
}
}
}
=======================
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Friday, February 10, 2017 3:43 AM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
On 10 Feb 2017, at 11:32, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
Good to learn that ARM/ARCH64 behavior. That is interesting.
Yes, if that is the case, we need figure out a way to make it work.
IMHO, “Undo the protection” directly is also risky.
Maybe the protection is used or setup by OS purposely before EBS (WoW. ☺). Unprotecting them in BIOS might break the OS expectation.
Would you please provide more info on ARM Linux? When the OSLoader or OS takes over the page table? After EBS?
On ARM, we call SetVirtualAddressMap directly after EBS, in the OS loader. The caches are disabled much later. It is the OS itself that reenables the MMU, and install the UEFI virtual mapping as per-process mapping, which is only live when a runtime services call is in progress.
Therefore, we use a low virtual mapping for runtime services, which may conflict with the 1:1 mapping, so we never map any uefi regions 1:1 under the os.
This implies that the virtual mapping we install is not yet live when we install it, and so the easiest way to do that is to install it immediately after ebs, when the firmware's 1:1 mapping is still live.
Thank you
Yao Jiewen
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Thursday, February 9, 2017 10:42 PM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Leif Lindholm <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Fan, Jeff <jeff.fan@intel.com<mailto:jeff.fan@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
> On 10 Feb 2017, at 06:34, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> wrote:
>
>
>
>> On 10 Feb 2017, at 02:26, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
>>
>> Very good question.
>>
>> 1) Yes, I did test UEFI OS boot, which is mentioned in V1 summary:
>> ======
>> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
>> ======
>>
>> 2) Star helps double confirm that OS already takes over the control of page table on SetVirtualAddressMap().
>> See below log on UEFI Win10.
>> ======
>> DXEIPL CR3 0x88140000
>> RUNTIMEDXE CR3 0x1AB000
>> ======
>>
>
> Not on AArch64/ARM linux, and the spec does not mandate it, so we need to deal with this imo
>
I think we should probably undo the protections for runtime drivers in EBS()
>> Thank you
>> Yao Jiewen
>>
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
>> Sent: Thursday, February 9, 2017 8:48 AM
>> To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
>> Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Leif Lindholm <leif.lindholm@linaro.org<mailto:leif.lindholm@linaro.org>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Fan, Jeff <jeff.fan@intel.com<mailto:jeff.fan@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
>> Subject: Re: [edk2] [PATCH V3 0/4] DXE Memory Protection
>>
>> On 9 February 2017 at 16:30, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> wrote:
>> > On 9 February 2017 at 16:29, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
>> >> Very good point.
>> >>
>> >> Can ARCH64 set 4K paging for 64K aligned runtime memory?
>> >>
>> >
>> > UEFI always uses 4 KB, but the OS may use 64 KB, so to create the
>> > virtual address map it needs the runtime regions to be 64 KB aligned.
>> >
>> >>
>> >>
>> >> If yes, how about we use
>> >>
>> >> “ImageRecord->ImageSize = ALIGN_VALUE(LoadedImage->ImageSize,
>> >> EFI_PAGE_SIZE);”
>> >>
>> >
>>
>> Another question: did you try SetVirtualAddressMap()? It looks like we
>> need to lift read-only permissions to allow the runtime PE/COFF
>> relocation to apply the fixups
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
>> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-10 12:59 ` Yao, Jiewen
@ 2017-02-10 14:16 ` Ard Biesheuvel
0 siblings, 0 replies; 27+ messages in thread
From: Ard Biesheuvel @ 2017-02-10 14:16 UTC (permalink / raw)
To: Yao, Jiewen
Cc: Tian, Feng, edk2-devel@lists.01.org, Leif Lindholm,
Kinney, Michael D, Fan, Jeff, Zeng, Star
On 10 February 2017 at 12:59, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> Thanks for the info.
>
>
>
> Mike/Vincent also mentioned that FW does not own page tables after
> ExitBootServices(), so the OS would have to relax NX protection of RT code
> pages across SVA.
>
> Or delay setting NX protections on RT code pages until after SVA.
>
>
>
> I agree with you that we can mark RT region to be RW.
>
> Here is the pseudo code I plan to put to CoreExitBootServices().
>
>
>
> =======================
>
> VOID
>
> MemoryprotectionExitBootServicesCallback (
>
> VOID
>
> )
>
> {
>
> EFI_RUNTIME_IMAGE_ENTRY *RuntimeImage;
>
> LIST_ENTRY *Link;
>
>
>
> //
>
> // We need remove the RT protection, because RT relocation need write code
> segment
>
> // at SetVirtualAddressMap(). We cannot assume OS/Loader has taken over
> page table at that time.
>
> //
>
> // Firmware does not own page tables after ExitBootServices(), so the OS
> would
>
> // have to relax protection of RT code pages across
> SetVirtualAddressMap(), or
>
> // delay setting protections on RT code pages until after
> SetVirtualAddressMap().
>
> // OS may set protection on RT based upon EFI_MEMORY_ATTRIBUTES_TABLE
> later.
>
> //
>
> if (mImageProtectionPolicy != 0) {
>
> for (Link = gRuntime->ImageHead.ForwardLink; Link !=
> &gRuntime->ImageHead; Link = Link->ForwardLink) {
>
> RuntimeImage = BASE_CR (Link, EFI_RUNTIME_IMAGE_ENTRY, Link);
>
> SetUefiImageMemoryAttributes ((UINT64)(UINTN)RuntimeImage->ImageBase,
> ALIGN_VALUE(RuntimeImage->ImageSize, EFI_PAGE_SIZE), 0);
>
> }
>
> }
>
> }
This looks correct to me, and it is the only place where we can deal
with this situation.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH V3 0/4] DXE Memory Protection
2017-02-09 7:20 [PATCH V3 0/4] DXE Memory Protection Jiewen Yao
` (4 preceding siblings ...)
2017-02-09 7:43 ` [PATCH V3 0/4] DXE Memory Protection Yao, Jiewen
@ 2017-02-09 14:23 ` Yao, Jiewen
5 siblings, 0 replies; 27+ messages in thread
From: Yao, Jiewen @ 2017-02-09 14:23 UTC (permalink / raw)
To: Yao, Jiewen, edk2-devel@lists.01.org
Cc: Tian, Feng, Ard Biesheuvel, Leif Lindholm, Kinney, Michael D,
Fan, Jeff, Zeng, Star
I forget mentioning the V3 update also include below 2 feedback:
=============================
4) Rename file PageTableLib.h/.c to CpuPageTable.h/.c file (from Jeff Fan)
5) Remove multi-entrypoint usage (from Liming Gao/Mike Kinney)
=============================
Thank you
Yao Jiewen
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jiewen
> Yao
> Sent: Wednesday, February 8, 2017 11:20 PM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Fan, Jeff <jeff.fan@intel.com>; Zeng,
> Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH V3 0/4] DXE Memory Protection
>
> ==== V3 ====
> 1) Add PCD for policy control (feedback from Ard Biesheuvel)
> (Discussed with Mike Kinney)
> + # BIT0 - Image from unknown device. <BR>
> + # BIT1 - Image from firmware volume.<BR>
> + # @Prompt Set image protection policy.
> + # @ValidRange 0x80000002 | 0x00000000 - 0x0000001F
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x00000002|UIN
> T32|0x00001047
>
> 2) Remove unused function in CpuDxe.(feedback from Liming Gao)
> 3) Add commit log on link option assumption (feedback from Feng Tian)
>
> ==== V2 ====
> 1) Clean up ArmPkg, (feedback from Leif Lindholm)
>
> ==== V1 ====
> This series patch provides capability to protect PE/COFF image
> in DXE memory.
> If the UEFI image is page aligned, the image code section is set to read
> only and the image data section is set to non-executable.
>
> The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
> the image.
>
> Tested platform: NT32/Quark IA32/OVMF IA32/OVMF IA32X64/Intel internal X64/
> Tested OS: UEFI Win10, UEFI Ubuntu 16.04.
>
> Untested platform: ARM/AARCH64.
> Can ARM/AARCH64 owner help to take a look and try the ARM platform?
>
>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
>
> Jiewen Yao (4):
> UefiCpuPkg/CpuDxe: Add memory attribute setting.
> ArmPkg/CpuDxe: Correct EFI_MEMORY_RO usage
> MdeModulePkg/dec: add PcdImageProtectionPolicy.
> MdeModulePkg/DxeCore: Add UEFI image protection.
>
> ArmPkg/Drivers/CpuDxe/AArch64/Mmu.c | 3 +-
> ArmPkg/Drivers/CpuDxe/Arm/Mmu.c | 14 +-
> ArmPkg/Drivers/CpuDxe/CpuMmuCommon.c | 5 +-
> ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 3 +-
> MdeModulePkg/Core/Dxe/DxeMain.h | 53 ++
> MdeModulePkg/Core/Dxe/DxeMain.inf | 5 +-
> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 3 +-
> MdeModulePkg/Core/Dxe/Image/Image.c | 7 +-
> MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 735
> ++++++++++++++++++
> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 24 +-
> MdeModulePkg/MdeModulePkg.dec | 10 +
> UefiCpuPkg/CpuDxe/CpuDxe.c | 141 ++--
> UefiCpuPkg/CpuDxe/CpuDxe.inf | 5 +-
> UefiCpuPkg/CpuDxe/CpuPageTable.c | 779
> ++++++++++++++++++++
> UefiCpuPkg/CpuDxe/CpuPageTable.h | 113 +++
> 15 files changed, 1801 insertions(+), 99 deletions(-)
> create mode 100644 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.c
> create mode 100644 UefiCpuPkg/CpuDxe/CpuPageTable.h
>
> --
> 2.7.4.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 27+ messages in thread