[prev in list] [next in list] [prev in thread] [next in thread]
List: oss-security
Subject: [oss-security] Xen Security Advisory 302 v5 (CVE-2019-18424) - passed through PCI devices may corrup
From: Xen.org security team <security () xen ! org>
Date: 2019-10-31 12:30:59
Message-ID: E1iQ9bP-0004PX-AC () xenbits ! xenproject ! org
[Download RAW message or body]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Xen Security Advisory CVE-2019-18424 / XSA-302
version 5
passed through PCI devices may corrupt host memory after deassignment
UPDATES IN VERSION 5
====================
Public release.
The patches are broken on ARM (which is not affected by the issue).
Don't apply the patches on ARM. See Resolution.
ISSUE DESCRIPTION
=================
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, or the device is
deassigned, the device is assigned back to dom0, thus allowing any
in-flight DMA to potentially target critical host data.
IMPACT
======
An untrusted domain with access to a physical device can DMA into host
memory, leading to privilege escalation.
VULNERABLE SYSTEMS
==================
Only systems where guests are given direct access to physical devices
capable of DMA (PCI pass-through) are vulnerable. Systems which do
not use PCI pass-through are not vulnerable.
MITIGATION
==========
In some configurations, use of passthrough can be replaced with a
higher-level protocol such as Xen PV block or network devices.
CREDITS
=======
This issue was discovered by Paul Durrant of Citrix.
RESOLUTION
==========
Applying the appropriate attached patchset should resolve this issue.
For Xen 4.9 and earlier at least the first patch of XSA-299
(whitespace cleanup) is also needed for XSA-302 to apply.
Unfortunately, at the time of writing, these patches have not been
tested to our satisfaction.
The patches are known to break on ARM. ARM is not affected by the
issue, so do not apply these patches on ARM systems. (On x86, there
is a latent bug but the patches are good to use.)
xsa302/*.patch xen-unstable
xsa302-4.12/*.patch Xen 4.12.x
xsa302-4.11/*.patch Xen 4.11.x
xsa302-4.10/*.patch Xen 4.10.x
xsa302-4.9/*.patch Xen 4.9.x, Xen 4.8.x
$ sha256sum xsa302* xsa302*/*
d722d1bed2440a5d35f0fd041e4a77966b7d26980a0f874d38d48710db0b9ebd xsa302.meta
703faced133ca21142f484acd8cf16578258e12ae0cf1413a5d9252f1e099465 \
xsa302-4.9/0001-IOMMU-add-missing-HVM-check.patch \
edb4753b91fa66e2f4b51d0075d106fc28d8451241ba482a33c2db4be53f21d1 \
xsa302-4.9/0002-passthrough-quarantine-PCI-devices.patch \
3c79107d8fd94807543443192fb31f3d188912c208f4dbda61f1f2ff92701afc \
xsa302-4.10/0001-IOMMU-add-missing-HVM-check.patch \
2a76add5a907baf0217e57e2a4dca91a6a8ce84c67b9ff87be1bcbb1f29efdc6 \
xsa302-4.10/0002-passthrough-quarantine-PCI-devices.patch \
a75723160c52c2c65d563905d0904b587beda1cfb6ca3ee18fb70e79818d3faa \
xsa302-4.11/0001-IOMMU-add-missing-HVM-check.patch \
48b9dae7adbe2438dcaa00f969532d835061cb4a06ab2bf47ada2afb644de4c5 \
xsa302-4.11/0002-passthrough-quarantine-PCI-devices.patch \
a21efa6cae14e87318ca3927f0ac310aee2dd1323f2dbf040c0fe80789d78712 \
xsa302-4.12/0001-IOMMU-add-missing-HVM-check.patch \
0a95f750ad1d5eb1838b6488e4ac188acdc2e568eb21b26306d5af2980bffb58 \
xsa302-4.12/0002-passthrough-quarantine-PCI-devices.patch \
11d7015960eab265b1f9ce372dd14597b6c4cc7907d77ed3eed14d161dd50e5c \
xsa302/0001-passthrough-quarantine-PCI-devices.patch $
DEPLOYMENT DURING EMBARGO
=========================
Deployment of the *patches* described above (or others which are
substantially similar) is permitted during the embargo, even on
public-facing systems with untrusted guest users and administrators.
But: Distribution of updated software is prohibited (except to other
members of the predisclosure list).
Also: deployment of the reconfiguration *mitigation* is NOT permitted
(except where all the affected systems and VMs are administered and
used only by organisations which are members of the Xen Project
Security Issues Predisclosure List). Specifically, deployment on
public cloud systems is NOT permitted.
This is because this reconfiguration reveals that a PCI passthrough
vulnerability is involved.
Deployment of that migitation is permitted only AFTER the embargo
ends.
Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.
(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable. This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)
For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----
iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAl260/wMHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZ2TYH/A+tmA2Wsw0NbdEhSzztj6cVFZpev16S75vOxLUm
/dFTQSxNVeqyzZjI7u9JPZUatQVIHwdDPi9Oiwygn8pFid1RBe+fn3saM3JdNQrA
pYVOCEYGoxnz/lpPLWfcI8aUIkdhU4Ns/hwXVa6lUNno9MaqqJR278k6nmB9/0QS
bFvsMirqTKHm7wQptY5mRcULdjcpn+u4W45nje3+TU0mMRQkbm+pnNX57qzn/LFI
/atzBQ8iyv9/y3e/soAXv3AkWzs/lUVIAZepaFhXCHi3WuMsMUyZAdDOUBmD0tBt
pjQzx408ZoMPtqqDKpY1qEn9Bu1MsIxx/4htqlgG0c9Kh1U=
=cUbr
-----END PGP SIGNATURE-----
["xsa302.meta" (application/octet-stream)]
["xsa302-4.9/0001-IOMMU-add-missing-HVM-check.patch" (application/octet-stream)]
From b1616045dc2fc3d28e6170164ceeef63a9f325b9 Mon Sep 17 00:00:00 2001
From: Jan Beulich <jbeulich@suse.com>
Date: Wed, 2 Oct 2019 13:36:59 +0200
Subject: [PATCH 1/2] IOMMU: add missing HVM check
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Fix an unguarded d->arch.hvm access in assign_device().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
(cherry picked from commit 41fd1009cd7416b73d745a77c24b4e8d1a296fe6)
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
xen/drivers/passthrough/pci.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index b00de243b8..ecb67ebc70 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1371,7 +1371,8 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
/* Prevent device assign if mem paging or mem sharing have been
* enabled for this domain */
if ( unlikely(!need_iommu(d) &&
- (d->arch.hvm_domain.mem_sharing_enabled ||
+ ((is_hvm_domain(d) &&
+ d->arch.hvm_domain.mem_sharing_enabled) ||
d->vm_event->paging.ring_page ||
p2m_get_hostp2m(d)->global_logdirty)) )
return -EXDEV;
--
2.11.0
["xsa302-4.9/0002-passthrough-quarantine-PCI-devices.patch" (application/octet-stream)]
From 88ce31b64b9cda93762db0b88d2943180afe3a33 Mon Sep 17 00:00:00 2001
From: Paul Durrant <paul.durrant@citrix.com>
Date: Mon, 14 Oct 2019 17:52:59 +0100
Subject: [PATCH 2/2] 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>
---
tools/libxl/libxl_pci.c | 25 +++++++++++-
xen/arch/x86/mm.c | 2 +
xen/common/domctl.c | 14 ++++++-
xen/drivers/passthrough/amd/pci_amd_iommu.c | 10 ++++-
xen/drivers/passthrough/iommu.c | 9 +++++
xen/drivers/passthrough/pci.c | 59 ++++++++++++++++++++++-------
xen/drivers/passthrough/vtd/iommu.c | 40 ++++++++++++++++---
xen/include/xen/pci.h | 3 ++
8 files changed, 138 insertions(+), 24 deletions(-)
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index b14df1629a..84a293cf99 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -761,6 +761,7 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
libxl_device_pci *pcidev,
int rebind)
{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
unsigned dom, bus, dev, func;
char *spath, *driver_path = NULL;
int rc;
@@ -786,7 +787,7 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
}
if ( rc ) {
LOG(WARN, 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 */
@@ -817,6 +818,19 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
return ERROR_FAIL;
}
+quarantine:
+ /*
+ * DOMID_IO is just a sentinel domain, without any actual mappings,
+ * so always pass XEN_DOMCTL_DEV_RDM_RELAXED to avoid assignment being
+ * unnecessarily denied.
+ */
+ rc = xc_assign_device(ctx->xch, DOMID_IO, pcidev_encode_bdf(pcidev),
+ XEN_DOMCTL_DEV_RDM_RELAXED);
+ if ( rc < 0 ) {
+ LOG(ERROR, "failed to quarantine "PCI_BDF, dom, bus, dev, func);
+ return ERROR_FAIL;
+ }
+
return 0;
}
@@ -824,9 +838,18 @@ static int libxl__device_pci_assignable_remove(libxl__gc *gc,
libxl_device_pci *pcidev,
int rebind)
{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
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 ) {
return ERROR_FAIL;
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index f7e0cc4508..df28391859 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -308,9 +308,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, NULL);
BUG_ON(IS_ERR(dom_io));
+ INIT_LIST_HEAD(&dom_io->arch.pdev_list);
/*
* Initialise our COW domain.
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 011a7de355..f8f7aa5f40 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -396,6 +396,16 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
case XEN_DOMCTL_gdbsx_guestmemio:
d = NULL;
break;
+ case XEN_DOMCTL_assign_device:
+ case XEN_DOMCTL_deassign_device:
+ if ( op->domain == DOMID_IO )
+ {
+ d = dom_io;
+ break;
+ }
+ else if ( op->domain == DOMID_INVALID )
+ return -ESRCH;
+ /* fall through */
default:
d = rcu_lock_domain_by_id(op->domain);
if ( !d && op->cmd != XEN_DOMCTL_getdomaininfo )
@@ -408,7 +418,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
if ( !domctl_lock_acquire() )
{
- if ( d )
+ if ( d && d != dom_io )
rcu_unlock_domain(d);
return hypercall_create_continuation(
__HYPERVISOR_domctl, "h", u_domctl);
@@ -1150,7 +1160,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
domctl_lock_release();
domctl_out_unlock_domonly:
- if ( d )
+ if ( d && d != dom_io )
rcu_unlock_domain(d);
if ( copyback && __copy_to_guest(u_domctl, op, 1) )
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 8c25110eae..d11dc9c94e 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -118,6 +118,10 @@ static void amd_iommu_setup_domain_device(
u8 bus = pdev->bus;
const struct domain_iommu *hd = dom_iommu(domain);
+ /* dom_io is used as a sentinel for quarantined devices */
+ if ( domain == dom_io )
+ return;
+
BUG_ON( !hd->arch.root_table || !hd->arch.paging_mode ||
!iommu->dev_table.buffer );
@@ -330,6 +334,10 @@ void amd_iommu_disable_domain_device(struct domain *domain,
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);
@@ -416,7 +424,7 @@ static int amd_iommu_assign_device(struct domain *d, u8 devfn,
ivrs_mappings[req_id].read_permission);
}
- return reassign_device(hardware_domain, d, devfn, pdev);
+ return reassign_device(pdev->domain, d, devfn, pdev);
}
static void deallocate_next_page_table(struct page_info *pg, int level)
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 5e81813942..9ebb8c6bc4 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -208,6 +208,9 @@ void iommu_teardown(struct domain *d)
{
const struct domain_iommu *hd = dom_iommu(d);
+ if ( d == dom_io )
+ return;
+
d->need_iommu = 0;
hd->platform_ops->teardown(d);
tasklet_schedule(&iommu_pt_cleanup_tasklet);
@@ -218,6 +221,9 @@ int iommu_construct(struct domain *d)
if ( need_iommu(d) > 0 )
return 0;
+ if ( d == dom_io )
+ return 0;
+
if ( !iommu_use_hap_pt(d) )
{
int rc;
@@ -393,6 +399,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");
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index ecb67ebc70..0250d19ae5 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1344,19 +1344,29 @@ int iommu_remove_device(struct pci_dev *pdev)
return hd->platform_ops->remove_device(pdev->devfn, pci_to_dev(pdev));
}
-/*
- * If the device isn't owned by the hardware domain, 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;
pcidevs_lock();
- pdev = pci_get_pdev_by_domain(hardware_domain, 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 != hardware_domain &&
+ pdev->domain != dom_io )
+ rc = -EBUSY;
+
pcidevs_unlock();
- return pdev ? 0 : -EBUSY;
+ return rc;
}
static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
@@ -1370,7 +1380,8 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
/* Prevent device assign if mem paging or mem sharing have been
* enabled for this domain */
- if ( unlikely(!need_iommu(d) &&
+ if ( d != dom_io &&
+ unlikely(!need_iommu(d) &&
((is_hvm_domain(d) &&
d->arch.hvm_domain.mem_sharing_enabled) ||
d->vm_event->paging.ring_page ||
@@ -1387,12 +1398,20 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
return rc;
}
- pdev = pci_get_pdev_by_domain(hardware_domain, 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 != hardware_domain &&
+ pdev->domain != dom_io )
+ goto done;
if ( pdev->msix )
msixtbl_init(d);
@@ -1415,6 +1434,10 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
}
done:
+ /* The device is assigned to dom_io so mark it as quarantined */
+ if ( !rc && d == dom_io )
+ pdev->quarantine = true;
+
if ( !has_arch_pdevs(d) && need_iommu(d) )
iommu_teardown(d);
pcidevs_unlock();
@@ -1427,6 +1450,7 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
{
const struct domain_iommu *hd = dom_iommu(d);
struct pci_dev *pdev = NULL;
+ struct domain *target;
int ret = 0;
if ( !iommu_enabled || !hd->platform_ops )
@@ -1437,12 +1461,16 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
if ( !pdev )
return -ENODEV;
+ /* De-assignment from dom_io should de-quarantine the device */
+ target = (pdev->quarantine && pdev->domain != dom_io) ?
+ dom_io : hardware_domain;
+
while ( pdev->phantom_stride )
{
devfn += pdev->phantom_stride;
if ( PCI_SLOT(devfn) != PCI_SLOT(pdev->devfn) )
break;
- ret = hd->platform_ops->reassign_device(d, hardware_domain, devfn,
+ ret = hd->platform_ops->reassign_device(d, target, devfn,
pci_to_dev(pdev));
if ( !ret )
continue;
@@ -1453,7 +1481,7 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
}
devfn = pdev->devfn;
- ret = hd->platform_ops->reassign_device(d, hardware_domain, devfn,
+ ret = hd->platform_ops->reassign_device(d, target, devfn,
pci_to_dev(pdev));
if ( ret )
{
@@ -1463,6 +1491,9 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
return ret;
}
+ if ( pdev->domain == hardware_domain )
+ pdev->quarantine = false;
+
pdev->fault.count = 0;
if ( !has_arch_pdevs(d) && need_iommu(d) )
@@ -1654,7 +1685,7 @@ int iommu_do_pci_domctl(
ret = hypercall_create_continuation(__HYPERVISOR_domctl,
"h", u_domctl);
else 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);
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 2093668328..0e0aa6d517 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1332,6 +1332,10 @@ int domain_context_mapping_one(
int agaw, rc, ret;
bool_t flush_dev_iotlb;
+ /* dom_io is used as a sentinel for quarantined devices */
+ if ( domain == dom_io )
+ return 0;
+
ASSERT(pcidevs_locked());
spin_lock(&iommu->lock);
maddr = bus_to_context_maddr(iommu, bus);
@@ -1567,6 +1571,10 @@ int domain_context_unmap_one(
int iommu_domid, rc, ret;
bool_t flush_dev_iotlb;
+ /* dom_io is used as a sentinel for quarantined devices */
+ if ( domain == dom_io )
+ return 0;
+
ASSERT(pcidevs_locked());
spin_lock(&iommu->lock);
@@ -1699,6 +1707,10 @@ static int domain_context_unmap(struct domain *domain, u8 devfn,
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
@@ -2383,6 +2395,15 @@ static int reassign_device_ownership(
if ( ret )
return ret;
+ if ( devfn == pdev->devfn )
+ {
+ list_move(&pdev->domain_list, &dom_io->arch.pdev_list);
+ pdev->domain = dom_io;
+ }
+
+ if ( !has_arch_pdevs(source) )
+ vmx_pi_hooks_deassign(source);
+
if ( !has_arch_pdevs(target) )
vmx_pi_hooks_assign(target);
@@ -2401,15 +2422,13 @@ static int reassign_device_ownership(
pdev->domain = target;
}
- if ( !has_arch_pdevs(source) )
- vmx_pi_hooks_deassign(source);
-
return ret;
}
static int intel_iommu_assign_device(
struct domain *d, u8 devfn, struct pci_dev *pdev, u32 flag)
{
+ struct domain *s = pdev->domain;
struct acpi_rmrr_unit *rmrr;
int ret = 0, i;
u16 bdf, seg;
@@ -2452,8 +2471,8 @@ static int intel_iommu_assign_device(
}
}
- ret = reassign_device_ownership(hardware_domain, d, devfn, pdev);
- if ( ret )
+ ret = reassign_device_ownership(s, d, devfn, pdev);
+ if ( ret || d == dom_io )
return ret;
/* Setup rmrr identity mapping */
@@ -2466,11 +2485,20 @@ static int intel_iommu_assign_device(
ret = rmrr_identity_mapping(d, 1, rmrr, flag);
if ( ret )
{
- reassign_device_ownership(d, hardware_domain, devfn, pdev);
+ int rc;
+
+ rc = reassign_device_ownership(d, s, devfn, pdev);
printk(XENLOG_G_ERR VTDPREFIX
" cannot map reserved region (%"PRIx64",%"PRIx64"] for Dom%d (%d)\n",
rmrr->base_address, rmrr->end_address,
d->domain_id, ret);
+ if ( rc )
+ {
+ printk(XENLOG_ERR VTDPREFIX
+ " failed to reclaim %04x:%02x:%02x.%u from %pd (%d)\n",
+ seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), d, rc);
+ domain_crash(d);
+ }
break;
}
}
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index da1bd22223..2917d00a47 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -68,6 +68,9 @@ struct pci_dev {
nodeid_t node; /* NUMA node */
+ /* Device to be quarantined, don't automatically re-assign to dom0 */
+ bool quarantine;
+
enum pdev_type {
DEV_TYPE_PCI_UNKNOWN,
DEV_TYPE_PCIe_ENDPOINT,
--
2.11.0
["xsa302-4.10/0001-IOMMU-add-missing-HVM-check.patch" (application/octet-stream)]
From 2bcbf2843250888b720bfea188ac9842c847f388 Mon Sep 17 00:00:00 2001
From: Jan Beulich <jbeulich@suse.com>
Date: Wed, 2 Oct 2019 13:36:59 +0200
Subject: [PATCH 1/2] IOMMU: add missing HVM check
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Fix an unguarded d->arch.hvm access in assign_device().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
(cherry picked from commit 41fd1009cd7416b73d745a77c24b4e8d1a296fe6)
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
xen/drivers/passthrough/pci.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index e021c7a317..e1668a1968 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1386,7 +1386,8 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
/* Prevent device assign if mem paging or mem sharing have been
* enabled for this domain */
if ( unlikely(!need_iommu(d) &&
- (d->arch.hvm_domain.mem_sharing_enabled ||
+ ((is_hvm_domain(d) &&
+ d->arch.hvm_domain.mem_sharing_enabled) ||
vm_event_check_ring(d->vm_event_paging) ||
p2m_get_hostp2m(d)->global_logdirty)) )
return -EXDEV;
--
2.11.0
["xsa302-4.10/0002-passthrough-quarantine-PCI-devices.patch" (application/octet-stream)]
From 02dd07e53b904570e0320d17d77022ddbc4e8225 Mon Sep 17 00:00:00 2001
From: Paul Durrant <paul.durrant@citrix.com>
Date: Mon, 14 Oct 2019 17:52:59 +0100
Subject: [PATCH 2/2] 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>
---
tools/libxl/libxl_pci.c | 25 +++++++++++-
xen/arch/x86/mm.c | 2 +
xen/common/domctl.c | 14 ++++++-
xen/drivers/passthrough/amd/pci_amd_iommu.c | 10 ++++-
xen/drivers/passthrough/iommu.c | 9 +++++
xen/drivers/passthrough/pci.c | 59 ++++++++++++++++++++++-------
xen/drivers/passthrough/vtd/iommu.c | 40 ++++++++++++++++---
xen/include/xen/pci.h | 3 ++
8 files changed, 138 insertions(+), 24 deletions(-)
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 88a55ce8bd..1b5c44f3e7 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -749,6 +749,7 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
libxl_device_pci *pcidev,
int rebind)
{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
unsigned dom, bus, dev, func;
char *spath, *driver_path = NULL;
int rc;
@@ -774,7 +775,7 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
}
if ( rc ) {
LOG(WARN, 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 */
@@ -805,6 +806,19 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
return ERROR_FAIL;
}
+quarantine:
+ /*
+ * DOMID_IO is just a sentinel domain, without any actual mappings,
+ * so always pass XEN_DOMCTL_DEV_RDM_RELAXED to avoid assignment being
+ * unnecessarily denied.
+ */
+ rc = xc_assign_device(ctx->xch, DOMID_IO, pcidev_encode_bdf(pcidev),
+ XEN_DOMCTL_DEV_RDM_RELAXED);
+ if ( rc < 0 ) {
+ LOG(ERROR, "failed to quarantine "PCI_BDF, dom, bus, dev, func);
+ return ERROR_FAIL;
+ }
+
return 0;
}
@@ -812,9 +826,18 @@ static int libxl__device_pci_assignable_remove(libxl__gc *gc,
libxl_device_pci *pcidev,
int rebind)
{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
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 ) {
return ERROR_FAIL;
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index ce2c082caf..0e42497cf7 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -295,9 +295,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, NULL);
BUG_ON(IS_ERR(dom_io));
+ INIT_LIST_HEAD(&dom_io->arch.pdev_list);
/*
* Initialise our COW domain.
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 3c6fa4ec67..a70f4b46f8 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -392,6 +392,16 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
switch ( op->cmd )
{
+ case XEN_DOMCTL_assign_device:
+ case XEN_DOMCTL_deassign_device:
+ if ( op->domain == DOMID_IO )
+ {
+ d = dom_io;
+ break;
+ }
+ else if ( op->domain == DOMID_INVALID )
+ return -ESRCH;
+ /* fall through */
case XEN_DOMCTL_test_assign_device:
if ( op->domain == DOMID_INVALID )
{
@@ -413,7 +423,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
if ( !domctl_lock_acquire() )
{
- if ( d )
+ if ( d && d != dom_io )
rcu_unlock_domain(d);
return hypercall_create_continuation(
__HYPERVISOR_domctl, "h", u_domctl);
@@ -1163,7 +1173,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
domctl_lock_release();
domctl_out_unlock_domonly:
- if ( d )
+ if ( d && d != dom_io )
rcu_unlock_domain(d);
if ( copyback && __copy_to_guest(u_domctl, op, 1) )
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 12d2695b89..ec8baae717 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -118,6 +118,10 @@ static void amd_iommu_setup_domain_device(
u8 bus = pdev->bus;
const struct domain_iommu *hd = dom_iommu(domain);
+ /* dom_io is used as a sentinel for quarantined devices */
+ if ( domain == dom_io )
+ return;
+
BUG_ON( !hd->arch.root_table || !hd->arch.paging_mode ||
!iommu->dev_table.buffer );
@@ -305,6 +309,10 @@ void amd_iommu_disable_domain_device(struct domain *domain,
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);
@@ -391,7 +399,7 @@ static int amd_iommu_assign_device(struct domain *d, u8 devfn,
ivrs_mappings[req_id].read_permission);
}
- return reassign_device(hardware_domain, d, devfn, pdev);
+ return reassign_device(pdev->domain, d, devfn, pdev);
}
static void deallocate_next_page_table(struct page_info *pg, int level)
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index b5f8044439..ad2ce8f39b 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -219,6 +219,9 @@ void iommu_teardown(struct domain *d)
{
const struct domain_iommu *hd = dom_iommu(d);
+ if ( d == dom_io )
+ return;
+
d->need_iommu = 0;
hd->platform_ops->teardown(d);
tasklet_schedule(&iommu_pt_cleanup_tasklet);
@@ -229,6 +232,9 @@ int iommu_construct(struct domain *d)
if ( need_iommu(d) > 0 )
return 0;
+ if ( d == dom_io )
+ return 0;
+
if ( !iommu_use_hap_pt(d) )
{
int rc;
@@ -404,6 +410,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");
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index e1668a1968..6b2e9d2896 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1359,19 +1359,29 @@ static int iommu_remove_device(struct pci_dev *pdev)
return hd->platform_ops->remove_device(pdev->devfn, pci_to_dev(pdev));
}
-/*
- * If the device isn't owned by the hardware domain, 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;
pcidevs_lock();
- pdev = pci_get_pdev_by_domain(hardware_domain, 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 != hardware_domain &&
+ pdev->domain != dom_io )
+ rc = -EBUSY;
+
pcidevs_unlock();
- return pdev ? 0 : -EBUSY;
+ return rc;
}
static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
@@ -1385,7 +1395,8 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
/* Prevent device assign if mem paging or mem sharing have been
* enabled for this domain */
- if ( unlikely(!need_iommu(d) &&
+ if ( d != dom_io &&
+ unlikely(!need_iommu(d) &&
((is_hvm_domain(d) &&
d->arch.hvm_domain.mem_sharing_enabled) ||
vm_event_check_ring(d->vm_event_paging) ||
@@ -1402,12 +1413,20 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
return rc;
}
- pdev = pci_get_pdev_by_domain(hardware_domain, 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 != hardware_domain &&
+ pdev->domain != dom_io )
+ goto done;
if ( pdev->msix )
msixtbl_init(d);
@@ -1430,6 +1449,10 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
}
done:
+ /* The device is assigned to dom_io so mark it as quarantined */
+ if ( !rc && d == dom_io )
+ pdev->quarantine = true;
+
if ( !has_arch_pdevs(d) && need_iommu(d) )
iommu_teardown(d);
pcidevs_unlock();
@@ -1442,6 +1465,7 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
{
const struct domain_iommu *hd = dom_iommu(d);
struct pci_dev *pdev = NULL;
+ struct domain *target;
int ret = 0;
if ( !iommu_enabled || !hd->platform_ops )
@@ -1452,12 +1476,16 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
if ( !pdev )
return -ENODEV;
+ /* De-assignment from dom_io should de-quarantine the device */
+ target = (pdev->quarantine && pdev->domain != dom_io) ?
+ dom_io : hardware_domain;
+
while ( pdev->phantom_stride )
{
devfn += pdev->phantom_stride;
if ( PCI_SLOT(devfn) != PCI_SLOT(pdev->devfn) )
break;
- ret = hd->platform_ops->reassign_device(d, hardware_domain, devfn,
+ ret = hd->platform_ops->reassign_device(d, target, devfn,
pci_to_dev(pdev));
if ( !ret )
continue;
@@ -1468,7 +1496,7 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
}
devfn = pdev->devfn;
- ret = hd->platform_ops->reassign_device(d, hardware_domain, devfn,
+ ret = hd->platform_ops->reassign_device(d, target, devfn,
pci_to_dev(pdev));
if ( ret )
{
@@ -1478,6 +1506,9 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
return ret;
}
+ if ( pdev->domain == hardware_domain )
+ pdev->quarantine = false;
+
pdev->fault.count = 0;
if ( !has_arch_pdevs(d) && need_iommu(d) )
@@ -1656,7 +1687,7 @@ int iommu_do_pci_domctl(
ret = hypercall_create_continuation(__HYPERVISOR_domctl,
"h", u_domctl);
else 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);
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 481efef2b0..1d16127d8f 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1332,6 +1332,10 @@ int domain_context_mapping_one(
int agaw, rc, ret;
bool_t flush_dev_iotlb;
+ /* dom_io is used as a sentinel for quarantined devices */
+ if ( domain == dom_io )
+ return 0;
+
ASSERT(pcidevs_locked());
spin_lock(&iommu->lock);
maddr = bus_to_context_maddr(iommu, bus);
@@ -1567,6 +1571,10 @@ int domain_context_unmap_one(
int iommu_domid, rc, ret;
bool_t flush_dev_iotlb;
+ /* dom_io is used as a sentinel for quarantined devices */
+ if ( domain == dom_io )
+ return 0;
+
ASSERT(pcidevs_locked());
spin_lock(&iommu->lock);
@@ -1699,6 +1707,10 @@ static int domain_context_unmap(struct domain *domain, u8 devfn,
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
@@ -2383,6 +2395,15 @@ static int reassign_device_ownership(
if ( ret )
return ret;
+ if ( devfn == pdev->devfn )
+ {
+ list_move(&pdev->domain_list, &dom_io->arch.pdev_list);
+ pdev->domain = dom_io;
+ }
+
+ if ( !has_arch_pdevs(source) )
+ vmx_pi_hooks_deassign(source);
+
if ( !has_arch_pdevs(target) )
vmx_pi_hooks_assign(target);
@@ -2401,15 +2422,13 @@ static int reassign_device_ownership(
pdev->domain = target;
}
- if ( !has_arch_pdevs(source) )
- vmx_pi_hooks_deassign(source);
-
return ret;
}
static int intel_iommu_assign_device(
struct domain *d, u8 devfn, struct pci_dev *pdev, u32 flag)
{
+ struct domain *s = pdev->domain;
struct acpi_rmrr_unit *rmrr;
int ret = 0, i;
u16 bdf, seg;
@@ -2452,8 +2471,8 @@ static int intel_iommu_assign_device(
}
}
- ret = reassign_device_ownership(hardware_domain, d, devfn, pdev);
- if ( ret )
+ ret = reassign_device_ownership(s, d, devfn, pdev);
+ if ( ret || d == dom_io )
return ret;
/* Setup rmrr identity mapping */
@@ -2466,11 +2485,20 @@ static int intel_iommu_assign_device(
ret = rmrr_identity_mapping(d, 1, rmrr, flag);
if ( ret )
{
- reassign_device_ownership(d, hardware_domain, devfn, pdev);
+ int rc;
+
+ rc = reassign_device_ownership(d, s, devfn, pdev);
printk(XENLOG_G_ERR VTDPREFIX
" cannot map reserved region (%"PRIx64",%"PRIx64"] for Dom%d (%d)\n",
rmrr->base_address, rmrr->end_address,
d->domain_id, ret);
+ if ( rc )
+ {
+ printk(XENLOG_ERR VTDPREFIX
+ " failed to reclaim %04x:%02x:%02x.%u from %pd (%d)\n",
+ seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), d, rc);
+ domain_crash(d);
+ }
break;
}
}
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 43f21251a5..3241e51e3c 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -68,6 +68,9 @@ struct pci_dev {
nodeid_t node; /* NUMA node */
+ /* Device to be quarantined, don't automatically re-assign to dom0 */
+ bool quarantine;
+
enum pdev_type {
DEV_TYPE_PCI_UNKNOWN,
DEV_TYPE_PCIe_ENDPOINT,
--
2.11.0
["xsa302-4.11/0001-IOMMU-add-missing-HVM-check.patch" (application/octet-stream)]
From bbca29f88d9ad9c7e91125a3b5d5f13a23e5801f Mon Sep 17 00:00:00 2001
From: Jan Beulich <jbeulich@suse.com>
Date: Wed, 2 Oct 2019 13:36:59 +0200
Subject: [PATCH 1/2] IOMMU: add missing HVM check
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Fix an unguarded d->arch.hvm access in assign_device().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
(cherry picked from commit 41fd1009cd7416b73d745a77c24b4e8d1a296fe6)
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
xen/drivers/passthrough/pci.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index f51cae7f4e..037aba7c94 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1416,7 +1416,8 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
/* Prevent device assign if mem paging or mem sharing have been
* enabled for this domain */
if ( unlikely(!need_iommu(d) &&
- (d->arch.hvm_domain.mem_sharing_enabled ||
+ ((is_hvm_domain(d) &&
+ d->arch.hvm_domain.mem_sharing_enabled) ||
vm_event_check_ring(d->vm_event_paging) ||
p2m_get_hostp2m(d)->global_logdirty)) )
return -EXDEV;
--
2.11.0
["xsa302-4.11/0002-passthrough-quarantine-PCI-devices.patch" (application/octet-stream)]
From ec99857f59f7f06236f11ca8b0b2303e5e745cc4 Mon Sep 17 00:00:00 2001
From: Paul Durrant <paul.durrant@citrix.com>
Date: Mon, 14 Oct 2019 17:52:59 +0100
Subject: [PATCH 2/2] 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>
---
tools/libxl/libxl_pci.c | 25 +++++++++++-
xen/arch/x86/mm.c | 2 +
xen/common/domctl.c | 14 ++++++-
xen/drivers/passthrough/amd/pci_amd_iommu.c | 10 ++++-
xen/drivers/passthrough/iommu.c | 9 +++++
xen/drivers/passthrough/pci.c | 59 ++++++++++++++++++++++-------
xen/drivers/passthrough/vtd/iommu.c | 40 ++++++++++++++++---
xen/include/xen/pci.h | 3 ++
8 files changed, 138 insertions(+), 24 deletions(-)
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 4755a0c93c..81890a91ac 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -754,6 +754,7 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
libxl_device_pci *pcidev,
int rebind)
{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
unsigned dom, bus, dev, func;
char *spath, *driver_path = NULL;
int rc;
@@ -779,7 +780,7 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
}
if ( rc ) {
LOG(WARN, 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 */
@@ -810,6 +811,19 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
return ERROR_FAIL;
}
+quarantine:
+ /*
+ * DOMID_IO is just a sentinel domain, without any actual mappings,
+ * so always pass XEN_DOMCTL_DEV_RDM_RELAXED to avoid assignment being
+ * unnecessarily denied.
+ */
+ rc = xc_assign_device(ctx->xch, DOMID_IO, pcidev_encode_bdf(pcidev),
+ XEN_DOMCTL_DEV_RDM_RELAXED);
+ if ( rc < 0 ) {
+ LOG(ERROR, "failed to quarantine "PCI_BDF, dom, bus, dev, func);
+ return ERROR_FAIL;
+ }
+
return 0;
}
@@ -817,9 +831,18 @@ static int libxl__device_pci_assignable_remove(libxl__gc *gc,
libxl_device_pci *pcidev,
int rebind)
{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
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 ) {
return ERROR_FAIL;
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index e6a4cb28f8..c1ab57f9a5 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -295,9 +295,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, NULL);
BUG_ON(IS_ERR(dom_io));
+ INIT_LIST_HEAD(&dom_io->arch.pdev_list);
/*
* Initialise our COW domain.
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 9b7bc083ee..741d774cd1 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -392,6 +392,16 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
switch ( op->cmd )
{
+ case XEN_DOMCTL_assign_device:
+ case XEN_DOMCTL_deassign_device:
+ if ( op->domain == DOMID_IO )
+ {
+ d = dom_io;
+ break;
+ }
+ else if ( op->domain == DOMID_INVALID )
+ return -ESRCH;
+ /* fall through */
case XEN_DOMCTL_test_assign_device:
if ( op->domain == DOMID_INVALID )
{
@@ -413,7 +423,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
if ( !domctl_lock_acquire() )
{
- if ( d )
+ if ( d && d != dom_io )
rcu_unlock_domain(d);
return hypercall_create_continuation(
__HYPERVISOR_domctl, "h", u_domctl);
@@ -1148,7 +1158,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
domctl_lock_release();
domctl_out_unlock_domonly:
- if ( d )
+ if ( d && d != dom_io )
rcu_unlock_domain(d);
if ( copyback && __copy_to_guest(u_domctl, op, 1) )
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 12d2695b89..ec8baae717 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -118,6 +118,10 @@ static void amd_iommu_setup_domain_device(
u8 bus = pdev->bus;
const struct domain_iommu *hd = dom_iommu(domain);
+ /* dom_io is used as a sentinel for quarantined devices */
+ if ( domain == dom_io )
+ return;
+
BUG_ON( !hd->arch.root_table || !hd->arch.paging_mode ||
!iommu->dev_table.buffer );
@@ -305,6 +309,10 @@ void amd_iommu_disable_domain_device(struct domain *domain,
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);
@@ -391,7 +399,7 @@ static int amd_iommu_assign_device(struct domain *d, u8 devfn,
ivrs_mappings[req_id].read_permission);
}
- return reassign_device(hardware_domain, d, devfn, pdev);
+ return reassign_device(pdev->domain, d, devfn, pdev);
}
static void deallocate_next_page_table(struct page_info *pg, int level)
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 04b0be37d3..8027d96f1c 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -219,6 +219,9 @@ void iommu_teardown(struct domain *d)
{
const struct domain_iommu *hd = dom_iommu(d);
+ if ( d == dom_io )
+ return;
+
d->need_iommu = 0;
hd->platform_ops->teardown(d);
tasklet_schedule(&iommu_pt_cleanup_tasklet);
@@ -229,6 +232,9 @@ int iommu_construct(struct domain *d)
if ( need_iommu(d) > 0 )
return 0;
+ if ( d == dom_io )
+ return 0;
+
if ( !iommu_use_hap_pt(d) )
{
int rc;
@@ -404,6 +410,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");
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 037aba7c94..fb010a547b 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1389,19 +1389,29 @@ static int iommu_remove_device(struct pci_dev *pdev)
return hd->platform_ops->remove_device(pdev->devfn, pci_to_dev(pdev));
}
-/*
- * If the device isn't owned by the hardware domain, 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;
pcidevs_lock();
- pdev = pci_get_pdev_by_domain(hardware_domain, 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 != hardware_domain &&
+ pdev->domain != dom_io )
+ rc = -EBUSY;
+
pcidevs_unlock();
- return pdev ? 0 : -EBUSY;
+ return rc;
}
static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
@@ -1415,7 +1425,8 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
/* Prevent device assign if mem paging or mem sharing have been
* enabled for this domain */
- if ( unlikely(!need_iommu(d) &&
+ if ( d != dom_io &&
+ unlikely(!need_iommu(d) &&
((is_hvm_domain(d) &&
d->arch.hvm_domain.mem_sharing_enabled) ||
vm_event_check_ring(d->vm_event_paging) ||
@@ -1432,12 +1443,20 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
return rc;
}
- pdev = pci_get_pdev_by_domain(hardware_domain, 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 != hardware_domain &&
+ pdev->domain != dom_io )
+ goto done;
if ( pdev->msix )
msixtbl_init(d);
@@ -1460,6 +1479,10 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
}
done:
+ /* The device is assigned to dom_io so mark it as quarantined */
+ if ( !rc && d == dom_io )
+ pdev->quarantine = true;
+
if ( !has_arch_pdevs(d) && need_iommu(d) )
iommu_teardown(d);
pcidevs_unlock();
@@ -1472,6 +1495,7 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
{
const struct domain_iommu *hd = dom_iommu(d);
struct pci_dev *pdev = NULL;
+ struct domain *target;
int ret = 0;
if ( !iommu_enabled || !hd->platform_ops )
@@ -1482,12 +1506,16 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
if ( !pdev )
return -ENODEV;
+ /* De-assignment from dom_io should de-quarantine the device */
+ target = (pdev->quarantine && pdev->domain != dom_io) ?
+ dom_io : hardware_domain;
+
while ( pdev->phantom_stride )
{
devfn += pdev->phantom_stride;
if ( PCI_SLOT(devfn) != PCI_SLOT(pdev->devfn) )
break;
- ret = hd->platform_ops->reassign_device(d, hardware_domain, devfn,
+ ret = hd->platform_ops->reassign_device(d, target, devfn,
pci_to_dev(pdev));
if ( !ret )
continue;
@@ -1498,7 +1526,7 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
}
devfn = pdev->devfn;
- ret = hd->platform_ops->reassign_device(d, hardware_domain, devfn,
+ ret = hd->platform_ops->reassign_device(d, target, devfn,
pci_to_dev(pdev));
if ( ret )
{
@@ -1508,6 +1536,9 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
return ret;
}
+ if ( pdev->domain == hardware_domain )
+ pdev->quarantine = false;
+
pdev->fault.count = 0;
if ( !has_arch_pdevs(d) && need_iommu(d) )
@@ -1686,7 +1717,7 @@ int iommu_do_pci_domctl(
ret = hypercall_create_continuation(__HYPERVISOR_domctl,
"h", u_domctl);
else 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);
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 4c719d4ee7..19f7d13013 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1338,6 +1338,10 @@ int domain_context_mapping_one(
int agaw, rc, ret;
bool_t flush_dev_iotlb;
+ /* dom_io is used as a sentinel for quarantined devices */
+ if ( domain == dom_io )
+ return 0;
+
ASSERT(pcidevs_locked());
spin_lock(&iommu->lock);
maddr = bus_to_context_maddr(iommu, bus);
@@ -1573,6 +1577,10 @@ int domain_context_unmap_one(
int iommu_domid, rc, ret;
bool_t flush_dev_iotlb;
+ /* dom_io is used as a sentinel for quarantined devices */
+ if ( domain == dom_io )
+ return 0;
+
ASSERT(pcidevs_locked());
spin_lock(&iommu->lock);
@@ -1705,6 +1713,10 @@ static int domain_context_unmap(struct domain *domain, u8 devfn,
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
@@ -2389,6 +2401,15 @@ static int reassign_device_ownership(
if ( ret )
return ret;
+ if ( devfn == pdev->devfn )
+ {
+ list_move(&pdev->domain_list, &dom_io->arch.pdev_list);
+ pdev->domain = dom_io;
+ }
+
+ if ( !has_arch_pdevs(source) )
+ vmx_pi_hooks_deassign(source);
+
if ( !has_arch_pdevs(target) )
vmx_pi_hooks_assign(target);
@@ -2407,15 +2428,13 @@ static int reassign_device_ownership(
pdev->domain = target;
}
- if ( !has_arch_pdevs(source) )
- vmx_pi_hooks_deassign(source);
-
return ret;
}
static int intel_iommu_assign_device(
struct domain *d, u8 devfn, struct pci_dev *pdev, u32 flag)
{
+ struct domain *s = pdev->domain;
struct acpi_rmrr_unit *rmrr;
int ret = 0, i;
u16 bdf, seg;
@@ -2458,8 +2477,8 @@ static int intel_iommu_assign_device(
}
}
- ret = reassign_device_ownership(hardware_domain, d, devfn, pdev);
- if ( ret )
+ ret = reassign_device_ownership(s, d, devfn, pdev);
+ if ( ret || d == dom_io )
return ret;
/* Setup rmrr identity mapping */
@@ -2472,11 +2491,20 @@ static int intel_iommu_assign_device(
ret = rmrr_identity_mapping(d, 1, rmrr, flag);
if ( ret )
{
- reassign_device_ownership(d, hardware_domain, devfn, pdev);
+ int rc;
+
+ rc = reassign_device_ownership(d, s, devfn, pdev);
printk(XENLOG_G_ERR VTDPREFIX
" cannot map reserved region (%"PRIx64",%"PRIx64"] for Dom%d (%d)\n",
rmrr->base_address, rmrr->end_address,
d->domain_id, ret);
+ if ( rc )
+ {
+ printk(XENLOG_ERR VTDPREFIX
+ " failed to reclaim %04x:%02x:%02x.%u from %pd (%d)\n",
+ seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), d, rc);
+ domain_crash(d);
+ }
break;
}
}
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 4cfa774615..066364bdef 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -88,6 +88,9 @@ struct pci_dev {
nodeid_t node; /* NUMA node */
+ /* Device to be quarantined, don't automatically re-assign to dom0 */
+ bool quarantine;
+
enum pdev_type {
DEV_TYPE_PCI_UNKNOWN,
DEV_TYPE_PCIe_ENDPOINT,
--
2.11.0
["xsa302-4.12/0001-IOMMU-add-missing-HVM-check.patch" (application/octet-stream)]
From 0c9c0fbb356e3210cb77b3d738be50981b26058a Mon Sep 17 00:00:00 2001
From: Jan Beulich <jbeulich@suse.com>
Date: Wed, 2 Oct 2019 13:36:59 +0200
Subject: [PATCH 1/2] IOMMU: add missing HVM check
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Fix an unguarded d->arch.hvm access in assign_device().
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
(cherry picked from commit 41fd1009cd7416b73d745a77c24b4e8d1a296fe6)
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
xen/drivers/passthrough/pci.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 8108ed5f9a..d7420bd8bf 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1452,7 +1452,8 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
/* Prevent device assign if mem paging or mem sharing have been
* enabled for this domain */
- if ( unlikely(d->arch.hvm.mem_sharing_enabled ||
+ if ( unlikely((is_hvm_domain(d) &&
+ d->arch.hvm.mem_sharing_enabled) ||
vm_event_check_ring(d->vm_event_paging) ||
p2m_get_hostp2m(d)->global_logdirty) )
return -EXDEV;
--
2.11.0
["xsa302-4.12/0002-passthrough-quarantine-PCI-devices.patch" (application/octet-stream)]
From 278d8e585a9f110a1af0bd92a9fc43733c9c7227 Mon Sep 17 00:00:00 2001
From: Paul Durrant <paul.durrant@citrix.com>
Date: Mon, 14 Oct 2019 17:52:59 +0100
Subject: [PATCH 2/2] 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>
(cherry picked from commit ec99857f59f7f06236f11ca8b0b2303e5e745cc4)
---
tools/libxl/libxl_pci.c | 25 +++++++++++-
xen/arch/x86/mm.c | 2 +
xen/common/domctl.c | 14 ++++++-
xen/drivers/passthrough/amd/pci_amd_iommu.c | 10 ++++-
xen/drivers/passthrough/iommu.c | 9 +++++
xen/drivers/passthrough/pci.c | 59 ++++++++++++++++++++++-------
xen/drivers/passthrough/vtd/iommu.c | 40 ++++++++++++++++---
xen/include/xen/pci.h | 3 ++
8 files changed, 138 insertions(+), 24 deletions(-)
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 88c324ea23..d6a23fb5f8 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -754,6 +754,7 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
libxl_device_pci *pcidev,
int rebind)
{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
unsigned dom, bus, dev, func;
char *spath, *driver_path = NULL;
int rc;
@@ -779,7 +780,7 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
}
if ( rc ) {
LOG(WARN, 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 */
@@ -810,6 +811,19 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
return ERROR_FAIL;
}
+quarantine:
+ /*
+ * DOMID_IO is just a sentinel domain, without any actual mappings,
+ * so always pass XEN_DOMCTL_DEV_RDM_RELAXED to avoid assignment being
+ * unnecessarily denied.
+ */
+ rc = xc_assign_device(ctx->xch, DOMID_IO, pcidev_encode_bdf(pcidev),
+ XEN_DOMCTL_DEV_RDM_RELAXED);
+ if ( rc < 0 ) {
+ LOG(ERROR, "failed to quarantine "PCI_BDF, dom, bus, dev, func);
+ return ERROR_FAIL;
+ }
+
return 0;
}
@@ -817,9 +831,18 @@ static int libxl__device_pci_assignable_remove(libxl__gc *gc,
libxl_device_pci *pcidev,
int rebind)
{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
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 ) {
return ERROR_FAIL;
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 3557cd1178..11d753d8d2 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -295,9 +295,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, NULL, false);
BUG_ON(IS_ERR(dom_io));
+ INIT_LIST_HEAD(&dom_io->arch.pdev_list);
/*
* Initialise our COW domain.
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index d08b6274e2..e3c4be2b48 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -391,6 +391,16 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
switch ( op->cmd )
{
+ case XEN_DOMCTL_assign_device:
+ case XEN_DOMCTL_deassign_device:
+ if ( op->domain == DOMID_IO )
+ {
+ d = dom_io;
+ break;
+ }
+ else if ( op->domain == DOMID_INVALID )
+ return -ESRCH;
+ /* fall through */
case XEN_DOMCTL_test_assign_device:
if ( op->domain == DOMID_INVALID )
{
@@ -412,7 +422,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
if ( !domctl_lock_acquire() )
{
- if ( d )
+ if ( d && d != dom_io )
rcu_unlock_domain(d);
return hypercall_create_continuation(
__HYPERVISOR_domctl, "h", u_domctl);
@@ -1074,7 +1084,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
domctl_lock_release();
domctl_out_unlock_domonly:
- if ( d )
+ if ( d && d != dom_io )
rcu_unlock_domain(d);
if ( copyback && __copy_to_guest(u_domctl, op, 1) )
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 33a3798f36..15c13e1163 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -120,6 +120,10 @@ static void amd_iommu_setup_domain_device(
u8 bus = pdev->bus;
const struct domain_iommu *hd = dom_iommu(domain);
+ /* dom_io is used as a sentinel for quarantined devices */
+ if ( domain == dom_io )
+ return;
+
BUG_ON( !hd->arch.root_table || !hd->arch.paging_mode ||
!iommu->dev_table.buffer );
@@ -277,6 +281,10 @@ void amd_iommu_disable_domain_device(struct domain *domain,
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);
@@ -363,7 +371,7 @@ static int amd_iommu_assign_device(struct domain *d, u8 devfn,
ivrs_mappings[req_id].read_permission);
}
- return reassign_device(hardware_domain, d, devfn, pdev);
+ return reassign_device(pdev->domain, d, devfn, pdev);
}
static void deallocate_next_page_table(struct page_info *pg, int level)
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index a6697d58fb..2762e1342f 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -232,6 +232,9 @@ void iommu_teardown(struct domain *d)
{
struct domain_iommu *hd = dom_iommu(d);
+ if ( d == dom_io )
+ return;
+
hd->status = IOMMU_STATUS_disabled;
hd->platform_ops->teardown(d);
tasklet_schedule(&iommu_pt_cleanup_tasklet);
@@ -241,6 +244,9 @@ int iommu_construct(struct domain *d)
{
struct domain_iommu *hd = dom_iommu(d);
+ if ( d == dom_io )
+ return 0;
+
if ( hd->status == IOMMU_STATUS_initialized )
return 0;
@@ -521,6 +527,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_hwdom_passthrough ? "Passthrough" :
iommu_hwdom_strict ? "Strict" : "Relaxed");
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index d7420bd8bf..d66a8a1daf 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1426,19 +1426,29 @@ static int iommu_remove_device(struct pci_dev *pdev)
return hd->platform_ops->remove_device(pdev->devfn, pci_to_dev(pdev));
}
-/*
- * If the device isn't owned by the hardware domain, 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;
pcidevs_lock();
- pdev = pci_get_pdev_by_domain(hardware_domain, 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 != hardware_domain &&
+ pdev->domain != dom_io )
+ rc = -EBUSY;
+
pcidevs_unlock();
- return pdev ? 0 : -EBUSY;
+ return rc;
}
static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
@@ -1452,7 +1462,8 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
/* Prevent device assign if mem paging or mem sharing have been
* enabled for this domain */
- if ( unlikely((is_hvm_domain(d) &&
+ if ( d != dom_io &&
+ unlikely((is_hvm_domain(d) &&
d->arch.hvm.mem_sharing_enabled) ||
vm_event_check_ring(d->vm_event_paging) ||
p2m_get_hostp2m(d)->global_logdirty) )
@@ -1468,12 +1479,20 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
return rc;
}
- pdev = pci_get_pdev_by_domain(hardware_domain, 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 != hardware_domain &&
+ pdev->domain != dom_io )
+ goto done;
if ( pdev->msix )
msixtbl_init(d);
@@ -1496,6 +1515,10 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
}
done:
+ /* The device is assigned to dom_io so mark it as quarantined */
+ if ( !rc && d == dom_io )
+ pdev->quarantine = true;
+
if ( !has_arch_pdevs(d) && has_iommu_pt(d) )
iommu_teardown(d);
pcidevs_unlock();
@@ -1508,6 +1531,7 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
{
const struct domain_iommu *hd = dom_iommu(d);
struct pci_dev *pdev = NULL;
+ struct domain *target;
int ret = 0;
if ( !iommu_enabled || !hd->platform_ops )
@@ -1518,12 +1542,16 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
if ( !pdev )
return -ENODEV;
+ /* De-assignment from dom_io should de-quarantine the device */
+ target = (pdev->quarantine && pdev->domain != dom_io) ?
+ dom_io : hardware_domain;
+
while ( pdev->phantom_stride )
{
devfn += pdev->phantom_stride;
if ( PCI_SLOT(devfn) != PCI_SLOT(pdev->devfn) )
break;
- ret = hd->platform_ops->reassign_device(d, hardware_domain, devfn,
+ ret = hd->platform_ops->reassign_device(d, target, devfn,
pci_to_dev(pdev));
if ( !ret )
continue;
@@ -1534,7 +1562,7 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
}
devfn = pdev->devfn;
- ret = hd->platform_ops->reassign_device(d, hardware_domain, devfn,
+ ret = hd->platform_ops->reassign_device(d, target, devfn,
pci_to_dev(pdev));
if ( ret )
{
@@ -1544,6 +1572,9 @@ int deassign_device(struct domain *d, u16 seg, u8 bus, u8 devfn)
return ret;
}
+ if ( pdev->domain == hardware_domain )
+ pdev->quarantine = false;
+
pdev->fault.count = 0;
if ( !has_arch_pdevs(d) && has_iommu_pt(d) )
@@ -1722,7 +1753,7 @@ int iommu_do_pci_domctl(
ret = hypercall_create_continuation(__HYPERVISOR_domctl,
"h", u_domctl);
else 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);
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 1db1cd9f2d..a8d1baa064 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1338,6 +1338,10 @@ int domain_context_mapping_one(
int agaw, rc, ret;
bool_t flush_dev_iotlb;
+ /* dom_io is used as a sentinel for quarantined devices */
+ if ( domain == dom_io )
+ return 0;
+
ASSERT(pcidevs_locked());
spin_lock(&iommu->lock);
maddr = bus_to_context_maddr(iommu, bus);
@@ -1573,6 +1577,10 @@ int domain_context_unmap_one(
int iommu_domid, rc, ret;
bool_t flush_dev_iotlb;
+ /* dom_io is used as a sentinel for quarantined devices */
+ if ( domain == dom_io )
+ return 0;
+
ASSERT(pcidevs_locked());
spin_lock(&iommu->lock);
@@ -1705,6 +1713,10 @@ static int domain_context_unmap(struct domain *domain, u8 devfn,
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
@@ -2441,6 +2453,15 @@ static int reassign_device_ownership(
if ( ret )
return ret;
+ if ( devfn == pdev->devfn )
+ {
+ list_move(&pdev->domain_list, &dom_io->arch.pdev_list);
+ pdev->domain = dom_io;
+ }
+
+ if ( !has_arch_pdevs(source) )
+ vmx_pi_hooks_deassign(source);
+
if ( !has_arch_pdevs(target) )
vmx_pi_hooks_assign(target);
@@ -2459,15 +2480,13 @@ static int reassign_device_ownership(
pdev->domain = target;
}
- if ( !has_arch_pdevs(source) )
- vmx_pi_hooks_deassign(source);
-
return ret;
}
static int intel_iommu_assign_device(
struct domain *d, u8 devfn, struct pci_dev *pdev, u32 flag)
{
+ struct domain *s = pdev->domain;
struct acpi_rmrr_unit *rmrr;
int ret = 0, i;
u16 bdf, seg;
@@ -2510,8 +2529,8 @@ static int intel_iommu_assign_device(
}
}
- ret = reassign_device_ownership(hardware_domain, d, devfn, pdev);
- if ( ret )
+ ret = reassign_device_ownership(s, d, devfn, pdev);
+ if ( ret || d == dom_io )
return ret;
/* Setup rmrr identity mapping */
@@ -2524,11 +2543,20 @@ static int intel_iommu_assign_device(
ret = rmrr_identity_mapping(d, 1, rmrr, flag);
if ( ret )
{
- reassign_device_ownership(d, hardware_domain, devfn, pdev);
+ int rc;
+
+ rc = reassign_device_ownership(d, s, devfn, pdev);
printk(XENLOG_G_ERR VTDPREFIX
" cannot map reserved region (%"PRIx64",%"PRIx64"] for Dom%d (%d)\n",
rmrr->base_address, rmrr->end_address,
d->domain_id, ret);
+ if ( rc )
+ {
+ printk(XENLOG_ERR VTDPREFIX
+ " failed to reclaim %04x:%02x:%02x.%u from %pd (%d)\n",
+ seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), d, rc);
+ domain_crash(d);
+ }
break;
}
}
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 8b21e8dc84..a031fd6020 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -88,6 +88,9 @@ struct pci_dev {
nodeid_t node; /* NUMA node */
+ /* Device to be quarantined, don't automatically re-assign to dom0 */
+ bool quarantine;
+
/* Device with errata, ignore the BARs. */
bool ignore_bars;
--
2.11.0
["xsa302/0001-passthrough-quarantine-PCI-devices.patch" (application/octet-stream)]
From 8a9fec53f174022adc88f25372c57b2eb63fa538 Mon Sep 17 00:00:00 2001
From: Paul Durrant <paul@xen.org>
Date: Fri, 18 Oct 2019 17:41:44 +0100
Subject: [PATCH] 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>
---
tools/libxl/libxl_pci.c | 25 +++++++++++-
xen/common/domain.c | 1 +
xen/common/domctl.c | 14 ++++++-
xen/drivers/passthrough/amd/pci_amd_iommu.c | 10 ++++-
xen/drivers/passthrough/iommu.c | 6 ++-
xen/drivers/passthrough/pci.c | 59 ++++++++++++++++++++++-------
xen/drivers/passthrough/vtd/iommu.c | 40 ++++++++++++++++---
xen/include/xen/pci.h | 3 ++
8 files changed, 133 insertions(+), 25 deletions(-)
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index b5444d1552..2ccab033b4 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -766,6 +766,7 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
libxl_device_pci *pcidev,
int rebind)
{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
unsigned dom, bus, dev, func;
char *spath, *driver_path = NULL;
int rc;
@@ -791,7 +792,7 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
}
if ( rc ) {
LOG(WARN, 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 */
@@ -822,6 +823,19 @@ static int libxl__device_pci_assignable_add(libxl__gc *gc,
return ERROR_FAIL;
}
+quarantine:
+ /*
+ * DOMID_IO is just a sentinel domain, without any actual mappings,
+ * so always pass XEN_DOMCTL_DEV_RDM_RELAXED to avoid assignment being
+ * unnecessarily denied.
+ */
+ rc = xc_assign_device(ctx->xch, DOMID_IO, pcidev_encode_bdf(pcidev),
+ XEN_DOMCTL_DEV_RDM_RELAXED);
+ if ( rc < 0 ) {
+ LOG(ERROR, "failed to quarantine "PCI_BDF, dom, bus, dev, func);
+ return ERROR_FAIL;
+ }
+
return 0;
}
@@ -829,9 +843,18 @@ static int libxl__device_pci_assignable_remove(libxl__gc *gc,
libxl_device_pci *pcidev,
int rebind)
{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
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 ) {
return ERROR_FAIL;
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 9c7360ed2a..fc2423a5d2 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -548,6 +548,7 @@ void __init setup_system_domains(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, NULL, false);
if ( IS_ERR(dom_io) )
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index d597a09f98..03d0226039 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -383,6 +383,16 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
switch ( op->cmd )
{
+ case XEN_DOMCTL_assign_device:
+ case XEN_DOMCTL_deassign_device:
+ if ( op->domain == DOMID_IO )
+ {
+ d = dom_io;
+ break;
+ }
+ else if ( op->domain == DOMID_INVALID )
+ return -ESRCH;
+ /* fall through */
case XEN_DOMCTL_test_assign_device:
case XEN_DOMCTL_vm_event_op:
if ( op->domain == DOMID_INVALID )
@@ -405,7 +415,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
if ( !domctl_lock_acquire() )
{
- if ( d )
+ if ( d && d != dom_io )
rcu_unlock_domain(d);
return hypercall_create_continuation(
__HYPERVISOR_domctl, "h", u_domctl);
@@ -1064,7 +1074,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
domctl_lock_release();
domctl_out_unlock_domonly:
- if ( d )
+ if ( d && d != dom_io )
rcu_unlock_domain(d);
if ( copyback && __copy_to_guest(u_domctl, op, 1) )
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 053e7355c4..b2046245d7 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -95,6 +95,10 @@ static void amd_iommu_setup_domain_device(
u8 bus = pdev->bus;
const struct domain_iommu *hd = dom_iommu(domain);
+ /* dom_io is used as a sentinel for quarantined devices */
+ if ( domain == dom_io )
+ return;
+
BUG_ON( !hd->arch.root_table || !hd->arch.paging_mode ||
!iommu->dev_table.buffer );
@@ -276,6 +280,10 @@ static void amd_iommu_disable_domain_device(const struct domain *domain,
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));
table = iommu->dev_table.buffer;
@@ -373,7 +381,7 @@ static int amd_iommu_assign_device(struct domain *d, u8 devfn,
ivrs_mappings[req_id].read_permission);
}
- return reassign_device(hardware_domain, d, devfn, pdev);
+ return reassign_device(pdev->domain, d, devfn, pdev);
}
static void deallocate_next_page_table(struct page_info *pg, int level)
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 9ef5f376c5..8cbe908fff 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -181,7 +181,7 @@ int iommu_domain_init(struct domain *d, unsigned int opts)
hd->platform_ops = iommu_get_ops();
ret = hd->platform_ops->init(d);
- if ( ret )
+ if ( ret || is_system_domain(d) )
return ret;
if ( is_hardware_domain(d) )
@@ -473,6 +473,10 @@ int __init iommu_setup(void)
}
else
{
+ dom_io->options |= XEN_DOMCTL_CDF_iommu;
+ if ( iommu_domain_init(dom_io, 0) )
+ panic("Could not set up quarantine\n");
+
printk(" - Dom0 mode: %s\n",
iommu_hwdom_passthrough ? "Passthrough" :
iommu_hwdom_strict ? "Strict" : "Relaxed");
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index bdcc482d81..18a7dc7224 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -910,6 +910,7 @@ static int deassign_device(struct domain *d, uint16_t seg, uint8_t bus,
{
const struct domain_iommu *hd = dom_iommu(d);
struct pci_dev *pdev;
+ struct domain *target;
int ret = 0;
if ( !is_iommu_enabled(d) )
@@ -920,12 +921,16 @@ static int deassign_device(struct domain *d, uint16_t seg, uint8_t bus,
if ( !pdev )
return -ENODEV;
+ /* De-assignment from dom_io should de-quarantine the device */
+ target = (pdev->quarantine && pdev->domain != dom_io) ?
+ dom_io : hardware_domain;
+
while ( pdev->phantom_stride )
{
devfn += pdev->phantom_stride;
if ( PCI_SLOT(devfn) != PCI_SLOT(pdev->devfn) )
break;
- ret = hd->platform_ops->reassign_device(d, hardware_domain, devfn,
+ ret = hd->platform_ops->reassign_device(d, target, devfn,
pci_to_dev(pdev));
if ( !ret )
continue;
@@ -936,7 +941,7 @@ static int deassign_device(struct domain *d, uint16_t seg, uint8_t bus,
}
devfn = pdev->devfn;
- ret = hd->platform_ops->reassign_device(d, hardware_domain, devfn,
+ ret = hd->platform_ops->reassign_device(d, target, devfn,
pci_to_dev(pdev));
if ( ret )
{
@@ -946,6 +951,9 @@ static int deassign_device(struct domain *d, uint16_t seg, uint8_t bus,
return ret;
}
+ if ( pdev->domain == hardware_domain )
+ pdev->quarantine = false;
+
pdev->fault.count = 0;
return ret;
@@ -1462,19 +1470,29 @@ static int iommu_remove_device(struct pci_dev *pdev)
return hd->platform_ops->remove_device(pdev->devfn, pci_to_dev(pdev));
}
-/*
- * If the device isn't owned by the hardware domain, 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;
pcidevs_lock();
- pdev = pci_get_pdev_by_domain(hardware_domain, 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 != hardware_domain &&
+ pdev->domain != dom_io )
+ rc = -EBUSY;
+
pcidevs_unlock();
- return pdev ? 0 : -EBUSY;
+ return rc;
}
static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
@@ -1488,7 +1506,8 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
/* Prevent device assign if mem paging or mem sharing have been
* enabled for this domain */
- if ( unlikely((is_hvm_domain(d) &&
+ if ( d != dom_io &&
+ unlikely((is_hvm_domain(d) &&
d->arch.hvm.mem_sharing_enabled) ||
vm_event_check_ring(d->vm_event_paging) ||
p2m_get_hostp2m(d)->global_logdirty) )
@@ -1497,12 +1516,20 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
if ( !pcidevs_trylock() )
return -ERESTART;
- pdev = pci_get_pdev_by_domain(hardware_domain, 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 != hardware_domain &&
+ pdev->domain != dom_io )
+ goto done;
if ( pdev->msix )
{
@@ -1530,6 +1557,10 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
}
done:
+ /* The device is assigned to dom_io so mark it as quarantined */
+ if ( !rc && d == dom_io )
+ pdev->quarantine = true;
+
pcidevs_unlock();
return rc;
@@ -1705,7 +1736,7 @@ int iommu_do_pci_domctl(
ret = hypercall_create_continuation(__HYPERVISOR_domctl,
"h", u_domctl);
else 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);
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index f08eec070d..53493efccc 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1290,6 +1290,10 @@ int domain_context_mapping_one(
int agaw, rc, ret;
bool_t flush_dev_iotlb;
+ /* dom_io is used as a sentinel for quarantined devices */
+ if ( domain == dom_io )
+ return 0;
+
ASSERT(pcidevs_locked());
spin_lock(&iommu->lock);
maddr = bus_to_context_maddr(iommu, bus);
@@ -1536,6 +1540,10 @@ int domain_context_unmap_one(
int iommu_domid, rc, ret;
bool_t flush_dev_iotlb;
+ /* dom_io is used as a sentinel for quarantined devices */
+ if ( domain == dom_io )
+ return 0;
+
ASSERT(pcidevs_locked());
spin_lock(&iommu->lock);
@@ -1668,6 +1676,10 @@ static int domain_context_unmap(struct domain *domain, u8 devfn,
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
@@ -2394,6 +2406,15 @@ static int reassign_device_ownership(
if ( ret )
return ret;
+ if ( devfn == pdev->devfn )
+ {
+ list_move(&pdev->domain_list, &dom_io->pdev_list);
+ pdev->domain = dom_io;
+ }
+
+ if ( !has_arch_pdevs(source) )
+ vmx_pi_hooks_deassign(source);
+
if ( !has_arch_pdevs(target) )
vmx_pi_hooks_assign(target);
@@ -2412,15 +2433,13 @@ static int reassign_device_ownership(
pdev->domain = target;
}
- if ( !has_arch_pdevs(source) )
- vmx_pi_hooks_deassign(source);
-
return ret;
}
static int intel_iommu_assign_device(
struct domain *d, u8 devfn, struct pci_dev *pdev, u32 flag)
{
+ struct domain *s = pdev->domain;
struct acpi_rmrr_unit *rmrr;
int ret = 0, i;
u16 bdf, seg;
@@ -2463,8 +2482,8 @@ static int intel_iommu_assign_device(
}
}
- ret = reassign_device_ownership(hardware_domain, d, devfn, pdev);
- if ( ret )
+ ret = reassign_device_ownership(s, d, devfn, pdev);
+ if ( ret || d == dom_io )
return ret;
/* Setup rmrr identity mapping */
@@ -2477,11 +2496,20 @@ static int intel_iommu_assign_device(
ret = rmrr_identity_mapping(d, 1, rmrr, flag);
if ( ret )
{
- reassign_device_ownership(d, hardware_domain, devfn, pdev);
+ int rc;
+
+ rc = reassign_device_ownership(d, s, devfn, pdev);
printk(XENLOG_G_ERR VTDPREFIX
" cannot map reserved region (%"PRIx64",%"PRIx64"] for Dom%d (%d)\n",
rmrr->base_address, rmrr->end_address,
d->domain_id, ret);
+ if ( rc )
+ {
+ printk(XENLOG_ERR VTDPREFIX
+ " failed to reclaim %04x:%02x:%02x.%u from %pd (%d)\n",
+ seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), d, rc);
+ domain_crash(d);
+ }
break;
}
}
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 393cb45de3..2bc4aaf453 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -99,6 +99,9 @@ struct pci_dev {
nodeid_t node; /* NUMA node */
+ /* Device to be quarantined, don't automatically re-assign to dom0 */
+ bool quarantine;
+
/* Device with errata, ignore the BARs. */
bool ignore_bars;
--
2.11.0
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic