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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

>
>>> --- 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.

~Andrew

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