From 061eebe0e99ad45c9c3b1a778b06140de4a91f25 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Tue, 22 Apr 2014 12:04:20 +0200 Subject: [PATCH] x86/MSI: drop workaround for insecure Dom0 kernels Considering that - the workaround is expensive (iterating through the entire P2M space of a domain), - the planned elimination of the expensiveness (by propagating the type change step by step to the individual P2M leaves) wouldn't address the IOMMU side of things (as for it to obey to the changed permissions the adjustments must be pushed down immediately through the entire tree) - the proper solution (PHYSDEVOP_msix_prepare) should by now be implemented by all security conscious Dom0 kernels remove the workaround, killing eventual guests that would be known to become a security risk instead. Signed-off-by: Jan Beulich Acked-by: Kevin Tian --- xen/arch/x86/mm/p2m-ept.c | 2 +- xen/arch/x86/msi.c | 40 +++++++++++++++------------------------ 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c index 76fb654a12..b0ab3ceba5 100644 --- a/xen/arch/x86/mm/p2m-ept.c +++ b/xen/arch/x86/mm/p2m-ept.c @@ -775,7 +775,7 @@ static void ept_change_entry_type_global(struct p2m_domain *p2m, return; BUG_ON(p2m_is_grant(ot) || p2m_is_grant(nt)); - BUG_ON(ot != nt && (ot == p2m_mmio_direct || nt == p2m_mmio_direct)); + BUG_ON(p2m_is_mmio(ot) || p2m_is_mmio(nt)); ept_change_entry_type_page(_mfn(ept_get_asr(ept)), ept_get_wl(ept), ot, nt); diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c index 61d6dd09d5..7dbb79d3f2 100644 --- a/xen/arch/x86/msi.c +++ b/xen/arch/x86/msi.c @@ -825,32 +825,22 @@ static int msix_capability_init(struct pci_dev *dev, msix->pba.last) ) WARN(); - if ( dev->domain ) - p2m_change_entry_type_global(dev->domain, - p2m_mmio_direct, p2m_mmio_direct); - if ( desc && (!dev->domain || !paging_mode_translate(dev->domain)) ) + if ( desc ) { - struct domain *d = dev->domain; - - if ( !d ) - for_each_domain(d) - if ( !paging_mode_translate(d) && - (iomem_access_permitted(d, msix->table.first, - msix->table.last) || - iomem_access_permitted(d, msix->pba.first, - msix->pba.last)) ) - break; - if ( d ) - { - if ( !is_hardware_domain(d) && msix->warned != d->domain_id ) - { - msix->warned = d->domain_id; - printk(XENLOG_ERR - "Potentially insecure use of MSI-X on %04x:%02x:%02x.%u by Dom%d\n", - seg, bus, slot, func, d->domain_id); - } - /* XXX How to deal with existing mappings? */ - } + struct domain *currd = current->domain; + struct domain *d = dev->domain ?: currd; + + if ( !is_hardware_domain(currd) || d != currd ) + printk("%s use of MSI-X on %04x:%02x:%02x.%u by Dom%d\n", + is_hardware_domain(currd) + ? XENLOG_WARNING "Potentially insecure" + : XENLOG_ERR "Insecure", + seg, bus, slot, func, d->domain_id); + if ( !is_hardware_domain(d) && + /* Assume a domain without memory has no mappings yet. */ + (!is_hardware_domain(currd) || d->tot_pages) ) + domain_crash(d); + /* XXX How to deal with existing mappings? */ } } WARN_ON(msix->nr_entries != nr_entries); -- 2.39.5