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

[Laszlo Ersek <lersek@xxxxxxxxxx>]

On 09/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@xxxxxxxxxx> 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@xxxxxxxxxxxxx
>> http://lists.xen.org/xen-devel


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