From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (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 6CEF2208F7AD7 for ; Wed, 13 Sep 2017 01:41:58 -0700 (PDT) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Sep 2017 01:44:56 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,386,1500966000"; d="scan'208";a="128281282" Received: from jwang36-mobl2.ccr.corp.intel.com (HELO jwang36-MOBL2.local) ([10.239.192.50]) by orsmga004.jf.intel.com with SMTP; 13 Sep 2017 01:44:55 -0700 Date: Wed, 13 Sep 2017 01:07:48 -0700 From: Wang, Jian J Sender: Wang, Jian J To: edk2-devel@lists.01.org Message-ID: X-Mailer: TortoiseGit MIME-Version: 1.0 Subject: [PATCH 1/4] MdeModulePkg/Core: Implement NULL pointer detection in 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: Wed, 13 Sep 2017 08:41:58 -0000 Content-Type: text/plain; The mechanism behind is to trigger a page fault exception at address 0. This can be made by disabling page 0 (0-4095) during page table setup. So this feature can only be available on platform with paging enabled. Once this feature is enabled, any code, like CSM, which has to access memory in page 0 needs to enable this page temporarily in advance and disable it afterwards. PcdNullPointerDetectionPropertyMask is used to control and elaborate the use cases. Cc: Jiewen Yao Cc: Eric Dong Cc: Star Zeng Cc: Laszlo Ersek Cc: Justen, Jordan L Cc: Kinney, Michael D Cc: Wolman, Ayellet 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 | 21 ++++++---- MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 47 +++++++++++++++++++++ MdeModulePkg/Core/DxeIplPeim/DxeIpl.h | 15 +++++++ MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 3 +- MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 53 ++++++++++++++++++++++++ MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 8 +++- MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 2 + MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 23 ++++++---- MdeModulePkg/MdeModulePkg.dec | 12 ++++++ 10 files changed, 167 insertions(+), 20 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf index 30d5984f7c..273b8b7c0e 100644 --- a/MdeModulePkg/Core/Dxe/DxeMain.inf +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf @@ -179,7 +179,7 @@ gEfiWatchdogTimerArchProtocolGuid ## CONSUMES [FeaturePcd] - gEfiMdeModulePkgTokenSpaceGuid.PcdFrameworkCompatibilitySupport ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdFrameworkCompatibilitySupport ## CONSUMES [Pcd] gEfiMdeModulePkgTokenSpaceGuid.PcdLoadFixAddressBootTimeCodePageNumber ## SOMETIMES_CONSUMES @@ -192,6 +192,7 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask ## CONSUMES # [Hob] # RESOURCE_DESCRIPTOR ## CONSUMES diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c index a142c79ee2..2e0b72f864 100644 --- a/MdeModulePkg/Core/Dxe/Mem/Page.c +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c @@ -170,6 +170,7 @@ CoreAddRange ( { LIST_ENTRY *Link; MEMORY_MAP *Entry; + EFI_STATUS Status; ASSERT ((Start & EFI_PAGE_MASK) == 0); ASSERT (End > Start) ; @@ -188,7 +189,17 @@ CoreAddRange ( // used for other purposes. // if (Type == EfiConventionalMemory && Start == 0 && (End >= EFI_PAGE_SIZE - 1)) { - SetMem ((VOID *)(UINTN)Start, EFI_PAGE_SIZE, 0); + if ((PcdGet8(PcdNullPointerDetectionPropertyMask) & BIT0) == 0) { + SetMem ((VOID *)(UINTN)Start, EFI_PAGE_SIZE, 0); + } else if (gCpu != NULL) { + Status = gCpu->SetMemoryAttributes(gCpu, 0, EFI_PAGE_SIZE, 0); + ASSERT_EFI_ERROR(Status); + + SetMem ((VOID *)(UINTN)Start, EFI_PAGE_SIZE, 0); + + Status = gCpu->SetMemoryAttributes(gCpu, 0, EFI_PAGE_SIZE, EFI_MEMORY_RP); + ASSERT_EFI_ERROR(Status); + } } // @@ -1972,11 +1983,3 @@ Done: return Status; } - - - - - - - - diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c index a73c4ccd64..2367d674e1 100644 --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c @@ -995,6 +995,36 @@ MemoryProtectionExitBootServicesCallback ( } } +/** + Disable NULL pointer detection after EndOfDxe. This is a workaround resort in + order to skip unfixable NULL pointer access issues detected in OptionROM or + boot loaders. + + @param[in] Event The Event this notify function registered to. + @param[in] Context Pointer to the context data registered to the Event. +**/ +VOID +EFIAPI +DisableNullDetectionAtTheEndOfDxe ( + EFI_EVENT Event, + VOID *Context + ) +{ + EFI_STATUS Status; + + DEBUG((DEBUG_INFO, "DisableNullDetectionAtTheEndOfDxe(): start\r\n")); + // + // Disable NULL pointer detection by enabling first 4K page + // + Status = gCpu->SetMemoryAttributes(gCpu, 0, EFI_PAGE_SIZE, 0); + ASSERT_EFI_ERROR(Status); + + CoreCloseEvent (Event); + DEBUG((DEBUG_INFO, "DisableNullDetectionAtTheEndOfDxe(): end\r\n")); + + return; +} + /** Initialize Memory Protection support. **/ @@ -1006,6 +1036,7 @@ CoreInitializeMemoryProtection ( { EFI_STATUS Status; EFI_EVENT Event; + EFI_EVENT EndOfDxeEvent; VOID *Registration; mImageProtectionPolicy = PcdGet32(PcdImageProtectionPolicy); @@ -1044,6 +1075,22 @@ CoreInitializeMemoryProtection ( ); ASSERT_EFI_ERROR(Status); } + + // + // Register a callback to disable NULL pointer detection at EndOfDxe + // + if ((PcdGet8(PcdNullPointerDetectionPropertyMask) & (BIT0|BIT7)) == (BIT0|BIT7)) { + Status = CoreCreateEventEx ( + EVT_NOTIFY_SIGNAL, + TPL_NOTIFY, + DisableNullDetectionAtTheEndOfDxe, + NULL, + &gEfiEndOfDxeEventGroupGuid, + &EndOfDxeEvent + ); + ASSERT_EFI_ERROR (Status); + } + return ; } diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h index 72d2532f50..104599156c 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h @@ -52,6 +52,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #define STACK_SIZE 0x20000 #define BSP_STORE_SIZE 0x4000 +#define NULL_DETECTION_ENABLED ((PcdGet8(PcdNullPointerDetectionPropertyMask) & BIT0) != 0) // // This PPI is installed to indicate the end of the PEI usage of memory @@ -240,4 +241,18 @@ Decompress ( OUT UINTN *OutputSize ); +/** + Clear legacy memory located at the first 4K-page. + + This function traverses the whole HOB list to check if memory from 0 to 4095 + exists and has not been allocated, and then clear it if so. + + @param HoStart The start of HobList passed to DxeCore. + +**/ +VOID +ClearLegacyMemory( + IN VOID *HobStart + ); + #endif diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf index c54afe4aa6..fde70f94bb 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf @@ -110,11 +110,12 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplBuildPageTables ## CONSUMES [FeaturePcd] - gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSupportUefiDecompress ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSupportUefiDecompress ## CONSUMES [Pcd.IA32,Pcd.X64] gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable ## SOMETIMES_CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask ## CONSUMES [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64] gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ## SOMETIMES_CONSUMES diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c index 50b5440d15..b5f9d92f5b 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c +++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c @@ -825,3 +825,56 @@ UpdateStackHob ( Hob.Raw = GET_NEXT_HOB (Hob); } } + +/** + Clear legacy memory located at the first 4K-page, if available. + + This function traverses the whole HOB list to check if memory from 0 to 4095 + exists and has not been allocated, and then clear it if so. + + @param HoStart The start of HobList passed to DxeCore. + +**/ +VOID +ClearLegacyMemory( + IN VOID *HobStart + ) +{ + EFI_PEI_HOB_POINTERS RscDescHob; + EFI_PEI_HOB_POINTERS MemAllocHob; + BOOLEAN DoClear; + + RscDescHob.Raw = HobStart; + MemAllocHob.Raw = HobStart; + DoClear = FALSE; + + // + // Check if page 0 exists and free + // + while ((RscDescHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, RscDescHob.Raw)) != NULL) { + if (RscDescHob.ResourceDescriptor->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY && + RscDescHob.ResourceDescriptor->PhysicalStart == 0) { + DoClear = TRUE; + // + // Make sure memory at 0-4095 has not been allocated. + // + while ((MemAllocHob.Raw = GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, MemAllocHob.Raw)) != NULL) { + if (MemAllocHob.MemoryAllocation->AllocDescriptor.MemoryBaseAddress < EFI_PAGE_SIZE) { + DoClear = FALSE; + break; + } + MemAllocHob.Raw = GET_NEXT_HOB (MemAllocHob); + } + break; + } + RscDescHob.Raw = GET_NEXT_HOB (RscDescHob); + } + + if (DoClear) { + DEBUG((DEBUG_INFO, "Clearing first 4K-page!\r\n")); + SetMem(NULL, EFI_PAGE_SIZE, 0); + } + + return; +} + diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c index 1957326caf..a8aa0d5d1b 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 ((NULL_DETECTION_ENABLED && PhysicalAddress == 0) + || ((PhysicalAddress < StackBase + StackSize) && ((PhysicalAddress + SIZE_2MB) > StackBase))) { // // Need to split this 2M page that covers stack range. // @@ -240,6 +241,8 @@ HandOffToDxeCore ( EFI_PEI_VECTOR_HANDOFF_INFO_PPI *VectorHandoffInfoPpi; BOOLEAN BuildPageTablesIa32Pae; + ClearLegacyMemory(HobList.Raw); + Status = PeiServicesAllocatePages (EfiBootServicesData, EFI_SIZE_TO_PAGES (STACK_SIZE), &BaseOfStack); ASSERT_EFI_ERROR (Status); @@ -379,7 +382,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) || NULL_DETECTION_ENABLED)); if (BuildPageTablesIa32Pae) { PageTables = Create4GPageTablesIa32Pae (BaseOfStack, STACK_SIZE); EnableExecuteDisableBit (); diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c index 6488880eab..50a8d77a5b 100644 --- a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c +++ b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c @@ -42,6 +42,8 @@ HandOffToDxeCore ( EFI_VECTOR_HANDOFF_INFO *VectorInfo; EFI_PEI_VECTOR_HANDOFF_INFO_PPI *VectorHandoffInfoPpi; + ClearLegacyMemory(HobList.Raw); + // // Get Vector Hand-off Info PPI and build Guided HOB // diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c index 48150be4e1..ccd6e10cb2 100644 --- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c +++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c @@ -90,8 +90,14 @@ Split2MPageTo4K ( // PageTableEntry->Uint64 = (UINT64) PhysicalAddress4K | AddressEncMask; PageTableEntry->Bits.ReadWrite = 1; - PageTableEntry->Bits.Present = 1; - if ((PhysicalAddress4K >= StackBase) && (PhysicalAddress4K < StackBase + StackSize)) { + + if (NULL_DETECTION_ENABLED && PhysicalAddress4K == 0) { + PageTableEntry->Bits.Present = 0; + } else { + PageTableEntry->Bits.Present = 1; + } + + if (PcdGetBool (PcdSetNxForStack) && (PhysicalAddress4K >= StackBase) && (PhysicalAddress4K < StackBase + StackSize)) { // // Set Nx bit for stack. // @@ -137,9 +143,10 @@ Split1GPageTo2M ( PhysicalAddress2M = PhysicalAddress; for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PhysicalAddress2M += SIZE_2MB) { - if ((PhysicalAddress2M < StackBase + StackSize) && ((PhysicalAddress2M + SIZE_2MB) > StackBase)) { + if ((NULL_DETECTION_ENABLED && 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 +286,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 ((NULL_DETECTION_ENABLED && PageAddress == 0) + || (PcdGetBool (PcdSetNxForStack) && (PageAddress < StackBase + StackSize) && ((PageAddress + SIZE_1GB) > StackBase))) { Split1GPageTo2M (PageAddress, (UINT64 *) PageDirectory1GEntry, StackBase, StackSize); } else { // @@ -308,9 +316,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 ((NULL_DETECTION_ENABLED && 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..1cc84894af 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -860,6 +860,18 @@ # @ValidList 0x80000006 | 0x03058002 gEfiMdeModulePkgTokenSpaceGuid.PcdErrorCodeSetVariable|0x03058002|UINT32|0x30001040 + ## Mask to control the NULL address detection in code for different phases. + # If enabled, accessing NULL address in UEFI or SMM code can be caught.

+ # BIT0 - Enable NULL pointer detection for UEFI.
+ # BIT1 - Enable NULL pointer detection for SMM.
+ # BIT2..6 - Reserved for future uses.
+ # BIT7 - Disable NULL pointer detection just after EndOfDxe.
+ # This is a workaround for those unsolvable NULL access issues in OptionROM, boot loader, etc. + # It can also help to avoid unnecessary exception caused by legacy memory (0-4095) access after + # EndOfDxe, such as Windows 7 boot on Qemu.
+ # @Prompt Enable NULL address detection. + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask|0x0|UINT8|0x30001050 + [PcdsFixedAtBuild, PcdsPatchableInModule] ## Dynamic type PCD can be registered callback function for Pcd setting action. # PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum number of callback function -- 2.14.1.windows.1