From: "Zeng, Star" <star.zeng@intel.com>
To: "Wang, Jian J" <jian.j.wang@intel.com>,
"edk2-devel@lists.01.org" <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
Date: Thu, 28 Sep 2017 03:34:37 +0000 [thread overview]
Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B97BEF7@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <20170928010353.11968-4-jian.j.wang@intel.com>
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
next prev parent reply other threads:[~2017-09-28 3:31 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-28 1:03 [PATCH v3 0/6] Add NULL pointer detection feature Jian J Wang
2017-09-28 1:03 ` [PATCH v3 1/6] MdeModulePkg/MdeModulePkg.dec, .uni: Add NULL pointer detection PCD Jian J Wang
2017-09-28 3:35 ` Zeng, Star
2017-09-28 1:03 ` [PATCH v3 2/6] MdeModulePkg/DxeIpl: Implement NULL pointer detection Jian J Wang
2017-09-28 3:23 ` Zeng, Star
2017-09-28 3:31 ` Zeng, Star
2017-09-28 3:55 ` Wang, Jian J
2017-09-28 5:09 ` Zeng, Star
2017-09-28 5:33 ` Wang, Jian J
2017-09-28 3:50 ` Wang, Jian J
2017-09-28 5:11 ` Zeng, Star
2017-09-28 1:03 ` [PATCH v3 3/6] MdeModulePkg/Core/Dxe: Add EndOfDxe workaround Jian J Wang
2017-09-28 3:34 ` Zeng, Star [this message]
2017-09-28 5:08 ` Wang, Jian J
2017-09-28 1:03 ` [PATCH v3 4/6] UefiCpuPkg/PiSmmCpuDxeSmm: Implement NULL pointer detection for SMM code Jian J Wang
2017-09-28 1:03 ` [PATCH v3 5/6] IntelFrameworkModulePkg/Csm: Add code to bypass NULL pointer detection Jian J Wang
2017-09-28 1:03 ` [PATCH v3 6/6] OvmfPkg/QemuVideoDxe: Bypass NULL pointer detection during VBE SHIM installing Jian J Wang
2017-09-28 7:59 ` Laszlo Ersek
2017-10-02 17:58 ` Jordan Justen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0C09AFA07DD0434D9E2A0C6AEB0483103B97BEF7@shsmsx102.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox