Re: [PATCH 2/2] x86/altp2m: Add a hvmop for setting the suppress #VE bit

[Adrian Pop <apop@bitdefender.com>]

On Fri, Jun 16, 2017 at 02:39:10AM -0600, Jan Beulich wrote:
> >>> On 15.06.17 at 21:01, <tamas@tklengyel.com> wrote:
> > On Fri, Jun 9, 2017 at 10:51 AM, Adrian Pop <apop@bitdefender.com> wrote:
> >> --- a/xen/arch/x86/mm/mem_access.c
> >> +++ b/xen/arch/x86/mm/mem_access.c
> >> @@ -466,6 +466,58 @@ int p2m_get_mem_access(struct domain *d, gfn_t gfn, xenmem_access_t *access)
> >>  }
> >>
> >>  /*
> >> + * Set/clear the #VE suppress bit for a page.  Only available on VMX.
> >> + */
> >> +int p2m_set_suppress_ve(struct domain *d, gfn_t gfn, bool suppress_ve,
> >> +                        unsigned int altp2m_idx)
> >> +{
> >> +    struct p2m_domain *host_p2m = p2m_get_hostp2m(d);
> >> +    struct p2m_domain *ap2m = NULL;
> >> +    struct p2m_domain *p2m;
> >> +    mfn_t mfn;
> >> +    p2m_access_t a;
> >> +    p2m_type_t t;
> >> +    int rc;
> >> +
> >> +    if ( !cpu_has_vmx_virt_exceptions )
> >> +        return -EOPNOTSUPP;
> >> +
> >> +    /* This subop should only be used from a privileged domain. */
> >> +    if ( !current->domain->is_privileged )
> >> +        return -EINVAL;
> > 
> > This check looks wrong to me. If this subop should only be used by an
> > external (privileged) domain then I don't think this should be
> > implemented as an HVMOP, looks more like a domctl to me.
> 
> I think this wants to be an XSM_DM_PRIV check instead.

I'm not sure, but I expect that to not behave as intended security-wise
if Xen is compiled without XSM.  Would it?  It would be great if this
feature worked well without XSM too.

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