Re: [Xen-devel] [PATCH v2] domctl: relax getdomaininfo permissions

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.08.16 at 19:07, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 05/08/16 14:54, Jan Beulich wrote:
>>>>> On 05.08.16 at 15:10, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 05/08/16 12:20, Jan Beulich wrote:
>>>> I wonder what good the duplication of the returned domain ID does: I'm
>>>> tempted to remove the one in the command-specific structure. Does
>>>> anyone have insight into why it was done that way?
>>> I wonder whether the first incarnation of this hypercall lacked a domid
>>> field in the returned structure?  It seems like the kind of thing which
>>> would be omitted, until the sysctl list version got introduced.
>> Oh, good point - that makes clear why the field can't be dropped:
>> That sysctl would break then.
> 
> Which domid were you referring to then?
> 
> The domid in the xen_domctl_getdomaininfo structure clearly needs to
> stay, but the domctl "op->domain = op->u.getdomaininfo.domain;"
> needn't.  OTOH, as we need to copy back the entire domctl structure
> anyway, it doesn't hurt to keep it.

The comment was about removal of the field, not just the
assignment. But as you did make obvious, the sysctl side needs
it to stay.

>>>> --- a/xen/include/xsm/dummy.h
>>>> +++ b/xen/include/xsm/dummy.h
>>>> @@ -61,7 +61,12 @@ static always_inline int xsm_default_act
>>>>          return 0;
>>>>      case XSM_TARGET:
>>>>          if ( src == target )
>>>> +        {
>>>>              return 0;
>>>> +    case XSM_XS_PRIV:
>>>> +            if ( src->is_xenstore )
>>>> +                return 0;
>>>> +        }
>>>>          /* fall through */
>>>>      case XSM_DM_PRIV:
>>>>          if ( target && src->target == target )
>>>> @@ -71,10 +76,6 @@ static always_inline int xsm_default_act
>>>>          if ( src->is_privileged )
>>>>              return 0;
>>>>          return -EPERM;
>>>> -    case XSM_XS_PRIV:
>>>> -        if ( src->is_xenstore || src->is_privileged )
>>>> -            return 0;
>>>> -        return -EPERM;
>>>>      default:
>>>>          LINKER_BUG_ON(1);
>>>>          return -EPERM;
>>> What is this change in relation to?  I can't see how it is related to
>>> the XSM changes mentioned in the commit, as that is strictly for the use
>>> of XSM_OTHER.
>> I don't see any XSM changes mentioned in the description, there
>> was only the XSM_OTHER related question outside the description.
>> Anyway - the change above is what guarantees the XSM_XS_PRIV
>> check, as invoked by xsm_domctl()'s XEN_DOMCTL_getdomaininfo
>> case, to fall through into XSM_DM_PRIV - after all that's what the
>> whole patch is about.
> 
> But the patch is about a qemu stubdom, which would be DM_PRIV, not XS_PRIV.

The point of the patch is to _extend_ permissions of this domctl
from XS_PRIV to DM_PRIV.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel