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

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

[Anthony PERARD]

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@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 <pbonzini@redhat.com>
> >> Cc: Jordan Justen <jordan.l.justen@intel.com>
> >> Cc: "Zeng, Star" <star.zeng@intel.com>
> >> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> > 
> > Reviewed by: Star Zeng <star.zeng@intel.com>
> 
> 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.

Thanks,

-- 
Anthony PERARD

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

[Laszlo Ersek]

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@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 <pbonzini@redhat.com>
>>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>>> Cc: "Zeng, Star" <star.zeng@intel.com>
>>>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>>>
>>> Reviewed by: Star Zeng <star.zeng@intel.com>
>>
>> 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

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

[Jan Beulich]

>>> On 09.09.15 at 00:23, <lersek@redhat.com> wrote:
> On 09/08/15 19:26, Anthony PERARD wrote:
>> 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/Runtime
> Dxe/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;

I don't think the exact instruction at that address really matters. The
main question appears to be why RIP and RSP both point into the
same page (see also the subject of Anthony's mail). I.e. we need to
spot the entity setting the stack to a page that also contains code,
or placing code on the stack. That's unlikely to be found by identifying
the instruction RIP points to, but rather (sadly not part of the state
dump) something higher up the call chain.

Jan

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

[Laszlo Ersek]

On 09/09/15 09:06, Jan Beulich wrote:
>>>> On 09.09.15 at 00:23, <lersek@redhat.com> wrote:
>> On 09/08/15 19:26, Anthony PERARD wrote:
>>> 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/Runtime
>> Dxe/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;
> 
> I don't think the exact instruction at that address really matters. The
> main question appears to be why RIP and RSP both point into the
> same page (see also the subject of Anthony's mail). I.e. we need to
> spot the entity setting the stack to a page that also contains code,
> or placing code on the stack.

Good point!

(... FWIW, I've had luck on several occasions in the past deducing the
the origin of the data from the data itself. So if we can see the "code
on the stack", maybe that could help us figure out where it comes from.
Just an idea; might not apply very well here.)

> That's unlikely to be found by identifying
> the instruction RIP points to, but rather (sadly not part of the state
> dump) something higher up the call chain.

As far as I can see, Debian switched from grub2 v1.99 to v2.02, from
Wheezy to Jessie. Based on the grub2 commit history, quite a few things
happened in grub2 in that timeframe. Should we perhaps ask the grub2
developers?

Thanks
Laszlo

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

[Ian Campbell]

On Wed, 2015-09-09 at 01:06 -0600, Jan Beulich wrote:
> > > > On 09.09.15 at 00:23, <lersek@redhat.com> wrote:
> > On 09/08/15 19:26, Anthony PERARD wrote:
> > > 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/R
> > untime
> > Dxe/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;
> 
> I don't think the exact instruction at that address really matters. The
> main question appears to be why RIP and RSP both point into the
> same page (see also the subject of Anthony's mail).

I'm not 100% what is going on, but if this (executable code on stack) is
happening in grub is there something which is explicitly forbidden to UEFI
apps by the UEFI spec?

Or is it happening within UEFI itself based on a call from grub.efi?


>  I.e. we need to
> spot the entity setting the stack to a page that also contains code,
> or placing code on the stack. That's unlikely to be found by identifying
> the instruction RIP points to, but rather (sadly not part of the state
> dump) something higher up the call chain.
> 
> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

[Jan Beulich]

>>> On 09.09.15 at 11:37, <ian.campbell@citrix.com> wrote:
> I'm not 100% what is going on, but if this (executable code on stack) is
> happening in grub is there something which is explicitly forbidden to UEFI
> apps by the UEFI spec?

Whether it's spelled out explicitly I don't know, but the separation
of memory types (*Code vs *Data) is clearly with the intention to
limit permissions. Hence an entity allocating *Data should not place
code there (as much as an entity allocating *Code shouldn't expect
to be able to write to that area, which kind of implies that such
allocations aren't useful from outside of UEFI, since then you have
no way to fill in the code you mean to execute).

> Or is it happening within UEFI itself based on a call from grub.efi?

That's still unclear at this point.

Jan

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

[Laszlo Ersek]

On 09/09/15 11:37, Ian Campbell wrote:
> On Wed, 2015-09-09 at 01:06 -0600, Jan Beulich wrote:
>>>>> On 09.09.15 at 00:23, <lersek@redhat.com> wrote:
>>> On 09/08/15 19:26, Anthony PERARD wrote:
>>>> 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/R
>>> untime
>>> Dxe/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;
>>
>> I don't think the exact instruction at that address really matters. The
>> main question appears to be why RIP and RSP both point into the
>> same page (see also the subject of Anthony's mail).
> 
> I'm not 100% what is going on,

me neither :)

> but if this (executable code on stack) is
> happening in grub is there something which is explicitly forbidden to UEFI
> apps by the UEFI spec?

Yes, there is. This small OvmfPkg patch only enables the edk2 feature
added by Star Zeng in
<https://github.com/tianocore/edk2/commit/5630cdfe> for OVMF. That patch
(also referenced in my commit message by SVN rev) says,

    This feature is added for UEFI spec that says
    "Stack may be marked as non-executable in identity mapped page
    tables".
    A PCD PcdSetNxForStack is added to turn on/off this feature, and it
    is FALSE by default.

A UEFI app runs (well, *starts*, anyway) before ExitBootServices() /
SetVirtualAddressMap(), so it's bound by the above.

The spec passage above is quoted from "2.3.2 IA-32 Platforms", and
"2.3.4 x64 Platforms", in chapter "2.3 Calling Conventions", where the
boot services time environment is specified.

This is new in UEFI-2.5, and it comes from Mantis ticket 1224: "Adding
support for No executable data areas".

... The question could be then if grub (in Wheezy) should be adapted to
UEFI-2.5 (if that's possible) or if OVMF should be built without this
feature.

Hmmm. Actually, I'm torn about the default for PcdSetNxForStack.

Namely, Mantis ticket 1224 has come up before. There's another edk2
sub-feature related to this UEFI spec feature / Mantis ticket; the
properties table (controlled by "PcdPropertiesTableEnable"), and the
effects it has on the UEFI memory map, and the requirements it presents
for UEFI OSes.

*That* sub-feature is extremely intrusive.
"MdeModulePkg/MdeModulePkg.dec" sets "PcdPropertiesTableEnable" TRUE by
default, and OvmfPkg inherits it. I have not overridden that default
just yet in OvmfPkg because the properties table feature depends on
something *else* too: sections in runtime DXE driver binaries must be
aligned at 4KB, and that is not ensured for OvmfPkg (yet?). Therefore,
the properties table feature is not active in OVMF at the moment due to
a "random build tools circumstance" -- and not because the feature flag
is actually disabled.

Now, the properties table feature absolutely cannot be (effectively)
enabled in OVMF as a default. It would break Windows 7 / Windows Server
2008 R2. A huge number of users run such guests for GPU passthrough.

The idea that just crossed my mind was to introduce a new build flag for
OVMF, like -D NOEXEC_DATA_ENABLE. And this would then control
PcdSetNxForStack *and* PcdPropertiesTableEnable *together*:

if NOEXEC_DATA_ENABLE:
  PcdSetNxForStack := TRUE
  PcdPropertiesTableEnable := TRUE
else
  PcdSetNxForStack := FALSE
  PcdPropertiesTableEnable := FALSE

However, I think that's a bad idea.

"PcdPropertiesTableEnable" breaks a very widely used guest OS for which
there is no update in sight (ie. no Service Pack 2 for Windows 7 /
Windows Server 2008 R2), but is otherwise supposed to be supported for
years to come.

Whereas Debian Wheezy is not as widely used as a guest (for one: it
"competes" with all the other Linux distros from the same timeframe).
Debian Wheezy also provides a quite non-intrusive upgrade path (to
Jessie), which is not the case for Windows 7. So the breakage casued by
setting PcdSetNxForStack has much smaller impact, I think, and the
benefits might outweigh the breakage.

Which just means that a heavy-handed -D NOEXEC_DATA_ENABLE, like above,
would not be appropriate. PcdSetNxForStack set, and
PcdPropertiesTableEnable clear, is a valid configuration.

In any case, I sort of convinced myself that Wheezy's grub is not at
fault; it was simply written against an earlier release of the UEFI spec.

How about this:

- (somewhat unrelatedly, but for completeness):
  I post a patch that exposes PcdPropertiesTableEnable with a build
  time flag (NX_PROP_TABLE_ENABLE) with an OVMF-default of FALSE,

- I post another patch that exposes PcdSetNxForStack with a build time
  flag (NX_STACK_ENABLE) with an OVMF-default of *TRUE*. You could then
  pass -D NX_STACK_ENABLE=FALSE when you build OVMF for any environment
  where Wheezy guests are expected.

Jordan?

Yet another (quite complex) possibility:

- According to "MdeModulePkg/MdeModulePkg.dec",
  a platform DSC can already type PcdPropertiesTableEnable as a dynamic
  PCD. We could allow the same for PcdSetNxForStack. Star?

  Both PCDs are consumed "acceptably late" (the first in the DXE core,
  the second in the DXE IPL PEIM). This would allow
  OvmfPkg/PlatformPei to set the PCDs dynamically.

- Then the question is how we pass in the PCD values from the outside.
  For QEMU/KVM virtual machines, we could use Gabriel's QEMU feature

  -fw_cfg name=opt/my_item_name,file=./my_blob.bin

  ie. no changes for QEMU would be necessary. Xen virtual machines
  don't have fw_cfg however, therefore "some other" info passing should
  be implemented in "OvmfPkg/PlatformPei/Xen.c", and (I assume!) in the
  host-side Xen tools as well.

Personally I think that this dynamic approach is overkill (mainly
because I'm fine with being unable to install Debian Wheezy guests, both
wearing and not wearing my red fedora; and because the properties table
feature is not active for *any* OVMF guests anyway, in practice).

So I'd like to ask you guys to decide which one you prefer: a simple
build time flag called NX_STACK_ENABLE (which will allow you to install
Wheezy guests, but deprive all other guests from a non-exec stack), or
the dynamic switches (which will take host-side work in Xen, plus a
matching OvmfPkg patch).

> 
> Or is it happening within UEFI itself based on a call from grub.efi?

(
I wrote this before the above paragraphs, but it's moot now:

It's unclear. As far as I can see from the error report, things blow up
after grub starts, but before it calls ExitBootServices(). The fault RIP
points into a runtime DXE driver. Which is both a consistent and an
inconsistent symptom.

It is "consistent" because a runtime DXE driver is certainly in memory;
it even survives ExitBootServices(), so its blowing up cannot be
immediately excluded on the grounds that "it's not there". It *is* there.

However, the symptom is also inconsistent / inexplicable because this
runtime DXE driver doesn't really have anything to do with grub. And
it's unclear why some of its machine could would exist in stack / NX
pages. Some memory may possibly be corrupt by the time the edk2 fault
handler associates the RIP with the driver load addresses. Not sure.
)

Thanks
Laszlo

> 
> 
>>  I.e. we need to
>> spot the entity setting the stack to a page that also contains code,
>> or placing code on the stack. That's unlikely to be found by identifying
>> the instruction RIP points to, but rather (sadly not part of the state
>> dump) something higher up the call chain.
>>
>> Jan
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel

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

[Ian Campbell]

On Wed, 2015-09-09 at 12:48 +0200, Laszlo Ersek wrote:

Thanks for all the info, I think I get it (although its not clear to me
whether how an app can claim to be UEFI 2.5 capable and what the transition
plan for legacy applications was going to be).

> ... The question could be then if grub (in Wheezy) should be adapted to
> UEFI-2.5 (if that's possible)

I don't know either I'm afraid.

[...]
> Hmmm. Actually, I'm torn about the default for PcdSetNxForStack.

I have a question: What attack vector is setting the stack as Nx in OVMF
(or even UEFI generally) trying to protect against? Or is this being done
for a reason other than security?

I understand why it is done for kernels and apps, but where does the
untrusted element which is being protected against come from when running
UEFI?

Ian.

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

[Paolo Bonzini]

On 09/09/2015 13:07, Ian Campbell wrote:
> I have a question: What attack vector is setting the stack as Nx in OVMF
> (or even UEFI generally) trying to protect against? Or is this being done
> for a reason other than security?
> 
> I understand why it is done for kernels and apps, but where does the
> untrusted element which is being protected against come from when running
> UEFI?

I guess something could attack shim.efi or GRUB, and subvert secure
boot's chain of trust.

Paolo

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

[Laszlo Ersek]

On 09/09/15 13:07, Ian Campbell wrote:
> On Wed, 2015-09-09 at 12:48 +0200, Laszlo Ersek wrote:
> 
> Thanks for all the info, I think I get it (although its not clear to me
> whether how an app can claim to be UEFI 2.5 capable and what the transition
> plan for legacy applications was going to be).
> 
>> ... The question could be then if grub (in Wheezy) should be adapted to
>> UEFI-2.5 (if that's possible)
> 
> I don't know either I'm afraid.
> 
> [...]
>> Hmmm. Actually, I'm torn about the default for PcdSetNxForStack.
> 
> I have a question: What attack vector is setting the stack as Nx in OVMF
> (or even UEFI generally) trying to protect against? Or is this being done
> for a reason other than security?

I think it is being done for security, generally speaking (ie. as far as
edk2 and the UEFI spec are concerned).

Specifically for OVMF, I wrote the patch because Paolo suggested it,
after (I think) he saw Star's feature patch on the edk2 mailing list. I
thought it was a good suggestion, even in case we had no particular
attack in mind: OVMF is frequently used as a UEFI development / test
bed, and we try to enable new edk2 / UEFI features in it, unless the
firmware size increase would be large, or something would break. (In
which cases we usually bracket the new feature with build time flags.)

I did consider regressions while writing this patch. In the commit
message I mentioned "-cpu <model>,-nx" as a way to turn of NX support in
the virtual CPU (which the edk2 feature dynamically detects and depends
on), should anything break. I did not expect two things: (a) old grub
actually executes stuff from the stack, (b) Xen has no way (?) to
disable NX in a VCPU (or maybe that would be detrimental for other reasons).

> I understand why it is done for kernels and apps, but where does the
> untrusted element which is being protected against come from when running
> UEFI?

I'm sure this is explained in the five ECR documents attached to Mantis
ticket 1224:

https://mantis.uefi.org/mantis/view.php?id=1224

Unfortunately, I don't think I can just publish those ECRs here (or dump
the mantis ticket).

I have no clue why UEFI development is so secretive. I had been annoyed
by not seeing mantis tickets myself (and thereby missing the
justification behind new UEFI features), so at some point I applied for
"non-voter, observer-only" membership in both the PIWG and the USWG, and
I got them. (I'm fuzzy on the details if this was helped by my being
@redhat.com -- it probably was.)

Perhaps you can also apply for membership (and read the ECRs on the
ticket), or hopefully Star (who implemented the feature) could summarize
the purpose here?

(I won't try, based on the ECRs, because that will unavoidably put me in
the position where I have to explain and defend the feature. Thanks, but
no, thanks :))

Cheers
Laszlo

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

[Laszlo Ersek]

On 09/09/15 13:30, Paolo Bonzini wrote:
> 
> 
> On 09/09/2015 13:07, Ian Campbell wrote:
>> I have a question: What attack vector is setting the stack as Nx in OVMF
>> (or even UEFI generally) trying to protect against? Or is this being done
>> for a reason other than security?
>>
>> I understand why it is done for kernels and apps, but where does the
>> untrusted element which is being protected against come from when running
>> UEFI?
> 
> I guess something could attack shim.efi or GRUB, and subvert secure
> boot's chain of trust.

... or the firmware could be fed some malicious data over the network,
when the fw (e.g. shim) boots off PXE (or, in case of edk2, HTTP), and a
buffer overflow could lead to the execution of arbitrary code?...

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

[Jan Beulich]

>>> On 09.09.15 at 12:48, <lersek@redhat.com> wrote:
> Personally I think that this dynamic approach is overkill (mainly
> because I'm fine with being unable to install Debian Wheezy guests, both
> wearing and not wearing my red fedora; and because the properties table
> feature is not active for *any* OVMF guests anyway, in practice).

I can only guess that PCD stands for "Platform Configuration Data".
However, I would want to suggest an even more dynamic approach:
Assuming that within the core UEFI code it ought to be possible to
flip between executable and non-executable mapping of the stack,
and considering that PE headers can carry target version numbers,
how about reverting to an executable stack as long as there's at
least one binary loaded that isn't claiming to be 2.5 compatible?

Jan

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

[Laszlo Ersek]

On 09/09/15 14:08, Jan Beulich wrote:
>>>> On 09.09.15 at 12:48, <lersek@redhat.com> wrote:
>> Personally I think that this dynamic approach is overkill (mainly
>> because I'm fine with being unable to install Debian Wheezy guests, both
>> wearing and not wearing my red fedora; and because the properties table
>> feature is not active for *any* OVMF guests anyway, in practice).
> 
> I can only guess that PCD stands for "Platform Configuration Data".

Yes, PCD stands for Platform Configuration Database.

> However, I would want to suggest an even more dynamic approach:
> Assuming that within the core UEFI code it ought to be possible to
> flip between executable and non-executable mapping of the stack,
> and considering that PE headers can carry target version numbers,
> how about reverting to an executable stack as long as there's at
> least one binary loaded that isn't claiming to be 2.5 compatible?

This would require very intrusive changes (to be implemented by people
other than me). Other concerns I have:

- I'm not sure if UEFI applications have any means to advertize what
  revision of the specification they target. (As you mention.)

- I could be mistaken, but this looks like it could weaken the
  protection offered by the platform feature. UEFI applications and
  UEFI drivers are expected to come from third parties. It could be
  argued that a non-compatible UEFI app or driver (like Wheezy's grub
  in this case) *should* preferably crash, immediately, in a controlled
  manner, instead of silently downgrading the protection for the stack,
  and thereby opening up an avenue for attacks.

  I'm not fully convinced about this, but it appears this should be
  controlled somewhere in the platform code. (Like a knob in the BIOS
  setup on physical platforms, or some enlighted info channel in
  guests.) Assuming we'd like it dynamic.

Thanks
Laszlo

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

[Jan Beulich]

>>> On 09.09.15 at 15:04, <lersek@redhat.com> wrote:
> On 09/09/15 14:08, Jan Beulich wrote:
>>>>> On 09.09.15 at 12:48, <lersek@redhat.com> wrote:
>> However, I would want to suggest an even more dynamic approach:
>> Assuming that within the core UEFI code it ought to be possible to
>> flip between executable and non-executable mapping of the stack,
>> and considering that PE headers can carry target version numbers,
>> how about reverting to an executable stack as long as there's at
>> least one binary loaded that isn't claiming to be 2.5 compatible?
> 
> This would require very intrusive changes (to be implemented by people
> other than me). Other concerns I have:
> 
> - I'm not sure if UEFI applications have any means to advertize what
>   revision of the specification they target. (As you mention.)

They can (as I said). Whether the image loader in UEFI actually
looks at the data today is another thing, as is whether everyone
makes sure their binaries say so.

And then again I realize that usually one would rather state the
minimum version required than that of the specification a binary
was built against, so perhaps the idea was a bad one anyway.

Jan

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

[Zeng, Star]

On 2015/9/9 18:48, Laszlo Ersek wrote:
> me neither :)
>
>> but if this (executable code on stack) is
>> happening in grub is there something which is explicitly forbidden to UEFI
>> apps by the UEFI spec?
>
> Yes, there is. This small OvmfPkg patch only enables the edk2 feature
> added by Star Zeng in
> <https://github.com/tianocore/edk2/commit/5630cdfe> for OVMF. That patch
> (also referenced in my commit message by SVN rev) says,
>
>      This feature is added for UEFI spec that says
>      "Stack may be marked as non-executable in identity mapped page
>      tables".
>      A PCD PcdSetNxForStack is added to turn on/off this feature, and it
>      is FALSE by default.
>
> A UEFI app runs (well, *starts*, anyway) before ExitBootServices() /
> SetVirtualAddressMap(), so it's bound by the above.
>
> The spec passage above is quoted from "2.3.2 IA-32 Platforms", and
> "2.3.4 x64 Platforms", in chapter "2.3 Calling Conventions", where the
> boot services time environment is specified.
>
> This is new in UEFI-2.5, and it comes from Mantis ticket 1224: "Adding
> support for No executable data areas".
>
> ... The question could be then if grub (in Wheezy) should be adapted to
> UEFI-2.5 (if that's possible) or if OVMF should be built without this
> feature.
>
> Hmmm. Actually, I'm torn about the default for PcdSetNxForStack.
>
> Namely, Mantis ticket 1224 has come up before. There's another edk2
> sub-feature related to this UEFI spec feature / Mantis ticket; the
> properties table (controlled by "PcdPropertiesTableEnable"), and the
> effects it has on the UEFI memory map, and the requirements it presents
> for UEFI OSes.
>
> *That* sub-feature is extremely intrusive.
> "MdeModulePkg/MdeModulePkg.dec" sets "PcdPropertiesTableEnable" TRUE by
> default, and OvmfPkg inherits it. I have not overridden that default
> just yet in OvmfPkg because the properties table feature depends on
> something *else* too: sections in runtime DXE driver binaries must be
> aligned at 4KB, and that is not ensured for OvmfPkg (yet?). Therefore,
> the properties table feature is not active in OVMF at the moment due to
> a "random build tools circumstance" -- and not because the feature flag
> is actually disabled.
>
> Now, the properties table feature absolutely cannot be (effectively)
> enabled in OVMF as a default. It would break Windows 7 / Windows Server
> 2008 R2. A huge number of users run such guests for GPU passthrough.
>
> The idea that just crossed my mind was to introduce a new build flag for
> OVMF, like -D NOEXEC_DATA_ENABLE. And this would then control
> PcdSetNxForStack *and* PcdPropertiesTableEnable *together*:
>
> if NOEXEC_DATA_ENABLE:
>    PcdSetNxForStack := TRUE
>    PcdPropertiesTableEnable := TRUE
> else
>    PcdSetNxForStack := FALSE
>    PcdPropertiesTableEnable := FALSE
>
> However, I think that's a bad idea.
>
> "PcdPropertiesTableEnable" breaks a very widely used guest OS for which
> there is no update in sight (ie. no Service Pack 2 for Windows 7 /
> Windows Server 2008 R2), but is otherwise supposed to be supported for
> years to come.
>
> Whereas Debian Wheezy is not as widely used as a guest (for one: it
> "competes" with all the other Linux distros from the same timeframe).
> Debian Wheezy also provides a quite non-intrusive upgrade path (to
> Jessie), which is not the case for Windows 7. So the breakage casued by
> setting PcdSetNxForStack has much smaller impact, I think, and the
> benefits might outweigh the breakage.
>
> Which just means that a heavy-handed -D NOEXEC_DATA_ENABLE, like above,
> would not be appropriate. PcdSetNxForStack set, and
> PcdPropertiesTableEnable clear, is a valid configuration.
>
> In any case, I sort of convinced myself that Wheezy's grub is not at
> fault; it was simply written against an earlier release of the UEFI spec.
>
> How about this:
>
> - (somewhat unrelatedly, but for completeness):
>    I post a patch that exposes PcdPropertiesTableEnable with a build
>    time flag (NX_PROP_TABLE_ENABLE) with an OVMF-default of FALSE,
>
> - I post another patch that exposes PcdSetNxForStack with a build time
>    flag (NX_STACK_ENABLE) with an OVMF-default of *TRUE*. You could then
>    pass -D NX_STACK_ENABLE=FALSE when you build OVMF for any environment
>    where Wheezy guests are expected.
>
> Jordan?
>
> Yet another (quite complex) possibility:
>
> - According to "MdeModulePkg/MdeModulePkg.dec",
>    a platform DSC can already type PcdPropertiesTableEnable as a dynamic
>    PCD. We could allow the same for PcdSetNxForStack. Star?

I think PcdSetNxForStack could be extended to support dynamic type if 
the platform really needs to set the PCD dynamically after some 
condition check.

>
>    Both PCDs are consumed "acceptably late" (the first in the DXE core,
>    the second in the DXE IPL PEIM). This would allow
>    OvmfPkg/PlatformPei to set the PCDs dynamically.
>
> - Then the question is how we pass in the PCD values from the outside.
>    For QEMU/KVM virtual machines, we could use Gabriel's QEMU feature
>
>    -fw_cfg name=opt/my_item_name,file=./my_blob.bin
>
>    ie. no changes for QEMU would be necessary. Xen virtual machines
>    don't have fw_cfg however, therefore "some other" info passing should
>    be implemented in "OvmfPkg/PlatformPei/Xen.c", and (I assume!) in the
>    host-side Xen tools as well.
>
> Personally I think that this dynamic approach is overkill (mainly
> because I'm fine with being unable to install Debian Wheezy guests, both
> wearing and not wearing my red fedora; and because the properties table
> feature is not active for *any* OVMF guests anyway, in practice).
>
> So I'd like to ask you guys to decide which one you prefer: a simple
> build time flag called NX_STACK_ENABLE (which will allow you to install
> Wheezy guests, but deprive all other guests from a non-exec stack), or
> the dynamic switches (which will take host-side work in Xen, plus a
> matching OvmfPkg patch).
>
>>
>> Or is it happening within UEFI itself based on a call from grub.efi?
>
> (
> I wrote this before the above paragraphs, but it's moot now:
>
> It's unclear. As far as I can see from the error report, things blow up
> after grub starts, but before it calls ExitBootServices(). The fault RIP
> points into a runtime DXE driver. Which is both a consistent and an
> inconsistent symptom.
>
> It is "consistent" because a runtime DXE driver is certainly in memory;
> it even survives ExitBootServices(), so its blowing up cannot be
> immediately excluded on the grounds that "it's not there". It *is* there.
>
> However, the symptom is also inconsistent / inexplicable because this
> runtime DXE driver doesn't really have anything to do with grub. And
> it's unclear why some of its machine could would exist in stack / NX
> pages. Some memory may possibly be corrupt by the time the edk2 fault
> handler associates the RIP with the driver load addresses. Not sure.
> )
>
> Thanks
> Laszlo
>

Thanks,
Star

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

[Zeng, Star]

On 2015/9/9 19:30, Laszlo Ersek wrote:
> On 09/09/15 13:07, Ian Campbell wrote:
>> On Wed, 2015-09-09 at 12:48 +0200, Laszlo Ersek wrote:
>>
>> Thanks for all the info, I think I get it (although its not clear to me
>> whether how an app can claim to be UEFI 2.5 capable and what the transition
>> plan for legacy applications was going to be).
>>
>>> ... The question could be then if grub (in Wheezy) should be adapted to
>>> UEFI-2.5 (if that's possible)
>>
>> I don't know either I'm afraid.
>>
>> [...]
>>> Hmmm. Actually, I'm torn about the default for PcdSetNxForStack.
>>
>> I have a question: What attack vector is setting the stack as Nx in OVMF
>> (or even UEFI generally) trying to protect against? Or is this being done
>> for a reason other than security?
>
> I think it is being done for security, generally speaking (ie. as far as
> edk2 and the UEFI spec are concerned).
>
> Specifically for OVMF, I wrote the patch because Paolo suggested it,
> after (I think) he saw Star's feature patch on the edk2 mailing list. I
> thought it was a good suggestion, even in case we had no particular
> attack in mind: OVMF is frequently used as a UEFI development / test
> bed, and we try to enable new edk2 / UEFI features in it, unless the
> firmware size increase would be large, or something would break. (In
> which cases we usually bracket the new feature with build time flags.)
>
> I did consider regressions while writing this patch. In the commit
> message I mentioned "-cpu <model>,-nx" as a way to turn of NX support in
> the virtual CPU (which the edk2 feature dynamically detects and depends
> on), should anything break. I did not expect two things: (a) old grub
> actually executes stuff from the stack, (b) Xen has no way (?) to
> disable NX in a VCPU (or maybe that would be detrimental for other reasons).
>
>> I understand why it is done for kernels and apps, but where does the
>> untrusted element which is being protected against come from when running
>> UEFI?
>
> I'm sure this is explained in the five ECR documents attached to Mantis
> ticket 1224:
>
> https://mantis.uefi.org/mantis/view.php?id=1224
>
> Unfortunately, I don't think I can just publish those ECRs here (or dump
> the mantis ticket).
>
> I have no clue why UEFI development is so secretive. I had been annoyed
> by not seeing mantis tickets myself (and thereby missing the
> justification behind new UEFI features), so at some point I applied for
> "non-voter, observer-only" membership in both the PIWG and the USWG, and
> I got them. (I'm fuzzy on the details if this was helped by my being
> @redhat.com -- it probably was.)
>
> Perhaps you can also apply for membership (and read the ECRs on the
> ticket), or hopefully Star (who implemented the feature) could summarize
> the purpose here?

According to the mantis, the idea should come from 
http://www.uefi.org/sites/default/files/resources/3_-_UEFI_Summit_-_Security_Defenses_PTEC.pdf 
and adopted by USWG to UEFI has more abilities to provide defense in depth.

>
> (I won't try, based on the ECRs, because that will unavoidably put me in
> the position where I have to explain and defend the feature. Thanks, but
> no, thanks :))
>
> Cheers
> Laszlo
>

Thanks,
Star

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

[Laszlo Ersek]

On 09/10/15 05:05, Zeng, Star wrote:
> On 2015/9/9 18:48, Laszlo Ersek wrote:
>> me neither :)
>>
>>> but if this (executable code on stack) is
>>> happening in grub is there something which is explicitly forbidden to
>>> UEFI
>>> apps by the UEFI spec?
>>
>> Yes, there is. This small OvmfPkg patch only enables the edk2 feature
>> added by Star Zeng in
>> <https://github.com/tianocore/edk2/commit/5630cdfe> for OVMF. That patch
>> (also referenced in my commit message by SVN rev) says,
>>
>>      This feature is added for UEFI spec that says
>>      "Stack may be marked as non-executable in identity mapped page
>>      tables".
>>      A PCD PcdSetNxForStack is added to turn on/off this feature, and it
>>      is FALSE by default.
>>
>> A UEFI app runs (well, *starts*, anyway) before ExitBootServices() /
>> SetVirtualAddressMap(), so it's bound by the above.
>>
>> The spec passage above is quoted from "2.3.2 IA-32 Platforms", and
>> "2.3.4 x64 Platforms", in chapter "2.3 Calling Conventions", where the
>> boot services time environment is specified.
>>
>> This is new in UEFI-2.5, and it comes from Mantis ticket 1224: "Adding
>> support for No executable data areas".
>>
>> ... The question could be then if grub (in Wheezy) should be adapted to
>> UEFI-2.5 (if that's possible) or if OVMF should be built without this
>> feature.
>>
>> Hmmm. Actually, I'm torn about the default for PcdSetNxForStack.
>>
>> Namely, Mantis ticket 1224 has come up before. There's another edk2
>> sub-feature related to this UEFI spec feature / Mantis ticket; the
>> properties table (controlled by "PcdPropertiesTableEnable"), and the
>> effects it has on the UEFI memory map, and the requirements it presents
>> for UEFI OSes.
>>
>> *That* sub-feature is extremely intrusive.
>> "MdeModulePkg/MdeModulePkg.dec" sets "PcdPropertiesTableEnable" TRUE by
>> default, and OvmfPkg inherits it. I have not overridden that default
>> just yet in OvmfPkg because the properties table feature depends on
>> something *else* too: sections in runtime DXE driver binaries must be
>> aligned at 4KB, and that is not ensured for OvmfPkg (yet?). Therefore,
>> the properties table feature is not active in OVMF at the moment due to
>> a "random build tools circumstance" -- and not because the feature flag
>> is actually disabled.
>>
>> Now, the properties table feature absolutely cannot be (effectively)
>> enabled in OVMF as a default. It would break Windows 7 / Windows Server
>> 2008 R2. A huge number of users run such guests for GPU passthrough.
>>
>> The idea that just crossed my mind was to introduce a new build flag for
>> OVMF, like -D NOEXEC_DATA_ENABLE. And this would then control
>> PcdSetNxForStack *and* PcdPropertiesTableEnable *together*:
>>
>> if NOEXEC_DATA_ENABLE:
>>    PcdSetNxForStack := TRUE
>>    PcdPropertiesTableEnable := TRUE
>> else
>>    PcdSetNxForStack := FALSE
>>    PcdPropertiesTableEnable := FALSE
>>
>> However, I think that's a bad idea.
>>
>> "PcdPropertiesTableEnable" breaks a very widely used guest OS for which
>> there is no update in sight (ie. no Service Pack 2 for Windows 7 /
>> Windows Server 2008 R2), but is otherwise supposed to be supported for
>> years to come.
>>
>> Whereas Debian Wheezy is not as widely used as a guest (for one: it
>> "competes" with all the other Linux distros from the same timeframe).
>> Debian Wheezy also provides a quite non-intrusive upgrade path (to
>> Jessie), which is not the case for Windows 7. So the breakage casued by
>> setting PcdSetNxForStack has much smaller impact, I think, and the
>> benefits might outweigh the breakage.
>>
>> Which just means that a heavy-handed -D NOEXEC_DATA_ENABLE, like above,
>> would not be appropriate. PcdSetNxForStack set, and
>> PcdPropertiesTableEnable clear, is a valid configuration.
>>
>> In any case, I sort of convinced myself that Wheezy's grub is not at
>> fault; it was simply written against an earlier release of the UEFI spec.
>>
>> How about this:
>>
>> - (somewhat unrelatedly, but for completeness):
>>    I post a patch that exposes PcdPropertiesTableEnable with a build
>>    time flag (NX_PROP_TABLE_ENABLE) with an OVMF-default of FALSE,
>>
>> - I post another patch that exposes PcdSetNxForStack with a build time
>>    flag (NX_STACK_ENABLE) with an OVMF-default of *TRUE*. You could then
>>    pass -D NX_STACK_ENABLE=FALSE when you build OVMF for any environment
>>    where Wheezy guests are expected.
>>
>> Jordan?
>>
>> Yet another (quite complex) possibility:
>>
>> - According to "MdeModulePkg/MdeModulePkg.dec",
>>    a platform DSC can already type PcdPropertiesTableEnable as a dynamic
>>    PCD. We could allow the same for PcdSetNxForStack. Star?
> 
> I think PcdSetNxForStack could be extended to support dynamic type if
> the platform really needs to set the PCD dynamically after some
> condition check.

Thank you. (Also for the PDF reference in your other email.)

Laszlo

> 
>>
>>    Both PCDs are consumed "acceptably late" (the first in the DXE core,
>>    the second in the DXE IPL PEIM). This would allow
>>    OvmfPkg/PlatformPei to set the PCDs dynamically.
>>
>> - Then the question is how we pass in the PCD values from the outside.
>>    For QEMU/KVM virtual machines, we could use Gabriel's QEMU feature
>>
>>    -fw_cfg name=opt/my_item_name,file=./my_blob.bin
>>
>>    ie. no changes for QEMU would be necessary. Xen virtual machines
>>    don't have fw_cfg however, therefore "some other" info passing should
>>    be implemented in "OvmfPkg/PlatformPei/Xen.c", and (I assume!) in the
>>    host-side Xen tools as well.
>>
>> Personally I think that this dynamic approach is overkill (mainly
>> because I'm fine with being unable to install Debian Wheezy guests, both
>> wearing and not wearing my red fedora; and because the properties table
>> feature is not active for *any* OVMF guests anyway, in practice).
>>
>> So I'd like to ask you guys to decide which one you prefer: a simple
>> build time flag called NX_STACK_ENABLE (which will allow you to install
>> Wheezy guests, but deprive all other guests from a non-exec stack), or
>> the dynamic switches (which will take host-side work in Xen, plus a
>> matching OvmfPkg patch).
>>
>>>
>>> Or is it happening within UEFI itself based on a call from grub.efi?
>>
>> (
>> I wrote this before the above paragraphs, but it's moot now:
>>
>> It's unclear. As far as I can see from the error report, things blow up
>> after grub starts, but before it calls ExitBootServices(). The fault RIP
>> points into a runtime DXE driver. Which is both a consistent and an
>> inconsistent symptom.
>>
>> It is "consistent" because a runtime DXE driver is certainly in memory;
>> it even survives ExitBootServices(), so its blowing up cannot be
>> immediately excluded on the grounds that "it's not there". It *is* there.
>>
>> However, the symptom is also inconsistent / inexplicable because this
>> runtime DXE driver doesn't really have anything to do with grub. And
>> it's unclear why some of its machine could would exist in stack / NX
>> pages. Some memory may possibly be corrupt by the time the edk2 fault
>> handler associates the RIP with the driver load addresses. Not sure.
>> )
>>
>> Thanks
>> Laszlo
>>
> 
> Thanks,
> Star
> 

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

[Laszlo Ersek]

On 09/09/15 12:48, Laszlo Ersek wrote:
> On 09/09/15 11:37, Ian Campbell wrote:
>> On Wed, 2015-09-09 at 01:06 -0600, Jan Beulich wrote:
>>>>>> On 09.09.15 at 00:23, <lersek@redhat.com> wrote:
>>>> On 09/08/15 19:26, Anthony PERARD wrote:
>>>>> 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/R
>>>> untime
>>>> Dxe/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;
>>>
>>> I don't think the exact instruction at that address really matters. The
>>> main question appears to be why RIP and RSP both point into the
>>> same page (see also the subject of Anthony's mail).
>>
>> I'm not 100% what is going on,
> 
> me neither :)
> 
>> but if this (executable code on stack) is
>> happening in grub is there something which is explicitly forbidden to UEFI
>> apps by the UEFI spec?
> 
> Yes, there is. This small OvmfPkg patch only enables the edk2 feature
> added by Star Zeng in
> <https://github.com/tianocore/edk2/commit/5630cdfe> for OVMF. That patch
> (also referenced in my commit message by SVN rev) says,
> 
>     This feature is added for UEFI spec that says
>     "Stack may be marked as non-executable in identity mapped page
>     tables".
>     A PCD PcdSetNxForStack is added to turn on/off this feature, and it
>     is FALSE by default.
> 
> A UEFI app runs (well, *starts*, anyway) before ExitBootServices() /
> SetVirtualAddressMap(), so it's bound by the above.
> 
> The spec passage above is quoted from "2.3.2 IA-32 Platforms", and
> "2.3.4 x64 Platforms", in chapter "2.3 Calling Conventions", where the
> boot services time environment is specified.
> 
> This is new in UEFI-2.5, and it comes from Mantis ticket 1224: "Adding
> support for No executable data areas".
> 
> ... The question could be then if grub (in Wheezy) should be adapted to
> UEFI-2.5 (if that's possible) or if OVMF should be built without this
> feature.
> 
> Hmmm. Actually, I'm torn about the default for PcdSetNxForStack.
> 
> Namely, Mantis ticket 1224 has come up before. There's another edk2
> sub-feature related to this UEFI spec feature / Mantis ticket; the
> properties table (controlled by "PcdPropertiesTableEnable"), and the
> effects it has on the UEFI memory map, and the requirements it presents
> for UEFI OSes.
> 
> *That* sub-feature is extremely intrusive.
> "MdeModulePkg/MdeModulePkg.dec" sets "PcdPropertiesTableEnable" TRUE by
> default, and OvmfPkg inherits it. I have not overridden that default
> just yet in OvmfPkg because the properties table feature depends on
> something *else* too: sections in runtime DXE driver binaries must be
> aligned at 4KB, and that is not ensured for OvmfPkg (yet?). Therefore,
> the properties table feature is not active in OVMF at the moment due to
> a "random build tools circumstance" -- and not because the feature flag
> is actually disabled.
> 
> Now, the properties table feature absolutely cannot be (effectively)
> enabled in OVMF as a default. It would break Windows 7 / Windows Server
> 2008 R2. A huge number of users run such guests for GPU passthrough.
> 
> The idea that just crossed my mind was to introduce a new build flag for
> OVMF, like -D NOEXEC_DATA_ENABLE. And this would then control
> PcdSetNxForStack *and* PcdPropertiesTableEnable *together*:
> 
> if NOEXEC_DATA_ENABLE:
>   PcdSetNxForStack := TRUE
>   PcdPropertiesTableEnable := TRUE
> else
>   PcdSetNxForStack := FALSE
>   PcdPropertiesTableEnable := FALSE
> 
> However, I think that's a bad idea.
> 
> "PcdPropertiesTableEnable" breaks a very widely used guest OS for which
> there is no update in sight (ie. no Service Pack 2 for Windows 7 /
> Windows Server 2008 R2), but is otherwise supposed to be supported for
> years to come.
> 
> Whereas Debian Wheezy is not as widely used as a guest (for one: it
> "competes" with all the other Linux distros from the same timeframe).
> Debian Wheezy also provides a quite non-intrusive upgrade path (to
> Jessie), which is not the case for Windows 7. So the breakage casued by
> setting PcdSetNxForStack has much smaller impact, I think, and the
> benefits might outweigh the breakage.
> 
> Which just means that a heavy-handed -D NOEXEC_DATA_ENABLE, like above,
> would not be appropriate. PcdSetNxForStack set, and
> PcdPropertiesTableEnable clear, is a valid configuration.
> 
> In any case, I sort of convinced myself that Wheezy's grub is not at
> fault; it was simply written against an earlier release of the UEFI spec.
> 
> How about this:
> 
> - (somewhat unrelatedly, but for completeness):
>   I post a patch that exposes PcdPropertiesTableEnable with a build
>   time flag (NX_PROP_TABLE_ENABLE) with an OVMF-default of FALSE,
> 
> - I post another patch that exposes PcdSetNxForStack with a build time
>   flag (NX_STACK_ENABLE) with an OVMF-default of *TRUE*. You could then
>   pass -D NX_STACK_ENABLE=FALSE when you build OVMF for any environment
>   where Wheezy guests are expected.
> 
> Jordan?
> 
> Yet another (quite complex) possibility:
> 
> - According to "MdeModulePkg/MdeModulePkg.dec",
>   a platform DSC can already type PcdPropertiesTableEnable as a dynamic
>   PCD. We could allow the same for PcdSetNxForStack. Star?
> 
>   Both PCDs are consumed "acceptably late" (the first in the DXE core,
>   the second in the DXE IPL PEIM). This would allow
>   OvmfPkg/PlatformPei to set the PCDs dynamically.
> 
> - Then the question is how we pass in the PCD values from the outside.
>   For QEMU/KVM virtual machines, we could use Gabriel's QEMU feature
> 
>   -fw_cfg name=opt/my_item_name,file=./my_blob.bin
> 
>   ie. no changes for QEMU would be necessary. Xen virtual machines
>   don't have fw_cfg however, therefore "some other" info passing should
>   be implemented in "OvmfPkg/PlatformPei/Xen.c", and (I assume!) in the
>   host-side Xen tools as well.
> 
> Personally I think that this dynamic approach is overkill (mainly
> because I'm fine with being unable to install Debian Wheezy guests, both
> wearing and not wearing my red fedora; and because the properties table
> feature is not active for *any* OVMF guests anyway, in practice).
> 
> So I'd like to ask you guys to decide which one you prefer: a simple
> build time flag called NX_STACK_ENABLE (which will allow you to install
> Wheezy guests, but deprive all other guests from a non-exec stack), or
> the dynamic switches (which will take host-side work in Xen, plus a
> matching OvmfPkg patch).

I might go ahead and implement the -fw_cfg solution (for when OVMF runs
on QEMU), leaving any parallel Xen functionality to Xen developers, for
later.

Because, I just found out that the GRUB binary built into the bits-2005
release ISO <http://biosbits.org/download/> blows up the exact same way,
and *that* is very annoying to me personally.

Maybe we should invert the default. I'm not so convinced any longer that
the current default is right. I didn't know that the GRUB version with
code on the stack was this wide-spread.

Thanks
Laszlo

> 
>>
>> Or is it happening within UEFI itself based on a call from grub.efi?
> 
> (
> I wrote this before the above paragraphs, but it's moot now:
> 
> It's unclear. As far as I can see from the error report, things blow up
> after grub starts, but before it calls ExitBootServices(). The fault RIP
> points into a runtime DXE driver. Which is both a consistent and an
> inconsistent symptom.
> 
> It is "consistent" because a runtime DXE driver is certainly in memory;
> it even survives ExitBootServices(), so its blowing up cannot be
> immediately excluded on the grounds that "it's not there". It *is* there.
> 
> However, the symptom is also inconsistent / inexplicable because this
> runtime DXE driver doesn't really have anything to do with grub. And
> it's unclear why some of its machine could would exist in stack / NX
> pages. Some memory may possibly be corrupt by the time the edk2 fault
> handler associates the RIP with the driver load addresses. Not sure.
> )
> 
> Thanks
> Laszlo
> 
>>
>>
>>>  I.e. we need to
>>> spot the entity setting the stack to a page that also contains code,
>>> or placing code on the stack. That's unlikely to be found by identifying
>>> the instruction RIP points to, but rather (sadly not part of the state
>>> dump) something higher up the call chain.
>>>
>>> Jan
>>>
>>>
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@lists.xen.org
>>> http://lists.xen.org/xen-devel
> 

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

[Josh Triplett]

On Fri, Sep 11, 2015 at 01:43:53PM +0200, Laszlo Ersek wrote:
> On 09/09/15 12:48, Laszlo Ersek wrote:
> > On 09/09/15 11:37, Ian Campbell wrote:
> >> On Wed, 2015-09-09 at 01:06 -0600, Jan Beulich wrote:
> >>>>>> On 09.09.15 at 00:23, <lersek@redhat.com> wrote:
> >>>> On 09/08/15 19:26, Anthony PERARD wrote:
> >>>>> 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/R
> >>>> untime
> >>>> Dxe/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;
> >>>
> >>> I don't think the exact instruction at that address really matters. The
> >>> main question appears to be why RIP and RSP both point into the
> >>> same page (see also the subject of Anthony's mail).
> >>
> >> I'm not 100% what is going on,
> > 
> > me neither :)
> > 
> >> but if this (executable code on stack) is
> >> happening in grub is there something which is explicitly forbidden to UEFI
> >> apps by the UEFI spec?
> > 
> > Yes, there is. This small OvmfPkg patch only enables the edk2 feature
> > added by Star Zeng in
> > <https://github.com/tianocore/edk2/commit/5630cdfe> for OVMF. That patch
> > (also referenced in my commit message by SVN rev) says,
> > 
> >     This feature is added for UEFI spec that says
> >     "Stack may be marked as non-executable in identity mapped page
> >     tables".
> >     A PCD PcdSetNxForStack is added to turn on/off this feature, and it
> >     is FALSE by default.
> > 
> > A UEFI app runs (well, *starts*, anyway) before ExitBootServices() /
> > SetVirtualAddressMap(), so it's bound by the above.
> > 
> > The spec passage above is quoted from "2.3.2 IA-32 Platforms", and
> > "2.3.4 x64 Platforms", in chapter "2.3 Calling Conventions", where the
> > boot services time environment is specified.
> > 
> > This is new in UEFI-2.5, and it comes from Mantis ticket 1224: "Adding
> > support for No executable data areas".
> > 
> > ... The question could be then if grub (in Wheezy) should be adapted to
> > UEFI-2.5 (if that's possible) or if OVMF should be built without this
> > feature.
> > 
> > Hmmm. Actually, I'm torn about the default for PcdSetNxForStack.
> > 
> > Namely, Mantis ticket 1224 has come up before. There's another edk2
> > sub-feature related to this UEFI spec feature / Mantis ticket; the
> > properties table (controlled by "PcdPropertiesTableEnable"), and the
> > effects it has on the UEFI memory map, and the requirements it presents
> > for UEFI OSes.
> > 
> > *That* sub-feature is extremely intrusive.
> > "MdeModulePkg/MdeModulePkg.dec" sets "PcdPropertiesTableEnable" TRUE by
> > default, and OvmfPkg inherits it. I have not overridden that default
> > just yet in OvmfPkg because the properties table feature depends on
> > something *else* too: sections in runtime DXE driver binaries must be
> > aligned at 4KB, and that is not ensured for OvmfPkg (yet?). Therefore,
> > the properties table feature is not active in OVMF at the moment due to
> > a "random build tools circumstance" -- and not because the feature flag
> > is actually disabled.
> > 
> > Now, the properties table feature absolutely cannot be (effectively)
> > enabled in OVMF as a default. It would break Windows 7 / Windows Server
> > 2008 R2. A huge number of users run such guests for GPU passthrough.
> > 
> > The idea that just crossed my mind was to introduce a new build flag for
> > OVMF, like -D NOEXEC_DATA_ENABLE. And this would then control
> > PcdSetNxForStack *and* PcdPropertiesTableEnable *together*:
> > 
> > if NOEXEC_DATA_ENABLE:
> >   PcdSetNxForStack := TRUE
> >   PcdPropertiesTableEnable := TRUE
> > else
> >   PcdSetNxForStack := FALSE
> >   PcdPropertiesTableEnable := FALSE
> > 
> > However, I think that's a bad idea.
> > 
> > "PcdPropertiesTableEnable" breaks a very widely used guest OS for which
> > there is no update in sight (ie. no Service Pack 2 for Windows 7 /
> > Windows Server 2008 R2), but is otherwise supposed to be supported for
> > years to come.
> > 
> > Whereas Debian Wheezy is not as widely used as a guest (for one: it
> > "competes" with all the other Linux distros from the same timeframe).
> > Debian Wheezy also provides a quite non-intrusive upgrade path (to
> > Jessie), which is not the case for Windows 7. So the breakage casued by
> > setting PcdSetNxForStack has much smaller impact, I think, and the
> > benefits might outweigh the breakage.
> > 
> > Which just means that a heavy-handed -D NOEXEC_DATA_ENABLE, like above,
> > would not be appropriate. PcdSetNxForStack set, and
> > PcdPropertiesTableEnable clear, is a valid configuration.
> > 
> > In any case, I sort of convinced myself that Wheezy's grub is not at
> > fault; it was simply written against an earlier release of the UEFI spec.
> > 
> > How about this:
> > 
> > - (somewhat unrelatedly, but for completeness):
> >   I post a patch that exposes PcdPropertiesTableEnable with a build
> >   time flag (NX_PROP_TABLE_ENABLE) with an OVMF-default of FALSE,
> > 
> > - I post another patch that exposes PcdSetNxForStack with a build time
> >   flag (NX_STACK_ENABLE) with an OVMF-default of *TRUE*. You could then
> >   pass -D NX_STACK_ENABLE=FALSE when you build OVMF for any environment
> >   where Wheezy guests are expected.
> > 
> > Jordan?
> > 
> > Yet another (quite complex) possibility:
> > 
> > - According to "MdeModulePkg/MdeModulePkg.dec",
> >   a platform DSC can already type PcdPropertiesTableEnable as a dynamic
> >   PCD. We could allow the same for PcdSetNxForStack. Star?
> > 
> >   Both PCDs are consumed "acceptably late" (the first in the DXE core,
> >   the second in the DXE IPL PEIM). This would allow
> >   OvmfPkg/PlatformPei to set the PCDs dynamically.
> > 
> > - Then the question is how we pass in the PCD values from the outside.
> >   For QEMU/KVM virtual machines, we could use Gabriel's QEMU feature
> > 
> >   -fw_cfg name=opt/my_item_name,file=./my_blob.bin
> > 
> >   ie. no changes for QEMU would be necessary. Xen virtual machines
> >   don't have fw_cfg however, therefore "some other" info passing should
> >   be implemented in "OvmfPkg/PlatformPei/Xen.c", and (I assume!) in the
> >   host-side Xen tools as well.
> > 
> > Personally I think that this dynamic approach is overkill (mainly
> > because I'm fine with being unable to install Debian Wheezy guests, both
> > wearing and not wearing my red fedora; and because the properties table
> > feature is not active for *any* OVMF guests anyway, in practice).
> > 
> > So I'd like to ask you guys to decide which one you prefer: a simple
> > build time flag called NX_STACK_ENABLE (which will allow you to install
> > Wheezy guests, but deprive all other guests from a non-exec stack), or
> > the dynamic switches (which will take host-side work in Xen, plus a
> > matching OvmfPkg patch).
> 
> I might go ahead and implement the -fw_cfg solution (for when OVMF runs
> on QEMU), leaving any parallel Xen functionality to Xen developers, for
> later.
> 
> Because, I just found out that the GRUB binary built into the bits-2005
> release ISO <http://biosbits.org/download/> blows up the exact same way,
> and *that* is very annoying to me personally.
> 
> Maybe we should invert the default. I'm not so convinced any longer that
> the current default is right. I didn't know that the GRUB version with
> code on the stack was this wide-spread.

Heh.  Our fault for still using old (2.00) GRUB2, which we do plan to
upgrade at some point, though doing so doesn't top the TODO list.  But I
do agree that with OVMF not having previously enforced NX in the UEFI
environment, going from that to immediately enforcing it *by default*
seems a bit quick.  I would be surprised if doing so didn't break other
UEFI programs too.

Could OVMF build with NX support available, but disabled by default, so
that qemu can then turn it *on* with a -fw_cfg option?

Do any UEFI stacks on systems in the wild have NX turned on?  I don't
think it makes sense for OVMF to enable NX by default until a majority
of new physical systems do so.

- Josh Triplett

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

[Laszlo Ersek]

On 09/11/15 16:10, Josh Triplett wrote:
> On Fri, Sep 11, 2015 at 01:43:53PM +0200, Laszlo Ersek wrote:
>> On 09/09/15 12:48, Laszlo Ersek wrote:
>>> On 09/09/15 11:37, Ian Campbell wrote:
>>>> On Wed, 2015-09-09 at 01:06 -0600, Jan Beulich wrote:
>>>>>>>> On 09.09.15 at 00:23, <lersek@redhat.com> wrote:
>>>>>> On 09/08/15 19:26, Anthony PERARD wrote:
>>>>>>> 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/R
>>>>>> untime
>>>>>> Dxe/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;
>>>>>
>>>>> I don't think the exact instruction at that address really matters. The
>>>>> main question appears to be why RIP and RSP both point into the
>>>>> same page (see also the subject of Anthony's mail).
>>>>
>>>> I'm not 100% what is going on,
>>>
>>> me neither :)
>>>
>>>> but if this (executable code on stack) is
>>>> happening in grub is there something which is explicitly forbidden to UEFI
>>>> apps by the UEFI spec?
>>>
>>> Yes, there is. This small OvmfPkg patch only enables the edk2 feature
>>> added by Star Zeng in
>>> <https://github.com/tianocore/edk2/commit/5630cdfe> for OVMF. That patch
>>> (also referenced in my commit message by SVN rev) says,
>>>
>>>     This feature is added for UEFI spec that says
>>>     "Stack may be marked as non-executable in identity mapped page
>>>     tables".
>>>     A PCD PcdSetNxForStack is added to turn on/off this feature, and it
>>>     is FALSE by default.
>>>
>>> A UEFI app runs (well, *starts*, anyway) before ExitBootServices() /
>>> SetVirtualAddressMap(), so it's bound by the above.
>>>
>>> The spec passage above is quoted from "2.3.2 IA-32 Platforms", and
>>> "2.3.4 x64 Platforms", in chapter "2.3 Calling Conventions", where the
>>> boot services time environment is specified.
>>>
>>> This is new in UEFI-2.5, and it comes from Mantis ticket 1224: "Adding
>>> support for No executable data areas".
>>>
>>> ... The question could be then if grub (in Wheezy) should be adapted to
>>> UEFI-2.5 (if that's possible) or if OVMF should be built without this
>>> feature.
>>>
>>> Hmmm. Actually, I'm torn about the default for PcdSetNxForStack.
>>>
>>> Namely, Mantis ticket 1224 has come up before. There's another edk2
>>> sub-feature related to this UEFI spec feature / Mantis ticket; the
>>> properties table (controlled by "PcdPropertiesTableEnable"), and the
>>> effects it has on the UEFI memory map, and the requirements it presents
>>> for UEFI OSes.
>>>
>>> *That* sub-feature is extremely intrusive.
>>> "MdeModulePkg/MdeModulePkg.dec" sets "PcdPropertiesTableEnable" TRUE by
>>> default, and OvmfPkg inherits it. I have not overridden that default
>>> just yet in OvmfPkg because the properties table feature depends on
>>> something *else* too: sections in runtime DXE driver binaries must be
>>> aligned at 4KB, and that is not ensured for OvmfPkg (yet?). Therefore,
>>> the properties table feature is not active in OVMF at the moment due to
>>> a "random build tools circumstance" -- and not because the feature flag
>>> is actually disabled.
>>>
>>> Now, the properties table feature absolutely cannot be (effectively)
>>> enabled in OVMF as a default. It would break Windows 7 / Windows Server
>>> 2008 R2. A huge number of users run such guests for GPU passthrough.
>>>
>>> The idea that just crossed my mind was to introduce a new build flag for
>>> OVMF, like -D NOEXEC_DATA_ENABLE. And this would then control
>>> PcdSetNxForStack *and* PcdPropertiesTableEnable *together*:
>>>
>>> if NOEXEC_DATA_ENABLE:
>>>   PcdSetNxForStack := TRUE
>>>   PcdPropertiesTableEnable := TRUE
>>> else
>>>   PcdSetNxForStack := FALSE
>>>   PcdPropertiesTableEnable := FALSE
>>>
>>> However, I think that's a bad idea.
>>>
>>> "PcdPropertiesTableEnable" breaks a very widely used guest OS for which
>>> there is no update in sight (ie. no Service Pack 2 for Windows 7 /
>>> Windows Server 2008 R2), but is otherwise supposed to be supported for
>>> years to come.
>>>
>>> Whereas Debian Wheezy is not as widely used as a guest (for one: it
>>> "competes" with all the other Linux distros from the same timeframe).
>>> Debian Wheezy also provides a quite non-intrusive upgrade path (to
>>> Jessie), which is not the case for Windows 7. So the breakage casued by
>>> setting PcdSetNxForStack has much smaller impact, I think, and the
>>> benefits might outweigh the breakage.
>>>
>>> Which just means that a heavy-handed -D NOEXEC_DATA_ENABLE, like above,
>>> would not be appropriate. PcdSetNxForStack set, and
>>> PcdPropertiesTableEnable clear, is a valid configuration.
>>>
>>> In any case, I sort of convinced myself that Wheezy's grub is not at
>>> fault; it was simply written against an earlier release of the UEFI spec.
>>>
>>> How about this:
>>>
>>> - (somewhat unrelatedly, but for completeness):
>>>   I post a patch that exposes PcdPropertiesTableEnable with a build
>>>   time flag (NX_PROP_TABLE_ENABLE) with an OVMF-default of FALSE,
>>>
>>> - I post another patch that exposes PcdSetNxForStack with a build time
>>>   flag (NX_STACK_ENABLE) with an OVMF-default of *TRUE*. You could then
>>>   pass -D NX_STACK_ENABLE=FALSE when you build OVMF for any environment
>>>   where Wheezy guests are expected.
>>>
>>> Jordan?
>>>
>>> Yet another (quite complex) possibility:
>>>
>>> - According to "MdeModulePkg/MdeModulePkg.dec",
>>>   a platform DSC can already type PcdPropertiesTableEnable as a dynamic
>>>   PCD. We could allow the same for PcdSetNxForStack. Star?
>>>
>>>   Both PCDs are consumed "acceptably late" (the first in the DXE core,
>>>   the second in the DXE IPL PEIM). This would allow
>>>   OvmfPkg/PlatformPei to set the PCDs dynamically.
>>>
>>> - Then the question is how we pass in the PCD values from the outside.
>>>   For QEMU/KVM virtual machines, we could use Gabriel's QEMU feature
>>>
>>>   -fw_cfg name=opt/my_item_name,file=./my_blob.bin
>>>
>>>   ie. no changes for QEMU would be necessary. Xen virtual machines
>>>   don't have fw_cfg however, therefore "some other" info passing should
>>>   be implemented in "OvmfPkg/PlatformPei/Xen.c", and (I assume!) in the
>>>   host-side Xen tools as well.
>>>
>>> Personally I think that this dynamic approach is overkill (mainly
>>> because I'm fine with being unable to install Debian Wheezy guests, both
>>> wearing and not wearing my red fedora; and because the properties table
>>> feature is not active for *any* OVMF guests anyway, in practice).
>>>
>>> So I'd like to ask you guys to decide which one you prefer: a simple
>>> build time flag called NX_STACK_ENABLE (which will allow you to install
>>> Wheezy guests, but deprive all other guests from a non-exec stack), or
>>> the dynamic switches (which will take host-side work in Xen, plus a
>>> matching OvmfPkg patch).
>>
>> I might go ahead and implement the -fw_cfg solution (for when OVMF runs
>> on QEMU), leaving any parallel Xen functionality to Xen developers, for
>> later.
>>
>> Because, I just found out that the GRUB binary built into the bits-2005
>> release ISO <http://biosbits.org/download/> blows up the exact same way,
>> and *that* is very annoying to me personally.
>>
>> Maybe we should invert the default. I'm not so convinced any longer that
>> the current default is right. I didn't know that the GRUB version with
>> code on the stack was this wide-spread.
> 
> Heh.  Our fault for still using old (2.00) GRUB2, which we do plan to
> upgrade at some point, though doing so doesn't top the TODO list.  But I
> do agree that with OVMF not having previously enforced NX in the UEFI
> environment, going from that to immediately enforcing it *by default*
> seems a bit quick.  I would be surprised if doing so didn't break other
> UEFI programs too.
> 
> Could OVMF build with NX support available, but disabled by default, so
> that qemu can then turn it *on* with a -fw_cfg option?

It would be possible, yes.

I just posted a patch series that adds that dynamism, but it leaves the
NX stack turned on by default. (Ie. one needs to add the cmdline option
to turn it off.) If there's consensus to revert the default to off, I
can submit a v2.

> Do any UEFI stacks on systems in the wild have NX turned on?  I don't
> think it makes sense for OVMF to enable NX by default until a majority
> of new physical systems do so.

For me that's not so clear-cut. OVMF is frequently used as a UEFI
development environment (it's better to brick a virtual machine than
your physical dev platform...), so upstream should embrace new UEFI
features reasonably early, unless there are *grave* regressions.

One example I named was the properties table feature (new in UEFI-2.5).
It would break Windows 7. Given the large number of users running
Windows 7 on OVMF (mainly for GPU passthrough), such a regression would
be serious.

Breaking Debian Wheezy's and BITS's GRUB is also bad, but the former is
very old (and has a clear upgrade path), while the latter is mainly used
by developers (who can learn about the -fw_cfg switch by googling or
asking on the least without huge trouble). In this case I'm leaning
towards OVMF being "bleeding edge" by default. But, I could be convinced
otherwise.

Perhaps Xen users (who won't be helped by -fw_cfg) would also like to
see the default reverted? (At the price of not having accces to the
nx-stack feature at all?)

Thanks!
Laszlo

> 
> - Josh Triplett
> 

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

[Josh Triplett]

On Fri, Sep 11, 2015 at 05:28:06PM +0200, Laszlo Ersek wrote:
> On 09/11/15 16:10, Josh Triplett wrote:
> > On Fri, Sep 11, 2015 at 01:43:53PM +0200, Laszlo Ersek wrote:
> >> On 09/09/15 12:48, Laszlo Ersek wrote:
> >>> On 09/09/15 11:37, Ian Campbell wrote:
> >>>> On Wed, 2015-09-09 at 01:06 -0600, Jan Beulich wrote:
> >>>>>>>> On 09.09.15 at 00:23, <lersek@redhat.com> wrote:
> >>>>>> On 09/08/15 19:26, Anthony PERARD wrote:
> >>>>>>> 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/R
> >>>>>> untime
> >>>>>> Dxe/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;
> >>>>>
> >>>>> I don't think the exact instruction at that address really matters. The
> >>>>> main question appears to be why RIP and RSP both point into the
> >>>>> same page (see also the subject of Anthony's mail).
> >>>>
> >>>> I'm not 100% what is going on,
> >>>
> >>> me neither :)
> >>>
> >>>> but if this (executable code on stack) is
> >>>> happening in grub is there something which is explicitly forbidden to UEFI
> >>>> apps by the UEFI spec?
> >>>
> >>> Yes, there is. This small OvmfPkg patch only enables the edk2 feature
> >>> added by Star Zeng in
> >>> <https://github.com/tianocore/edk2/commit/5630cdfe> for OVMF. That patch
> >>> (also referenced in my commit message by SVN rev) says,
> >>>
> >>>     This feature is added for UEFI spec that says
> >>>     "Stack may be marked as non-executable in identity mapped page
> >>>     tables".
> >>>     A PCD PcdSetNxForStack is added to turn on/off this feature, and it
> >>>     is FALSE by default.
> >>>
> >>> A UEFI app runs (well, *starts*, anyway) before ExitBootServices() /
> >>> SetVirtualAddressMap(), so it's bound by the above.
> >>>
> >>> The spec passage above is quoted from "2.3.2 IA-32 Platforms", and
> >>> "2.3.4 x64 Platforms", in chapter "2.3 Calling Conventions", where the
> >>> boot services time environment is specified.
> >>>
> >>> This is new in UEFI-2.5, and it comes from Mantis ticket 1224: "Adding
> >>> support for No executable data areas".
> >>>
> >>> ... The question could be then if grub (in Wheezy) should be adapted to
> >>> UEFI-2.5 (if that's possible) or if OVMF should be built without this
> >>> feature.
> >>>
> >>> Hmmm. Actually, I'm torn about the default for PcdSetNxForStack.
> >>>
> >>> Namely, Mantis ticket 1224 has come up before. There's another edk2
> >>> sub-feature related to this UEFI spec feature / Mantis ticket; the
> >>> properties table (controlled by "PcdPropertiesTableEnable"), and the
> >>> effects it has on the UEFI memory map, and the requirements it presents
> >>> for UEFI OSes.
> >>>
> >>> *That* sub-feature is extremely intrusive.
> >>> "MdeModulePkg/MdeModulePkg.dec" sets "PcdPropertiesTableEnable" TRUE by
> >>> default, and OvmfPkg inherits it. I have not overridden that default
> >>> just yet in OvmfPkg because the properties table feature depends on
> >>> something *else* too: sections in runtime DXE driver binaries must be
> >>> aligned at 4KB, and that is not ensured for OvmfPkg (yet?). Therefore,
> >>> the properties table feature is not active in OVMF at the moment due to
> >>> a "random build tools circumstance" -- and not because the feature flag
> >>> is actually disabled.
> >>>
> >>> Now, the properties table feature absolutely cannot be (effectively)
> >>> enabled in OVMF as a default. It would break Windows 7 / Windows Server
> >>> 2008 R2. A huge number of users run such guests for GPU passthrough.
> >>>
> >>> The idea that just crossed my mind was to introduce a new build flag for
> >>> OVMF, like -D NOEXEC_DATA_ENABLE. And this would then control
> >>> PcdSetNxForStack *and* PcdPropertiesTableEnable *together*:
> >>>
> >>> if NOEXEC_DATA_ENABLE:
> >>>   PcdSetNxForStack := TRUE
> >>>   PcdPropertiesTableEnable := TRUE
> >>> else
> >>>   PcdSetNxForStack := FALSE
> >>>   PcdPropertiesTableEnable := FALSE
> >>>
> >>> However, I think that's a bad idea.
> >>>
> >>> "PcdPropertiesTableEnable" breaks a very widely used guest OS for which
> >>> there is no update in sight (ie. no Service Pack 2 for Windows 7 /
> >>> Windows Server 2008 R2), but is otherwise supposed to be supported for
> >>> years to come.
> >>>
> >>> Whereas Debian Wheezy is not as widely used as a guest (for one: it
> >>> "competes" with all the other Linux distros from the same timeframe).
> >>> Debian Wheezy also provides a quite non-intrusive upgrade path (to
> >>> Jessie), which is not the case for Windows 7. So the breakage casued by
> >>> setting PcdSetNxForStack has much smaller impact, I think, and the
> >>> benefits might outweigh the breakage.
> >>>
> >>> Which just means that a heavy-handed -D NOEXEC_DATA_ENABLE, like above,
> >>> would not be appropriate. PcdSetNxForStack set, and
> >>> PcdPropertiesTableEnable clear, is a valid configuration.
> >>>
> >>> In any case, I sort of convinced myself that Wheezy's grub is not at
> >>> fault; it was simply written against an earlier release of the UEFI spec.
> >>>
> >>> How about this:
> >>>
> >>> - (somewhat unrelatedly, but for completeness):
> >>>   I post a patch that exposes PcdPropertiesTableEnable with a build
> >>>   time flag (NX_PROP_TABLE_ENABLE) with an OVMF-default of FALSE,
> >>>
> >>> - I post another patch that exposes PcdSetNxForStack with a build time
> >>>   flag (NX_STACK_ENABLE) with an OVMF-default of *TRUE*. You could then
> >>>   pass -D NX_STACK_ENABLE=FALSE when you build OVMF for any environment
> >>>   where Wheezy guests are expected.
> >>>
> >>> Jordan?
> >>>
> >>> Yet another (quite complex) possibility:
> >>>
> >>> - According to "MdeModulePkg/MdeModulePkg.dec",
> >>>   a platform DSC can already type PcdPropertiesTableEnable as a dynamic
> >>>   PCD. We could allow the same for PcdSetNxForStack. Star?
> >>>
> >>>   Both PCDs are consumed "acceptably late" (the first in the DXE core,
> >>>   the second in the DXE IPL PEIM). This would allow
> >>>   OvmfPkg/PlatformPei to set the PCDs dynamically.
> >>>
> >>> - Then the question is how we pass in the PCD values from the outside.
> >>>   For QEMU/KVM virtual machines, we could use Gabriel's QEMU feature
> >>>
> >>>   -fw_cfg name=opt/my_item_name,file=./my_blob.bin
> >>>
> >>>   ie. no changes for QEMU would be necessary. Xen virtual machines
> >>>   don't have fw_cfg however, therefore "some other" info passing should
> >>>   be implemented in "OvmfPkg/PlatformPei/Xen.c", and (I assume!) in the
> >>>   host-side Xen tools as well.
> >>>
> >>> Personally I think that this dynamic approach is overkill (mainly
> >>> because I'm fine with being unable to install Debian Wheezy guests, both
> >>> wearing and not wearing my red fedora; and because the properties table
> >>> feature is not active for *any* OVMF guests anyway, in practice).
> >>>
> >>> So I'd like to ask you guys to decide which one you prefer: a simple
> >>> build time flag called NX_STACK_ENABLE (which will allow you to install
> >>> Wheezy guests, but deprive all other guests from a non-exec stack), or
> >>> the dynamic switches (which will take host-side work in Xen, plus a
> >>> matching OvmfPkg patch).
> >>
> >> I might go ahead and implement the -fw_cfg solution (for when OVMF runs
> >> on QEMU), leaving any parallel Xen functionality to Xen developers, for
> >> later.
> >>
> >> Because, I just found out that the GRUB binary built into the bits-2005
> >> release ISO <http://biosbits.org/download/> blows up the exact same way,
> >> and *that* is very annoying to me personally.
> >>
> >> Maybe we should invert the default. I'm not so convinced any longer that
> >> the current default is right. I didn't know that the GRUB version with
> >> code on the stack was this wide-spread.
> > 
> > Heh.  Our fault for still using old (2.00) GRUB2, which we do plan to
> > upgrade at some point, though doing so doesn't top the TODO list.  But I
> > do agree that with OVMF not having previously enforced NX in the UEFI
> > environment, going from that to immediately enforcing it *by default*
> > seems a bit quick.  I would be surprised if doing so didn't break other
> > UEFI programs too.
> > 
> > Could OVMF build with NX support available, but disabled by default, so
> > that qemu can then turn it *on* with a -fw_cfg option?
> 
> It would be possible, yes.
> 
> I just posted a patch series that adds that dynamism, but it leaves the
> NX stack turned on by default. (Ie. one needs to add the cmdline option
> to turn it off.) If there's consensus to revert the default to off, I
> can submit a v2.
> 
> > Do any UEFI stacks on systems in the wild have NX turned on?  I don't
> > think it makes sense for OVMF to enable NX by default until a majority
> > of new physical systems do so.
> 
> For me that's not so clear-cut. OVMF is frequently used as a UEFI
> development environment (it's better to brick a virtual machine than
> your physical dev platform...), so upstream should embrace new UEFI
> features reasonably early, unless there are *grave* regressions.
> 
> One example I named was the properties table feature (new in UEFI-2.5).
> It would break Windows 7. Given the large number of users running
> Windows 7 on OVMF (mainly for GPU passthrough), such a regression would
> be serious.
> 
> Breaking Debian Wheezy's and BITS's GRUB is also bad, but the former is
> very old (and has a clear upgrade path), while the latter is mainly used
> by developers (who can learn about the -fw_cfg switch by googling or
> asking on the least without huge trouble). In this case I'm leaning
> towards OVMF being "bleeding edge" by default. But, I could be convinced
> otherwise.

I certainly think it makes sense for OVMF to adopt the feature sooner
than normal, and I agree that OVMF serves as a test case.  But going
directly from "not possible to turn on" to "turned on by default",
without any period of "off by default but possible to turn on", seems a
bit unfortunate.

That said, we could certainly fix BITS to use newer GRUB2, and use
(and document) -fw_cfg in the meantime.  So I won't push *too* hard for
changing the default, just mildly.

On a vaguely related note, what's the canonical place to report bugs in
OVMF?  Github issues on tianocore/edk2 (which a few people have filed
issues on) or tianocore/edk2-OvmfPkg (which nobody has filed issues on),
or some other bug tracker I'm unaware of and haven't found a link to?

- Josh Triplett

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

[Laszlo Ersek]

On 09/11/15 21:30, Josh Triplett wrote:
> On Fri, Sep 11, 2015 at 05:28:06PM +0200, Laszlo Ersek wrote:
>> On 09/11/15 16:10, Josh Triplett wrote:
>>> On Fri, Sep 11, 2015 at 01:43:53PM +0200, Laszlo Ersek wrote:
>>>> On 09/09/15 12:48, Laszlo Ersek wrote:
>>>>> On 09/09/15 11:37, Ian Campbell wrote:
>>>>>> On Wed, 2015-09-09 at 01:06 -0600, Jan Beulich wrote:
>>>>>>>>>> On 09.09.15 at 00:23, <lersek@redhat.com> wrote:
>>>>>>>> On 09/08/15 19:26, Anthony PERARD wrote:
>>>>>>>>> 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/R
>>>>>>>> untime
>>>>>>>> Dxe/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;
>>>>>>>
>>>>>>> I don't think the exact instruction at that address really matters. The
>>>>>>> main question appears to be why RIP and RSP both point into the
>>>>>>> same page (see also the subject of Anthony's mail).
>>>>>>
>>>>>> I'm not 100% what is going on,
>>>>>
>>>>> me neither :)
>>>>>
>>>>>> but if this (executable code on stack) is
>>>>>> happening in grub is there something which is explicitly forbidden to UEFI
>>>>>> apps by the UEFI spec?
>>>>>
>>>>> Yes, there is. This small OvmfPkg patch only enables the edk2 feature
>>>>> added by Star Zeng in
>>>>> <https://github.com/tianocore/edk2/commit/5630cdfe> for OVMF. That patch
>>>>> (also referenced in my commit message by SVN rev) says,
>>>>>
>>>>>     This feature is added for UEFI spec that says
>>>>>     "Stack may be marked as non-executable in identity mapped page
>>>>>     tables".
>>>>>     A PCD PcdSetNxForStack is added to turn on/off this feature, and it
>>>>>     is FALSE by default.
>>>>>
>>>>> A UEFI app runs (well, *starts*, anyway) before ExitBootServices() /
>>>>> SetVirtualAddressMap(), so it's bound by the above.
>>>>>
>>>>> The spec passage above is quoted from "2.3.2 IA-32 Platforms", and
>>>>> "2.3.4 x64 Platforms", in chapter "2.3 Calling Conventions", where the
>>>>> boot services time environment is specified.
>>>>>
>>>>> This is new in UEFI-2.5, and it comes from Mantis ticket 1224: "Adding
>>>>> support for No executable data areas".
>>>>>
>>>>> ... The question could be then if grub (in Wheezy) should be adapted to
>>>>> UEFI-2.5 (if that's possible) or if OVMF should be built without this
>>>>> feature.
>>>>>
>>>>> Hmmm. Actually, I'm torn about the default for PcdSetNxForStack.
>>>>>
>>>>> Namely, Mantis ticket 1224 has come up before. There's another edk2
>>>>> sub-feature related to this UEFI spec feature / Mantis ticket; the
>>>>> properties table (controlled by "PcdPropertiesTableEnable"), and the
>>>>> effects it has on the UEFI memory map, and the requirements it presents
>>>>> for UEFI OSes.
>>>>>
>>>>> *That* sub-feature is extremely intrusive.
>>>>> "MdeModulePkg/MdeModulePkg.dec" sets "PcdPropertiesTableEnable" TRUE by
>>>>> default, and OvmfPkg inherits it. I have not overridden that default
>>>>> just yet in OvmfPkg because the properties table feature depends on
>>>>> something *else* too: sections in runtime DXE driver binaries must be
>>>>> aligned at 4KB, and that is not ensured for OvmfPkg (yet?). Therefore,
>>>>> the properties table feature is not active in OVMF at the moment due to
>>>>> a "random build tools circumstance" -- and not because the feature flag
>>>>> is actually disabled.
>>>>>
>>>>> Now, the properties table feature absolutely cannot be (effectively)
>>>>> enabled in OVMF as a default. It would break Windows 7 / Windows Server
>>>>> 2008 R2. A huge number of users run such guests for GPU passthrough.
>>>>>
>>>>> The idea that just crossed my mind was to introduce a new build flag for
>>>>> OVMF, like -D NOEXEC_DATA_ENABLE. And this would then control
>>>>> PcdSetNxForStack *and* PcdPropertiesTableEnable *together*:
>>>>>
>>>>> if NOEXEC_DATA_ENABLE:
>>>>>   PcdSetNxForStack := TRUE
>>>>>   PcdPropertiesTableEnable := TRUE
>>>>> else
>>>>>   PcdSetNxForStack := FALSE
>>>>>   PcdPropertiesTableEnable := FALSE
>>>>>
>>>>> However, I think that's a bad idea.
>>>>>
>>>>> "PcdPropertiesTableEnable" breaks a very widely used guest OS for which
>>>>> there is no update in sight (ie. no Service Pack 2 for Windows 7 /
>>>>> Windows Server 2008 R2), but is otherwise supposed to be supported for
>>>>> years to come.
>>>>>
>>>>> Whereas Debian Wheezy is not as widely used as a guest (for one: it
>>>>> "competes" with all the other Linux distros from the same timeframe).
>>>>> Debian Wheezy also provides a quite non-intrusive upgrade path (to
>>>>> Jessie), which is not the case for Windows 7. So the breakage casued by
>>>>> setting PcdSetNxForStack has much smaller impact, I think, and the
>>>>> benefits might outweigh the breakage.
>>>>>
>>>>> Which just means that a heavy-handed -D NOEXEC_DATA_ENABLE, like above,
>>>>> would not be appropriate. PcdSetNxForStack set, and
>>>>> PcdPropertiesTableEnable clear, is a valid configuration.
>>>>>
>>>>> In any case, I sort of convinced myself that Wheezy's grub is not at
>>>>> fault; it was simply written against an earlier release of the UEFI spec.
>>>>>
>>>>> How about this:
>>>>>
>>>>> - (somewhat unrelatedly, but for completeness):
>>>>>   I post a patch that exposes PcdPropertiesTableEnable with a build
>>>>>   time flag (NX_PROP_TABLE_ENABLE) with an OVMF-default of FALSE,
>>>>>
>>>>> - I post another patch that exposes PcdSetNxForStack with a build time
>>>>>   flag (NX_STACK_ENABLE) with an OVMF-default of *TRUE*. You could then
>>>>>   pass -D NX_STACK_ENABLE=FALSE when you build OVMF for any environment
>>>>>   where Wheezy guests are expected.
>>>>>
>>>>> Jordan?
>>>>>
>>>>> Yet another (quite complex) possibility:
>>>>>
>>>>> - According to "MdeModulePkg/MdeModulePkg.dec",
>>>>>   a platform DSC can already type PcdPropertiesTableEnable as a dynamic
>>>>>   PCD. We could allow the same for PcdSetNxForStack. Star?
>>>>>
>>>>>   Both PCDs are consumed "acceptably late" (the first in the DXE core,
>>>>>   the second in the DXE IPL PEIM). This would allow
>>>>>   OvmfPkg/PlatformPei to set the PCDs dynamically.
>>>>>
>>>>> - Then the question is how we pass in the PCD values from the outside.
>>>>>   For QEMU/KVM virtual machines, we could use Gabriel's QEMU feature
>>>>>
>>>>>   -fw_cfg name=opt/my_item_name,file=./my_blob.bin
>>>>>
>>>>>   ie. no changes for QEMU would be necessary. Xen virtual machines
>>>>>   don't have fw_cfg however, therefore "some other" info passing should
>>>>>   be implemented in "OvmfPkg/PlatformPei/Xen.c", and (I assume!) in the
>>>>>   host-side Xen tools as well.
>>>>>
>>>>> Personally I think that this dynamic approach is overkill (mainly
>>>>> because I'm fine with being unable to install Debian Wheezy guests, both
>>>>> wearing and not wearing my red fedora; and because the properties table
>>>>> feature is not active for *any* OVMF guests anyway, in practice).
>>>>>
>>>>> So I'd like to ask you guys to decide which one you prefer: a simple
>>>>> build time flag called NX_STACK_ENABLE (which will allow you to install
>>>>> Wheezy guests, but deprive all other guests from a non-exec stack), or
>>>>> the dynamic switches (which will take host-side work in Xen, plus a
>>>>> matching OvmfPkg patch).
>>>>
>>>> I might go ahead and implement the -fw_cfg solution (for when OVMF runs
>>>> on QEMU), leaving any parallel Xen functionality to Xen developers, for
>>>> later.
>>>>
>>>> Because, I just found out that the GRUB binary built into the bits-2005
>>>> release ISO <http://biosbits.org/download/> blows up the exact same way,
>>>> and *that* is very annoying to me personally.
>>>>
>>>> Maybe we should invert the default. I'm not so convinced any longer that
>>>> the current default is right. I didn't know that the GRUB version with
>>>> code on the stack was this wide-spread.
>>>
>>> Heh.  Our fault for still using old (2.00) GRUB2, which we do plan to
>>> upgrade at some point, though doing so doesn't top the TODO list.  But I
>>> do agree that with OVMF not having previously enforced NX in the UEFI
>>> environment, going from that to immediately enforcing it *by default*
>>> seems a bit quick.  I would be surprised if doing so didn't break other
>>> UEFI programs too.
>>>
>>> Could OVMF build with NX support available, but disabled by default, so
>>> that qemu can then turn it *on* with a -fw_cfg option?
>>
>> It would be possible, yes.
>>
>> I just posted a patch series that adds that dynamism, but it leaves the
>> NX stack turned on by default. (Ie. one needs to add the cmdline option
>> to turn it off.) If there's consensus to revert the default to off, I
>> can submit a v2.
>>
>>> Do any UEFI stacks on systems in the wild have NX turned on?  I don't
>>> think it makes sense for OVMF to enable NX by default until a majority
>>> of new physical systems do so.
>>
>> For me that's not so clear-cut. OVMF is frequently used as a UEFI
>> development environment (it's better to brick a virtual machine than
>> your physical dev platform...), so upstream should embrace new UEFI
>> features reasonably early, unless there are *grave* regressions.
>>
>> One example I named was the properties table feature (new in UEFI-2.5).
>> It would break Windows 7. Given the large number of users running
>> Windows 7 on OVMF (mainly for GPU passthrough), such a regression would
>> be serious.
>>
>> Breaking Debian Wheezy's and BITS's GRUB is also bad, but the former is
>> very old (and has a clear upgrade path), while the latter is mainly used
>> by developers (who can learn about the -fw_cfg switch by googling or
>> asking on the least without huge trouble). In this case I'm leaning
>> towards OVMF being "bleeding edge" by default. But, I could be convinced
>> otherwise.
> 
> I certainly think it makes sense for OVMF to adopt the feature sooner
> than normal, and I agree that OVMF serves as a test case.  But going
> directly from "not possible to turn on" to "turned on by default",
> without any period of "off by default but possible to turn on", seems a
> bit unfortunate.
> 
> That said, we could certainly fix BITS to use newer GRUB2, and use
> (and document) -fw_cfg in the meantime.  So I won't push *too* hard for
> changing the default, just mildly.

Okay. If I'll need to send a v2 for any reason, I'll incorporate this.
If not, then I can post a followup patch later (stating that it's due to
community feedback).

> On a vaguely related note, what's the canonical place to report bugs in
> OVMF?

(Bugs? What bugs? :))

It's this list, <edk2-devel@lists.01.org>.

Thanks!
Laszlo

> Github issues on tianocore/edk2 (which a few people have filed
> issues on) or tianocore/edk2-OvmfPkg (which nobody has filed issues on),
> or some other bug tracker I'm unaware of and haven't found a link to?
> 
> - Josh Triplett
> 

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

[Josh Triplett]

On Fri, Sep 11, 2015 at 11:27:32PM +0200, Laszlo Ersek wrote:
> On 09/11/15 21:30, Josh Triplett wrote:
> > On Fri, Sep 11, 2015 at 05:28:06PM +0200, Laszlo Ersek wrote:
> >> Breaking Debian Wheezy's and BITS's GRUB is also bad, but the former is
> >> very old (and has a clear upgrade path), while the latter is mainly used
> >> by developers (who can learn about the -fw_cfg switch by googling or
> >> asking on the least without huge trouble). In this case I'm leaning
> >> towards OVMF being "bleeding edge" by default. But, I could be convinced
> >> otherwise.
> > 
> > I certainly think it makes sense for OVMF to adopt the feature sooner
> > than normal, and I agree that OVMF serves as a test case.  But going
> > directly from "not possible to turn on" to "turned on by default",
> > without any period of "off by default but possible to turn on", seems a
> > bit unfortunate.
> > 
> > That said, we could certainly fix BITS to use newer GRUB2, and use
> > (and document) -fw_cfg in the meantime.  So I won't push *too* hard for
> > changing the default, just mildly.
> 
> Okay. If I'll need to send a v2 for any reason, I'll incorporate this.
> If not, then I can post a followup patch later (stating that it's due to
> community feedback).

Thanks!

> > On a vaguely related note, what's the canonical place to report bugs in
> > OVMF?
> 
> (Bugs? What bugs? :))
> 
> It's this list, <edk2-devel@lists.01.org>.

There isn't a tracker of some kind?  That's unfortunate.

But thanks; I'll send mail to the list when we discover an issue while
experimenting with BITS.

(Also, if you don't intend to use github's issue tracker, you might want
to turn it off so people don't file things there and expect a response.)

- Josh Triplett

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

[Ian Campbell]

On Fri, 2015-09-11 at 17:28 +0200, Laszlo Ersek wrote:
> 
[...]
> For me that's not so clear-cut. OVMF is frequently used as a UEFI
> development environment (it's better to brick a virtual machine than
> your physical dev platform...)

One flip side to this is that people often virtualize in order to continue
running their older platforms and applications, because upgrading them
would be difficult for whatever reason.

There's an obvious tension between that and the desire to use OVMF as a
development platform, but it seems to me that the developers are the ones
who can more readily be expected to mess with the defaults.

> , so upstream should embrace new UEFI
> features reasonably early, unless there are *grave* regressions.
> 
> One example I named was the properties table feature (new in UEFI-2.5).
> It would break Windows 7. Given the large number of users running
> Windows 7 on OVMF (mainly for GPU passthrough), such a regression would
> be serious.
> 
> Breaking Debian Wheezy's and BITS's GRUB is also bad, but the former is
> very old (and has a clear upgrade path)

Debian Wheezy is not very old, it's only a year older than RHEL7 (May 2013
vs June 2014) and only a bit older than two years in absolute terms. It is
also the subject of an LTS effort, which extends its lifetime to 2018.

For comparison Windows 7 (which you argue regressing would be serious) was
released in 2009 and there have been two major Windows releases since then.

Given that and with consideration between the desire to run older platforms
vs. a development environment it seems to me that Debian Wheezy has not yet
reached the threshold for being ignored or for saying to users "you must
now upgrade".

Ian.

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

[Laszlo Ersek]

On 09/12/15 01:06, Josh Triplett wrote:
> On Fri, Sep 11, 2015 at 11:27:32PM +0200, Laszlo Ersek wrote:
>> On 09/11/15 21:30, Josh Triplett wrote:
>>> On Fri, Sep 11, 2015 at 05:28:06PM +0200, Laszlo Ersek wrote:
>>>> Breaking Debian Wheezy's and BITS's GRUB is also bad, but the former is
>>>> very old (and has a clear upgrade path), while the latter is mainly used
>>>> by developers (who can learn about the -fw_cfg switch by googling or
>>>> asking on the least without huge trouble). In this case I'm leaning
>>>> towards OVMF being "bleeding edge" by default. But, I could be convinced
>>>> otherwise.
>>>
>>> I certainly think it makes sense for OVMF to adopt the feature sooner
>>> than normal, and I agree that OVMF serves as a test case.  But going
>>> directly from "not possible to turn on" to "turned on by default",
>>> without any period of "off by default but possible to turn on", seems a
>>> bit unfortunate.
>>>
>>> That said, we could certainly fix BITS to use newer GRUB2, and use
>>> (and document) -fw_cfg in the meantime.  So I won't push *too* hard for
>>> changing the default, just mildly.
>>
>> Okay. If I'll need to send a v2 for any reason, I'll incorporate this.
>> If not, then I can post a followup patch later (stating that it's due to
>> community feedback).
> 
> Thanks!
> 
>>> On a vaguely related note, what's the canonical place to report bugs in
>>> OVMF?
>>
>> (Bugs? What bugs? :))
>>
>> It's this list, <edk2-devel@lists.01.org>.
> 
> There isn't a tracker of some kind?  That's unfortunate.

I won't disagree with you, but I'll note three things:

(1) There isn't much use to a bug tracker if there aren't enough human
resources to actually monitor that tracker, and work hard on the bugs. I
can offer to monitor this list and work on bugs reported here the best I
can. Bug fixing is hard and taxing; for *official* long-term bug
tracking, some form of legal relationship is usually necessary. I do
take my RHBZs very seriously.

(2) OvmfPkg is one platform in edk2. I don't think OVMF / OvmfPkg should
have its own separate tracker. And regarding a tracker for the entirety
of edk2, there used to be one (still on sf.net), and nobody (no
contributor or maintainer) cared. Goto (1).

(3) I've seen first hand how Fedora bug tracker entries, Debian bug
tracker entries, and upstream QEMU bug tracker entries are handled. Goto
(1). As I said, I try to do my best with bugs reported on the list, both
in tracking them and in fixing them, as my load allows.

> But thanks; I'll send mail to the list when we discover an issue while
> experimenting with BITS.

Yes, please do that. And thank you. In my experience, other package
maintainers (not just us in OvmfPkg) are pretty responsive if you report
bugs for their packages on the list, especially if you can narrow it
down (bisection, good reproducer etc).

> 
> (Also, if you don't intend to use github's issue tracker, you might want
> to turn it off so people don't file things there and expect a response.)

That's a very good point. Jordan, can you please disable the issue
tracker on github?

Thanks
Laszlo

> 
> - Josh Triplett
> 

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

[Laszlo Ersek]

On 09/14/15 11:22, Ian Campbell wrote:
> On Fri, 2015-09-11 at 17:28 +0200, Laszlo Ersek wrote:
>>
> [...]
>> For me that's not so clear-cut. OVMF is frequently used as a UEFI
>> development environment (it's better to brick a virtual machine than
>> your physical dev platform...)
> 
> One flip side to this is that people often virtualize in order to continue
> running their older platforms and applications, because upgrading them
> would be difficult for whatever reason.
> 
> There's an obvious tension between that and the desire to use OVMF as a
> development platform, but it seems to me that the developers are the ones
> who can more readily be expected to mess with the defaults.

Good points!

>> , so upstream should embrace new UEFI
>> features reasonably early, unless there are *grave* regressions.
>>
>> One example I named was the properties table feature (new in UEFI-2.5).
>> It would break Windows 7. Given the large number of users running
>> Windows 7 on OVMF (mainly for GPU passthrough), such a regression would
>> be serious.
>>
>> Breaking Debian Wheezy's and BITS's GRUB is also bad, but the former is
>> very old (and has a clear upgrade path)
> 
> Debian Wheezy is not very old, it's only a year older than RHEL7 (May 2013
> vs June 2014) and only a bit older than two years in absolute terms. It is
> also the subject of an LTS effort, which extends its lifetime to 2018.

(*)

> For comparison Windows 7 (which you argue regressing would be serious) was
> released in 2009 and there have been two major Windows releases since then.

(**)

> Given that and with consideration between the desire to run older platforms
> vs. a development environment it seems to me that Debian Wheezy has not yet
> reached the threshold for being ignored or for saying to users "you must
> now upgrade".

I believe I could argue against both (*) and (**), but it would not be
productive. :)

Instead, what matters is the (now) clear, significant user demand for
turning off PcdSetNxForStack by default. I'll send a followup patch for
my series to that end.

And, sorry about the inconvenience the regression may have caused, of
course ;)

Thanks!
Laszlo

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

[Ian Campbell]

On Mon, 2015-09-14 at 13:07 +0200, Laszlo Ersek wrote:
> Debian Wheezy is not very old, it's only a year older than RHEL7 (May
> > 2013
> > vs June 2014) and only a bit older than two years in absolute terms. It is
> > also the subject of an LTS effort, which extends its lifetime to 2018.
> 
> (*)
> 
> > For comparison Windows 7 (which you argue regressing would be serious) was
> > released in 2009 and there have been two major Windows releases since then.
> 
> (**)
> 
> > Given that and with consideration between the desire to run older platforms
> > vs. a development environment it seems to me that Debian Wheezy has not yet
> > reached the threshold for being ignored or for saying to users "you must
> > now upgrade".
> 
> I believe I could argue against both (*) and (**), but it would not be
> productive. :)

Yes, I'm sure we could be here until the cows come home to roost ;-)

> Instead, what matters is the (now) clear, significant user demand for
> turning off PcdSetNxForStack by default. I'll send a followup patch for
> my series to that end.

Thanks.

> And, sorry about the inconvenience the regression may have caused, of
> course ;)

No need to apologise, it was an experiment worth performing IMHO.

Ian.