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

Stubdoms need to be able to make all the passthrough related hypercalls
on behalf of the guest.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---

diff -r 65a13cafbbef xen/arch/x86/irq.c
--- a/xen/arch/x86/irq.c	Wed Jul 22 15:59:44 2009 +0100
+++ b/xen/arch/x86/irq.c	Wed Jul 22 16:15:14 2009 +0100
@@ -917,7 +917,7 @@
     ASSERT(spin_is_locked(&pcidevs_lock));
     ASSERT(spin_is_locked(&d->event_lock));
 
-    if ( !IS_PRIV(current->domain) )
+    if ( !IS_PRIV_FOR(current->domain, d) )
         return -EPERM;
 
     if ( pirq < 0 || pirq >= d->nr_pirqs || vector < 0 || vector
>= NR_VECTORS )
diff -r 65a13cafbbef xen/arch/x86/physdev.c
--- a/xen/arch/x86/physdev.c	Wed Jul 22 15:59:44 2009 +0100
+++ b/xen/arch/x86/physdev.c	Wed Jul 22 16:15:14 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,12 @@
     if ( d == NULL )
     {
         ret = -ESRCH;
+        goto free_domain;
+    }
+
+    if ( !IS_PRIV_FOR(current->domain, d) )
+    {
+        ret = -EPERM;
         goto free_domain;
     }
 
@@ -158,10 +161,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 +169,13 @@
         d = rcu_lock_domain_by_id(unmap->domid);
 
     if ( d == NULL )
-        return -ESRCH;
+        goto free_domain;
+
+    if ( !IS_PRIV_FOR(current->domain, d) )
+    {
+        ret = -EPERM;
+        goto free_domain;
+    }
 
     spin_lock(&pcidevs_lock);
     spin_lock(&d->event_lock);
@@ -177,6 +183,7 @@
     spin_unlock(&d->event_lock);
     spin_unlock(&pcidevs_lock);
 
+free_domain:
     rcu_unlock_domain(d);
 
     return ret;
diff -r 65a13cafbbef xen/common/domctl.c
--- a/xen/common/domctl.c	Wed Jul 22 15:59:44 2009 +0100
+++ b/xen/common/domctl.c	Wed Jul 22 16:15:14 2009 +0100
@@ -220,14 +220,36 @@
     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:
+            {
+                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;
+        default:
+            if ( !IS_PRIV(current->domain) )
+                return -EPERM;
+            break;
+    }
+
 
     if ( !domctl_lock_acquire() )
         return hypercall_create_continuation(

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

Hello,

Stefano Stabellini, le Mon 12 Oct 2009 18:20:09 +0100, a écrit
:
> Stubdoms need to be able to make all the passthrough related hypercalls
> on behalf of the guest.
Tried before, nacked by Keir :)
> diff -r 65a13cafbbef xen/arch/x86/irq.c
> --- a/xen/arch/x86/irq.c	Wed Jul 22 15:59:44 2009 +0100
> +++ b/xen/arch/x86/irq.c	Wed Jul 22 16:15:14 2009 +0100
> @@ -917,7 +917,7 @@
>      ASSERT(spin_is_locked(&pcidevs_lock));
>      ASSERT(spin_is_locked(&d->event_lock));
>  
> -    if ( !IS_PRIV(current->domain) )
> +    if ( !IS_PRIV_FOR(current->domain, d) )
>          return -EPERM;
>  
>      if ( pirq < 0 || pirq >= d->nr_pirqs || vector < 0 ||
vector >= NR_VECTORS )
For instance, here.  It''s not because a stubdomain has privilege over
another domain that it suddendly is allowed to reroute all IRQs of the
machine :)

This needs proper accounting: xend should tell the hypervisor which IRQs
domains are allowed to use. Same for physdev, pci functions, etc.

Samuel

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

On Mon, 12 Oct 2009, Samuel Thibault wrote:
> Hello,
> 
> Stefano Stabellini, le Mon 12 Oct 2009 18:20:09 +0100, a écrit :
> > Stubdoms need to be able to make all the passthrough related
hypercalls
> > on behalf of the guest.
> 
> Tried before, nacked by Keir :)
Last time I spoke with Keir I got the impression that allowing only the
hypercalls strictly needed for passthrough was OK.

In any case I would greately appreciate if all the other patches were
accepted in any case because I wouldn''t want them to bitrot.


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

On Mon, 12 Oct 2009, Stefano Stabellini wrote:
> On Mon, 12 Oct 2009, Samuel Thibault wrote:
> > Hello,
> > 
> > Stefano Stabellini, le Mon 12 Oct 2009 18:20:09 +0100, a écrit :
> > > Stubdoms need to be able to make all the passthrough related
hypercalls
> > > on behalf of the guest.
> > 
> > Tried before, nacked by Keir :)
> 
> Last time I spoke with Keir I got the impression that allowing only the
> hypercalls strictly needed for passthrough was OK.
> 
> In any case I would greately appreciate if all the other patches were
> accepted in any case because I wouldn''t want them to bitrot.
>
BTW: it cannot be worse than opening /dev/mem O_RDWR in qemu-xen, that
is exactly what happens at the moment.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Stefano Stabellini, le Mon 12 Oct 2009 19:10:54 +0100, a écrit
:
> On Mon, 12 Oct 2009, Samuel Thibault wrote:
> > Stefano Stabellini, le Mon 12 Oct 2009 18:20:09 +0100, a écrit :
> > > Stubdoms need to be able to make all the passthrough related
hypercalls
> > > on behalf of the guest.
> > 
> > Tried before, nacked by Keir :)
> 
> Last time I spoke with Keir I got the impression that allowing only the
> hypercalls strictly needed for passthrough was OK.
Ok, but doesn''t that open a door to passthrough an IRQ which was not
supposed to be passed through?  Doesn''t it let a stubdom passthrough
IRQ0 for instance?
> In any case I would greately appreciate if all the other patches were
> accepted in any case because I wouldn''t want them to bitrot.
I think they can go in, yes.

Samuel

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

Stefano Stabellini, le Mon 12 Oct 2009 19:14:11 +0100, a écrit
:
> > In any case I would greately appreciate if all the other patches were
> > accepted in any case because I wouldn''t want them to bitrot.
> >
> 
> BTW: it cannot be worse than opening /dev/mem O_RDWR in qemu-xen, that
> is exactly what happens at the moment.
Sure, but the difference is that qemu-xen is known to be run as root
in dom0, while allowing things from the hypervisor potentially hides
security leaks in its source code.

Samuel

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

On 12/10/2009 19:19, "Samuel Thibault"
<samuel.thibault@ens-lyon.org> wrote:
>> BTW: it cannot be worse than opening /dev/mem O_RDWR in qemu-xen, that
>> is exactly what happens at the moment.
> 
> Sure, but the difference is that qemu-xen is known to be run as root
> in dom0, while allowing things from the hypervisor potentially hides
> security leaks in its source code.
Also, one of the advantages of stubdom is supposed to be that it contains
qemu''s large attack surface within a deprivileged environment.

The hard thing about passthru in a stubdom is that every relevant hypercall
really needs to be audited and potentially redesigned so that it all works
but according to principle of least privilege.

The alternative is rather undesirable, but perhaps acceptable if we make
that choice with our eyes open to it.

 -- Keir



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

On 12/10/2009 18:20, "Stefano Stabellini"
<stefano.stabellini@eu.citrix.com>
wrote:
> +        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:
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! :-)

 -- Keir



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

On Mon, 12 Oct 2009, Keir Fraser wrote:
> On 12/10/2009 18:20, "Stefano Stabellini"
<stefano.stabellini@eu.citrix.com>
> wrote:
> 
> > +        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:
> 
> 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.

In any case it is just a matter of ordering the items in the todo list,
all these things have to happen anyway.


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

Stefano Stabellini, le Tue 13 Oct 2009 13:00:09 +0100, a écrit
:
> 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.
For that you could just provide testers with this patch along with a
strong warning that it''s not safe (from a security point of view).

Samuel

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

On Tue, 13 Oct 2009, Samuel Thibault wrote:
> Stefano Stabellini, le Tue 13 Oct 2009 13:00:09 +0100, a écrit :
> > 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.
> 
> For that you could just provide testers with this patch along with a
> strong warning that it''s not safe (from a security point of view).
In theory yes, but in practice not so many people read the list
compared to the people that actually chase xen-unstable.

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

Stefano Stabellini, le Tue 13 Oct 2009 13:10:20 +0100, a écrit
:
> On Tue, 13 Oct 2009, Samuel Thibault wrote:
> > Stefano Stabellini, le Tue 13 Oct 2009 13:00:09 +0100, a écrit :
> > > 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.
> > 
> > For that you could just provide testers with this patch along with a
> > strong warning that it''s not safe (from a security point of
view).
> 
> In theory yes, but in practice not so many people read the list
> compared to the people that actually chase xen-unstable.
Would the latter report bugs they encounter?

