Re: [PATCH v4 3/5] xen: add new domctl get_changed_domain

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.12.2024 12:12, Juergen Gross wrote:
> Add a new domctl sub-function to get data of a domain having changed
> state (this is needed by Xenstore).
> 
> The returned state just contains the domid, the domain unique id,
> and some flags (existing, shutdown, dying).
> 
> In order to enable Xenstore stubdom being built for multiple Xen
> versions, make this domctl stable.  For stable domctls the
> interface_version is always 0.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> # non-XSM/Flask
albeit ...

> +int get_domain_state(struct xen_domctl_get_domain_state *info, struct domain 
> *d,
> +                     domid_t *domid)
> +{
> +    unsigned int dom;
> +    int rc = -ENOENT;
> +
> +    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;
> +
> +    spin_lock(&dom_state_changed_lock);
> +
> +    if ( dom_state_changed )
> +    {
> +        dom = find_first_bit(dom_state_changed, DOMID_MASK + 1);
> +        if ( dom < DOMID_FIRST_RESERVED )
> +        {
> +            __clear_bit(dom, dom_state_changed);
> +
> +            *domid = dom;
> +
> +            d = rcu_lock_domain_by_id(dom);
> +
> +            if ( d )
> +            {
> +                set_domain_state_info(info, d);
> +
> +                rcu_unlock_domain(d);
> +            }
> +            else
> +            {
> +                info->state = 0;
> +                info->unique_id = 0;
> +            }

... memset() may be prudent to use on the "else" path, to avoid this becoming
a pitfall if/when one of the padding fields have some meaning assigned.

Jan