Re: [Xen-devel] [PATCH v4 2/7] xen: xsm: flask: introduce XENMAPSPACE_gmfn_share for memory sharing

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.02.18 at 19:11, <blackskygg@xxxxxxxxx> wrote:
> 2018-02-01 18:23 GMT+08:00 Jan Beulich <JBeulich@xxxxxxxx>:
>>>>> On 30.01.18 at 18:50, <blackskygg@xxxxxxxxx> wrote:
>>> --- a/xen/include/xsm/dummy.h
>>> +++ b/xen/include/xsm/dummy.h
>>> @@ -521,6 +521,12 @@ static XSM_INLINE int 
>>> xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *d, str
>>>      return xsm_default_action(action, d, t);
>>>  }
>>>
>>> +static XSM_INLINE int xsm_map_gmfn_share(XSM_DEFAULT_ARG struct domain *d, 
>>> struct domain *t)
>>> +{
>>> +    XSM_ASSERT_ACTION(XSM_TARGET);
>>> +    return xsm_default_action(action, current->domain, t);
>>
>> How does this represent a proper default equivalent of ...
>>
>> --- a/xen/xsm/flask/hooks.c
>> +++ b/xen/xsm/flask/hooks.c
>> @@ -1196,6 +1196,12 @@ static int flask_map_gmfn_foreign(struct domain *d, 
>> struct domain *t)
>>      return domain_has_perm(d, t, SECCLASS_MMU, MMU__MAP_READ | 
>> MMU__MAP_WRITE);
>>  }
>>
>> +static int flask_map_gmfn_share(struct domain *d, struct domain *t)
>> +{
>> +    return current_has_perm(t, SECCLASS_MMU, MMU__MAP_READ | 
>> MMU__MAP_WRITE) ?:
>> +        domain_has_perm(d, t, SECCLASS_MMU, MMU__SHARE_MEM);
>>
>> ... this?
> 
> The full flask check tries to guarantee that:
>   1) (c) over (d), which will always be checked somewhere higher in
>       the call stack for these kinds of calls;

This is in no way apparent here. At the very least a comment to
that effect should be added, but perhaps even better would be
if you had an explicit xsm_default_action(..., current->domain, d)
call there. Whether one or the other is preferable I'd leave to
Daniel.

>   2) (c) over (t), namely, MMU__MAP_READ/WRITE;
>   3) (d) over (t), namely, MMU__SHARE_MEM
> 
> In my default builtin actions, checks 1) and 2) are done by the
> xsm_default_action
> function, but I can't think of a way to do check 3), because we don't
> have the proper
> equivalence of MMU__SHARE_MEM (xsm_default_action is definitely not a 
> choice).
> Want to hear your and other maintainers' suggestions about how to do
> this properly.

"(d) over (t)" would suggest xsm_default_action(action, d, t), which
I don't think is appropriate here. In fact aiui d and t are unrelated to
one another in terms of mutual privilege. I don't think 3) needs
expressing in the dummy version; it's really the apparent lack of 1)
which I've been commenting on. But again, I'll leave it to Daniel to
tell you otherwise. What is imperative in any event is that you
extend the description to also reason about the dummy logic, at
least as long as it isn't a clear equivalent of the flask variant.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel