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

[Jan Beulich <jbeulich@xxxxxxxx>]

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;
>>   
>> @@ -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,39 @@ int get_domain_state(struct 
>> xen_domctl_get_domain_state *info, struct domain *d,
> 
> 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.

Jan