> According to Jiewen's feedback, change the page split condition > for NULL pointer detection to exclude IsExecuteDisableBitAvailable() > (Ia32/DxeLoadFunc.c)
NULL pointer detection is done by making use of paging mechanism of CPU. During page table setup, if enabled, the first 4-K page (0-4095) will be marked as NOT PRESENT. Any code which unintentionally access memory between 0-4095 will trigger a Page Fault exception which warns users that there's potential illegal code in BIOS. This also means that legacy code which has to access memory between 0-4095 should be cautious to temporarily disable this feature before the access and re-enable it afterwards; or disalbe this feature at all. Cc: Star Zeng <star.z...@intel.com> Cc: Eric Dong <eric.d...@intel.com> Cc: Laszlo Ersek <ler...@redhat.com> Cc: Jiewen Yao <jiewen....@intel.com> Cc: Michael Kinney <michael.d.kin...@intel.com> Cc: Jordan Justen <jordan.l.jus...@intel.com> Cc: Ayellet Wolman <ayellet.wol...@intel.com> Suggested-by: Ayellet Wolman <ayellet.wol...@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang <jian.j.w...@intel.com> --- MdeModulePkg/Core/DxeIplPeim/DxeIpl.h | 25 +++++++++ MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 + MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 65 ++++++++++++++++++++++++ MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 11 +++- MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 2 + MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 31 ++++++++--- 6 files changed, 126 insertions(+), 9 deletions(-) diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h index 72d2532f50..1654bcd2dc 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h @@ -240,4 +240,29 @@ 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 + ); + +/** + Return configure status of NULL pointer detection feature + + @return TRUE NULL pointer detection feature is enabled + @return FALSE NULL pointer detection feature is disabled +**/ +BOOLEAN +IsNullDetectionEnabled ( + VOID + ); + #endif diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf index c54afe4aa6..9d0e76a293 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf @@ -115,6 +115,7 @@ [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..0a71b1f3de 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c +++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c @@ -825,3 +825,68 @@ 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 RscHob; + EFI_PEI_HOB_POINTERS MemHob; + BOOLEAN DoClear; + + RscHob.Raw = HobStart; + MemHob.Raw = HobStart; + DoClear = FALSE; + + // + // Check if page 0 exists and free + // + while ((RscHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, + RscHob.Raw)) != NULL) { + if (RscHob.ResourceDescriptor->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY && + RscHob.ResourceDescriptor->PhysicalStart == 0) { + DoClear = TRUE; + // + // Make sure memory at 0-4095 has not been allocated. + // + while ((MemHob.Raw = GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, + MemHob.Raw)) != NULL) { + if (MemHob.MemoryAllocation->AllocDescriptor.MemoryBaseAddress + < EFI_PAGE_SIZE) { + DoClear = FALSE; + break; + } + MemHob.Raw = GET_NEXT_HOB (MemHob); + } + break; + } + RscHob.Raw = GET_NEXT_HOB (RscHob); + } + + if (DoClear) { + DEBUG ((DEBUG_INFO, "Clearing first 4K-page!\r\n")); + SetMem (NULL, EFI_PAGE_SIZE, 0); + } + + return; +} + +BOOLEAN +IsNullDetectionEnabled ( + VOID + ) +{ + return (((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) != 0) ? + TRUE : FALSE); +} + diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c index 1957326caf..7a8421dd16 100644 --- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c +++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c @@ -123,7 +123,9 @@ Create4GPageTablesIa32Pae ( PageDirectoryPointerEntry->Bits.Present = 1; for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PhysicalAddress += SIZE_2MB) { - if ((PhysicalAddress < StackBase + StackSize) && ((PhysicalAddress + SIZE_2MB) > StackBase)) { + if ((IsNullDetectionEnabled () && PhysicalAddress == 0) + || ((PhysicalAddress < StackBase + StackSize) + && ((PhysicalAddress + SIZE_2MB) > StackBase))) { // // Need to split this 2M page that covers stack range. // @@ -240,6 +242,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 +383,10 @@ HandOffToDxeCore ( TopOfStack = (EFI_PHYSICAL_ADDRESS) (UINTN) ALIGN_POINTER (TopOfStack, CPU_STACK_ALIGNMENT); PageTables = 0; - BuildPageTablesIa32Pae = (BOOLEAN) (PcdGetBool (PcdSetNxForStack) && IsIa32PaeSupport () && IsExecuteDisableBitAvailable ()); + BuildPageTablesIa32Pae = (BOOLEAN) (IsIa32PaeSupport () && + (IsNullDetectionEnabled () || + (PcdGetBool (PcdSetNxForStack) && + IsExecuteDisableBitAvailable ()))); 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..d93a9c5a2d 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..80c1821eca 100644 --- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c +++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c @@ -90,8 +90,16 @@ Split2MPageTo4K ( // PageTableEntry->Uint64 = (UINT64) PhysicalAddress4K | AddressEncMask; PageTableEntry->Bits.ReadWrite = 1; - PageTableEntry->Bits.Present = 1; - if ((PhysicalAddress4K >= StackBase) && (PhysicalAddress4K < StackBase + StackSize)) { + + if (IsNullDetectionEnabled () && 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 +145,12 @@ Split1GPageTo2M ( PhysicalAddress2M = PhysicalAddress; for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PhysicalAddress2M += SIZE_2MB) { - if ((PhysicalAddress2M < StackBase + StackSize) && ((PhysicalAddress2M + SIZE_2MB) > StackBase)) { + if ((IsNullDetectionEnabled () && 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 +290,10 @@ 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 ((IsNullDetectionEnabled () && PageAddress == 0) + || (PcdGetBool (PcdSetNxForStack) + && (PageAddress < StackBase + StackSize) + && ((PageAddress + SIZE_1GB) > StackBase))) { Split1GPageTo2M (PageAddress, (UINT64 *) PageDirectory1GEntry, StackBase, StackSize); } else { // @@ -308,9 +322,12 @@ 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 ((IsNullDetectionEnabled () && 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 { -- 2.14.1.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel