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

[Adrian Pop <apop@bitdefender.com>]

On Thu, Jun 22, 2017 at 06:13:22AM -0600, Jan Beulich wrote:
> >>> On 22.06.17 at 14:04, <apop@bitdefender.com> wrote:
> > 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.
> 
> Well, without you explaining why you think this wouldn't work
> without XSM, I don't really know what to answer. I suppose
> you've grep-ed for other uses of this and/or other XSM_* values,
> finding that these exist in various places where all is fine without
> XSM?

I'll check what it does then because it's not very clear to me either.

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