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

["Jan Beulich" <JBeulich@xxxxxxxx>]

First of all I think your Cc list is too short here - all of REST should be
included imo.

>>> On 31.07.18 at 20:23, <sstabellini@xxxxxxxxxx> wrote:
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -535,6 +535,21 @@ static XSM_INLINE int 
> xsm_map_gmfn_foreign(XSM_DEFAULT_ARG struct domain *d, str
>      return xsm_default_action(action, d, t);
>  }
>  
> +/*
> + * This action also requires that @current targets @d, but it has already 
> been
> + * checked somewhere higher in the call stack.

I'm not convinced it is a good idea to have such a dependency, even
more so with this cloudy a reference. If there's another XSM check
that has necessarily been done before, you should at least name it
here so it's easy to later verify that the assumption still holds. But
even better would imo be to re-do the check here, just in case.

> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -1198,6 +1198,17 @@ 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);
>  }
>  
> +/*
> + * This action also requires that @current has MMU__MAP_READ/WRITE over @d,
> + * but that has already been checked somewhere higher in the call stack (for
> + * example, by flask_add_to_physmap()).

This one's better in that it at least names the other hook. Still I'm
not fully convinced that omitting the other half of the check here
is prudent. We'll see what others think ...

Jan



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