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

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

On 08/24/2017 08:39 AM, Jan Beulich wrote:
> > > > 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
This is really just a choice of whatever looks better.  There's a very minor
performance penalty from not calling current->domain over and over, but there
might also be a performance gain if current_has_perm is not inlined and the
call results in smaller code size.

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