Hi I do not think NULL pointer is related to NX. As long as NullPointer PCD is true and IA32PAE is supported, we can build page table.
+ BuildPageTablesIa32Pae = (BOOLEAN) (IsIa32PaeSupport () + && IsExecuteDisableBitAvailable () + && (PcdGetBool (PcdSetNxForStack) + || IsNullDetectionEnabled ())); > -----Original Message----- > From: Wang, Jian J > Sent: Thursday, September 21, 2017 1:20 PM > To: edk2-devel@lists.01.org > Cc: Zeng, Star <star.z...@intel.com>; Dong, Eric <eric.d...@intel.com>; Laszlo > Ersek <ler...@redhat.com>; Yao, Jiewen <jiewen....@intel.com>; Kinney, > Michael D <michael.d.kin...@intel.com>; Justen, Jordan L > <jordan.l.jus...@intel.com>; Wolman, Ayellet <ayellet.wol...@intel.com> > Subject: [PATCH v2 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer > detection > > 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..503cf88382 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 () > + && IsExecuteDisableBitAvailable > () > + && (PcdGetBool > (PcdSetNxForStack) > + || IsNullDetectionEnabled > ())); > 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