From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Durrant Subject: Re: [PATCH] x86/hvm: Allow the guest to permit the use of userspace hypercalls Date: Mon, 11 Jan 2016 14:32:07 +0000 Message-ID: <4bb76cf725844b92929bd6e10f9cc9dd@AMSPEX02CL03.citrite.net> References: <1452520774-16794-1-git-send-email-andrew.cooper3@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1452520774-16794-1-git-send-email-andrew.cooper3@citrix.com> Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Xen-devel Cc: Andrew Cooper , Stefano Stabellini , Ian Campbell , Jan Beulich List-Id: xen-devel@lists.xenproject.org > -----Original Message----- > From: xen-devel-bounces@lists.xen.org [mailto:xen-devel- > bounces@lists.xen.org] On Behalf Of Andrew Cooper > Sent: 11 January 2016 14:00 > To: Xen-devel > Cc: Andrew Cooper; Stefano Stabellini; Ian Campbell; Jan Beulich > Subject: [Xen-devel] [PATCH] x86/hvm: Allow the guest to permit the use of > userspace hypercalls > > Currently, hypercalls issued from HVM userspace will unconditionally fail with > -EPERM. > > This is inflexible, and a guest may wish to allow userspace to make > hypercalls. > > Introduce HVMOP_set_hypercall_dpl which allows the guest to alter the > permissions check for hypercalls. It behaves exactly like the dpl field for > GDT/LDT/IDT entries. > > As the dpl is initialised to 0, hypercalls are restricted to cpl0 code until > the OS explicitly chooses an alternative. > > Signed-off-by: Andrew Cooper > -- > CC: Jan Beulich > CC: Ian Campbell > CC: Stefano Stabellini > > Arm folks: Is something like this sufficiently generic to be useful on Arm, > perhaps with more generic naming? > > PV guest support for userspace hypercalls is substantially more involved, and > will take longer to complete. > --- > xen/arch/x86/hvm/hvm.c | 25 ++++++++++++++++++++++++- > xen/include/asm-x86/hvm/domain.h | 2 ++ > xen/include/public/hvm/hvm_op.h | 8 ++++++++ > 3 files changed, 34 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 21470ec..e5a08db 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -5228,7 +5228,8 @@ int hvm_do_hypercall(struct cpu_user_regs *regs) > case 4: > case 2: > hvm_get_segment_register(curr, x86_seg_ss, &sreg); > - if ( unlikely(sreg.attr.fields.dpl) ) > + if ( unlikely(sreg.attr.fields.dpl < > + currd->arch.hvm_domain.hypercall_dpl) ) Either I'm going mad, or that check should be '>' shouldn't it? (After all the test is currently effectively a test for > 0) > { > default: > regs->eax = -EPERM; > @@ -6839,6 +6840,28 @@ long do_hvm_op(unsigned long op, > XEN_GUEST_HANDLE_PARAM(void) arg) > rc = do_altp2m_op(arg); > break; > > + case HVMOP_set_hypercall_dpl: > + { > + xen_hvm_hypercall_dpl_t a; > + struct domain *d; > + > + if ( copy_from_guest(&a, arg, 1 ) ) > + return -EFAULT; > + > + rc = rcu_lock_remote_domain_by_id(a.domid, &d); > + if ( rc != 0 ) > + return rc; > + > + if ( current->domain != d ) > + return -EPERM; > + > + if ( !is_hvm_domain(d) || a.dpl > 3 ) > + return -EINVAL; > + > + d->arch.hvm_domain.hypercall_dpl = a.dpl; > + break; > + } > + > default: > { > gdprintk(XENLOG_DEBUG, "Bad HVM op %ld.\n", op); > diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm- > x86/hvm/domain.h > index a8cc2ad..006a142 100644 > --- a/xen/include/asm-x86/hvm/domain.h > +++ b/xen/include/asm-x86/hvm/domain.h > @@ -137,6 +137,8 @@ struct hvm_domain { > bool_t qemu_mapcache_invalidate; > bool_t is_s3_suspended; > > + uint32_t hypercall_dpl; > + Why burn 32-bits for a value that's only 2 bits wide? Paul > /* > * TSC value that VCPUs use to calculate their tsc_offset value. > * Used during initialization and save/restore. > diff --git a/xen/include/public/hvm/hvm_op.h > b/xen/include/public/hvm/hvm_op.h > index 1606185..f8247db 100644 > --- a/xen/include/public/hvm/hvm_op.h > +++ b/xen/include/public/hvm/hvm_op.h > @@ -489,6 +489,14 @@ struct xen_hvm_altp2m_op { > typedef struct xen_hvm_altp2m_op xen_hvm_altp2m_op_t; > DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_op_t); > > +#define HVMOP_set_hypercall_dpl 26 > +struct xen_hvm_hypercall_dpl { > + domid_t domid; > + uint16_t dpl; /* IN[1:0] cpl required to make hypercalls. */ > +}; > +typedef struct xen_hvm_hypercall_dpl xen_hvm_hypercall_dpl_t; > +DEFINE_XEN_GUEST_HANDLE(xen_hvm_hypercall_dpl_t); > + > #endif /* __XEN_PUBLIC_HVM_HVM_OP_H__ */ > > /* > -- > 2.1.4 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel