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 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@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).

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


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