On 03/07/2019 13:18, Jan Beulich wrote: > There are currently three more or less different checks: > - _get_page_type() adjusts the IOMMU mappings according to the new type > alone, > - arch_iommu_populate_page_table() wants just the type to be > PGT_writable_page, > - iommu_hwdom_init() additionally permits all other types with a type > refcount of zero. > The canonical one is in _get_page_type(), and as of XSA-288 > arch_iommu_populate_page_table() also has no need anymore to deal with > PGT_none pages. In the PV Dom0 case, however, PGT_none pages are still > necessary to consider, since in that case pages don't get handed to > guest_physmap_add_entry(). Furthermore, the function so far also > established r/o mappings, which is not in line with the rules set forth > by the XSA-288 change. > > For arch_iommu_populate_page_table() to not encounter PGT_none pages > anymore even in cases where the IOMMU gets enabled for a domain only > when it is already running, replace the IOMMU dependency in > guest_physmap_add_entry()'s handling of PV guests to check just the > system wide state instead of the domain property.
And this is the problem. We have an enormous amount of complexity, and a hypercall which even after an XSA, we could only reduce to "will livelock under adversarial conditions (inc. the toolstack thread which started it)", so support a corner case which doesn't (AFACIT) look like it can work correctly, and surely isn't used in practice. IMO, the only sane thing to do is to have a "create an IOMMU context" flag in domaincreate (and a shared vs split while we're at it, for the EPT case), and have the IOMMU context properly known from the very start of the domain. Even if this does end up restricting a corner case which is believed to work, I do not see it as an inconvenience or problem to require a domain config file to specify whether it wants an IOMMU context (directly, or indirectly via things like PCI=), and the reduction in complexity in Xen would be massive. How many security bugs have we already found here? How may are still lurking, or liable to be re-introduced because this code is just too damn complicated for you, me and George to review sensibly? Reducing the complexity is the responsible thing to do. I'm afraid that I don't view anything other than moving towards an up-front declaration as making the situation better. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
