[Xen-devel] [PATCH v3 5/5] x86/MSI: drop workaround for insecure Dom0 kernels

["Jan Beulich" <JBeulich@xxxxxxxx>]

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 <jbeulich@xxxxxxxx>

--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -770,7 +770,7 @@ static void ept_change_entry_type_global
         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);
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -825,32 +825,22 @@ static int msix_capability_init(struct p
                                 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;
+            struct domain *currd = current->domain;
+            struct domain *d = dev->domain ?: currd;
 
-            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? */
-            }
+            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);

Attachment: x86-MSI-drop-workaround.patch
Description: Text document

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