From: Jan Beulich <jbeulich@suse.com>
Subject: domctl: handle XEN_DOMCTL_set_target without acquiring domctl lock

The only locking required here is that between checking d->target and
setting it. To avoid the need for an explicit lock, use cmpxchgptr() to
update d->target.

Move the handling not only ahead of acquiring the lock, but also ahead
of the XSM check, leveraging that the sub-op has its own hook.

This is part of XSA-492.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -489,6 +489,30 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
         goto domctl_out_unlock_domonly;
     }
 
+    case XEN_DOMCTL_set_target:
+    {
+        struct domain *e = get_domain_by_id(op->u.set_target.target);
+
+        ret = -ESRCH;
+        if ( !e )
+            goto domctl_out_unlock_domonly;
+
+        if ( d == e )
+            ret = -EINVAL;
+        else if ( !is_hvm_domain(e) )
+            ret = -EOPNOTSUPP;
+        else
+            ret = xsm_set_target(XSM_PRIV, d, e);
+
+        /* Hold reference on @e until we destroy @d. */
+        if ( !ret && cmpxchgptr(&d->target, NULL, e) )
+            ret = -EINVAL;
+
+        if ( ret )
+            put_domain(e);
+        goto domctl_out_unlock_domonly;
+    }
+
     case XEN_DOMCTL_vm_event_op:
         if ( op->u.vm_event_op.op == XEN_VM_EVENT_GET_VERSION )
         {
@@ -845,36 +869,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
         domain_set_time_offset(d, op->u.settimeoffset.time_offset_seconds);
         break;
 
-    case XEN_DOMCTL_set_target:
-    {
-        struct domain *e;
-
-        ret = -ESRCH;
-        e = get_domain_by_id(op->u.set_target.target);
-        if ( e == NULL )
-            break;
-
-        ret = -EINVAL;
-        if ( (d == e) || (d->target != NULL) )
-        {
-            put_domain(e);
-            break;
-        }
-
-        ret = -EOPNOTSUPP;
-        if ( is_hvm_domain(e) )
-            ret = xsm_set_target(XSM_HOOK, d, e);
-        if ( ret )
-        {
-            put_domain(e);
-            break;
-        }
-
-        /* Hold reference on @e until we destroy @d. */
-        d->target = e;
-        break;
-    }
-
     case XEN_DOMCTL_subscribe:
         d->suspend_evtchn = op->u.subscribe.port;
         break;
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -150,7 +150,7 @@ static XSM_INLINE int cf_check xsm_sysct
 static XSM_INLINE int cf_check xsm_set_target(
     XSM_DEFAULT_ARG struct domain *d, struct domain *e)
 {
-    XSM_ASSERT_ACTION(XSM_HOOK);
+    XSM_ASSERT_ACTION(XSM_PRIV);
     return xsm_default_action(action, current->domain, NULL);
 }
 
@@ -168,6 +168,7 @@ static XSM_INLINE int cf_check xsm_domct
     case XEN_DOMCTL_ioport_permission:
     case XEN_DOMCTL_irq_permission:
     case XEN_DOMCTL_memory_mapping:
+    case XEN_DOMCTL_set_target:
     case XEN_DOMCTL_unbind_pt_irq:
         ASSERT_UNREACHABLE();
         return -EILSEQ;
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -699,14 +699,11 @@ static int cf_check flask_domctl(struct
     case XEN_DOMCTL_ioport_permission:
     case XEN_DOMCTL_irq_permission:
     case XEN_DOMCTL_memory_mapping:
+    case XEN_DOMCTL_set_target:
     case XEN_DOMCTL_unbind_pt_irq:
         ASSERT_UNREACHABLE();
         return -EILSEQ;
 
-    /* These have individual XSM hooks (common/domctl.c) */
-    case XEN_DOMCTL_set_target:
-        return 0;
-
     case XEN_DOMCTL_destroydomain:
         return current_has_perm(d, SECCLASS_DOMAIN, DOMAIN__DESTROY);