IOMMU: default to always quarantining PCI devices

XSA-302 relies on the use of libxl's "assignable-add" feature to prepare
devices to be assigned to untrusted guests.

Unfortunately, this is not considered a strictly required step for
device assignment. The PCI passthrough documentation on the wiki
describes alternate ways of preparing devices for assignment, and
libvirt uses its own ways as well. Hosts where these alternate methods
are used will still leave the system in a vulnerable state after the
device comes back from a guest.

Default to always quarantining PCI devices, but provide a command line
option to revert back to prior behavior (such that people who both
sufficiently trust their guests and want to be able to use devices in
Dom0 again after they had been in use by a guest wouldn't need to
"manually" move such devices back from DomIO to Dom0).

This is XSA-306.

Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Wei Liu <wl@xen.org>
master commit: ba2ab00bbb8c74e311a252d816d68dee47c779a0
master date: 2019-11-26 14:15:01 +0100

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 8084862f60d2ae983469e3697bc19195e06e6212..d09f35723a4d249b4dae8377053cecb445a8f5af 100644 (file)
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1137,7 +1137,7 @@ detection of systems known to misbehave upon accesses to that port.
 > Default: `new` unless directed-EOI is supported
 
 ### iommu
-> `= List of [ <boolean> | force | required | intremap | intpost | qinval | snoop | sharept | dom0-passthrough | dom0-strict | amd-iommu-perdev-intremap | workaround_bios_bug | igfx | crash-disable | verbose | debug ]`
+> `= List of [ <boolean> | force | required | quarantine | intremap | intpost | qinval | snoop | sharept | dom0-passthrough | dom0-strict | amd-iommu-perdev-intremap | workaround_bios_bug | igfx | crash-disable | verbose | debug ]`
 
 > Sub-options:
 
@@ -1157,6 +1157,15 @@ detection of systems known to misbehave upon accesses to that port.
 >> Don't continue booting unless IOMMU support is found and can be initialized
 >> successfully.
 
+> `quarantine`
+
+> Default: `true`
+
+>> Control Xen's behavior when de-assigning devices from guests.  If enabled,
+>> Xen always quarantines such devices; they must be explicitly assigned back
+>> to Dom0 before they can be used there again.  If disabled, Xen will only
+>> quarantine devices the toolstack hass arranged for getting quarantined.
+
 > `intremap`
 
 > Default: `true`

diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 8027d96f1c9940c65db8bf4acd6d1352d7262b5f..132bbb37629b9a41efb144a7a5558aa670e72aed 100644 (file)
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -52,6 +52,7 @@ custom_param("iommu", parse_iommu_param);
 bool_t __initdata iommu_enable = 1;
 bool_t __read_mostly iommu_enabled;
 bool_t __read_mostly force_iommu;
+bool __read_mostly iommu_quarantine = true;
 bool_t __hwdom_initdata iommu_dom0_strict;
 bool_t __read_mostly iommu_verbose;
 bool_t __read_mostly iommu_workaround_bios_bug;
@@ -99,6 +100,8 @@ static int __init parse_iommu_param(const char *s)
         else if ( !cmdline_strcmp(s, "force") ||
                   !cmdline_strcmp(s, "required") )
             force_iommu = val;
+        else if ( !cmdline_strcmp(s, "quarantine") )
+            iommu_quarantine = val;
         else if ( !cmdline_strcmp(s, "workaround_bios_bug") )
             iommu_workaround_bios_bug = val;
         else if ( !cmdline_strcmp(s, "igfx") )

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index b7b8aea9a33e7ded04f3153a23fd9d87c777d065..482ee1633e5628c39941a57ff8c9494b6b4bc5c8 100644 (file)
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1511,7 +1511,8 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
         return -ENODEV;
 
     /* De-assignment from dom_io should de-quarantine the device */
-    target = (pdev->quarantine && pdev->domain != dom_io) ?
+    target = ((pdev->quarantine || iommu_quarantine) &&
+              pdev->domain != dom_io) ?
         dom_io : hardware_domain;
 
     while ( pdev->phantom_stride )

diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 33c8b221dc97f454a6e8056180b0780fccf76ce1..235d2a620b14f97774447c4fc06c7eaf56483397 100644 (file)
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -29,7 +29,7 @@
 #include <asm/iommu.h>
 
 extern bool_t iommu_enable, iommu_enabled;
-extern bool_t force_iommu, iommu_verbose;
+extern bool force_iommu, iommu_quarantine, iommu_verbose;
 extern bool_t iommu_workaround_bios_bug, iommu_igfx, iommu_passthrough;
 extern bool_t iommu_snoop, iommu_qinval, iommu_intremap, iommu_intpost;
 extern bool_t iommu_hap_pt_share;