Samuel

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

On Tue, 13 Oct 2009, Samuel Thibault wrote:
> Stefano Stabellini, le Tue 13 Oct 2009 13:10:20 +0100, a écrit :
> > On Tue, 13 Oct 2009, Samuel Thibault wrote:
> > > Stefano Stabellini, le Tue 13 Oct 2009 13:00:09 +0100, a écrit :
> > > > 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.
> > > 
> > > For that you could just provide testers with this patch along
with a
> > > strong warning that it''s not safe (from a security point
of view).
> > 
> > In theory yes, but in practice not so many people read the list
> > compared to the people that actually chase xen-unstable.
> 
> Would the latter report bugs they encounter?
> 
I hope so, at least on xen-users.

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

Stefano Stabellini, le Tue 13 Oct 2009 13:18:50 +0100, a écrit
:
> On Tue, 13 Oct 2009, Samuel Thibault wrote:
> > Stefano Stabellini, le Tue 13 Oct 2009 13:10:20 +0100, a écrit :
> > > On Tue, 13 Oct 2009, Samuel Thibault wrote:
> > > > Stefano Stabellini, le Tue 13 Oct 2009 13:00:09 +0100, a
écrit :
> > > > > 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.
> > > > 
> > > > For that you could just provide testers with this patch
along with a
> > > > strong warning that it''s not safe (from a security
point of view).
> > > 
> > > In theory yes, but in practice not so many people read the list
> > > compared to the people that actually chase xen-unstable.
> > 
> > Would the latter report bugs they encounter?
> 
> I hope so, at least on xen-users.
Well, I''d tend to think that if they''re not following either,
they''re
probably not building from the hg repository.  Anyway, I let it to
Keir whether he''s keen on opening a temporary security breach to allow
testing :)

Samuel

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

On 13/10/2009 13:00, "Stefano Stabellini"
<Stefano.Stabellini@eu.citrix.com>
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.

 -- Keir



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

Keir Fraser, le Tue 13 Oct 2009 13:32:32 +0100, a écrit
:
> 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.
Can be a solution yes.

Samuel

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

On Tue, 13 Oct 2009, Keir Fraser wrote:
> On 13/10/2009 13:00, "Stefano Stabellini"
<Stefano.Stabellini@eu.citrix.com>
> 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@eu.citrix.com>

---

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@lists.xensource.com
http://lists.xensource.com/xen-devel

Stefano Stabellini, le Tue 13 Oct 2009 15:24:31 +0100, a écrit
:
>  
> -    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() )
The cases should be inside the #ifdef, or there should be a #else.

Samuel

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

On Tue, 13 Oct 2009, Samuel Thibault wrote:
> The cases should be inside the #ifdef, or there should be a #else.
>
Why? Just for aesthetic?

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

Stefano Stabellini, le Tue 13 Oct 2009 15:40:23 +0100, a écrit
:
> On Tue, 13 Oct 2009, Samuel Thibault wrote:
> > The cases should be inside the #ifdef, or there should be a #else.
> 
> Why? Just for aesthetic?
Ah, sorry, I missed that it was exactly along break;
This shows that yes, at least for readability it''d be useful.

Samuel

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

Samuel Thibault, le Tue 13 Oct 2009 16:42:36 +0200, a écrit
:
> Stefano Stabellini, le Tue 13 Oct 2009 15:40:23 +0100, a écrit :
> > On Tue, 13 Oct 2009, Samuel Thibault wrote:
> > > The cases should be inside the #ifdef, or there should be a
#else.
> > 
> > Why? Just for aesthetic?
> 
> Ah, sorry, I missed that it was exactly along break;
Along default: I meant.
> This shows that yes, at least for readability it''d be useful.
> 
> Samuel
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
> 
-- 
Samuel
"c''est pas nous qui sommes à la rue, c''est la rue qui est
à nous"

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

On 13/10/2009 15:36, "Samuel Thibault"
<samuel.thibault@ens-lyon.org> wrote:
> The cases should be inside the #ifdef, or there should be a #else.
I''ll do the aesthetic cleanup when I check in.

 -- Keir



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

On 13/10/2009 15:24, "Stefano Stabellini"
<Stefano.Stabellini@eu.citrix.com>
wrote:
>> 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.
Fixed and reworked and applied.

 K.



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