[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[xen staging-4.17] domctl: handle XEN_DOMCTL_getdomaininfo without acquiring domctl lock



commit fc4b4423afc1001f9840f6dc7f77427ce9d23d7f
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Thu Jun 4 21:42:55 2026 +0100
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Thu Jun 4 22:29:12 2026 +0100

    domctl: handle XEN_DOMCTL_getdomaininfo without acquiring domctl lock
    
    getdomaininfo() is not called under consistently the same lock. Thus,
    with caller side locking irrelevant, it can as well be called with the
    domctl lock not held. (Callers not pausing the domain they want to
    retrieve information for already need to be aware that not all of the
    data returned can be relied on as being consistent; most data will also
    be stale by the time the caller gets to look at it.)
    
    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.
    
    Fixes: 5513bd0b4675 ("add xenstore domain flag to hypervisor")
    Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
    Acked-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
    (cherry picked from commit 784bd4dc0bc440b978b80b6945c4bc380b28e32d)
---
 xen/common/domctl.c     | 91 +++++++++++++++++++++++++++----------------------
 xen/include/xsm/dummy.h |  5 ++-
 xen/xsm/flask/hooks.c   |  6 +++-
 3 files changed, 59 insertions(+), 43 deletions(-)

diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 5824398183..5f0ad71660 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -318,6 +318,56 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
u_domctl)
             return -ESRCH;
     }
 
+    /* Handle sub-ops not requiring the domctl lock. */
+    switch ( op->cmd )
+    {
+    case XEN_DOMCTL_getdomaininfo:
+    {
+        domid_t dom = DOMID_INVALID;
+
+        if ( !d )
+        {
+            ret = -EINVAL;
+            if ( op->domain >= DOMID_FIRST_RESERVED )
+                goto domctl_out_unlock_domonly;
+
+            rcu_read_lock(&domlist_read_lock);
+
+            dom = op->domain;
+            for_each_domain ( d )
+                if ( d->domain_id >= dom )
+                    break;
+        }
+
+        ret = -ESRCH;
+        if ( !d )
+            goto getdomaininfo_out;
+
+        ret = xsm_getdomaininfo(XSM_XS_PRIV, d);
+        if ( !ret )
+        {
+            getdomaininfo(d, &op->u.getdomaininfo);
+
+            op->domain = op->u.getdomaininfo.domain;
+            copyback = true;
+
+    getdomaininfo_out:
+            /* When d was non-NULL upon entry, no cleanup is needed. */
+            if ( dom == DOMID_INVALID )
+                goto domctl_out_unlock_domonly;
+
+            rcu_read_unlock(&domlist_read_lock);
+            d = NULL;
+        }
+
+        goto domctl_out_unlock_domonly;
+    }
+
+    default:
+        /* Everything else handled further down. */
+        break;
+    }
+
     ret = xsm_domctl(XSM_OTHER, d, op->cmd,
                      /* SSIDRef only applicable for cmd == createdomain */
                      op->u.createdomain.ssidref);
@@ -534,47 +584,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
u_domctl)
         copyback = 1;
         break;
 
-    case XEN_DOMCTL_getdomaininfo:
-    {
-        domid_t dom = DOMID_INVALID;
-
-        if ( !d )
-        {
-            ret = -EINVAL;
-            if ( op->domain >= DOMID_FIRST_RESERVED )
-                break;
-
-            rcu_read_lock(&domlist_read_lock);
-
-            dom = op->domain;
-            for_each_domain ( d )
-                if ( d->domain_id >= dom )
-                    break;
-        }
-
-        ret = -ESRCH;
-        if ( d == NULL )
-            goto getdomaininfo_out;
-
-        ret = xsm_getdomaininfo(XSM_XS_PRIV, d);
-        if ( ret )
-            goto getdomaininfo_out;
-
-        getdomaininfo(d, &op->u.getdomaininfo);
-
-        op->domain = op->u.getdomaininfo.domain;
-        copyback = 1;
-
-    getdomaininfo_out:
-        /* When d was non-NULL upon entry, no cleanup is needed. */
-        if ( dom == DOMID_INVALID )
-            break;
-
-        rcu_read_unlock(&domlist_read_lock);
-        d = NULL;
-        break;
-    }
-
     case XEN_DOMCTL_getvcpucontext:
     {
         vcpu_guest_context_u c = { .nat = NULL };
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 4b5f07ad27..2ed193689f 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -172,8 +172,11 @@ static XSM_INLINE int cf_check xsm_domctl(
     case XEN_DOMCTL_bind_pt_irq:
     case XEN_DOMCTL_unbind_pt_irq:
         return xsm_default_action(XSM_DM_PRIV, current->domain, d);
+
     case XEN_DOMCTL_getdomaininfo:
-        return xsm_default_action(XSM_XS_PRIV, current->domain, d);
+        ASSERT_UNREACHABLE();
+        return -EILSEQ;
+
     default:
         return xsm_default_action(XSM_PRIV, current->domain, d);
     }
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 42857c2122..c4c0ed596f 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -678,8 +678,12 @@ static int cf_check flask_domctl(struct domain *d, 
unsigned int cmd,
          */
         return avc_current_has_perm(ssidref, SECCLASS_DOMAIN, DOMAIN__CREATE, 
NULL);
 
-    /* These have individual XSM hooks (common/domctl.c) */
+    /* These have individual XSM hooks and don't make it here. */
     case XEN_DOMCTL_getdomaininfo:
+        ASSERT_UNREACHABLE();
+        return -EILSEQ;
+
+    /* These have individual XSM hooks (common/domctl.c) */
     case XEN_DOMCTL_scheduler_op:
     case XEN_DOMCTL_irq_permission:
     case XEN_DOMCTL_iomem_permission:
--
generated by git-patchbot for /home/xen/git/xen.git#staging-4.17



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.