Re: [Xen-devel] [PATCH 3 of 7] xen: allows more hypercalls from stubdoms

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 13 Oct 2009, Keir Fraser wrote:
> On 13/10/2009 13:00, "Stefano Stabellini" <Stefano.Stabellini@xxxxxxxxxxxxx>
> wrote:
> 
> >> This kind of thing, for example, while we're talking about least
> >> privilege... I think it's wrong-headed in the first place for this kind of
> >> control-plane activity to be going on in qemu. Surely it belongs in the
> >> toolstack? Yes, I know it's a pain in the bum that this means modifying
> >> multiple toolstacks! :-)
> >> 
> > 
> > I agree with you that we need to redesign these hypercalls, but I am a
> > fan of "doing one thing at a time" so I think we should decouple this
> > goal from the other one of making passthrough work with stubdom for the
> > moment.
> > This way we could first let people test it as it is, fix some bugs that
> > probably still affect the code, fix pci coldplug and add MSI-X support,
> > then redesign the hypercalls.
> 
> Perhaps acceptable then if the changes are placed in clear ifdef'ed regions.
> This ifdef would be default-disabled for a stable release, if the hypercalls
> are not redone by then.
> 


This is the updated version of the patch, with all the controversial
changes ifdef'ed.


Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>

---

diff -r 0035117b3a88 Config.mk
--- a/Config.mk Tue Oct 13 14:38:45 2009 +0100
+++ b/Config.mk Tue Oct 13 15:23:05 2009 +0100
@@ -2,6 +2,10 @@
 
 # A debug build of Xen and tools?
 debug ?= y
+
+# Allow some delicate passthrough related hypercalls to be made from a
+# stubdom
+privileged_stubdoms ?= y
 
 XEN_COMPILE_ARCH    ?= $(shell uname -m | sed -e s/i.86/x86_32/ \
                          -e s/i86pc/x86_32/ -e s/amd64/x86_64/)
@@ -114,6 +118,10 @@
 CFLAGS += -g
 endif
 
+ifeq ($(privileged_stubdoms),y)
+CFLAGS += -DPRIVILEGED_STUBDOMS
+endif
+
 CFLAGS += -fno-strict-aliasing
 
 CFLAGS += -std=gnu99
diff -r 0035117b3a88 xen/arch/x86/irq.c
--- a/xen/arch/x86/irq.c        Tue Oct 13 14:38:45 2009 +0100
+++ b/xen/arch/x86/irq.c        Tue Oct 13 15:23:05 2009 +0100
@@ -1340,7 +1340,11 @@
     ASSERT(spin_is_locked(&pcidevs_lock));
     ASSERT(spin_is_locked(&d->event_lock));
 
+#ifdef PRIVILEGED_STUBDOMS
+    if ( !IS_PRIV_FOR(current->domain, d) )
+#else
     if ( !IS_PRIV(current->domain) )
+#endif
         return -EPERM;
 
     if ( pirq < 0 || pirq >= d->nr_pirqs || irq < 0 || irq >= nr_irqs )
diff -r 0035117b3a88 xen/arch/x86/physdev.c
--- a/xen/arch/x86/physdev.c    Tue Oct 13 14:38:45 2009 +0100
+++ b/xen/arch/x86/physdev.c    Tue Oct 13 15:23:05 2009 +0100
@@ -34,9 +34,6 @@
     struct msi_info _msi;
     void *map_data = NULL;
 
-    if ( !IS_PRIV(current->domain) )
-        return -EPERM;
-
     if ( !map )
         return -EINVAL;
 
@@ -48,6 +45,16 @@
     if ( d == NULL )
     {
         ret = -ESRCH;
+        goto free_domain;
+    }
+
+#ifdef PRIVILEGED_STUBDOMS
+    if ( !IS_PRIV_FOR(current->domain, d) )
+#else
+    if ( !IS_PRIV(current->domain) )
+#endif
+    {
+        ret = -EPERM;
         goto free_domain;
     }
 
@@ -158,10 +165,7 @@
 static int physdev_unmap_pirq(struct physdev_unmap_pirq *unmap)
 {
     struct domain *d;
-    int ret;
-
-    if ( !IS_PRIV(current->domain) )
-        return -EPERM;
+    int ret = -ESRCH;
 
     if ( unmap->domid == DOMID_SELF )
         d = rcu_lock_domain(current->domain);
@@ -169,7 +173,17 @@
         d = rcu_lock_domain_by_id(unmap->domid);
 
     if ( d == NULL )
-        return -ESRCH;
+        goto free_domain;
+
+#ifdef PRIVILEGED_STUBDOMS
+    if ( !IS_PRIV_FOR(current->domain, d) )
+#else
+    if ( !IS_PRIV(current->domain) )
+#endif
+    {
+        ret = -EPERM;
+        goto free_domain;
+    }
 
     spin_lock(&pcidevs_lock);
     spin_lock(&d->event_lock);
@@ -177,6 +191,7 @@
     spin_unlock(&d->event_lock);
     spin_unlock(&pcidevs_lock);
 
+free_domain:
     rcu_unlock_domain(d);
 
     return ret;
diff -r 0035117b3a88 xen/common/domctl.c
--- a/xen/common/domctl.c       Tue Oct 13 14:38:45 2009 +0100
+++ b/xen/common/domctl.c       Tue Oct 13 15:23:05 2009 +0100
@@ -220,14 +220,38 @@
     long ret = 0;
     struct xen_domctl curop, *op = &curop;
 
-    if ( !IS_PRIV(current->domain) )
-        return -EPERM;
-
     if ( copy_from_guest(op, u_domctl, 1) )
         return -EFAULT;
 
     if ( op->interface_version != XEN_DOMCTL_INTERFACE_VERSION )
         return -EACCES;
+
+    switch ( op->cmd )
+    {
+        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:
+#ifdef PRIVILEGED_STUBDOMS
+            {
+                struct domain *d = get_domain_by_id(op->domain);
+                if ( !IS_PRIV_FOR(current->domain, d) )
+                {
+                    put_domain(d);
+                    return -EPERM;
+                }
+                put_domain(d);
+            }
+            break;
+#endif
+        default:
+            if ( !IS_PRIV(current->domain) )
+                return -EPERM;
+            break;
+    }
+
 
     if ( !domctl_lock_acquire() )
         return hypercall_create_continuation(

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel