Cui, Dexuan
2009-Oct-26 08:33 UTC
[Xen-devel] RE: [Xen-changelog] [xen-unstable] passthrough/stubdom: clean up hypercall privilege checking
Hi Stefano,
Looks after your patchset to enable passthrough for stubdomain was checked in,
guest hotplug (at least for hvm guest in the non-stubdomain case) is broken,
e.g., multiple times of "xm pci-attach/pci-detach" would fail. Can
you try that?
Thanks,
-- Dexuan
-----Original Message-----
From: xen-changelog-bounces@lists.xensource.com
[mailto:xen-changelog-bounces@lists.xensource.com] On Behalf Of Xen
patchbot-unstable
Sent: 2009?10?23? 17:40
To: xen-changelog@lists.xensource.com
Subject: [Xen-changelog] [xen-unstable] passthrough/stubdom: clean up hypercall
privilege checking
# HG changeset patch
# User Keir Fraser <keir.fraser@citrix.com>
# Date 1256288643 -3600
# Node ID ecc649ec367598e63e256a8c6515242ff5aa9afd
# Parent c7381d04e2b3882a1cd5d0fc6abb13786c88b9ee
passthrough/stubdom: clean up hypercall privilege checking
This patch adds securty checks for pci passthrough related hypercalls
to enforce that the current domain owns the resources that it is about
to remap. It also adds a call to xc_assign_device to xend and removes
the PRIVILEGED_STUBDOMS flags.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
Config.mk | 6 +++---
tools/python/xen/xend/server/pciif.py | 10 +++++++++-
xen/Rules.mk | 4 ----
xen/arch/x86/domctl.c | 21 +++++++++++++++++++++
xen/arch/x86/irq.c | 4 +++-
xen/arch/x86/physdev.c | 4 ++--
xen/common/domctl.c | 6 ++----
xen/include/xen/config.h | 6 ------
8 files changed, 40 insertions(+), 21 deletions(-)
diff -r c7381d04e2b3 -r ecc649ec3675 Config.mk
--- a/Config.mk Fri Oct 23 10:02:09 2009 +0100
+++ b/Config.mk Fri Oct 23 10:04:03 2009 +0100
@@ -150,9 +150,9 @@ QEMU_REMOTE=http://xenbits.xensource.com
# CONFIG_QEMU ?= ../qemu-xen.git
CONFIG_QEMU ?= $(QEMU_REMOTE)
-QEMU_TAG ?= a3285ff385d2568f0226f15fee2b9808ec3b6deb
-# Tue Oct 20 15:16:34 2009 +0100
-# usb hotplug in qemu-dm via xm
+QEMU_TAG ?= b4bb8b3f09d1c873f522f6aebe1f125a6d1854d0
+# Wed Oct 21 16:42:15 2009 +0100
+# passthrough: fix security issue with stubdoms
OCAML_XENSTORED_REPO=http://xenbits.xensource.com/ext/xen-ocaml-tools.hg
diff -r c7381d04e2b3 -r ecc649ec3675 tools/python/xen/xend/server/pciif.py
--- a/tools/python/xen/xend/server/pciif.py Fri Oct 23 10:02:09 2009 +0100
+++ b/tools/python/xen/xend/server/pciif.py Fri Oct 23 10:04:03 2009 +0100
@@ -444,7 +444,15 @@ class PciController(DevController):
# For hvm guest, (from c/s 19679 on) assigning device statically and
# dynamically both go through reconfigureDevice(), so HERE the
# setupOneDevice() is not necessary.
- if not self.vm.info.is_hvm():
+ if self.vm.info.is_hvm():
+ for pci_dev in pci_dev_list:
+ # Setup IOMMU device assignment
+ bdf = xc.assign_device(self.getDomid(),
pci_dict_to_xc_str(pci_dev))
+ pci_str = pci_dict_to_bdf_str(pci_dev)
+ if bdf > 0:
+ raise VmError("Failed to assign device to IOMMU
(%s)" % pci_str)
+ log.debug("pci: assign device %s" % pci_str)
+ else :
for d in pci_dev_list:
self.setupOneDevice(d)
wPath = ''/local/domain/0/backend/pci/%u/0/aerState'' %
(self.getDomid())
diff -r c7381d04e2b3 -r ecc649ec3675 xen/Rules.mk
--- a/xen/Rules.mk Fri Oct 23 10:02:09 2009 +0100
+++ b/xen/Rules.mk Fri Oct 23 10:04:03 2009 +0100
@@ -10,9 +10,6 @@ crash_debug ?= n
crash_debug ?= n
gdbsx ?= n
frame_pointer ?= n
-
-# Allow some delicate passthrough related hypercalls to be made from a stubdom
-privileged_stubdoms ?= y
XEN_ROOT=$(BASEDIR)/..
include $(XEN_ROOT)/Config.mk
@@ -56,7 +53,6 @@ CFLAGS-$(perfc_arrays) += -DPERF_ARRAYS
CFLAGS-$(perfc_arrays) += -DPERF_ARRAYS
CFLAGS-$(lock_profile) += -DLOCK_PROFILE
CFLAGS-$(frame_pointer) += -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER
-CFLAGS-$(privileged_stubdoms) += -DPRIVILEGED_STUBDOMS
CFLAGS-$(gdbsx) += -DXEN_GDBSX_CONFIG
ifneq ($(max_phys_cpus),)
diff -r c7381d04e2b3 -r ecc649ec3675 xen/arch/x86/domctl.c
--- a/xen/arch/x86/domctl.c Fri Oct 23 10:02:09 2009 +0100
+++ b/xen/arch/x86/domctl.c Fri Oct 23 10:04:03 2009 +0100
@@ -796,6 +796,11 @@ long arch_do_domctl(
if ( ret )
goto bind_out;
+ ret = -EPERM;
+ if ( !IS_PRIV(current->domain) &&
+ !irq_access_permitted(current->domain, bind->machine_irq) )
+ goto bind_out;
+
ret = -ESRCH;
if ( iommu_enabled )
{
@@ -820,6 +825,12 @@ long arch_do_domctl(
if ( (d = rcu_lock_domain_by_id(domctl->domain)) == NULL )
break;
bind = &(domctl->u.bind_pt_irq);
+
+ ret = -EPERM;
+ if ( !IS_PRIV(current->domain) &&
+ !irq_access_permitted(current->domain, bind->machine_irq) )
+ goto bind_out;
+
if ( iommu_enabled )
{
spin_lock(&pcidevs_lock);
@@ -846,6 +857,11 @@ long arch_do_domctl(
ret = -ESRCH;
if ( unlikely((d = rcu_lock_domain_by_id(domctl->domain)) == NULL) )
+ break;
+
+ ret = -EPERM;
+ if ( !IS_PRIV(current->domain) &&
+ !iomem_access_permitted(current->domain, mfn, mfn + nr_mfns -
1) )
break;
ret=0;
@@ -894,6 +910,11 @@ long arch_do_domctl(
fgp, fmp, np);
break;
}
+
+ ret = -EPERM;
+ if ( !IS_PRIV(current->domain) &&
+ !ioports_access_permitted(current->domain, fmp, fmp + np - 1) )
+ break;
ret = -ESRCH;
if ( unlikely((d = rcu_lock_domain_by_id(domctl->domain)) == NULL) )
diff -r c7381d04e2b3 -r ecc649ec3675 xen/arch/x86/irq.c
--- a/xen/arch/x86/irq.c Fri Oct 23 10:02:09 2009 +0100
+++ b/xen/arch/x86/irq.c Fri Oct 23 10:04:03 2009 +0100
@@ -1343,7 +1343,9 @@ int map_domain_pirq(
ASSERT(spin_is_locked(&pcidevs_lock));
ASSERT(spin_is_locked(&d->event_lock));
- if ( !STUBDOM_IS_PRIV_FOR(current->domain, d) )
+ if ( !IS_PRIV(current->domain) &&
+ !(IS_PRIV_FOR(current->domain, d) &&
+ irq_access_permitted(current->domain, pirq)))
return -EPERM;
if ( pirq < 0 || pirq >= d->nr_pirqs || irq < 0 || irq >=
nr_irqs )
diff -r c7381d04e2b3 -r ecc649ec3675 xen/arch/x86/physdev.c
--- a/xen/arch/x86/physdev.c Fri Oct 23 10:02:09 2009 +0100
+++ b/xen/arch/x86/physdev.c Fri Oct 23 10:04:03 2009 +0100
@@ -45,7 +45,7 @@ static int physdev_map_pirq(struct physd
if ( d == NULL )
return -ESRCH;
- if ( !STUBDOM_IS_PRIV_FOR(current->domain, d) )
+ if ( !IS_PRIV_FOR(current->domain, d) )
{
ret = -EPERM;
goto free_domain;
@@ -169,7 +169,7 @@ static int physdev_unmap_pirq(struct phy
return -ESRCH;
ret = -EPERM;
- if ( !STUBDOM_IS_PRIV_FOR(current->domain, d) )
+ if ( !IS_PRIV_FOR(current->domain, d) )
goto free_domain;
spin_lock(&pcidevs_lock);
diff -r c7381d04e2b3 -r ecc649ec3675 xen/common/domctl.c
--- a/xen/common/domctl.c Fri Oct 23 10:02:09 2009 +0100
+++ b/xen/common/domctl.c Fri Oct 23 10:04:03 2009 +0100
@@ -231,14 +231,12 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
case XEN_DOMCTL_ioport_mapping:
case XEN_DOMCTL_memory_mapping:
case XEN_DOMCTL_bind_pt_irq:
- case XEN_DOMCTL_unbind_pt_irq:
- case XEN_DOMCTL_assign_device:
- case XEN_DOMCTL_deassign_device: {
+ case XEN_DOMCTL_unbind_pt_irq: {
struct domain *d;
bool_t is_priv = IS_PRIV(current->domain);
if ( !is_priv && ((d = rcu_lock_domain_by_id(op->domain)) !=
NULL) )
{
- is_priv = STUBDOM_IS_PRIV_FOR(current->domain, d);
+ is_priv = IS_PRIV_FOR(current->domain, d);
rcu_unlock_domain(d);
}
if ( !is_priv )
diff -r c7381d04e2b3 -r ecc649ec3675 xen/include/xen/config.h
--- a/xen/include/xen/config.h Fri Oct 23 10:02:09 2009 +0100
+++ b/xen/include/xen/config.h Fri Oct 23 10:04:03 2009 +0100
@@ -95,10 +95,4 @@ int current_domain_id(void);
#define __cpuinitdata
#define __cpuinit
-#ifdef PRIVILEGED_STUBDOMS
-#define STUBDOM_IS_PRIV_FOR(x,y) IS_PRIV_FOR(x,y)
-#else
-#define STUBDOM_IS_PRIV_FOR(x,y) IS_PRIV(x)
-#endif
-
#endif /* __XEN_CONFIG_H__ */
_______________________________________________
Xen-changelog mailing list
Xen-changelog@lists.xensource.com
http://lists.xensource.com/xen-changelog
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Stefano Stabellini
2009-Oct-26 11:49 UTC
[Xen-devel] RE: [Xen-changelog] [xen-unstable] passthrough/stubdom: clean up hypercall privilege checking
On Mon, 26 Oct 2009, Cui, Dexuan wrote:> Hi Stefano, > Looks after your patchset to enable passthrough for stubdomain was checked in, guest hotplug (at least for hvm guest in the non-stubdomain case) is broken, e.g., multiple times of "xm pci-attach/pci-detach" would fail. Can you try that? >I tried several times and I only had problems with one particular device for which the find_parent() function returns None therefore breaking the following code in find_all_the_multi_functions(). This patch fixes it. However I doubt that this bug is related to the problem you are seeing, it would probably help if you could post xend and qemu logs. --- diff -r 8ca4e32583b6 tools/python/xen/util/pci.py --- a/tools/python/xen/util/pci.py Fri Oct 23 10:15:17 2009 +0100 +++ b/tools/python/xen/util/pci.py Mon Oct 26 11:46:38 2009 +0000 @@ -830,7 +830,10 @@ def find_all_the_multi_functions(self): sysfs_mnt = find_sysfs_mnt() - parent = pci_dict_to_bdf_str(self.find_parent()) + parentdict = self.find_parent() + if parentdict is None : + return [ self.name ] + parent = pci_dict_to_bdf_str(parentdict) pci_names = os.popen(''ls '' + sysfs_mnt + SYSFS_PCI_DEVS_PATH + ''/'' + \ parent + ''/'').read() funcs = extract_the_exact_pci_names(pci_names) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel