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)

["Zeng, Star" <star.zeng@xxxxxxxxx>]

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


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