xen: avoid calling rcu_lock_*target_domain when an XSM hook exists

The rcu_lock_{,remote_}target_domain_by_id functions are wrappers
around an IS_PRIV_FOR check for the current domain. This is now
redundant with XSM hooks, so replace these calls with
rcu_lock_domain_by_any_id or rcu_lock_remote_domain_by_id to remove
the duplicate permission checks.

When XSM_ENABLE is not defined or when the dummy XSM module is used,
this patch should not change any functionality. Because the locations
of privilege checks have sometimes moved below argument validation,
error returns of some functions may change from EPERM to EINVAL when
called with invalid arguments and from a domain without permission to
perform the operation.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Acked-by: Jan Beulich <jbeulich@suse.com>
Committed-by: Keir Fraser <keir@xen.org>

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 123a147a7951fb832eb93b8dc3cf1468721c52ba..e17b7356de24bd4da4cf356fc15cb19ff2f1380e 100644 (file)
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3388,7 +3388,7 @@ static int hvmop_set_pci_intx_level(
     if ( (op.domain > 0) || (op.bus > 0) || (op.device > 31) || (op.intx > 3) )
         return -EINVAL;
 
-    rc = rcu_lock_remote_target_domain_by_id(op.domid, &d);
+    rc = rcu_lock_remote_domain_by_id(op.domid, &d);
     if ( rc != 0 )
         return rc;
 
@@ -3555,7 +3555,7 @@ static int hvmop_set_isa_irq_level(
     if ( op.isa_irq > 15 )
         return -EINVAL;
 
-    rc = rcu_lock_remote_target_domain_by_id(op.domid, &d);
+    rc = rcu_lock_remote_domain_by_id(op.domid, &d);
     if ( rc != 0 )
         return rc;
 
@@ -3599,7 +3599,7 @@ static int hvmop_set_pci_link_route(
     if ( (op.link > 3) || (op.isa_irq > 15) )
         return -EINVAL;
 
-    rc = rcu_lock_remote_target_domain_by_id(op.domid, &d);
+    rc = rcu_lock_remote_domain_by_id(op.domid, &d);
     if ( rc != 0 )
         return rc;
 
@@ -3629,7 +3629,7 @@ static int hvmop_inject_msi(
     if ( copy_from_guest(&op, uop, 1) )
         return -EFAULT;
 
-    rc = rcu_lock_remote_target_domain_by_id(op.domid, &d);
+    rc = rcu_lock_remote_domain_by_id(op.domid, &d);
     if ( rc != 0 )
         return rc;
 
@@ -3726,9 +3726,9 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( a.index >= HVM_NR_PARAMS )
             return -EINVAL;
 
-        rc = rcu_lock_target_domain_by_id(a.domid, &d);
-        if ( rc != 0 )
-            return rc;
+        d = rcu_lock_domain_by_any_id(a.domid);
+        if ( d == NULL )
+            return -ESRCH;
 
         rc = -EINVAL;
         if ( !is_hvm_domain(d) )
@@ -3972,7 +3972,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( copy_from_guest(&a, arg, 1) )
             return -EFAULT;
 
-        rc = rcu_lock_remote_target_domain_by_id(a.domid, &d);
+        rc = rcu_lock_remote_domain_by_id(a.domid, &d);
         if ( rc != 0 )
             return rc;
 
@@ -4013,7 +4013,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( copy_from_guest(&a, arg, 1) )
             return -EFAULT;
 
-        rc = rcu_lock_remote_target_domain_by_id(a.domid, &d);
+        rc = rcu_lock_remote_domain_by_id(a.domid, &d);
         if ( rc != 0 )
             return rc;
 
@@ -4078,9 +4078,9 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( copy_from_guest(&a, arg, 1) )
             return -EFAULT;
 
-        rc = rcu_lock_target_domain_by_id(a.domid, &d);
-        if ( rc != 0 )
-            return rc;
+        d = rcu_lock_domain_by_any_id(a.domid);
+        if ( d == NULL )
+            return -ESRCH;
 
         rc = xsm_hvm_param(d, op);
         if ( rc )
@@ -4128,7 +4128,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( copy_from_guest(&a, arg, 1) )
             return -EFAULT;
 
-        rc = rcu_lock_remote_target_domain_by_id(a.domid, &d);
+        rc = rcu_lock_remote_domain_by_id(a.domid, &d);
         if ( rc != 0 )
             return rc;
 
@@ -4221,7 +4221,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( copy_from_guest(&a, arg, 1) )
             return -EFAULT;
 
-        rc = rcu_lock_remote_target_domain_by_id(a.domid, &d);
+        rc = rcu_lock_remote_domain_by_id(a.domid, &d);
         if ( rc != 0 )
             return rc;
 
@@ -4256,7 +4256,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( copy_from_guest(&a, arg, 1) )
             return -EFAULT;
 
-        rc = rcu_lock_remote_target_domain_by_id(a.domid, &d);
+        rc = rcu_lock_remote_domain_by_id(a.domid, &d);
         if ( rc != 0 )
             return rc;
 
@@ -4292,9 +4292,9 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( copy_from_guest(&a, arg, 1) )
             return -EFAULT;
 
-        rc = rcu_lock_target_domain_by_id(a.domid, &d);
-        if ( rc != 0 )
-            return rc;
+        d = rcu_lock_domain_by_any_id(a.domid);
+        if ( d == NULL )
+            return -ESRCH;
 
         rc = -EINVAL;
         if ( !is_hvm_domain(d) || !paging_mode_shadow(d) )
@@ -4346,7 +4346,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( copy_from_guest(&tr, arg, 1 ) )
             return -EFAULT;
 
-        rc = rcu_lock_remote_target_domain_by_id(tr.domid, &d);
+        rc = rcu_lock_remote_domain_by_id(tr.domid, &d);
         if ( rc != 0 )
             return rc;
 

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index af2eafa6f553d6d02fad069c8fb3e1faa15cbd58..c8e39c0a87b26549061f44833006b7e63d1cfadd 100644 (file)
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4375,9 +4375,9 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( copy_from_guest(&xatp, arg, 1) )
             return -EFAULT;
 
-        rc = rcu_lock_target_domain_by_id(xatp.domid, &d);
-        if ( rc != 0 )
-            return rc;
+        d = rcu_lock_domain_by_any_id(xatp.domid);
+        if ( d == NULL )
+            return -ESRCH;
 
         if ( xsm_add_to_physmap(current->domain, d) )
         {
@@ -4414,9 +4414,9 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( fmap.map.nr_entries > E820MAX )
             return -EINVAL;
 
-        rc = rcu_lock_target_domain_by_id(fmap.domid, &d);
-        if ( rc != 0 )
-            return rc;
+        d = rcu_lock_domain_by_any_id(fmap.domid);
+        if ( d == NULL )
+            return -ESRCH;
 
         rc = xsm_domain_memory_map(d);
         if ( rc )
@@ -4569,16 +4569,12 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
         struct domain *d;
         struct p2m_domain *p2m;
 
-        /* Support DOMID_SELF? */
-        if ( !IS_PRIV(current->domain) )
-            return -EPERM;
-
         if ( copy_from_guest(&target, arg, 1) )
             return -EFAULT;
 
-        rc = rcu_lock_target_domain_by_id(target.domid, &d);
-        if ( rc != 0 )
-            return rc;
+        d = rcu_lock_domain_by_any_id(target.domid);
+        if ( d == NULL )
+            return -ESRCH;
 
         if ( op == XENMEM_set_pod_target )
             rc = xsm_set_pod_target(d);

diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
index 89f0ca762032a0d9614c4021b717cd0e3f499cfe..f620966ce259223c51a4e741d47d166d910eaaab 100644 (file)
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -165,9 +165,9 @@ static long evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc)
     domid_t        dom = alloc->dom;
     long           rc;
 
-    rc = rcu_lock_target_domain_by_id(dom, &d);
-    if ( rc )
-        return rc;
+    d = rcu_lock_domain_by_any_id(dom);
+    if ( d == NULL )
+        return -ESRCH;
 
     spin_lock(&d->event_lock);
 
@@ -798,9 +798,9 @@ static long evtchn_status(evtchn_status_t *status)
     struct evtchn   *chn;
     long             rc = 0;
 
-    rc = rcu_lock_target_domain_by_id(dom, &d);
-    if ( rc )
-        return rc;
+    d = rcu_lock_domain_by_any_id(dom);
+    if ( d == NULL )
+        return -ESRCH;
 
     spin_lock(&d->event_lock);
 
@@ -950,9 +950,9 @@ static long evtchn_reset(evtchn_reset_t *r)
     struct domain *d;
     int i, rc;
 
-    rc = rcu_lock_target_domain_by_id(dom, &d);
-    if ( rc )
-        return rc;
+    d = rcu_lock_domain_by_any_id(dom);
+    if ( d == NULL )
+        return -ESRCH;
 
     rc = xsm_evtchn_reset(current->domain, d);
     if ( rc )

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index ce66d7598f736e81335fb7b5b75d775554d7743a..59708c33e423a353d3d2da53549be340ca42d4a8 100644 (file)
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -230,30 +230,6 @@ double_gt_unlock(struct grant_table *lgt, struct grant_table *rgt)
         spin_unlock(&rgt->lock);
 }
 
-static struct domain *gt_lock_target_domain_by_id(domid_t dom)
-{
-    struct domain *d;
-    int rc = GNTST_general_error;
-
-    switch ( rcu_lock_target_domain_by_id(dom, &d) )
-    {
-    case 0:
-        return d;
-
-    case -ESRCH:
-        gdprintk(XENLOG_INFO, "Bad domid %d.\n", dom);
-        rc = GNTST_bad_domain;
-        break;
-
-    case -EPERM:
-        rc = GNTST_permission_denied;
-        break;
-    }
-
-    ASSERT(rc < 0 && -rc <= MAX_ERRNO);
-    return ERR_PTR(rc);
-}
-
 static inline int
 __get_maptrack_handle(
     struct grant_table *t)
@@ -1352,11 +1328,12 @@ gnttab_setup_table(
     if ( !guest_handle_okay(op.frame_list, op.nr_frames) )
         return -EFAULT;
 
-    d = gt_lock_target_domain_by_id(op.dom);
-    if ( IS_ERR(d) )
+    d = rcu_lock_domain_by_any_id(op.dom);
+    if ( d == NULL )
     {
-        op.status = PTR_ERR(d);
-        goto out1;
+        gdprintk(XENLOG_INFO, "Bad domid %d.\n", op.dom);
+        op.status = GNTST_bad_domain;
+        goto out2;
     }
 
     if ( xsm_grant_setup(current->domain, d) )
@@ -1421,10 +1398,11 @@ gnttab_query_size(
         return -EFAULT;
     }
 
-    d = gt_lock_target_domain_by_id(op.dom);
-    if ( IS_ERR(d) )
+    d = rcu_lock_domain_by_any_id(op.dom);
+    if ( d == NULL )
     {
-        op.status = PTR_ERR(d);
+        gdprintk(XENLOG_INFO, "Bad domid %d.\n", op.dom);
+        op.status = GNTST_bad_domain;
         goto query_out;
     }
 
@@ -2296,10 +2274,10 @@ gnttab_get_status_frames(XEN_GUEST_HANDLE_PARAM(gnttab_get_status_frames_t) uop,
         return -EFAULT;
     }
 
-    d = gt_lock_target_domain_by_id(op.dom);
-    if ( IS_ERR(d) )
+    d = rcu_lock_domain_by_any_id(op.dom);
+    if ( d == NULL )
     {
-        op.status = PTR_ERR(d);
+        op.status = GNTST_bad_domain;
         goto out1;
     }
     rc = xsm_grant_setup(current->domain, d);
@@ -2349,14 +2327,15 @@ gnttab_get_version(XEN_GUEST_HANDLE_PARAM(gnttab_get_version_t) uop)
     if ( copy_from_guest(&op, uop, 1) )
         return -EFAULT;
 
-    rc = rcu_lock_target_domain_by_id(op.dom, &d);
-    if ( rc < 0 )
-        return rc;
+    d = rcu_lock_domain_by_any_id(op.dom);
+    if ( d == NULL )
+        return -ESRCH;
 
-    if ( xsm_grant_query_size(current->domain, d) )
+    rc = xsm_grant_query_size(current->domain, d);
+    if ( rc )
     {
         rcu_unlock_domain(d);
-        return -EPERM;
+        return rc;
     }
 
     op.version = d->grant_table->gt_version;

diff --git a/xen/common/memory.c b/xen/common/memory.c
index c8c1ef2896c895fd773b298355da8ca0b2ac95b2..35acf1cb881ca177511dfff9d6f6be5ebe7dae79 100644 (file)
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -585,7 +585,8 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
              && (reservation.mem_flags & XENMEMF_populate_on_demand) )
             args.memflags |= MEMF_populate_on_demand;
 
-        if ( unlikely(rcu_lock_target_domain_by_id(reservation.domid, &d)) )
+        d = rcu_lock_domain_by_any_id(reservation.domid);
+        if ( d == NULL )
             return start_extent;
         args.domain = d;
 
@@ -634,9 +635,9 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( copy_from_guest(&domid, arg, 1) )
             return -EFAULT;
 
-        rc = rcu_lock_target_domain_by_id(domid, &d);
-        if ( rc )
-            return rc;
+        d = rcu_lock_domain_by_any_id(domid);
+        if ( d == NULL )
+            return -ESRCH;
 
         rc = xsm_memory_stat_reservation(current->domain, d);
         if ( rc )
@@ -672,9 +673,9 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( copy_from_guest(&xrfp, arg, 1) )
             return -EFAULT;
 
-        rc = rcu_lock_target_domain_by_id(xrfp.domid, &d);
-        if ( rc != 0 )
-            return rc;
+        d = rcu_lock_domain_by_any_id(xrfp.domid);
+        if ( d == NULL )
+            return -ESRCH;
 
         if ( xsm_remove_from_physmap(current->domain, d) )
         {

diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index fb00a01febc8d89dcc82e53ba1d30110b69a6fa3..dc16684e8016598ef9943e42ed52f296b757967b 100644 (file)
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -194,6 +194,8 @@ static XSM_INLINE int xsm_grant_unmapref(struct domain *d1, struct domain *d2)
 
 static XSM_INLINE int xsm_grant_setup(struct domain *d1, struct domain *d2)
 {
+    if ( d1 != d2 && !IS_PRIV_FOR(d1, d2) )
+        return -EPERM;
     return 0;
 }
 
@@ -209,17 +211,23 @@ static XSM_INLINE int xsm_grant_copy(struct domain *d1, struct domain *d2)
 
 static XSM_INLINE int xsm_grant_query_size(struct domain *d1, struct domain *d2)
 {
+    if ( d1 != d2 && !IS_PRIV_FOR(d1, d2) )
+        return -EPERM;
     return 0;
 }
 
 static XSM_INLINE int xsm_memory_adjust_reservation(struct domain *d1,
                                                             struct domain *d2)
 {
+    if ( d1 != d2 && !IS_PRIV_FOR(d1, d2) )
+        return -EPERM;
     return 0;
 }
 
 static XSM_INLINE int xsm_memory_stat_reservation(struct domain *d1, struct domain *d2)
 {
+    if ( d1 != d2 && !IS_PRIV_FOR(d1, d2) )
+        return -EPERM;
     return 0;
 }
 
@@ -260,6 +268,8 @@ static XSM_INLINE int xsm_memory_pin_page(struct domain *d1, struct domain *d2,
 static XSM_INLINE int xsm_evtchn_unbound(struct domain *d, struct evtchn *chn,
                                          domid_t id2)
 {
+    if ( current->domain != d && !IS_PRIV_FOR(current->domain, d) )
+        return -EPERM;
     return 0;
 }
 
@@ -281,11 +291,15 @@ static XSM_INLINE int xsm_evtchn_send(struct domain *d, struct evtchn *chn)
 
 static XSM_INLINE int xsm_evtchn_status(struct domain *d, struct evtchn *chn)
 {
+    if ( current->domain != d && !IS_PRIV_FOR(current->domain, d) )
+        return -EPERM;
     return 0;
 }
 
 static XSM_INLINE int xsm_evtchn_reset(struct domain *d1, struct domain *d2)
 {
+    if ( d1 != d2 && !IS_PRIV_FOR(d1, d2) )
+        return -EPERM;
     return 0;
 }
 
@@ -306,11 +320,15 @@ static XSM_INLINE char *xsm_show_security_evtchn(struct domain *d, const struct
 
 static XSM_INLINE int xsm_get_pod_target(struct domain *d)
 {
+    if ( !IS_PRIV(current->domain) )
+        return -EPERM;
     return 0;
 }
 
 static XSM_INLINE int xsm_set_pod_target(struct domain *d)
 {
+    if ( !IS_PRIV(current->domain) )
+        return -EPERM;
     return 0;
 }
 
@@ -481,26 +499,36 @@ static XSM_INLINE int xsm_machine_address_size(struct domain *d, uint32_t cmd)
 
 static XSM_INLINE int xsm_hvm_param(struct domain *d, unsigned long op)
 {
+    if ( current->domain != d && !IS_PRIV_FOR(current->domain, d) )
+        return -EPERM;
     return 0;
 }
 
 static XSM_INLINE int xsm_hvm_set_pci_intx_level(struct domain *d)
 {
+    if ( !IS_PRIV_FOR(current->domain, d) )
+        return -EPERM;
     return 0;
 }
 
 static XSM_INLINE int xsm_hvm_set_isa_irq_level(struct domain *d)
 {
+    if ( !IS_PRIV_FOR(current->domain, d) )
+        return -EPERM;
     return 0;
 }
 
 static XSM_INLINE int xsm_hvm_set_pci_link_route(struct domain *d)
 {
+    if ( !IS_PRIV_FOR(current->domain, d) )
+        return -EPERM;
     return 0;
 }
 
 static XSM_INLINE int xsm_hvm_inject_msi(struct domain *d)
 {
+    if ( !IS_PRIV_FOR(current->domain, d) )
+        return -EPERM;
     return 0;
 }
 
@@ -582,6 +610,8 @@ static XSM_INLINE int xsm_machine_memory_map(void)
 
 static XSM_INLINE int xsm_domain_memory_map(struct domain *d)
 {
+    if ( current->domain != d && !IS_PRIV_FOR(current->domain, d) )
+        return -EPERM;
     return 0;
 }
 
@@ -605,11 +635,15 @@ static XSM_INLINE int xsm_update_va_mapping(struct domain *d, struct domain *f,
 
 static XSM_INLINE int xsm_add_to_physmap(struct domain *d1, struct domain *d2)
 {
+    if ( d1 != d2 && !IS_PRIV_FOR(d1, d2) )
+        return -EPERM;
     return 0;
 }
 
 static XSM_INLINE int xsm_remove_from_physmap(struct domain *d1, struct domain *d2)
 {
+    if ( d1 != d2 && !IS_PRIV_FOR(d1, d2) )
+        return -EPERM;
     return 0;
 }