One of issue caused by enabling NULL pointer detection is that some PCI
device OptionROM, binary drivers and binary OS boot loaders may have NULL
pointer access bugs, which will prevent BIOS from booting and is almost
impossible to fix. BIT7 of PCD PcdNullPointerDetectionPropertyMask is used
as a workaround to indicate BIOS to disable NULL pointer detection right
after event gEfiEndOfDxeEventGroupGuid, and then let boot continue.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ayellet Wolman <ayellet.wolman@intel.com>
Suggested-by: Ayellet Wolman <ayellet.wolman@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
MdeModulePkg/Core/Dxe/Mem/Page.c | 4 ++-
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 48 +++++++++++++++++++++++++++
3 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 30d5984f7c..0a161ffd71 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -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..0468df3171 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -188,7 +188,9 @@ 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);
+ }
}
//
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index a73c4ccd64..73e3b269f3 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,23 @@ 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 ;
}
--
2.14.1.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Some comments to this patch.
1. How about using lower TPL TPL_CALLBACK instead of TPL_NOTIFY for the notification?
2. Should GCD SetMemorySpaceCapabilities + SetMemorySpaceAttributes be used instead of gCpu->SetMemoryAttributes()?
Thanks,
Star
-----Original Message-----
From: Wang, Jian J
Sent: Thursday, September 28, 2017 9:04 AM
To: edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Wolman, Ayellet <ayellet.wolman@intel.com>
Subject: [PATCH v3 3/6] MdeModulePkg/Core/Dxe: Add EndOfDxe workaround
One of issue caused by enabling NULL pointer detection is that some PCI device OptionROM, binary drivers and binary OS boot loaders may have NULL pointer access bugs, which will prevent BIOS from booting and is almost impossible to fix. BIT7 of PCD PcdNullPointerDetectionPropertyMask is used as a workaround to indicate BIOS to disable NULL pointer detection right after event gEfiEndOfDxeEventGroupGuid, and then let boot continue.
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ayellet Wolman <ayellet.wolman@intel.com>
Suggested-by: Ayellet Wolman <ayellet.wolman@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
MdeModulePkg/Core/Dxe/Mem/Page.c | 4 ++-
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 48 +++++++++++++++++++++++++++
3 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 30d5984f7c..0a161ffd71 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -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..0468df3171 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -188,7 +188,9 @@ 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);
+ }
}
//
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index a73c4ccd64..73e3b269f3 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,23 @@ 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 ;
}
--
2.14.1.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Thanks for the feedback. Please see my comments below.
> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, September 28, 2017 11:35 AM
> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
> Wolman, Ayellet <ayellet.wolman@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [PATCH v3 3/6] MdeModulePkg/Core/Dxe: Add EndOfDxe
> workaround
>
> Some comments to this patch.
>
> 1. How about using lower TPL TPL_CALLBACK instead of TPL_NOTIFY for the
> notification?
I think it's safe to use TPL_CALLBACK.
> 2. Should GCD SetMemorySpaceCapabilities + SetMemorySpaceAttributes be
> used instead of gCpu->SetMemoryAttributes()?
Yes. Since the GCG out-of-sync issue has been fixed, GCD service
should be used instead.
>
> Thanks,
> Star
> -----Original Message-----
> From: Wang, Jian J
> Sent: Thursday, September 28, 2017 9:04 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Laszlo
> Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Wolman, Ayellet <ayellet.wolman@intel.com>
> Subject: [PATCH v3 3/6] MdeModulePkg/Core/Dxe: Add EndOfDxe workaround
>
> One of issue caused by enabling NULL pointer detection is that some PCI device
> OptionROM, binary drivers and binary OS boot loaders may have NULL pointer
> access bugs, which will prevent BIOS from booting and is almost impossible to
> fix. BIT7 of PCD PcdNullPointerDetectionPropertyMask is used as a workaround
> to indicate BIOS to disable NULL pointer detection right after event
> gEfiEndOfDxeEventGroupGuid, and then let boot continue.
>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ayellet Wolman <ayellet.wolman@intel.com>
> Suggested-by: Ayellet Wolman <ayellet.wolman@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
> MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
> MdeModulePkg/Core/Dxe/Mem/Page.c | 4 ++-
> MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 48
> +++++++++++++++++++++++++++
> 3 files changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
> b/MdeModulePkg/Core/Dxe/DxeMain.inf
> index 30d5984f7c..0a161ffd71 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> @@ -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..0468df3171 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> @@ -188,7 +188,9 @@ 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);
> + }
> }
>
> //
> diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> index a73c4ccd64..73e3b269f3 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,23 @@ 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 ;
> }
>
> --
> 2.14.1.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.