On 02.05.2022 09:11, Demi Marie Obenour wrote: > On Mon, May 02, 2022 at 08:24:30AM +0200, Jan Beulich wrote: >> 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. > > I will make one at some point. > >>>> 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. > > Then it is good that none is necessary :) > > Also, should the various bug checks I added be replaced by ASSERT()?
You mean those in the earlier patch(es)? Not sure - depends on what you would be doing for release builds. In the cases where you simply re- check what was checked earlier on, ASSERT() would probably indeed be preferable over BUG_ON() (and there I wouldn't even see a strong need to consider alternatives for release builds). Jan
