From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id D211521E11D2E for ; Sun, 27 Aug 2017 19:50:51 -0700 (PDT) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga104.jf.intel.com with ESMTP; 27 Aug 2017 19:53:30 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,439,1498546800"; d="scan'208";a="1188708375" Received: from jwang36-mobl.ccr.corp.intel.com ([10.239.197.59]) by fmsmga001.fm.intel.com with ESMTP; 27 Aug 2017 19:53:28 -0700 From: "Wang, Jian J" To: edk2-devel@lists.01.org Cc: Star Zeng , Jiewen Yao , Eric Dong Date: Mon, 28 Aug 2017 10:51:08 +0800 Message-Id: <20170828025109.5032-2-jian.j.wang@intel.com> X-Mailer: git-send-email 2.11.0.windows.1 In-Reply-To: <20170828025109.5032-1-jian.j.wang@intel.com> References: <20170828025109.5032-1-jian.j.wang@intel.com> Subject: [PATCH 1/2] Implement NULL pointer detection for EDK-II Core X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Aug 2017 02:50:52 -0000 This feature is for debug purpose which helps to detect potential NULL pointer access in code at run-time. Cc: Star Zeng Cc: Jiewen Yao Cc: Eric Dong Suggested-by: Wolman, Ayellet Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Wang, Jian J --- 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.

+ # TRUE - NULL address detection will be enabled.
+ # FALSE - NULL address detection will be disabled.
+ # @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