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

[Jürgen Groß <jgross@xxxxxxxx>]

On 19.02.26 12:31, Jason Andryuk wrote:
> On 2026-02-19 06:21, Jürgen Groß wrote:
> > 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;
> > > > > > > @@ -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.
> > > > 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.
> Yes, this could happen, so the bits should remain set.
In the above scenario, I think VIRQ_DOM_EXC should be triggered after a flask
policy update in order to let xenstored look for previously missed domain state
changes.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature