From patchwork Wed Jan 27 19:13:04 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Cooper X-Patchwork-Id: 8138721 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id DF048BEEE5 for ; Wed, 27 Jan 2016 19:16:20 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D268120251 for ; Wed, 27 Jan 2016 19:16:15 +0000 (UTC) Received: from lists.xen.org (lists.xenproject.org [50.57.142.19]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 16A872022D for ; Wed, 27 Jan 2016 19:16:14 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xen.org) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aOVWi-0003Sr-2E; Wed, 27 Jan 2016 19:13:12 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aOVWg-0003Sm-4r for xen-devel@lists.xen.org; Wed, 27 Jan 2016 19:13:10 +0000 Received: from [85.158.139.211] by server-9.bemta-5.messagelabs.com id CA/49-29478-5C619A65; Wed, 27 Jan 2016 19:13:09 +0000 X-Env-Sender: prvs=82734b35b=Andrew.Cooper3@citrix.com X-Msg-Ref: server-6.tower-206.messagelabs.com!1453921986!18322483!1 X-Originating-IP: [66.165.176.63] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogNjYuMTY1LjE3Ni42MyA9PiAzMDYwNDg=\n, received_headers: No Received headers X-StarScan-Received: X-StarScan-Version: 7.35.1; banners=-,-,- X-VirusChecked: Checked Received: (qmail 49470 invoked from network); 27 Jan 2016 19:13:08 -0000 Received: from smtp02.citrix.com (HELO SMTP02.CITRIX.COM) (66.165.176.63) by server-6.tower-206.messagelabs.com with RC4-SHA encrypted SMTP; 27 Jan 2016 19:13:08 -0000 X-IronPort-AV: E=Sophos;i="5.22,356,1449532800"; d="scan'208";a="334386691" From: Andrew Cooper To: Xen-devel Date: Wed, 27 Jan 2016 19:13:04 +0000 Message-ID: <1453921984-29197-1-git-send-email-andrew.cooper3@citrix.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <56A91122.50502@oracle.com> References: <56A91122.50502@oracle.com> MIME-Version: 1.0 X-DLP: MIA1 Cc: Kevin Tian , Jan Beulich , Andrew Cooper , Aravind Gopalakrishnan , Jun Nakajima , Boris Ostrovsky , Suravee Suthikulpanit Subject: [Xen-devel] [PATCH v2 2/2] x86/hvm: Don't intercept #UD exceptions in general X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP c/s 0f1cb96e "x86 hvm: Allow cross-vendor migration" caused HVM domains to unconditionally intercept #UD exceptions. While cross-vendor migration is cool as a demo, it is extremely niche. Intercepting #UD allows userspace code in a multi-vcpu guest to execute arbitrary instructions in the x86 emulator by having one thread execute a ud2a instruction, and having a second thread rewrite the instruction before the emulator performs an instruction fetch. XSAs 105, 106 and 110 are all examples where guest userspace can use bugs in the x86 emulator to compromise security of the domain, either by privilege escalation or causing a crash. c/s 2d67a7a4 "x86: synchronize PCI config space access decoding" introduced (amongst other things) a per-domain vendor, based on the guests cpuid policy. Use the per-guest vendor to enable #UD interception only when a domain is configured for a vendor different to the current hardware. (#UD interception is also enabled if hvm_fep is specified on the Xen command line. This is a debug-only option whose entire purpose is for testing the x86 emulator.) As a result, the overwhelming majority of usecases now have #UD interception disabled, removing an attack surface for malicious guest userspace. Signed-off-by: Andrew Cooper Reviewed-by: Boris Ostrovsky --- CC: Jan Beulich CC: Jun Nakajima CC: Kevin Tian CC: Boris Ostrovsky CC: Suravee Suthikulpanit CC: Aravind Gopalakrishnan v2: * Pause the domain while updating cpuid information. In practice, the set_cpuid hypercall is only made during domain construction. * Use vmcb_{get,set}_exception_intercepts() to provide appropriate manipulation of the clean bits. --- xen/arch/x86/domctl.c | 19 +++++++++++++++++++ xen/arch/x86/hvm/hvm.c | 6 ++---- xen/arch/x86/hvm/svm/svm.c | 16 ++++++++++++++++ xen/arch/x86/hvm/svm/vmcb.c | 1 + xen/arch/x86/hvm/vmx/vmcs.c | 1 + xen/arch/x86/hvm/vmx/vmx.c | 15 +++++++++++++++ xen/include/asm-x86/hvm/hvm.h | 15 ++++++++++++++- 7 files changed, 68 insertions(+), 5 deletions(-) diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c index 1d71216..316e13a 100644 --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -65,8 +65,18 @@ static void update_domain_cpuid_info(struct domain *d, .ecx = ctl->ecx } }; + int old_vendor = d->arch.x86_vendor; d->arch.x86_vendor = get_cpu_vendor(vendor_id.str, gcv_guest); + + if ( is_hvm_domain(d) && (d->arch.x86_vendor != old_vendor) ) + { + struct vcpu *v; + + for_each_vcpu( d, v ) + hvm_update_guest_vendor(v); + } + break; } @@ -707,6 +717,12 @@ long arch_do_domctl( xen_domctl_cpuid_t *ctl = &domctl->u.cpuid; cpuid_input_t *cpuid, *unused = NULL; + if ( d == currd ) /* no domain_pause() */ + { + ret = -EINVAL; + break; + } + for ( i = 0; i < MAX_CPUID_INPUT; i++ ) { cpuid = &d->arch.cpuids[i]; @@ -724,6 +740,8 @@ long arch_do_domctl( break; } + domain_pause(d); + if ( i < MAX_CPUID_INPUT ) *cpuid = *ctl; else if ( unused ) @@ -734,6 +752,7 @@ long arch_do_domctl( if ( !ret ) update_domain_cpuid_info(d, ctl); + domain_unpause(d); break; } diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 674feea..7a15d49 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -93,12 +93,10 @@ unsigned long __section(".bss.page_aligned") static bool_t __initdata opt_hap_enabled = 1; boolean_param("hap", opt_hap_enabled); -#ifndef NDEBUG +#ifndef opt_hvm_fep /* Permit use of the Forced Emulation Prefix in HVM guests */ -static bool_t opt_hvm_fep; +bool_t opt_hvm_fep; boolean_param("hvm_fep", opt_hvm_fep); -#else -#define opt_hvm_fep 0 #endif /* Xen command-line option to enable altp2m */ diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index 953e0b5..e62dfa1 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -597,6 +597,21 @@ static void svm_update_guest_efer(struct vcpu *v) vmcb_set_efer(vmcb, new_efer); } +static void svm_update_guest_vendor(struct vcpu *v) +{ + struct arch_svm_struct *arch_svm = &v->arch.hvm_svm; + struct vmcb_struct *vmcb = arch_svm->vmcb; + u32 bitmap = vmcb_get_exception_intercepts(vmcb); + + if ( opt_hvm_fep || + (v->domain->arch.x86_vendor != boot_cpu_data.x86_vendor) ) + bitmap |= (1U << TRAP_invalid_op); + else + bitmap &= ~(1U << TRAP_invalid_op); + + vmcb_set_exception_intercepts(vmcb, bitmap); +} + static void svm_sync_vmcb(struct vcpu *v) { struct arch_svm_struct *arch_svm = &v->arch.hvm_svm; @@ -2245,6 +2260,7 @@ static struct hvm_function_table __initdata svm_function_table = { .get_shadow_gs_base = svm_get_shadow_gs_base, .update_guest_cr = svm_update_guest_cr, .update_guest_efer = svm_update_guest_efer, + .update_guest_vendor = svm_update_guest_vendor, .set_guest_pat = svm_set_guest_pat, .get_guest_pat = svm_get_guest_pat, .set_tsc_offset = svm_set_tsc_offset, diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c index 9ea014f..be2dc32 100644 --- a/xen/arch/x86/hvm/svm/vmcb.c +++ b/xen/arch/x86/hvm/svm/vmcb.c @@ -192,6 +192,7 @@ static int construct_vmcb(struct vcpu *v) vmcb->_exception_intercepts = HVM_TRAP_MASK + | (opt_hvm_fep ? (1U << TRAP_invalid_op) : 0) | (1U << TRAP_no_device); if ( paging_mode_hap(v->domain) ) diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c index 5bc3c74..a12813a 100644 --- a/xen/arch/x86/hvm/vmx/vmcs.c +++ b/xen/arch/x86/hvm/vmx/vmcs.c @@ -1237,6 +1237,7 @@ static int construct_vmcs(struct vcpu *v) v->arch.hvm_vmx.exception_bitmap = HVM_TRAP_MASK | (paging_mode_hap(d) ? 0 : (1U << TRAP_page_fault)) + | (opt_hvm_fep ? (1U << TRAP_invalid_op) : 0) | (1U << TRAP_no_device); vmx_update_exception_bitmap(v); diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 4f9951f..195def6 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -73,6 +73,7 @@ static void vmx_free_vlapic_mapping(struct domain *d); static void vmx_install_vlapic_mapping(struct vcpu *v); static void vmx_update_guest_cr(struct vcpu *v, unsigned int cr); static void vmx_update_guest_efer(struct vcpu *v); +static void vmx_update_guest_vendor(struct vcpu *v); static void vmx_cpuid_intercept( unsigned int *eax, unsigned int *ebx, unsigned int *ecx, unsigned int *edx); @@ -398,6 +399,19 @@ void vmx_update_exception_bitmap(struct vcpu *v) __vmwrite(EXCEPTION_BITMAP, bitmap); } +static void vmx_update_guest_vendor(struct vcpu *v) +{ + if ( opt_hvm_fep || + (v->domain->arch.x86_vendor != boot_cpu_data.x86_vendor) ) + v->arch.hvm_vmx.exception_bitmap |= (1U << TRAP_invalid_op); + else + v->arch.hvm_vmx.exception_bitmap &= ~(1U << TRAP_invalid_op); + + vmx_vmcs_enter(v); + vmx_update_exception_bitmap(v); + vmx_vmcs_exit(v); +} + static int vmx_guest_x86_mode(struct vcpu *v) { unsigned long cs_ar_bytes; @@ -1963,6 +1977,7 @@ static struct hvm_function_table __initdata vmx_function_table = { .update_host_cr3 = vmx_update_host_cr3, .update_guest_cr = vmx_update_guest_cr, .update_guest_efer = vmx_update_guest_efer, + .update_guest_vendor = vmx_update_guest_vendor, .set_guest_pat = vmx_set_guest_pat, .get_guest_pat = vmx_get_guest_pat, .set_tsc_offset = vmx_set_tsc_offset, diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h index a87224b..0b15616 100644 --- a/xen/include/asm-x86/hvm/hvm.h +++ b/xen/include/asm-x86/hvm/hvm.h @@ -28,6 +28,13 @@ #include #include +#ifndef NDEBUG +/* Permit use of the Forced Emulation Prefix in HVM guests */ +extern bool_t opt_hvm_fep; +#else +#define opt_hvm_fep 0 +#endif + /* Interrupt acknowledgement sources. */ enum hvm_intsrc { hvm_intsrc_none, @@ -136,6 +143,8 @@ struct hvm_function_table { void (*update_guest_cr)(struct vcpu *v, unsigned int cr); void (*update_guest_efer)(struct vcpu *v); + void (*update_guest_vendor)(struct vcpu *v); + int (*get_guest_pat)(struct vcpu *v, u64 *); int (*set_guest_pat)(struct vcpu *v, u64); @@ -316,6 +325,11 @@ static inline void hvm_update_guest_efer(struct vcpu *v) hvm_funcs.update_guest_efer(v); } +static inline void hvm_update_guest_vendor(struct vcpu *v) +{ + hvm_funcs.update_guest_vendor(v); +} + /* * Called to ensure than all guest-specific mappings in a tagged TLB are * flushed; does *not* flush Xen's TLB entries, and on processors without a @@ -387,7 +401,6 @@ static inline int hvm_event_pending(struct vcpu *v) /* These exceptions must always be intercepted. */ #define HVM_TRAP_MASK ((1U << TRAP_debug) | \ - (1U << TRAP_invalid_op) | \ (1U << TRAP_alignment_check) | \ (1U << TRAP_machine_check))