[BUG v3] common/domctl: xsm update for get_domain_state access

Daniel P. Smith posted 1 patch 1 week, 4 days ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20260219143707.1588-1-dpsmith@apertussolutions.com
xen/common/domain.c | 37 ++++++++++++++++++++++---------------
xen/common/domctl.c |  7 ++-----
2 files changed, 24 insertions(+), 20 deletions(-)
[BUG v3] common/domctl: xsm update for get_domain_state access
Posted by Daniel P. Smith 1 week, 4 days ago
When using XSM Flask, passing DOMID_INVALID will result in a NULL pointer
reference from the passing of NULL as the target domain to
xsm_get_domain_state(). Simply not invoking xsm_get_domain_state() when the
target domain is NULL opens the opportunity to circumvent the XSM
get_domain_state access check. This is due to the fact that the call to
xsm_domctl() for get_domain_state op is a no-op check, deferring to
xsm_get_domain_state().

Modify the helper get_domain_state() to ensure the requesting domain has
get_domain_state access for the target domain, whether the target domain is
explicitly set or implicitly determined with a domain state search. In the case
of access not being allowed for a domain found during an implicit search, the
search will continue to the next domain whose state has changed.

Fixes: 3ad3df1bd0aa ("xen: add new domctl get_domain_state")
Reported-by: Chris Rogers <rogersc@ainfosec.com>
Reported-by: Dmytro Firsov <dmytro_firsov@epam.com>
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
Changes in v3:
- collapse if statements
- dropped unnecessary NULL
- use true for copyback

Changes in v2:
- fix commit message
- init dom as -1
- rework loop logic to use test_and_clear_bit()
---
 xen/common/domain.c | 37 ++++++++++++++++++++++---------------
 xen/common/domctl.c |  7 ++-----
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index de6fdf59236e..73d6c72d9709 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -210,7 +210,7 @@ static void set_domain_state_info(struct xen_domctl_get_domain_state *info,
 int get_domain_state(struct xen_domctl_get_domain_state *info, struct domain *d,
                      domid_t *domid)
 {
-    unsigned int dom;
+    unsigned int dom = -1;
     int rc = -ENOENT;
     struct domain *hdl;
 
@@ -219,6 +219,10 @@ int get_domain_state(struct xen_domctl_get_domain_state *info, struct domain *d,
 
     if ( d )
     {
+        rc = xsm_get_domain_state(XSM_XS_PRIV, d);
+        if ( rc )
+            return rc;
+
         set_domain_state_info(info, d);
 
         return 0;
@@ -238,28 +242,31 @@ int get_domain_state(struct xen_domctl_get_domain_state *info, struct domain *d,
 
     while ( dom_state_changed )
     {
-        dom = find_first_bit(dom_state_changed, DOMID_MASK + 1);
+        dom = find_next_bit(dom_state_changed, DOMID_MASK + 1, dom + 1);
         if ( dom >= DOMID_FIRST_RESERVED )
             break;
-        if ( test_and_clear_bit(dom, dom_state_changed) )
-        {
-            *domid = dom;
-
-            d = rcu_lock_domain_by_id(dom);
 
+        d = rcu_lock_domain_by_id(dom);
+        if ( (d && xsm_get_domain_state(XSM_XS_PRIV, d)) ||
+             !test_and_clear_bit(dom, dom_state_changed) )
+        {
             if ( d )
-            {
-                set_domain_state_info(info, d);
-
                 rcu_unlock_domain(d);
-            }
-            else
-                memset(info, 0, sizeof(*info));
+            continue;
+        }
 
-            rc = 0;
+        *domid = dom;
 
-            break;
+        if ( d )
+        {
+            set_domain_state_info(info, d);
+            rcu_unlock_domain(d);
         }
+        else
+            memset(info, 0, sizeof(*info));
+
+        rc = 0;
+        break;
     }
 
  out:
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 29a7726d32d0..93738931c575 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -860,12 +860,9 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         break;
 
     case XEN_DOMCTL_get_domain_state:
-        ret = xsm_get_domain_state(XSM_XS_PRIV, d);
-        if ( ret )
-            break;
-
-        copyback = 1;
         ret = get_domain_state(&op->u.get_domain_state, d, &op->domain);
+        if ( !ret )
+            copyback = true;
         break;
 
     default:
-- 
2.39.5
Re: [BUG v3] common/domctl: xsm update for get_domain_state access
Posted by Jan Beulich 1 week, 4 days ago
On 19.02.2026 15:37, Daniel P. Smith wrote:
> When using XSM Flask, passing DOMID_INVALID will result in a NULL pointer
> reference from the passing of NULL as the target domain to
> xsm_get_domain_state(). Simply not invoking xsm_get_domain_state() when the
> target domain is NULL opens the opportunity to circumvent the XSM
> get_domain_state access check. This is due to the fact that the call to
> xsm_domctl() for get_domain_state op is a no-op check, deferring to
> xsm_get_domain_state().
> 
> Modify the helper get_domain_state() to ensure the requesting domain has
> get_domain_state access for the target domain, whether the target domain is
> explicitly set or implicitly determined with a domain state search. In the case
> of access not being allowed for a domain found during an implicit search, the
> search will continue to the next domain whose state has changed.
> 
> Fixes: 3ad3df1bd0aa ("xen: add new domctl get_domain_state")
> Reported-by: Chris Rogers <rogersc@ainfosec.com>
> Reported-by: Dmytro Firsov <dmytro_firsov@epam.com>
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>