[PATCH 0/2] Reserve page 0 for NULL pointer detection

[PATCH 0/2] Reserve page 0 for NULL pointer detection

[Wang, Jian J]

When a boot loader examines the memory map, it can see that location 0
is available memory. If it chooses to use that memory, and
PcdNullPointerDetectionPropertyMask is enabled, use of memory in page 0
will cause an exception. This does occur when running the memtest86
program.

Leaving page 0 available is for legacy support purpose. Since we have
deprecated the support of legacy, the solution is just reserving it so
that it cannot be allocated for other uses.

Tests:
  - run memtest86 with PcdNullPointerDetectionPropertyMask set to 0x03
  - run memtest86 with PcdNullPointerDetectionPropertyMask set to 0x83
  - boot OVMF into Windows 7 with PcdNullPointerDetectionPropertyMask set to 0x83 

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1885
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>

Jian J Wang (2):
  MdeModulePkg/DxeIplPeim: reserve page 0 for NULL pointer detection
  MdeModulePkg/Core/Dxe: free page 0 after disabling NULL pointer
    detection

 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c   | 5 +++++
 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 4 ++++
 MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c  | 4 ++++
 3 files changed, 13 insertions(+)

-- 
2.17.1.windows.2

[PATCH 1/2] MdeModulePkg/DxeIplPeim: reserve page 0 for NULL pointer detection

[Wang, Jian J]

When a boot loader examines the memory map, it can see that location 0
is available memory. If it chooses to use that memory, and
PcdNullPointerDetectionPropertyMask is enabled, use of memory in page 0
will cause an exception. This does occur when running the memtest86
program.

Leaving page 0 available is for legacy support purpose. Since we have
deprecated the support of legacy, the solution is just reserving it so
that it cannot be allocated for other uses.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1885
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 4 ++++
 MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c  | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
index 172d7cd1c6..6e8ca824d4 100644
--- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
@@ -246,8 +246,12 @@ HandOffToDxeCore (
   EFI_PEI_VECTOR_HANDOFF_INFO_PPI *VectorHandoffInfoPpi;
   BOOLEAN                   BuildPageTablesIa32Pae;
 
+  //
+  // Clear page 0 and mark it as allocated if NULL pointer detection is enabled.
+  //
   if (IsNullDetectionEnabled ()) {
     ClearFirst4KPage (HobList.Raw);
+    BuildMemoryAllocationHob (0, EFI_PAGES_TO_SIZE (1), EfiBootServicesData);
   }
 
   Status = PeiServicesAllocatePages (EfiBootServicesData, EFI_SIZE_TO_PAGES (STACK_SIZE), &BaseOfStack);
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
index 2867610bff..f465eb1d8a 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c
@@ -36,8 +36,12 @@ HandOffToDxeCore (
   EFI_VECTOR_HANDOFF_INFO         *VectorInfo;
   EFI_PEI_VECTOR_HANDOFF_INFO_PPI *VectorHandoffInfoPpi;
 
+  //
+  // Clear page 0 and mark it as allocated if NULL pointer detection is enabled.
+  //
   if (IsNullDetectionEnabled ()) {
     ClearFirst4KPage (HobList.Raw);
+    BuildMemoryAllocationHob (0, EFI_PAGES_TO_SIZE (1), EfiBootServicesData);
   }
 
   //
-- 
2.17.1.windows.2

[PATCH 2/2] MdeModulePkg/Core/Dxe: free page 0 after disabling NULL pointer detection

[Wang, Jian J]

To solve access issue reported by BZ1885, page 0 will be allocated to
avoid misuses if NULL pointer detection is enabled. It should be better
to be freed after EndOfDxe if BIT7 of PcdNullPointerDetectionPropertyMask
is set, because NULL pointer detection is no longer available after
EndOfDxe and there will be no access conflict.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1885
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 7a24bd0781..47edf86dfb 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -1094,6 +1094,11 @@ DisableNullDetectionAtTheEndOfDxe (
             );
   ASSERT_EFI_ERROR (Status);
 
+  //
+  // Page 0 might have be allocated to avoid misuses. Free it here anyway.
+  //
+  CoreFreePages (0, 1);
+
   CoreCloseEvent (Event);
   DEBUG ((DEBUG_INFO, "DisableNullDetectionAtTheEndOfDxe(): end\r\n"));
 
-- 
2.17.1.windows.2

Re: [PATCH 0/2] Reserve page 0 for NULL pointer detection

[Ni, Ray]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Wang, Jian J <jian.j.wang@intel.com>
> Sent: Wednesday, November 6, 2019 9:14 PM
> To: devel@edk2.groups.io
> Cc: Bi, Dandan <dandan.bi@intel.com>; Gao, Liming <liming.gao@intel.com>; Ni, Ray <ray.ni@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Sean Brogan <sean.brogan@microsoft.com>
> Subject: [PATCH 0/2] Reserve page 0 for NULL pointer detection
> 
> When a boot loader examines the memory map, it can see that location 0
> is available memory. If it chooses to use that memory, and
> PcdNullPointerDetectionPropertyMask is enabled, use of memory in page 0
> will cause an exception. This does occur when running the memtest86
> program.
> 
> Leaving page 0 available is for legacy support purpose. Since we have
> deprecated the support of legacy, the solution is just reserving it so
> that it cannot be allocated for other uses.
> 
> Tests:
>   - run memtest86 with PcdNullPointerDetectionPropertyMask set to 0x03
>   - run memtest86 with PcdNullPointerDetectionPropertyMask set to 0x83
>   - boot OVMF into Windows 7 with PcdNullPointerDetectionPropertyMask set to 0x83
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1885
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> 
> Jian J Wang (2):
>   MdeModulePkg/DxeIplPeim: reserve page 0 for NULL pointer detection
>   MdeModulePkg/Core/Dxe: free page 0 after disabling NULL pointer
>     detection
> 
>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c   | 5 +++++
>  MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 4 ++++
>  MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c  | 4 ++++
>  3 files changed, 13 insertions(+)
> 
> --
> 2.17.1.windows.2

Re: [PATCH 0/2] Reserve page 0 for NULL pointer detection

[Wang, Jian J]

Pushed at fb92fe9e1817a53ca0fc985447f3c534201a62fa

Regards,
Jian

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Thursday, November 07, 2019 10:11 AM
> To: Wang, Jian J <jian.j.wang@intel.com>; devel@edk2.groups.io
> Cc: Bi, Dandan <dandan.bi@intel.com>; Gao, Liming <liming.gao@intel.com>;
> Wu, Hao A <hao.a.wu@intel.com>; Sean Brogan <sean.brogan@microsoft.com>
> Subject: RE: [PATCH 0/2] Reserve page 0 for NULL pointer detection
> 
> Reviewed-by: Ray Ni <ray.ni@intel.com>
> 
> > -----Original Message-----
> > From: Wang, Jian J <jian.j.wang@intel.com>
> > Sent: Wednesday, November 6, 2019 9:14 PM
> > To: devel@edk2.groups.io
> > Cc: Bi, Dandan <dandan.bi@intel.com>; Gao, Liming <liming.gao@intel.com>;
> Ni, Ray <ray.ni@intel.com>; Wu, Hao A
> > <hao.a.wu@intel.com>; Sean Brogan <sean.brogan@microsoft.com>
> > Subject: [PATCH 0/2] Reserve page 0 for NULL pointer detection
> >
> > When a boot loader examines the memory map, it can see that location 0
> > is available memory. If it chooses to use that memory, and
> > PcdNullPointerDetectionPropertyMask is enabled, use of memory in page 0
> > will cause an exception. This does occur when running the memtest86
> > program.
> >
> > Leaving page 0 available is for legacy support purpose. Since we have
> > deprecated the support of legacy, the solution is just reserving it so
> > that it cannot be allocated for other uses.
> >
> > Tests:
> >   - run memtest86 with PcdNullPointerDetectionPropertyMask set to 0x03
> >   - run memtest86 with PcdNullPointerDetectionPropertyMask set to 0x83
> >   - boot OVMF into Windows 7 with PcdNullPointerDetectionPropertyMask set
> to 0x83
> >
> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1885
> > Cc: Dandan Bi <dandan.bi@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Sean Brogan <sean.brogan@microsoft.com>
> >
> > Jian J Wang (2):
> >   MdeModulePkg/DxeIplPeim: reserve page 0 for NULL pointer detection
> >   MdeModulePkg/Core/Dxe: free page 0 after disabling NULL pointer
> >     detection
> >
> >  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c   | 5 +++++
> >  MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 4 ++++
> >  MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c  | 4 ++++
> >  3 files changed, 13 insertions(+)
> >
> > --
> > 2.17.1.windows.2