On 29.04.2022 19:06, Demi Marie Obenour wrote: > On Fri, Apr 29, 2022 at 10:40:42AM +0200, Jan Beulich wrote: >> On 29.04.2022 00:54, Demi Marie Obenour wrote: >>> On Thu, Apr 28, 2022 at 08:47:49AM +0200, Jan Beulich wrote: >>>> On 27.04.2022 21:08, Demi Marie Obenour wrote: >>>>> On further thought, I think the hypercall approach is actually better >>>>> than reserving the ESRT. I really do not want XEN_FW_EFI_MEM_INFO to >>>>> return anything other than the actual firmware-provided memory >>>>> information, and the current approach seems to require more and more >>>>> special-casing of the ESRT, not to mention potentially wasting memory >>>>> and splitting a potentially large memory region into two smaller ones. >>>>> By copying the entire ESRT into memory owned by Xen, the logic becomes >>>>> significantly simpler on both the Xen and dom0 sides. >>>> >>>> I actually did consider the option of making a private copy when you did >>>> send the initial version of this, but I'm not convinced this simplifies >>>> things from a kernel perspective: They'd now need to discover the table >>>> by some entirely different means. In Linux at least such divergence >>>> "just for Xen" hasn't been liked in the past. >>>> >>>> There's also the question of how to propagate the information across >>>> kexec. But I guess that question exists even outside of Xen, with the >>>> area living in memory which the OS is expected to recycle. >>> >>> Indeed it does. A simple rule might be, “Only trust the ESRT if it is >>> in memory of type EfiRuntimeServicesData.” That is easy to achieve by >>> monkeypatching the config table as you suggested below. >>> >>> I *am* worried that the config table might be mapped read-only on some >>> systems, in which case the overwrite would cause a fatal page fault. Is >>> there a way for Xen to check for this? >> >> While in boot mode, aiui page tables aren't supposed to be enforcing >> access restrictions. Recall that on other architectures EFI even runs >> with paging disabled; this simply is not possible for x86-64. > > Yikes! No wonder firmware has nonexistent exploit mitigations. They > really ought to start porting UEFI to Rust, with ASLR, NX, stack > canaries, a hardened allocator, and support for de-priviliged services > that run in user mode. > > That reminds me: Can Xen itself run from ROM?
I guess that could be possible in principle, but would certainly require some work. > Xen is being ported to > POWER for use in Qubes OS, and one approach under consideration is to > have Xen and a mini-dom0 be part of the firmware. Personally, I really > like this approach, as it makes untrusted storage domains much simpler. > If this should be a separate email thread, let me know. It probably should be. >> So >> portable firmware shouldn't map anything r/o. In principle the pointer >> could still be in ROM; I consider this unlikely, but we could check >> for that (just like we could do a page table walk to figure out >> whether a r/o mapping would prevent us from updating the field). > > Is there a utility function that could be used for this? I don't think there is. >>> It could also be undefined behavior to modify it. >> >> That's the bigger worry I have. > > Turns out that it is *not* undefined behavior, so long as > ExitBootServices() has not been called. This is becaues EFI drivers > will modify the config table, so firmware cannot assume it to be > read-only. Ah, right - we could even use InstallConfigurationTable() ourselves to make the adjustment. >>>>> Is using ebmalloc() to allocate a copy of the ESRT a reasonable option? >>>> >>>> I'd suggest to try hard to avoid ebmalloc(). It ought to be possible to >>>> make the copy before ExitBootServices(), via normal EFI allocation. If >>>> replacing a pointer in the config table was okay(ish), this could even >>>> be utilized to overcome the kexec problem. >>> >>> What type should I use for the allocation? EfiLoaderData looks like the >>> most consistent choice, but I am not sure if memory so allocated remains >>> valid when Xen hands off to the OS, so EfiRuntimeServicesData might be a >>> better choice. >> >> It definitely is. We do recycle EfiLoaderData ourselves. > > I wonder why the ESRT was not in EfiRuntimeServicesData to begin with. So do I. >>> To avoid memory leaks from repeated kexec(), this could >>> be made conditional on the ESRT not being in memory of type >>> EfiRuntimeServicesData to begin with. >> >> Of course - there's no point relocating the blob when it already is >> immune to recycling. > > Yup. Is it reasonable for dom0 to check that the ESRT is in > EfiRuntimeServicesData when under Xen? I think it is, but kernel folks may not like Xen specific code in this (or about any) area. Jan
