|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen-unstable] Clean up handling of IS_PRIV_FOR() and rcu_[un]lock_domain().
# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1206726610 0
# Node ID 4e2e98c2098eb78faaaca6982370900f11851013
# Parent b5fea3aeb04b14b8ba82d3db18449bd593051113
Clean up handling of IS_PRIV_FOR() and rcu_[un]lock_domain().
In particular this *removes* some IS_PRIV_FOR() checks. *Especially*
in particular, all domctls are executable only by dom0. Several of
them were really unsafe for execution by a stub domain as they can
affect global system resource usage.
This probably breaks stub domains. Where necessary, some of these
reversions can themselves be reverted where they are judged both
necessary and safe.
Signed-off-by: Keir Fraser <keir.fraser@xxxxxxxxxx>
---
xen/arch/x86/hvm/hvm.c | 11 ++-
xen/arch/x86/mm.c | 28 ++++----
xen/common/domain.c | 2
xen/common/domctl.c | 156 +++++++++++++--------------------------------
xen/common/event_channel.c | 49 ++++++++------
xen/common/grant_table.c | 43 ++++++------
xen/common/memory.c | 51 ++++++++------
7 files changed, 153 insertions(+), 187 deletions(-)
diff -r b5fea3aeb04b -r 4e2e98c2098e xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c Fri Mar 28 14:12:33 2008 +0000
+++ b/xen/arch/x86/hvm/hvm.c Fri Mar 28 17:50:10 2008 +0000
@@ -2160,12 +2160,15 @@ long do_hvm_op(unsigned long op, XEN_GUE
return -EINVAL;
if ( a.domid == DOMID_SELF )
+ {
d = rcu_lock_current_domain();
- else {
- d = rcu_lock_domain_by_id(a.domid);
- if ( d == NULL )
+ }
+ else
+ {
+ if ( (d = rcu_lock_domain_by_id(a.domid)) == NULL )
return -ESRCH;
- if ( !IS_PRIV_FOR(current->domain, d) ) {
+ if ( !IS_PRIV_FOR(current->domain, d) )
+ {
rc = -EPERM;
goto param_fail;
}
diff -r b5fea3aeb04b -r 4e2e98c2098e xen/arch/x86/mm.c
--- a/xen/arch/x86/mm.c Fri Mar 28 14:12:33 2008 +0000
+++ b/xen/arch/x86/mm.c Fri Mar 28 17:50:10 2008 +0000
@@ -2114,14 +2114,14 @@ static int set_foreigndom(domid_t domid)
info->foreign = rcu_lock_domain(dom_xen);
break;
default:
- e = rcu_lock_domain_by_id(domid);
- if ( e == NULL )
+ if ( (e = rcu_lock_domain_by_id(domid)) == NULL )
{
MEM_LOG("Unknown domain '%u'", domid);
okay = 0;
break;
}
- if (!IS_PRIV_FOR(d, e)) {
+ if ( !IS_PRIV_FOR(d, e) )
+ {
MEM_LOG("Cannot set foreign dom");
okay = 0;
rcu_unlock_domain(e);
@@ -3259,12 +3259,15 @@ long arch_memory_op(int op, XEN_GUEST_HA
return -EFAULT;
if ( xatp.domid == DOMID_SELF )
+ {
d = rcu_lock_current_domain();
- else {
- d = rcu_lock_domain_by_id(xatp.domid);
- if ( d == NULL )
+ }
+ else
+ {
+ if ( (d = rcu_lock_domain_by_id(xatp.domid)) == NULL )
return -ESRCH;
- if ( !IS_PRIV_FOR(current->domain, d) ) {
+ if ( !IS_PRIV_FOR(current->domain, d) )
+ {
rcu_unlock_domain(d);
return -EPERM;
}
@@ -3355,12 +3358,15 @@ long arch_memory_op(int op, XEN_GUEST_HA
return -EINVAL;
if ( fmap.domid == DOMID_SELF )
+ {
d = rcu_lock_current_domain();
- else {
- d = rcu_lock_domain_by_id(fmap.domid);
- if ( d == NULL )
+ }
+ else
+ {
+ if ( (d = rcu_lock_domain_by_id(fmap.domid)) == NULL )
return -ESRCH;
- if ( !IS_PRIV_FOR(current->domain, d) ) {
+ if ( !IS_PRIV_FOR(current->domain, d) )
+ {
rcu_unlock_domain(d);
return -EPERM;
}
diff -r b5fea3aeb04b -r 4e2e98c2098e xen/common/domain.c
--- a/xen/common/domain.c Fri Mar 28 14:12:33 2008 +0000
+++ b/xen/common/domain.c Fri Mar 28 17:50:10 2008 +0000
@@ -522,7 +522,7 @@ static void complete_domain_destroy(stru
if ( (v = d->vcpu[i]) != NULL )
free_vcpu_struct(v);
- if (d->target)
+ if ( d->target != NULL )
put_domain(d->target);
free_domain(d);
diff -r b5fea3aeb04b -r 4e2e98c2098e xen/common/domctl.c
--- a/xen/common/domctl.c Fri Mar 28 14:12:33 2008 +0000
+++ b/xen/common/domctl.c Fri Mar 28 17:50:10 2008 +0000
@@ -182,6 +182,9 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
struct xen_domctl curop, *op = &curop;
static DEFINE_SPINLOCK(domctl_lock);
+ if ( !IS_PRIV(current->domain) )
+ return -EPERM;
+
if ( copy_from_guest(op, u_domctl, 1) )
return -EFAULT;
@@ -203,10 +206,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
ret = -ESRCH;
if ( d == NULL )
break;
-
- ret = -EPERM;
- if ( !IS_PRIV_FOR(current->domain, d) )
- goto svc_out;
ret = xsm_setvcpucontext(d);
if ( ret )
@@ -259,10 +258,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
ret = -ESRCH;
if ( d != NULL )
{
- ret = -EPERM;
- if ( !IS_PRIV_FOR(current->domain, d) )
- goto pausedomain_out;
-
ret = xsm_pausedomain(d);
if ( ret )
goto pausedomain_out;
@@ -287,18 +282,16 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
if ( d == NULL )
break;
- ret = -EPERM;
- if ( !IS_PRIV_FOR(current->domain, d) )
- goto unpausedomain_out;
-
ret = xsm_unpausedomain(d);
if ( ret )
- goto unpausedomain_out;
+ {
+ rcu_unlock_domain(d);
+ break;
+ }
domain_unpause_by_systemcontroller(d);
- ret = 0;
-unpausedomain_out:
- rcu_unlock_domain(d);
+ rcu_unlock_domain(d);
+ ret = 0;
}
break;
@@ -310,18 +303,16 @@ unpausedomain_out:
if ( d == NULL )
break;
- ret = -EPERM;
- if ( !IS_PRIV_FOR(current->domain, d) )
- goto resumedomain_out;
-
ret = xsm_resumedomain(d);
if ( ret )
- goto resumedomain_out;
+ {
+ rcu_unlock_domain(d);
+ break;
+ }
domain_resume(d);
- ret = 0;
-resumedomain_out:
- rcu_unlock_domain(d);
+ rcu_unlock_domain(d);
+ ret = 0;
}
break;
@@ -331,10 +322,6 @@ resumedomain_out:
domid_t dom;
static domid_t rover = 0;
unsigned int domcr_flags;
-
- ret = -EPERM;
- if ( !IS_PRIV(current->domain) )
- break;
ret = -EINVAL;
if ( supervisor_mode_kernel ||
@@ -401,13 +388,12 @@ resumedomain_out:
if ( (d = rcu_lock_domain_by_id(op->domain)) == NULL )
break;
- ret = -EPERM;
- if ( !IS_PRIV_FOR(current->domain, d) )
- goto maxvcpu_out2;
-
ret = xsm_max_vcpus(d);
if ( ret )
- goto maxvcpu_out2;
+ {
+ rcu_unlock_domain(d);
+ break;
+ }
/* Needed, for example, to ensure writable p.t. state is synced. */
domain_pause(d);
@@ -435,7 +421,6 @@ resumedomain_out:
maxvcpu_out:
domain_unpause(d);
- maxvcpu_out2:
rcu_unlock_domain(d);
}
break;
@@ -446,9 +431,7 @@ resumedomain_out:
ret = -ESRCH;
if ( d != NULL )
{
- ret = -EPERM;
- if ( IS_PRIV_FOR(current->domain, d) )
- ret = xsm_destroydomain(d) ? : domain_kill(d);
+ ret = xsm_destroydomain(d) ? : domain_kill(d);
rcu_unlock_domain(d);
}
}
@@ -466,10 +449,6 @@ resumedomain_out:
if ( d == NULL )
break;
- ret = -EPERM;
- if ( !IS_PRIV_FOR(current->domain, d) )
- goto vcpuaffinity_out;
-
ret = xsm_vcpuaffinity(op->cmd, d);
if ( ret )
goto vcpuaffinity_out;
@@ -508,10 +487,6 @@ resumedomain_out:
if ( (d = rcu_lock_domain_by_id(op->domain)) == NULL )
break;
- ret = -EPERM;
- if ( !IS_PRIV_FOR(current->domain, d) )
- goto scheduler_op_out;
-
ret = xsm_scheduler(d);
if ( ret )
goto scheduler_op_out;
@@ -533,7 +508,7 @@ resumedomain_out:
rcu_read_lock(&domlist_read_lock);
for_each_domain ( d )
- if ( d->domain_id >= dom && IS_PRIV_FOR(current->domain, d))
+ if ( d->domain_id >= dom )
break;
if ( d == NULL )
@@ -567,10 +542,6 @@ resumedomain_out:
ret = -ESRCH;
if ( (d = rcu_lock_domain_by_id(op->domain)) == NULL )
break;
-
- ret = -EPERM;
- if ( !IS_PRIV_FOR(current->domain, d) )
- goto getvcpucontext_out;
ret = xsm_getvcpucontext(d);
if ( ret )
@@ -632,10 +603,6 @@ resumedomain_out:
if ( (d = rcu_lock_domain_by_id(op->domain)) == NULL )
break;
- ret = -EPERM;
- if ( !IS_PRIV_FOR(current->domain, d) )
- goto getvcpuinfo_out;
-
ret = xsm_getvcpuinfo(d);
if ( ret )
goto getvcpuinfo_out;
@@ -675,10 +642,6 @@ resumedomain_out:
if ( d == NULL )
break;
- ret = -EPERM;
- if ( !IS_PRIV_FOR(current->domain, d) )
- goto max_mem_out;
-
ret = xsm_setdomainmaxmem(d);
if ( ret )
goto max_mem_out;
@@ -695,8 +658,6 @@ resumedomain_out:
d->max_pages = new_max;
ret = 0;
}
- else
- printk("new max %ld, tot pages %d\n", new_max, d->tot_pages);
spin_unlock(&d->page_alloc_lock);
max_mem_out:
@@ -713,19 +674,17 @@ resumedomain_out:
if ( d == NULL )
break;
- ret = -EPERM;
- if ( !IS_PRIV_FOR(current->domain, d) )
- goto setdomainhandle_out;
-
ret = xsm_setdomainhandle(d);
if ( ret )
- goto setdomainhandle_out;
+ {
+ rcu_unlock_domain(d);
+ break;
+ }
memcpy(d->handle, op->u.setdomainhandle.handle,
sizeof(xen_domain_handle_t));
- ret = 0;
-setdomainhandle_out:
- rcu_unlock_domain(d);
+ rcu_unlock_domain(d);
+ ret = 0;
}
break;
@@ -738,20 +697,18 @@ setdomainhandle_out:
if ( d == NULL )
break;
- ret = -EPERM;
- if ( !IS_PRIV_FOR(current->domain, d) )
- goto setdebugging_out;
-
ret = xsm_setdebugging(d);
if ( ret )
- goto setdebugging_out;
+ {
+ rcu_unlock_domain(d);
+ break;
+ }
domain_pause(d);
d->debugger_attached = !!op->u.setdebugging.enable;
domain_unpause(d); /* causes guest to latch new status */
- ret = 0;
-setdebugging_out:
- rcu_unlock_domain(d);
+ rcu_unlock_domain(d);
+ ret = 0;
}
break;
@@ -768,10 +725,6 @@ setdebugging_out:
d = rcu_lock_domain_by_id(op->domain);
if ( d == NULL )
break;
-
- ret = -EPERM;
- if ( !IS_PRIV_FOR(current->domain, d) )
- goto irq_permission_out;
ret = xsm_irq_permission(d, pirq, op->u.irq_permission.allow_access);
if ( ret )
@@ -802,10 +755,6 @@ setdebugging_out:
if ( d == NULL )
break;
- ret = -EPERM;
- if ( !IS_PRIV_FOR(current->domain, d) )
- goto iomem_permission_out;
-
ret = xsm_iomem_permission(d, mfn,
op->u.iomem_permission.allow_access);
if ( ret )
goto iomem_permission_out;
@@ -829,19 +778,16 @@ setdebugging_out:
if ( d == NULL )
break;
- ret = -EPERM;
- if ( !IS_PRIV_FOR(current->domain, d) )
- goto settimeoffset_out;
-
ret = xsm_domain_settime(d);
if ( ret )
- goto settimeoffset_out;
+ {
+ rcu_unlock_domain(d);
+ break;
+ }
d->time_offset_seconds = op->u.settimeoffset.time_offset_seconds;
-
- ret = 0;
-settimeoffset_out:
- rcu_unlock_domain(d);
+ rcu_unlock_domain(d);
+ ret = 0;
}
break;
@@ -853,33 +799,25 @@ settimeoffset_out:
d = rcu_lock_domain_by_id(op->domain);
if ( d == NULL )
break;
-
- ret = -EPERM;
- if (!IS_PRIV_FOR(current->domain, d))
- goto set_target_out;
ret = -ESRCH;
e = get_domain_by_id(op->u.set_target.target);
if ( e == NULL )
goto set_target_out;
- if ( d == e ) {
- ret = -EINVAL;
+ ret = -EINVAL;
+ if ( (d == e) || (d->target != NULL) )
+ {
put_domain(e);
goto set_target_out;
}
- if (!IS_PRIV_FOR(current->domain, e)) {
- ret = -EPERM;
- put_domain(e);
- goto set_target_out;
- }
-
+ /* Hold reference on @e until we destroy @d. */
d->target = e;
- /* and we keep the reference on e, released when destroying d */
- ret = 0;
-
-set_target_out:
+
+ ret = 0;
+
+ set_target_out:
rcu_unlock_domain(d);
}
break;
diff -r b5fea3aeb04b -r 4e2e98c2098e xen/common/event_channel.c
--- a/xen/common/event_channel.c Fri Mar 28 14:12:33 2008 +0000
+++ b/xen/common/event_channel.c Fri Mar 28 17:50:10 2008 +0000
@@ -130,13 +130,17 @@ static long evtchn_alloc_unbound(evtchn_
long rc;
if ( dom == DOMID_SELF )
- d = current->domain;
- else {
+ {
+ d = rcu_lock_current_domain();
+ }
+ else
+ {
if ( (d = rcu_lock_domain_by_id(dom)) == NULL )
return -ESRCH;
- if ( !IS_PRIV_FOR(current->domain, d) ) {
- rc = -EPERM;
- goto out2;
+ if ( !IS_PRIV_FOR(current->domain, d) )
+ {
+ rcu_unlock_domain(d);
+ return -EPERM;
}
}
@@ -158,8 +162,6 @@ static long evtchn_alloc_unbound(evtchn_
out:
spin_unlock(&d->evtchn_lock);
-
- out2:
rcu_unlock_domain(d);
return rc;
@@ -201,7 +203,7 @@ static long evtchn_bind_interdomain(evtc
ERROR_EXIT_DOM(-EINVAL, rd);
rchn = evtchn_from_port(rd, rport);
if ( (rchn->state != ECS_UNBOUND) ||
- (rchn->u.unbound.remote_domid != ld->domain_id && !IS_PRIV_FOR(ld,
rd)))
+ (rchn->u.unbound.remote_domid != ld->domain_id) )
ERROR_EXIT_DOM(-EINVAL, rd);
rc = xsm_evtchn_interdomain(ld, lchn, rd, rchn);
@@ -631,13 +633,17 @@ static long evtchn_status(evtchn_status_
long rc = 0;
if ( dom == DOMID_SELF )
- d = current->domain;
- else {
+ {
+ d = rcu_lock_current_domain();
+ }
+ else
+ {
if ( (d = rcu_lock_domain_by_id(dom)) == NULL )
return -ESRCH;
- if ( !IS_PRIV_FOR(current->domain, d) ) {
- rc = -EPERM;
- goto out2;
+ if ( !IS_PRIV_FOR(current->domain, d) )
+ {
+ rcu_unlock_domain(d);
+ return -EPERM;
}
}
@@ -690,8 +696,8 @@ static long evtchn_status(evtchn_status_
out:
spin_unlock(&d->evtchn_lock);
- out2:
rcu_unlock_domain(d);
+
return rc;
}
@@ -742,6 +748,7 @@ long evtchn_bind_vcpu(unsigned int port,
out:
spin_unlock(&d->evtchn_lock);
+
return rc;
}
@@ -784,15 +791,18 @@ static long evtchn_reset(evtchn_reset_t
{
domid_t dom = r->dom;
struct domain *d;
- int i;
- int rc;
+ int i, rc;
if ( dom == DOMID_SELF )
- d = current->domain;
- else {
+ {
+ d = rcu_lock_current_domain();
+ }
+ else
+ {
if ( (d = rcu_lock_domain_by_id(dom)) == NULL )
return -ESRCH;
- if ( !IS_PRIV_FOR(current->domain, d) ) {
+ if ( !IS_PRIV_FOR(current->domain, d) )
+ {
rc = -EPERM;
goto out;
}
@@ -806,6 +816,7 @@ static long evtchn_reset(evtchn_reset_t
(void)__evtchn_close(d, i);
rc = 0;
+
out:
rcu_unlock_domain(d);
diff -r b5fea3aeb04b -r 4e2e98c2098e xen/common/grant_table.c
--- a/xen/common/grant_table.c Fri Mar 28 14:12:33 2008 +0000
+++ b/xen/common/grant_table.c Fri Mar 28 17:50:10 2008 +0000
@@ -828,32 +828,34 @@ gnttab_setup_table(
" per domain.\n",
max_nr_grant_frames);
op.status = GNTST_general_error;
- goto out;
+ goto out1;
}
dom = op.dom;
if ( dom == DOMID_SELF )
{
- d = current->domain;
- }
- else {
+ d = rcu_lock_current_domain();
+ }
+ else
+ {
if ( unlikely((d = rcu_lock_domain_by_id(dom)) == NULL) )
{
gdprintk(XENLOG_INFO, "Bad domid %d.\n", dom);
op.status = GNTST_bad_domain;
- goto out;
- }
- if ( unlikely(!IS_PRIV_FOR(current->domain, d)) ) {
+ goto out1;
+ }
+
+ if ( unlikely(!IS_PRIV_FOR(current->domain, d)) )
+ {
op.status = GNTST_permission_denied;
- goto setup_unlock_out2;
+ goto out2;
}
}
if ( xsm_grant_setup(current->domain, d) )
{
- rcu_unlock_domain(d);
op.status = GNTST_permission_denied;
- goto out;
+ goto out2;
}
spin_lock(&d->grant_table->lock);
@@ -867,7 +869,7 @@ gnttab_setup_table(
nr_grant_frames(d->grant_table),
max_nr_grant_frames);
op.status = GNTST_general_error;
- goto setup_unlock_out;
+ goto out3;
}
op.status = GNTST_okay;
@@ -877,13 +879,11 @@ gnttab_setup_table(
(void)copy_to_guest_offset(op.frame_list, i, &gmfn, 1);
}
- setup_unlock_out:
+ out3:
spin_unlock(&d->grant_table->lock);
-
- setup_unlock_out2:
+ out2:
rcu_unlock_domain(d);
-
- out:
+ out1:
if ( unlikely(copy_to_guest(uop, &op, 1)) )
return -EFAULT;
@@ -911,16 +911,19 @@ gnttab_query_size(
dom = op.dom;
if ( dom == DOMID_SELF )
{
- d = current->domain;
- }
- else {
+ d = rcu_lock_current_domain();
+ }
+ else
+ {
if ( unlikely((d = rcu_lock_domain_by_id(dom)) == NULL) )
{
gdprintk(XENLOG_INFO, "Bad domid %d.\n", dom);
op.status = GNTST_bad_domain;
goto query_out;
}
- if ( unlikely(!IS_PRIV_FOR(current->domain, d)) ) {
+
+ if ( unlikely(!IS_PRIV_FOR(current->domain, d)) )
+ {
op.status = GNTST_permission_denied;
goto query_out_unlock;
}
diff -r b5fea3aeb04b -r 4e2e98c2098e xen/common/memory.c
--- a/xen/common/memory.c Fri Mar 28 14:12:33 2008 +0000
+++ b/xen/common/memory.c Fri Mar 28 17:50:10 2008 +0000
@@ -232,12 +232,15 @@ static long translate_gpfn_list(
return -EFAULT;
if ( op.domid == DOMID_SELF )
- d = current->domain;
- else {
- d = rcu_lock_domain_by_id(op.domid);
- if ( d == NULL )
+ {
+ d = rcu_lock_current_domain();
+ }
+ else
+ {
+ if ( (d = rcu_lock_domain_by_id(op.domid)) == NULL )
return -ESRCH;
- if ( !IS_PRIV_FOR(current->domain, d) ) {
+ if ( !IS_PRIV_FOR(current->domain, d) )
+ {
rcu_unlock_domain(d);
return -EPERM;
}
@@ -539,12 +542,15 @@ long do_memory_op(unsigned long cmd, XEN
}
if ( likely(reservation.domid == DOMID_SELF) )
- d = current->domain;
- else {
- d = rcu_lock_domain_by_id(reservation.domid);
- if ( d == NULL)
+ {
+ d = rcu_lock_current_domain();
+ }
+ else
+ {
+ if ( (d = rcu_lock_domain_by_id(reservation.domid)) == NULL )
return start_extent;
- if ( !IS_PRIV_FOR(current->domain, d) ) {
+ if ( !IS_PRIV_FOR(current->domain, d) )
+ {
rcu_unlock_domain(d);
return start_extent;
}
@@ -554,8 +560,7 @@ long do_memory_op(unsigned long cmd, XEN
rc = xsm_memory_adjust_reservation(current->domain, d);
if ( rc )
{
- if ( reservation.domid != DOMID_SELF )
- rcu_unlock_domain(d);
+ rcu_unlock_domain(d);
return rc;
}
@@ -572,8 +577,7 @@ long do_memory_op(unsigned long cmd, XEN
break;
}
- if ( unlikely(reservation.domid != DOMID_SELF) )
- rcu_unlock_domain(d);
+ rcu_unlock_domain(d);
rc = args.nr_done;
@@ -599,12 +603,15 @@ long do_memory_op(unsigned long cmd, XEN
return -EFAULT;
if ( likely(domid == DOMID_SELF) )
- d = current->domain;
- else {
- d = rcu_lock_domain_by_id(domid);
- if ( d == NULL )
+ {
+ d = rcu_lock_current_domain();
+ }
+ else
+ {
+ if ( (d = rcu_lock_domain_by_id(domid)) == NULL )
return -ESRCH;
- if ( !IS_PRIV_FOR(current->domain, d) ) {
+ if ( !IS_PRIV_FOR(current->domain, d) )
+ {
rcu_unlock_domain(d);
return -EPERM;
}
@@ -613,8 +620,7 @@ long do_memory_op(unsigned long cmd, XEN
rc = xsm_memory_stat_reservation(current->domain, d);
if ( rc )
{
- if ( domid != DOMID_SELF )
- rcu_unlock_domain(d);
+ rcu_unlock_domain(d);
return rc;
}
@@ -632,8 +638,7 @@ long do_memory_op(unsigned long cmd, XEN
break;
}
- if ( unlikely(domid != DOMID_SELF) )
- rcu_unlock_domain(d);
+ rcu_unlock_domain(d);
break;
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |