Jordan, On 08/08/15 02:02, Zeng, Star wrote: >> -----Original Message----- >> From: edk2-devel [mailto:[email protected]] On Behalf Of >> Laszlo Ersek >> Sent: Saturday, August 8, 2015 12:00 AM >> To: edk2-devel-01 >> Cc: Paolo Bonzini; Zeng, Star; Justen, Jordan L >> Subject: [edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack >> >> SVN rev 18166 ("MdeModulePkg DxeIpl: Add stack NX support") enables >> platforms to request non-executable stack for the DXE phase, by setting >> PcdSetNxForStack to TRUE. >> >> The PCD defaults to FALSE, because: >> >> (a) A non-executable DXE stack is a new feature and causes changes in >> behavior. Some platform could rely on executing code from the stack. >> >> (b) The code enabling NX in the DXE IPL PEIM enforces the >> >> PcdSetNxForStack ==> PcdDxeIplBuildPageTables >> >> implication for "64-bit PEI + 64-bit DXE" platforms, with a new >> ASSERT(). Some platform might not comply with this requirement >> immediately. >> >> Regarding (a), in none of the OVMF builds do we try to execute code from >> the stack. >> >> Regarding (b): >> >> - In the OvmfPkgX64.dsc build (which is where (b) applies) we simply >> inherit the PcdDxeIplBuildPageTables|TRUE default from >> "MdeModulePkg/MdeModulePkg.dec". Therefore we can set >> PcdSetNxForStack >> to TRUE. >> >> - In OvmfPkgIa32X64.dsc, page tables are built by default for DXE. Hence >> we can set PcdSetNxForStack to TRUE. >> >> - In OvmfPkgIa32.dsc, page tables used not to be necessary until now. >> After we set PcdSetNxForStack to TRUE in this patch, the DXE IPL will >> construct page tables even when it is built as part of OvmfPkgIa32.dsc, >> provided the (virtual) hardware supports both PAE mode and the XD bit. >> >> Should this setting cause problems in a GPU (or other device) passthru >> scenario, with a UEFI_DRIVER in the PCI option rom attempting to execute >> code from the stack, the feature can be dynamically disabled on the QEMU >> command line, with "-cpu <MODEL>,-nx". >> >> Cc: Paolo Bonzini <[email protected]> >> Cc: Jordan Justen <[email protected]> >> Cc: "Zeng, Star" <[email protected]> >> Suggested-by: Paolo Bonzini <[email protected]> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek <[email protected]> > > Reviewed by: Star Zeng <[email protected]>
do you have any comments? Star's prerequisite patch "UefiCpuPkg CpuDxe: Sync up the settings of Execute Disable to APs" has been committed as git 21d0e75065 (SVN rev 18191), so this simple OvmfPkg patch should be good to go on top. If you're content with Star's R-b, I can go ahead and commit the patch with that. Thanks Laszlo > >> --- >> >> Notes: >> - This patch depends on Star's >> >> [edk2] [PATCH] UefiCpuPkg CpuDxe: Sync up the settings of Execute >> Disable to APs >> http://thread.gmane.org/gmane.comp.bios.edk2.devel/960 >> >> and should be applied only after that patch. >> >> OvmfPkg/OvmfPkgIa32.dsc | 1 + >> OvmfPkg/OvmfPkgIa32X64.dsc | 1 + >> OvmfPkg/OvmfPkgX64.dsc | 1 + >> 3 files changed, 3 insertions(+) >> >> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index >> 48118cc..38954109 100644 >> --- a/OvmfPkg/OvmfPkgIa32.dsc >> +++ b/OvmfPkg/OvmfPkgIa32.dsc >> @@ -303,6 +303,7 @@ [PcdsFeatureFlag] >> !endif >> >> [PcdsFixedAtBuild] >> + gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE >> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >> >> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationC >> hange|FALSE >> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 >> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc >> index 6860ad7..6f6517c 100644 >> --- a/OvmfPkg/OvmfPkgIa32X64.dsc >> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc >> @@ -308,6 +308,7 @@ [PcdsFeatureFlag] >> !endif >> >> [PcdsFixedAtBuild] >> + gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE >> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >> >> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationC >> hange|FALSE >> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 >> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index >> f877fda..6b7f955 100644 >> --- a/OvmfPkg/OvmfPkgX64.dsc >> +++ b/OvmfPkg/OvmfPkgX64.dsc >> @@ -308,6 +308,7 @@ [PcdsFeatureFlag] >> !endif >> >> [PcdsFixedAtBuild] >> + gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE >> gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 >> >> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationC >> hange|FALSE >> gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 >> -- >> 1.8.3.1 >> >> _______________________________________________ >> edk2-devel mailing list >> [email protected] >> https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
