From: "Wang, Jian J" <jian.j.wang@intel.com>
To: edk2-devel@lists.01.org
Cc: Star Zeng <star.zeng@intel.com>,
Jiewen Yao <jiewen.yao@intel.com>,
Eric Dong <eric.dong@intel.com>
Subject: [PATCH 1/2] Implement NULL pointer detection for EDK-II Core
Date: Mon, 28 Aug 2017 10:51:08 +0800 [thread overview]
Message-ID: <20170828025109.5032-2-jian.j.wang@intel.com> (raw)
In-Reply-To: <20170828025109.5032-1-jian.j.wang@intel.com>
This feature is for debug purpose which helps to detect potential
NULL pointer access in code at run-time.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Suggested-by: Wolman, Ayellet <ayellet.wolman@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wang, Jian J <jian.j.wang@intel.com>
---
MdeModulePkg/Core/Dxe/DxeMain.inf | 3 ++-
MdeModulePkg/Core/Dxe/Mem/Page.c | 5 +++--
MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 +
MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 6 ++++--
MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 26 ++++++++++++++++--------
MdeModulePkg/MdeModulePkg.dec | 7 +++++++
6 files changed, 35 insertions(+), 13 deletions(-)
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 30d5984f7c..3d75a0014d 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -179,7 +179,8 @@
gEfiWatchdogTimerArchProtocolGuid ## CONSUMES
[FeaturePcd]
- gEfiMdeModulePkgTokenSpaceGuid.PcdFrameworkCompatibilitySupport ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdFrameworkCompatibilitySupport ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetection ## CONSUMES
[Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdLoadFixAddressBootTimeCodePageNumber ## SOMETIMES_CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index a142c79ee2..3fe77391b7 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -185,9 +185,10 @@ CoreAddRange (
// compatibility with operating systems that may evaluate memory in this page
// for legacy data structures. If memory of any other type is added starting
// at address 0, then do not zero the page at address 0 because the page is being
- // used for other purposes.
+ // used for other purposes. But don't do this if NULL pointer detection mechanism
+ // is used.
//
- if (Type == EfiConventionalMemory && Start == 0 && (End >= EFI_PAGE_SIZE - 1)) {
+ if (!PcdGetBool(PcdNullPointerDetection) && Type == EfiConventionalMemory && Start == 0 && (End >= EFI_PAGE_SIZE - 1)) {
SetMem ((VOID *)(UINTN)Start, EFI_PAGE_SIZE, 0);
}
diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
index c54afe4aa6..6b4d68cfa1 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
@@ -111,6 +111,7 @@
[FeaturePcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSupportUefiDecompress ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetection ## CONSUMES
[Pcd.IA32,Pcd.X64]
gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable ## SOMETIMES_CONSUMES
diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
index 1957326caf..d4e1b7c858 100644
--- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
@@ -123,7 +123,8 @@ Create4GPageTablesIa32Pae (
PageDirectoryPointerEntry->Bits.Present = 1;
for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PhysicalAddress += SIZE_2MB) {
- if ((PhysicalAddress < StackBase + StackSize) && ((PhysicalAddress + SIZE_2MB) > StackBase)) {
+ if ((PcdGetBool(PcdNullPointerDetection) && PhysicalAddress == 0)
+ || ((PhysicalAddress < StackBase + StackSize) && ((PhysicalAddress + SIZE_2MB) > StackBase))) {
//
// Need to split this 2M page that covers stack range.
//
@@ -379,7 +380,8 @@ HandOffToDxeCore (
TopOfStack = (EFI_PHYSICAL_ADDRESS) (UINTN) ALIGN_POINTER (TopOfStack, CPU_STACK_ALIGNMENT);
PageTables = 0;
- BuildPageTablesIa32Pae = (BOOLEAN) (PcdGetBool (PcdSetNxForStack) && IsIa32PaeSupport () && IsExecuteDisableBitAvailable ());
+ BuildPageTablesIa32Pae = (BOOLEAN) (IsIa32PaeSupport () && IsExecuteDisableBitAvailable ()
+ && (PcdGetBool (PcdSetNxForStack) || PcdGetBool (PcdNullPointerDetection)));
if (BuildPageTablesIa32Pae) {
PageTables = Create4GPageTablesIa32Pae (BaseOfStack, STACK_SIZE);
EnableExecuteDisableBit ();
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index 48150be4e1..c69f889d9e 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -89,9 +89,16 @@ Split2MPageTo4K (
// Fill in the Page Table entries
//
PageTableEntry->Uint64 = (UINT64) PhysicalAddress4K | AddressEncMask;
- PageTableEntry->Bits.ReadWrite = 1;
- PageTableEntry->Bits.Present = 1;
- if ((PhysicalAddress4K >= StackBase) && (PhysicalAddress4K < StackBase + StackSize)) {
+
+ if (PcdGetBool(PcdNullPointerDetection) && PhysicalAddress4K == 0) {
+ PageTableEntry->Bits.ReadWrite = 0;
+ PageTableEntry->Bits.Present = 0;
+ } else {
+ PageTableEntry->Bits.ReadWrite = 1;
+ PageTableEntry->Bits.Present = 1;
+ }
+
+ if (PcdGetBool (PcdSetNxForStack) && (PhysicalAddress4K >= StackBase) && (PhysicalAddress4K < StackBase + StackSize)) {
//
// Set Nx bit for stack.
//
@@ -137,9 +144,10 @@ Split1GPageTo2M (
PhysicalAddress2M = PhysicalAddress;
for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PhysicalAddress2M += SIZE_2MB) {
- if ((PhysicalAddress2M < StackBase + StackSize) && ((PhysicalAddress2M + SIZE_2MB) > StackBase)) {
+ if ((PcdGetBool(PcdNullPointerDetection) && PhysicalAddress2M == 0)
+ || (PcdGetBool (PcdSetNxForStack) && (PhysicalAddress2M < StackBase + StackSize) && ((PhysicalAddress2M + SIZE_2MB) > StackBase))) {
//
- // Need to split this 2M page that covers stack range.
+ // Need to split this 2M page that covers NULL or stack range.
//
Split2MPageTo4K (PhysicalAddress2M, (UINT64 *) PageDirectoryEntry, StackBase, StackSize);
} else {
@@ -279,7 +287,8 @@ CreateIdentityMappingPageTables (
PageDirectory1GEntry = (VOID *) PageDirectoryPointerEntry;
for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectory1GEntry++, PageAddress += SIZE_1GB) {
- if (PcdGetBool (PcdSetNxForStack) && (PageAddress < StackBase + StackSize) && ((PageAddress + SIZE_1GB) > StackBase)) {
+ if ((PcdGetBool (PcdNullPointerDetection) && PageAddress == 0)
+ || (PcdGetBool (PcdSetNxForStack) && (PageAddress < StackBase + StackSize) && ((PageAddress + SIZE_1GB) > StackBase))) {
Split1GPageTo2M (PageAddress, (UINT64 *) PageDirectory1GEntry, StackBase, StackSize);
} else {
//
@@ -308,9 +317,10 @@ CreateIdentityMappingPageTables (
PageDirectoryPointerEntry->Bits.Present = 1;
for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PageAddress += SIZE_2MB) {
- if (PcdGetBool (PcdSetNxForStack) && (PageAddress < StackBase + StackSize) && ((PageAddress + SIZE_2MB) > StackBase)) {
+ if ((PcdGetBool (PcdNullPointerDetection) && PageAddress == 0)
+ || (PcdGetBool (PcdSetNxForStack) && (PageAddress < StackBase + StackSize) && ((PageAddress + SIZE_2MB) > StackBase))) {
//
- // Need to split this 2M page that covers stack range.
+ // Need to split this 2M page that covers NULL or stack range.
//
Split2MPageTo4K (PageAddress, (UINT64 *) PageDirectoryEntry, StackBase, StackSize);
} else {
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 593bff357a..713593dc38 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -802,6 +802,13 @@
# @Prompt Degrade 64-bit PCI MMIO BARs for legacy BIOS option ROMs
gEfiMdeModulePkgTokenSpaceGuid.PcdPciDegradeResourceForOptionRom|TRUE|BOOLEAN|0x0001003a
+ ## Indicates if NULL address detection will be enabled.
+ # If enabled, accessing NULL address in UEFI can be caught.<BR><BR>
+ # TRUE - NULL address detection will be enabled.<BR>
+ # FALSE - NULL address detection will be disabled.<BR>
+ # @Prompt Enable NULL address detection.
+ gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetection|TRUE|BOOLEAN|0x0004003d
+
[PcdsFeatureFlag.IA32, PcdsFeatureFlag.ARM, PcdsFeatureFlag.AARCH64]
gEfiMdeModulePkgTokenSpaceGuid.PcdPciDegradeResourceForOptionRom|FALSE|BOOLEAN|0x0001003a
--
2.11.0.windows.1
next prev parent reply other threads:[~2017-08-28 2:50 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-28 2:51 [PATCH 0/2] Implement NULL pointer detection feature Wang, Jian J
2017-08-28 2:51 ` Wang, Jian J [this message]
2017-08-28 2:51 ` [PATCH 2/2] Implement NULL pointer detection for EDK-II SMM Core and driver Wang, Jian J
2017-08-28 3:10 ` [PATCH 0/2] Implement NULL pointer detection feature Yao, Jiewen
2017-08-28 3:24 ` Wang, Jian J
2017-08-28 3:39 ` Yao, Jiewen
2017-08-29 15:16 ` Johnson, Brian (EXL - Eagan)
2017-08-29 16:02 ` Kinney, Michael D
2017-08-29 17:12 ` Johnson, Brian (EXL - Eagan)
2017-08-29 17:14 ` Tim Lewis
2017-08-30 14:55 ` Yao, Jiewen
2017-08-30 16:27 ` Andrew Fish
2017-08-30 21:36 ` Johnson, Brian (EXL - Eagan)
2017-08-31 1:16 ` Yao, Jiewen
2017-09-01 2:27 ` Wang, Jian J
2017-09-01 5:22 ` Wang, Jian J
2017-08-31 0:55 ` Yao, Jiewen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170828025109.5032-2-jian.j.wang@intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox