passthrough: quarantine PCI devices

When a PCI device is assigned to an untrusted domain, it is possible for
that domain to program the device to DMA to an arbitrary address. The
IOMMU is used to protect the host from malicious DMA by making sure that
the device addresses can only target memory assigned to the guest. However,
when the guest domain is torn down the device is assigned back to dom0,
thus allowing any in-flight DMA to potentially target critical host data.

This patch introduces a 'quarantine' for PCI devices using dom_io. When
the toolstack makes a device assignable (by binding it to pciback), it
will now also assign it to DOMID_IO and the device will only be assigned
back to dom0 when the device is made unassignable again. Whilst device is
assignable it will only ever transfer between dom_io and guest domains.
dom_io is actually only used as a sentinel domain for quarantining purposes;
it is not configured with any IOMMU mappings. Assignment to dom_io simply
means that the device's initiator (requestor) identifier is not present in
the IOMMU's device table and thus any DMA transactions issued will be
terminated with a fault condition.

In addition, a fix to assignment handling is made for VT-d.  Failure
during the assignment step should not lead to a device still being
associated with its prior owner. Hand the device to DomIO temporarily,
until the assignment step has completed successfully.  Remove the PI
hooks from the source domain then earlier as well.

Failure of the recovery reassign_device_ownership() may not go silent:
There e.g. may still be left over RMRR mappings in the domain assignment
to which has failed, and hence we can't allow that domain to continue
executing.

NOTE: This patch also includes one printk() cleanup; the
      "XEN_DOMCTL_assign_device: " tag is dropped in iommu_do_pci_domctl(),
      since similar printk()-s elsewhere also don't log such a tag.

This is XSA-302.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

--- xen-4.2.5-testing.orig/tools/libxl/libxl_pci.c
+++ xen-4.2.5-testing/tools/libxl/libxl_pci.c
@@ -627,6 +627,7 @@ static int libxl__device_pci_assignable_
     unsigned dom, bus, dev, func;
     char *spath, *driver_path = NULL;
     struct stat st;
+    int rc;
 
     /* Local copy for convenience */
     dom = pcidev->domain;
@@ -645,7 +646,7 @@ static int libxl__device_pci_assignable_
     if ( pciback_dev_is_assigned(gc, pcidev) ) {
         LIBXL__LOG(ctx, LIBXL__LOG_WARNING, PCI_BDF" already assigned to pciback",
                    dom, bus, dev, func);
-        return 0;
+        goto quarantine;
     }
 
     /* Check to see if there's already a driver that we need to unbind from */
@@ -672,6 +673,13 @@ static int libxl__device_pci_assignable_
         return ERROR_FAIL;
     }
 
+quarantine:
+    rc = xc_assign_device(ctx->xch, DOMID_IO, pcidev_encode_bdf(pcidev));
+    if ( rc < 0 ) {
+        LOG(ERROR, "failed to quarantine "PCI_BDF, dom, bus, dev, func);
+        return ERROR_FAIL;
+    }
+
     return 0;
 }
 
@@ -683,6 +691,14 @@ static int libxl__device_pci_assignable_
     int rc;
     char *driver_path;
 
+    /* De-quarantine */
+    rc = xc_deassign_device(ctx->xch, DOMID_IO, pcidev_encode_bdf(pcidev));
+    if ( rc < 0 ) {
+        LOG(ERROR, "failed to de-quarantine "PCI_BDF, pcidev->domain, pcidev->bus,
+            pcidev->dev, pcidev->func);
+        return ERROR_FAIL;
+    }
+
     /* Unbind from pciback */
     if ( (rc=pciback_dev_is_assigned(gc, pcidev)) < 0 ) {
         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Checking if pciback was assigned");
--- xen-4.2.5-testing.orig/xen/arch/x86/mm.c
+++ xen-4.2.5-testing/xen/arch/x86/mm.c
@@ -354,9 +354,11 @@ void __init arch_init_memory(void)
      * Initialise our DOMID_IO domain.
      * This domain owns I/O pages that are within the range of the page_info
      * array. Mappings occur at the priv of the caller.
+     * Quarantined PCI devices will be associated with this domain.
      */
     dom_io = domain_create(DOMID_IO, DOMCRF_dummy, 0);
     BUG_ON(IS_ERR(dom_io));
+    INIT_LIST_HEAD(&dom_io->arch.pdev_list);
 
     /*
      * Initialise our COW domain.
--- xen-4.2.5-testing.orig/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ xen-4.2.5-testing/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -95,6 +95,10 @@ static void amd_iommu_setup_domain_devic
 
     struct hvm_iommu *hd = domain_hvm_iommu(domain);
 
+    /* dom_io is used as a sentinel for quarantined devices */
+    if ( domain == dom_io )
+        return;
+
     BUG_ON( !hd->root_table || !hd->paging_mode || !iommu->dev_table.buffer );
 
     if ( iommu_passthrough && (domain->domain_id == 0) )
@@ -326,6 +330,10 @@ void amd_iommu_disable_domain_device(str
     int req_id;
     u8 bus = pdev->bus;
 
+    /* dom_io is used as a sentinel for quarantined devices */
+    if ( domain == dom_io )
+        return;
+
     BUG_ON ( iommu->dev_table.buffer == NULL );
     req_id = get_dma_requestor_id(iommu->seg, PCI_BDF2(bus, devfn));
     dte = iommu->dev_table.buffer + (req_id * IOMMU_DEV_TABLE_ENTRY_SIZE);
@@ -412,7 +420,7 @@ static int amd_iommu_assign_device(struc
             ivrs_mappings[req_id].read_permission);
     }
 
-    return reassign_device(dom0, d, devfn, pdev);
+    return reassign_device(pdev->domain, d, devfn, pdev);
 }
 
 static void deallocate_next_page_table(struct page_info* pg, int level)
--- xen-4.2.5-testing.orig/xen/drivers/passthrough/iommu.c
+++ xen-4.2.5-testing/xen/drivers/passthrough/iommu.c
@@ -233,19 +233,28 @@ int iommu_remove_device(struct pci_dev *
     return hd->platform_ops->remove_device(pdev->devfn, pdev);
 }
 
-/*
- * If the device isn't owned by dom0, it means it already
- * has been assigned to other domain, or it doesn't exist.
- */
 static int device_assigned(u16 seg, u8 bus, u8 devfn)
 {
     struct pci_dev *pdev;
+    int rc = 0;
 
     spin_lock(&pcidevs_lock);
-    pdev = pci_get_pdev_by_domain(dom0, seg, bus, devfn);
+
+    pdev = pci_get_pdev(seg, bus, devfn);
+
+    if ( !pdev )
+        rc = -ENODEV;
+    /*
+     * If the device exists and it is not owned by either the hardware
+     * domain or dom_io then it must be assigned to a guest, or be
+     * hidden (owned by dom_xen).
+     */
+    else if ( pdev->domain != dom0 && pdev->domain != dom_io )
+        rc = -EBUSY;
+
     spin_unlock(&pcidevs_lock);
 
-    return pdev ? 0 : -1;
+    return rc;
 }
 
 static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
@@ -259,18 +268,25 @@ static int assign_device(struct domain *
 
     /* Prevent device assign if mem paging or mem sharing have been 
      * enabled for this domain */
-    if ( unlikely(!need_iommu(d) &&
+    if ( unlikely(!need_iommu(d) && d != dom_io &&
             (d->arch.hvm_domain.mem_sharing_enabled ||
              d->mem_event->paging.ring_page)) )
         return -EXDEV;
 
     spin_lock(&pcidevs_lock);
-    pdev = pci_get_pdev_by_domain(dom0, seg, bus, devfn);
+    pdev = pci_get_pdev(seg, bus, devfn);
+
+    rc = -ENODEV;
     if ( !pdev )
-    {
-        rc = pci_get_pdev(seg, bus, devfn) ? -EBUSY : -ENODEV;
         goto done;
-    }
+
+    rc = 0;
+    if ( d == pdev->domain )
+        goto done;
+
+    rc = -EBUSY;
+    if ( pdev->domain != dom0 && pdev->domain != dom_io )
+        goto done;
 
     pdev->fault.count = 0;
 
@@ -289,7 +305,9 @@ static int assign_device(struct domain *
                    rc);
     }
 
-    if ( has_arch_pdevs(d) && !need_iommu(d) )
+    if ( d == dom_io )
+        d->need_iommu = 1;
+    else if ( has_arch_pdevs(d) && !need_iommu(d) )
     {
         d->need_iommu = 1;
         if ( !iommu_use_hap_pt(d) )
@@ -297,6 +315,9 @@ static int assign_device(struct domain *
         goto done;
     }
 done:
+    /* The device is assigned to dom_io so mark it as quarantined */
+    if ( !rc && d == dom_io )
+        pdev->quarantine = 1;
     spin_unlock(&pcidevs_lock);
     return rc;
 }
@@ -367,6 +388,9 @@ void iommu_domain_destroy(struct domain
     if ( !iommu_enabled || !hd->platform_ops )
         return;
 
+    if ( d == dom_io )
+        return;
+
     d->need_iommu = 0;
     hd->platform_ops->teardown(d);
 
@@ -450,6 +474,7 @@ int deassign_device(struct domain *d, u1
 {
     struct hvm_iommu *hd = domain_hvm_iommu(d);
     struct pci_dev *pdev = NULL;
+    struct domain *target;
     int ret = 0;
 
     if ( !iommu_enabled || !hd->platform_ops )
@@ -460,12 +485,15 @@ int deassign_device(struct domain *d, u1
     if ( !pdev )
         return -ENODEV;
 
+    /* De-assignment from dom_io should de-quarantine the device */
+    target = (pdev->quarantine && pdev->domain != dom_io) ? dom_io : dom0;
+
     while ( pdev->phantom_stride )
     {
         devfn += pdev->phantom_stride;
         if ( PCI_SLOT(devfn) != PCI_SLOT(pdev->devfn) )
             break;
-        ret = hd->platform_ops->reassign_device(d, dom0, devfn, pdev);
+        ret = hd->platform_ops->reassign_device(d, target, devfn, pdev);
         if ( !ret )
             continue;
 
@@ -475,7 +503,7 @@ int deassign_device(struct domain *d, u1
     }
 
     devfn = pdev->devfn;
-    ret = hd->platform_ops->reassign_device(d, dom0, devfn, pdev);
+    ret = hd->platform_ops->reassign_device(d, target, devfn, pdev);
     if ( ret )
     {
         dprintk(XENLOG_ERR VTDPREFIX,
@@ -484,6 +512,9 @@ int deassign_device(struct domain *d, u1
         return ret;
     }
 
+    if ( pdev->domain == dom0  )
+        pdev->quarantine = 0;
+
     pdev->fault.count = 0;
 
     if ( !has_arch_pdevs(d) && need_iommu(d) )
@@ -527,6 +558,9 @@ int __init iommu_setup(void)
     printk("I/O virtualisation %sabled\n", iommu_enabled ? "en" : "dis");
     if ( iommu_enabled )
     {
+        if ( iommu_domain_init(dom_io) )
+            panic("Could not set up quarantine\n");
+
         printk(" - Dom0 mode: %s\n",
                iommu_passthrough ? "Passthrough" :
                iommu_dom0_strict ? "Strict" : "Relaxed");
@@ -708,8 +742,12 @@ int iommu_do_domctl(
         break;
 
     case XEN_DOMCTL_assign_device:
-        if ( unlikely((d = get_domain_by_id(domctl->domain)) == NULL) ||
-             unlikely(d->is_dying) )
+        if ( domctl->domain == DOMID_IO )
+            d = dom_io;
+        else if ( domctl->domain == DOMID_INVALID )
+            return -ESRCH;
+        else if ( unlikely((d = get_domain_by_id(domctl->domain)) == NULL) ||
+                  unlikely(d->is_dying) )
         {
             printk(XENLOG_G_ERR
                    "XEN_DOMCTL_assign_device: get_domain_by_id() failed\n");
@@ -729,17 +767,22 @@ int iommu_do_domctl(
 
         ret = assign_device(d, seg, bus, devfn);
         if ( ret )
-            printk(XENLOG_G_ERR "XEN_DOMCTL_assign_device: "
+            printk(XENLOG_G_ERR
                    "assign %04x:%02x:%02x.%u to dom%d failed (%d)\n",
                    seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
                    d->domain_id, ret);
 
     assign_device_out:
-        put_domain(d);
+        if ( d != dom_io )
+            put_domain(d);
         break;
 
     case XEN_DOMCTL_deassign_device:
-        if ( unlikely((d = get_domain_by_id(domctl->domain)) == NULL) )
+        if ( domctl->domain == DOMID_IO )
+            d = dom_io;
+        else if ( domctl->domain == DOMID_INVALID )
+            return -ESRCH;
+        else if ( unlikely((d = get_domain_by_id(domctl->domain)) == NULL) )
         {
             printk(XENLOG_G_ERR
                    "XEN_DOMCTL_deassign_device: get_domain_by_id() failed\n");
@@ -765,7 +808,8 @@ int iommu_do_domctl(
                    d->domain_id, ret);
 
     deassign_device_out:
-        put_domain(d);
+        if ( d != dom_io )
+            put_domain(d);
         break;
 
     default:
--- xen-4.2.5-testing.orig/xen/drivers/passthrough/vtd/iommu.c
+++ xen-4.2.5-testing/xen/drivers/passthrough/vtd/iommu.c
@@ -1308,6 +1308,10 @@ int domain_context_mapping_one(
     u16 seg = iommu->intel->drhd->segment;
     int agaw;
 
+    /* dom_io is used as a sentinel for quarantined devices */
+    if ( domain == dom_io )
+        return 0;
+
     ASSERT(spin_is_locked(&pcidevs_lock));
     spin_lock(&iommu->lock);
     maddr = bus_to_context_maddr(iommu, bus);
@@ -1521,6 +1525,10 @@ int domain_context_unmap_one(
     u64 maddr;
     int iommu_domid;
 
+    /* dom_io is used as a sentinel for quarantined devices */
+    if ( domain == dom_io )
+        return 0;
+
     ASSERT(spin_is_locked(&pcidevs_lock));
     spin_lock(&iommu->lock);
 
@@ -1633,6 +1641,10 @@ static int domain_context_unmap(
         goto out;
     }
 
+    /* dom_io is used as a sentinel for quarantined devices */
+    if ( domain == dom_io )
+        goto out;
+
     /*
      * if no other devices under the same iommu owned by this domain,
      * clear iommu in iommu_bitmap and clear domain_id in domid_bitmp
@@ -1691,11 +1703,17 @@ static int reassign_device_ownership(
     if ( ret )
         return ret;
 
+    if ( devfn == pdev->devfn && pdev->domain != dom_io )
+    {
+        list_move(&pdev->domain_list, &dom_io->arch.pdev_list);
+        pdev->domain = dom_io;
+    }
+
     ret = domain_context_mapping(target, devfn, pdev);
     if ( ret )
         return ret;
 
-    if ( devfn == pdev->devfn )
+    if ( devfn == pdev->devfn && pdev->domain != target )
     {
         list_move(&pdev->domain_list, &target->arch.pdev_list);
         pdev->domain = target;
@@ -2252,6 +2270,7 @@ int __init intel_vtd_setup(void)
 static int intel_iommu_assign_device(
     struct domain *d, u8 devfn, struct pci_dev *pdev)
 {
+    struct domain *s = pdev->domain;
     struct acpi_rmrr_unit *rmrr;
     int ret = 0, i;
     u16 bdf, seg;
@@ -2260,8 +2279,8 @@ static int intel_iommu_assign_device(
     if ( list_empty(&acpi_drhd_units) )
         return -ENODEV;
 
-    ret = reassign_device_ownership(dom0, d, devfn, pdev);
-    if ( ret )
+    ret = reassign_device_ownership(s, d, devfn, pdev);
+    if ( ret || d == dom_io )
         goto done;
 
     /* FIXME: Because USB RMRR conflicts with guest bios region,
--- xen-4.2.5-testing.orig/xen/include/xen/pci.h
+++ xen-4.2.5-testing/xen/include/xen/pci.h
@@ -66,6 +66,9 @@ struct pci_dev {
 
     u8 phantom_stride;
 
+    /* Device to be quarantined, don't automatically re-assign to dom0 */
+    bool_t quarantine;
+
     enum pdev_type {
         DEV_TYPE_PCI_UNKNOWN,
         DEV_TYPE_PCIe_ENDPOINT,
