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

[Jürgen Groß]

On 19.02.26 12:13, Jan Beulich wrote:

On 19.02.2026 12:03, Juergen Gross wrote:

On 19.02.26 11:52, Jan Beulich wrote:

On 19.02.2026 00:04, Jason Andryuk wrote:

On 2026-02-18 14:08, Daniel P. Smith wrote:

--- 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;

        {
+        rc = xsm_get_domain_state(XSM_XS_PRIV, d);
+        if ( rc )
+            return rc;
+
            set_domain_state_info(info, d);

@@ -238,28 +242,39 @@ int get_domain_state(struct xen_domctl_get_domain_state 
*info, struct domain *d,

@@ -219,6 +219,10 @@ int get_domain_state(struct xen_domctl_get_domain_state *info, struct domain *d, if ( d ) return 0;

Between the two hunks is this:

       hdl = lock_dom_exc_handler();

       /*
        * Only domain registered for VIRQ_DOM_EXC event is allowed to query
        * domains having changed state.
        */
       if ( current->domain != hdl )
       {
           rc = -EACCES;
           goto out;
       }

So it is only the domain with VIRQ_DOM_EXC that can enter the while loop:

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;
+
+        d = rcu_lock_domain_by_id(dom);
+        if ( d && xsm_get_domain_state(XSM_XS_PRIV, d) )

... if the VIRQ_DOM_EXC owner is denied for domain d ...

+        {
+            rcu_unlock_domain(d);
+            d = NULL;
+            continue;

... the caller would continue ...

+        }
+
            if ( test_and_clear_bit(dom, dom_state_changed) )

... and this bit would never be cleared.  Should the VIRQ_DOM_EXC owner
always get to clear the bit even if it cannot see the result?

I don't think so, no. Whenever a legitimate consumer occurs (the owner of
VIRQ_DOM_EXC can change, after all), it'll then consume the bits as needed.
More generally, I think we're better off not making the code here depend
too much on that special VIRQ_DOM_EXC property.

OTOH a new VIRQ_DOM_EXC owner will result in a complete reset of the bitmap
anyway (that is: the bits for all existing domains will be set, while all
others will be cleared).

Yes, while writing my reply I wondered whether I should mention that. To keep
things a little more simple, I didn't. Plus for this aspect the last sentence
of my earlier reply also applies.

I believe flask settings can be changed, right?

If so, clearing the bit would be affecting the scenario where the VIRQ_DOM_EXC
is NOT changed, but the flask settings are updated to allow it obtaining info
about d.


Juergen

OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

OpenPGP_signature.asc
Description: OpenPGP digital signature