On 08/08/15 02:02, Zeng, Star wrote:
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] 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 <pbonz...@redhat.com>
>> Cc: Jordan Justen <jordan.l.jus...@intel.com>
>> Cc: "Zeng, Star" <star.z...@intel.com>
>> Suggested-by: Paolo Bonzini <pbonz...@redhat.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
> 
> Reviewed by: Star Zeng <star.z...@intel.com>

Committed as SVN r18360. 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
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to