On 16.12.24 11:41, Jan Beulich wrote:
On 13.12.2024 17:24, Juergen Gross wrote:--- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -193,6 +193,57 @@ static void domain_changed_state(const struct domain *d) spin_unlock(&dom_state_changed_lock); }+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; + + if ( info->pad0 || info->pad1 ) + return -EINVAL; + + if ( d ) + { + set_domain_state_info(info, d); + + return 0; + } + + while ( (dom = find_first_bit(dom_state_changed, DOMID_MASK + 1)) < + DOMID_FIRST_RESERVED ) + { + if ( test_and_clear_bit(dom, dom_state_changed) )For these two accesses to dom_state_changed don't you need to hold the lock patch 4 introduces? Also didn't you say you'd constrain the new sub-op to the sole domain having VIRQ_DOM_EXEC bound (which, ftaod, isn't enough to eliminate the race)?
Oh, indeed. Sorry for having missed both aspects.
+ { + *domid = dom; + + d = rcu_lock_domain_by_id(dom); + + if ( d ) + { + set_domain_state_info(info, d); + + rcu_unlock_domain(d); + }Oh, on the implicit "else" is where the original memset() would come into play: You want to make sure at least ->state, but perhaps also ->unique_id are cleared (rather than demanding the caller to clear them ahead of making the call).
Right. Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
