Re: [edk2] [PATCH 1/5] MdeModulePkg/DxeIplPeim: expire the use of PcdSetNxForStack

2018-09-11 Thread Ni, Ruiyu

On 9/11/2018 1:16 PM, Jian J Wang wrote:

+if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & BIT4) != 0


I suggest to use (1 << EfiBootServicesData) to replace BIT4.



--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH 1/5] MdeModulePkg/DxeIplPeim: expire the use of PcdSetNxForStack

2018-09-10 Thread Jian J Wang
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116

Since the stack memory is allocated as EfiBootServicesData, its NX protection
can be covered by BIT4 of PcdDxeNxMemoryProtectionPolicy. To avoid confusing
in setting related PCDs, PcdSetNxForStack will be expired. Instead, If BIT4
of PcdDxeNxMemoryProtectionPolicy is set, the DxeIpl will set NX bit in page
table entries mapping the stack memory.

Cc: Star Zeng 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Ruiyu Ni 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c   |  6 +-
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf  |  2 +-
 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  |  3 ++-
 MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c   |  2 +-
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 14 +++---
 5 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c 
b/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c
index 176d361f19..d44b845b76 100644
--- a/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c
@@ -45,7 +45,11 @@ HandOffToDxeCore (
   BaseOfStack = AllocatePages (EFI_SIZE_TO_PAGES (STACK_SIZE));
   ASSERT (BaseOfStack != NULL);
 
-  if (PcdGetBool (PcdSetNxForStack)) {
+  //
+  // Set stack to non-executable, if EfiBootServicesData type of memory is
+  // set for NX protection.
+  //
+  if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & BIT4) != 0) {
 Status = ArmSetMemoryRegionNoExec ((UINTN)BaseOfStack, STACK_SIZE);
 ASSERT_EFI_ERROR (Status);
   }
diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf 
b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
index fd82657404..44b6ea84ff 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
@@ -116,7 +116,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard   ## 
CONSUMES
 
 [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64]
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack   ## 
SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## 
SOMETIMES_CONSUMES
 
 [Depex]
   gEfiPeiLoadFilePpiGuid AND gEfiPeiMasterBootModePpiGuid
diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c 
b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
index d28baa3615..854078e6dd 100644
--- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
@@ -245,7 +245,8 @@ ToBuildPageTable (
 return TRUE;
   }
 
-  if (PcdGetBool (PcdSetNxForStack) && IsExecuteDisableBitAvailable ()) {
+  if (PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0 &&
+  IsExecuteDisableBitAvailable ()) {
 return TRUE;
   }
 
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c 
b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
index 81efcfe93d..eb53bc9417 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
@@ -94,7 +94,7 @@ HandOffToDxeCore (
 // Set NX for stack feature also require PcdDxeIplBuildPageTables be TRUE
 // for the DxeIpl and the DxeCore are both X64.
 //
-ASSERT (PcdGetBool (PcdSetNxForStack) == FALSE);
+ASSERT (PcdGet64 (PcdDxeNxMemoryProtectionPolicy) == 0);
 ASSERT (PcdGetBool (PcdCpuStackGuard) == FALSE);
   }
 
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c 
b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index 496e219913..27e9d6955d 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -152,7 +152,11 @@ ToSplitPageTable (
 }
   }
 
-  if (PcdGetBool (PcdSetNxForStack)) {
+  //
+  // Set stack to non-executable, if EfiBootServicesData type of memory is
+  // set for NX protection.
+  //
+  if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & BIT4) != 0) {
 if ((Address < StackBase + StackSize) && ((Address + Size) > StackBase)) {
   return TRUE;
 }
@@ -314,7 +318,11 @@ Split2MPageTo4K (
   PageTableEntry->Bits.Present = 1;
 }
 
-if (PcdGetBool (PcdSetNxForStack)
+//
+// Set stack to non-executable, if EfiBootServicesData type of memory is
+// set for NX protection.
+//
+if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & BIT4) != 0
 && (PhysicalAddress4K >= StackBase)
 && (PhysicalAddress4K < StackBase + StackSize)) {
   //
@@ -755,7 +763,7 @@ CreateIdentityMappingPageTables (
   //
   EnablePageTableProtection ((UINTN)PageMap, TRUE);
 
-  if (PcdGetBool (PcdSetNxForStack)) {
+  if (PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0) {
 EnableExecuteDisableBit ();
   }
 
-- 
2.16.2.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel