On 07.01.25 17:28, Jan Beulich wrote:
On 07.01.2025 11:17, Juergen Gross wrote:--- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -185,6 +185,76 @@ static void domain_changed_state(const struct domain *d) unlock_dom_exc_handler(hdl); }+static void set_domain_state_info(struct xen_domctl_get_domain_state *info,+ const struct domain *d) +{ + info->state = XEN_DOMCTL_GETDOMSTATE_STATE_EXIST; + if ( d->is_shut_down ) + info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_SHUTDOWN; + if ( d->is_dying == DOMDYING_dying ) + info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DYING; + if ( d->is_dying == DOMDYING_dead ) + info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DEAD; + info->unique_id = d->unique_id; +} + +int get_domain_state(struct xen_domctl_get_domain_state *info, struct domain *d, + domid_t *domid) +{ + unsigned int dom; + int rc = -ENOENT; + struct domain *hdl; + + if ( info->pad0 || info->pad1 ) + return -EINVAL; + + if ( d ) + { + set_domain_state_info(info, d); + + return 0; + } + + /* + * Only domain registered for VIRQ_DOM_EXC event is allowed to query + * domains having changed state. + */ + if ( !domain_handles_global_virq(current->domain, VIRQ_DOM_EXC) ) + return -EACCES; + + hdl = lock_dom_exc_handler();Instead of leaving a small window for races between the if() and this function call, can't you simply compare hdl against current->domain?
Good idea.
+ while ( dom_state_changed ) + { + dom = find_first_bit(dom_state_changed, DOMID_MASK + 1); + if ( dom >= DOMID_FIRST_RESERVED ) + break; + if ( test_and_clear_bit(dom, dom_state_changed) )As this is now running under lock, does it really need to be test-and-clear? What mechanism would allow the flag to be cleared between the find-1st and here? Plus, like for patch 4, I think it could be __clear_bit() here.
It is only under read_lock(), so there are concurrent calls possible. I don't think we want to use write_lock() here, do we? Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
