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/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
> 


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