Re: [Xen-devel] OVMF/Xen, Debian wheezy can't boot with NX on stack (Was: Re: [edk2] [PATCH] OvmfPkg: prevent code execution from DXE stack)

[Laszlo Ersek <lersek@xxxxxxxxxx>]

On 09/08/15 19:26, Anthony PERARD wrote:
> On Fri, Aug 28, 2015 at 10:17:28AM +0200, Laszlo Ersek wrote:
>> On 08/08/15 02:02, Zeng, Star wrote:
>>>> -----Original Message-----
>>>> From: edk2-devel [mailto:edk2-devel-bounces@xxxxxxxxxxxx] 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 <pbonzini@xxxxxxxxxx>
>>>> Cc: Jordan Justen <jordan.l.justen@xxxxxxxxx>
>>>> Cc: "Zeng, Star" <star.zeng@xxxxxxxxx>
>>>> Suggested-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Laszlo Ersek <lersek@xxxxxxxxxx>
>>>
>>> Reviewed by: Star Zeng <star.zeng@xxxxxxxxx>
>>
>> Committed as SVN r18360. Thanks!
>> Laszlo
> 
> Hi,
> 
> This change breaks Debian installer 7.2, or wheezy while running in a Xen
> guest.
> http://lists.xenproject.org/archives/html/xen-devel/2015-09/msg00845.html
> 
> I've reproduce this using this iso:
> http://ftp.uk.debian.org/debian/dists/wheezy/main/installer-amd64/current/images/netboot/mini.iso
> 
> And I get this on the console:
> Welcome to GRUB!
> 
> !!!! X64 Exception Type - 0E(#PF - Page-Fault)  CPU Apic ID - 00000000 !!!!
> RIP  - 000000000F5F8918, CS  - 0000000000000028, RFLAGS - 0000000000210206
> ExceptionData - 0000000000000011
> RAX  - 0000000000000000, RCX - 0000000007FCE000, RDX - 0000000000000000
> RBX  - 000000000B6092C0, RSP - 000000000F5F8590, RBP - 000000000B608EA0
> RSI  - 000000000F5F8838, RDI - 000000000B608EA0
> R8   - 0000000000000000, R9  - 000000000B609200, R10 - 0000000000000000
> R11  - 000000000000000A, R12 - 0000000000000000, R13 - 000000000000001B
> R14  - 000000000B609360, R15 - 0000000000000000
> DS   - 0000000000000008, ES  - 0000000000000008, FS  - 0000000000000008
> GS   - 0000000000000008, SS  - 0000000000000008
> CR0  - 0000000080000033, CR2 - 000000000F5F8918, CR3 - 000000000F597000
> CR4  - 0000000000000668, CR8 - 0000000000000000
> DR0  - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000
> DR3  - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400
> GDTR - 000000000F57BF18 000000000000003F, LDTR - 0000000000000000
> IDTR - 000000000EEA5018 0000000000000FFF,   TR - 0000000000000000
> FXSAVE_STATE - 000000000F5F81F0
> !!!! Find PE image 
> /build/xen-unstable/src/xen-unstable/tools/firmware/ovmf-dir-remote/Build/OvmfX64/DEBUG_GCC49/X64/IntelFrameworkModulePkg/Universal/StatusCode/RuntimeDxe/StatusCodeRuntimeDxe/DEBUG/StatusCodeRuntimeDxe.dll
>  (ImageBase=000000000F556000, EntryPoint=000000000F55628F) !!!!
> 
> I did check with other guest (Windows, Ubuntu, Debian Jessie), and they are
> working correctly. Debian Wheezy is the only one that fail.

I don't have an environment to reproduce this in. I think we should try
to understand this problem better, before deciding how to make it go away.

Please locate the "StatusCodeRuntimeDxe.debug" file in your Build
directory (ie. under the location listed in the error report). Then,
please disassemble it with "objdump -S". The fault location in the
disassembly can be found based on RIP, ImageBase and EntryPoint; please
see an example here:

http://thread.gmane.org/gmane.comp.bios.tianocore.devel/7300/focus=7305

First I'd like to understand if this is a bug in Debian Wheezy's grub,
or if the bug is in StatusCodeRuntimeDxe, and Debian Wheezy's grub only
exposes that bug.

Once we know that, we can decide how to fix the issue.

If StatusCodeRuntimeDxe is problematic (or something else in the
firmware), we should just fix that.

If Debian Wheezy's grub is broken, there should be at least three
possibilities:

- Turn off NX in the domU (as explained at the end of the commit
  message).

- Report a bug to Debian, get Wheezy's grub fixed. Should be rolled
  into the next Wheezy update (= 7.10).

- Add a build option to OVMF that turns off this feature PCD.
  (According to Wikipedia, Debian Wheezy ("oldstable") is expected to
  get long term support until 2018-05. I certainly wouldn't like to
  keep the PCD off that long, as the default, for the sake of one guest
  Linux distro, especially given that Jessie ("stable") seems to work.
  Therefore the default should remain on, and be overridden at specific
  build sites if necessary.) I'm certainly willing to write the patch
  for this, if we decide that would be best.

But, again, please try to dig a bit more into the page fault first.

Thanks!
Laszlo

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel