On 10.12.2024 16:52, Jürgen Groß wrote:
> On 09.12.24 18:04, Jan Beulich wrote:
>> On 06.12.2024 14:02, Juergen Gross wrote:
>>> --- a/xen/common/domain.c
>>> +++ b/xen/common/domain.c
>>> @@ -192,6 +192,54 @@ 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_dead )
>>> + info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DYING;
>>
>> The public constant saying "dying" isn't quite in line with the internal
>> constant saying "dead". It may well be that Xenstore only cares about the
>> "dead" state, but then it would better be nemaed this way also in the
>> public interface, I think.
>
> Okay, I'll rename it to "XEN_DOMCTL_GETDOMSTATE_STATE_DEAD".
Well, maybe have both DYING and DEAD, even if Xenstore right now needs only one?
>>> @@ -866,6 +873,15 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
>>> u_domctl)
>>> __HYPERVISOR_domctl, "h", u_domctl);
>>> break;
>>>
>>> + case XEN_DOMCTL_get_domain_state:
>>> + ret = xsm_get_domain_state(XSM_XS_PRIV, d);
>>> + if ( ret )
>>> + break;
>>> +
>>> + copyback = 1;
>>> + ret = get_domain_state(&op->u.get_domain_state, d, &op->domain);
>>> + break;
>>
>> Especially with this being a stable interface, surely the two padding fields
>> want checking to be zero on input (to possibly allow their future use for
>> something input-ish). Then even the memset() in the function may not really
>> be needed.
>
> I'll add the check. Removing the memset() is a little bit doubtful, as this
> might result in leaking hypervisor data e.g. in case a domain isn't existing
> (this will copy the internal struct to the user even in the -ENOENT case).
Which internal struct? The function is passed &op->... for both parameters.
And op is fully copied from guest space.
Jan
