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?
> + 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.
Jan
