Re: [Xen-devel] [PATCH 4/6] xsm: flask: change the interface and default policy for xsm_map_gmfn_foregin

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.08.17 at 13:33, <blackskygg@xxxxxxxxx> wrote:
> Hi Jan,
> 
> 2017-08-24 14:37 GMT+08:00 Jan Beulich <JBeulich@xxxxxxxx>:
>>>>> On 24.08.17 at 02:51, <blackskygg@xxxxxxxxx> wrote:
>>> 2017-08-23 17:55 GMT+08:00 Jan Beulich <JBeulich@xxxxxxxx>:
>>>>>>> On 22.08.17 at 20:08, <blackskygg@xxxxxxxxx> wrote:
>>>>> --- a/xen/include/xsm/dummy.h
>>>>> +++ b/xen/include/xsm/dummy.h
>>>>> @@ -525,10 +525,12 @@ static XSM_INLINE int 
> xsm_remove_from_physmap(XSM_DEFAULT_ARG struct domain *d1,
>>>>>      return xsm_default_action(action, d1, d2);
>>>>>  }
>>>>>
>>>>> -static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain 
> *d, struct domain *t)
>>>>> +static XSM_INLINE int xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain 
> *cd,
>>>>> +                                           struct domain *d, struct 
>>>>> domain 
> *t)
>>>>>  {
>>>>>      XSM_ASSERT_ACTION(XSM_TARGET);
>>>>> -    return xsm_default_action(action, d, t);
>>>>> +    return xsm_default_action(action, cd, d) ||
>>>>> +        xsm_default_action(action, cd, t);
>>>>>  }
>>>>
>>>> ... you use "or" here and ...
>>>
>>> This might be confusing. But think of returning 0 as "allowed", the
>>> only condition where this
>>> statement returns a 0 is when both calls return 0 -- so it's actually
>>> an "and". (Think of de-morgan's law.)
>>>
>>> But as Stefano has pointed out, I should preserve the error code.
>>
>> Ah, right - the code as written suggests boolean return values,
>> which gives it the wrong look. You really mean ?: instead of ||.
>> Why do you, btw, pass in current->domain (as cd) instead of
>> obtaining it here (just like various other hooks do)?
> 
> This was my original plan, but I'm not very sure about this, so I turn
> to Julien for help, and...
> Here is part of the irc log from a discussion with Julien on
> #xendevel, where Julien said:
> 
>    blackskygg: I think you want to pass the current domain in
> parameter, i.e having 3 domains argument.
>    because your solution only works when XSM is not enabled (this is
> the dummy callback)
>    when XSM is enabled, the policy would be specificed by the administrator
>    he needs to be able to know which domain was doing the configuration.

in flask/hooks.c there are quite a few uses of
avc_current_has_perm() in such cases, so I would think not
handing current->domain through the hook should be fine. But
of course Daniel may tell you I'm completely wrong here.

Jan


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