From patchwork Sat Jul 9 04:23:05 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Corneliu ZUZU X-Patchwork-Id: 9221997 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 530BB60572 for ; Sat, 9 Jul 2016 04:26:20 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 44FFE28622 for ; Sat, 9 Jul 2016 04:26:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 39C7A2862F; Sat, 9 Jul 2016 04:26:20 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 7A7CD28622 for ; Sat, 9 Jul 2016 04:26:19 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bLjo8-0003RP-2P; Sat, 09 Jul 2016 04:24:00 +0000 Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bLjo6-0003R2-PY for xen-devel@lists.xen.org; Sat, 09 Jul 2016 04:23:58 +0000 Received: from [85.158.137.68] by server-14.bemta-3.messagelabs.com id B6/1C-07949-D5C70875; Sat, 09 Jul 2016 04:23:57 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrJIsWRWlGSWpSXmKPExsUSfTxjoW5sTUO 4wYZX3BZLPi5mcWD0OLr7N1MAYxRrZl5SfkUCa0bDzo8sBVPNKo6dmcjUwHhHq4uRk0NIwF2i a9IZJgh7DaPElh/cEPZJRomNJ9MgbDeJw6d2M3cxcgHZ6xgldk/YzwKSYBPQljh36B5Ys4iAt MS1z5cZQYqYBf4wSuy83cMGkhAWSJX4vvIaUIKDg0VAVeLDDwGQMK+Ai8Sk/0fYQcISAnISCy 6kg4Q5BVwlzr19zQqx10Xixr5JYOMlBHIkVj6bBVUuJfG/VQlkk4TAUhaJh7tbGCFqZCQeTbz JNoFRaAEjwypGjeLUorLUIl0jQ72kosz0jJLcxMwcXUMDY73c1OLixPTUnMSkYr3k/NxNjMAg rGdgYNzB2LPX7xCjJAeTkijvBduGcCG+pPyUyozE4oz4otKc1OJDjDIcHEoSvJXVQDnBotT01 Iq0zBxgPMCkJTh4lER4J4OkeYsLEnOLM9MhUqcYFaXEeRtAEgIgiYzSPLg2WAxeYpSVEuZlZG BgEOIpSC3KzSxBlX/FKM7BqCTMuwBkCk9mXgnc9FdAi5mAFhsE1IMsLklESEk1MMY3nMlmSd1 0XyIiYbetc8anlS+lNu5WDLxSo3K5Z/kC17+XOmLVvQ/wbFbZ6DXzindv8pwFP1az5p6RdkhQ nzZ710RLjjUdhV8PlHZezAt+26l+1ChsRUPFk2dsepx/Vpu3l+9aVbZQOveVoOwaFnUVhpm7g 0P3Tk8L8P+2y3F68CnOWH/2XCWW4oxEQy3mouJEAEGKHxG8AgAA X-Env-Sender: czuzu@bitdefender.com X-Msg-Ref: server-7.tower-31.messagelabs.com!1468038236!42084607!1 X-Originating-IP: [91.199.104.161] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 8.46; banners=-,-,- X-VirusChecked: Checked Received: (qmail 18572 invoked from network); 9 Jul 2016 04:23:57 -0000 Received: from mx01.bbu.dsd.mx.bitdefender.com (HELO mx01.bbu.dsd.mx.bitdefender.com) (91.199.104.161) by server-7.tower-31.messagelabs.com with DHE-RSA-AES128-GCM-SHA256 encrypted SMTP; 9 Jul 2016 04:23:57 -0000 Received: (qmail 12216 invoked from network); 9 Jul 2016 07:23:56 +0300 Received: from unknown (HELO mx-sr.buh.bitdefender.com) (10.17.80.103) by mx01.bbu.dsd.mx.bitdefender.com with AES256-GCM-SHA384 encrypted SMTP; 9 Jul 2016 07:23:56 +0300 Received: from smtp03.buh.bitdefender.org (unknown [10.17.80.77]) by mx-sr.buh.bitdefender.com (Postfix) with ESMTP id 3FF817FBEA for ; Sat, 9 Jul 2016 07:23:56 +0300 (EEST) Received: (qmail 4650 invoked from network); 9 Jul 2016 07:23:56 +0300 Received: from 188-24-34-246.rdsnet.ro (HELO localhost.localdomain) (czuzu@bitdefender.com@188.24.34.246) by smtp03.buh.bitdefender.org with SMTP; 9 Jul 2016 07:23:56 +0300 From: Corneliu ZUZU To: xen-devel@lists.xen.org Date: Sat, 9 Jul 2016 07:23:05 +0300 Message-Id: <1468038185-7126-1-git-send-email-czuzu@bitdefender.com> X-Mailer: git-send-email 2.5.0 In-Reply-To: <1468037509-6428-1-git-send-email-czuzu@bitdefender.com> References: <1468037509-6428-1-git-send-email-czuzu@bitdefender.com> X-BitDefender-Scanner: Clean, Agent: BitDefender qmail 3.1.6 on smtp03.buh.bitdefender.org, sigver: 7.66242 X-BitDefender-Spam: No (0) X-BitDefender-SpamStamp: Build: [Engines: 2.15.6.911, Dats: 425852, Stamp: 3], Multi: [Enabled, t: (0.000014, 0.006698)], BW: [Enabled, t: (0.000008,0.000001)], RBL DNSBL: [Disabled], APM: [Enabled, Score: 500, t: (0.004948), Flags: 85D2ED72; NN_NO_CONTENT_TYPE; NN_LEGIT_SUMM_400_WORDS; NN_NO_LINK_NMD; NN_LEGIT_BITDEFENDER; NN_LEGIT_S_SQARE_BRACKETS; NN_LEGIT_MAILING_LIST_TO], SGN: [Enabled, t: (0.011106)], URL: [Enabled, t: (0.000005)], RTDA: [Enabled, t: (0.014020), Hit: No, Details: v2.3.10; Id: 2m1ghhh.1amv7hmgm.cqg5], total: 0(775) X-BitDefender-CF-Stamp: none Cc: Tamas K Lengyel , Razvan Cojocaru , George Dunlap , Andrew Cooper , Paul Durrant , Jan Beulich Subject: [Xen-devel] [PATCH 15/16] x86/monitor: fix: don't compromise a monitor_write_data with pending CR writes X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP The arch_vm_event.monitor structure is dynamically allocated and freed e.g. when the toolstack user calls xc_monitor_disable(), which in turn effectively discards any information that was in arch_vm_event.monitor->write_data. But this can yield unexpected behavior since if a CR-write was awaiting to be committed (non-zero write_data.writes_pending) on the scheduling tail (hvm_do_resume()->monitor_ctrlreg_write_data()) before xc_monitor_disable() is called, then the domain CR-write is wrongfully ignored, which of course, in these cases, can easily render a domain crash. To fix the issue, this patch: * makes arch_vm_event.monitor->emul_read_data dynamically allocated and only frees the entire arch_vm_event.monitor in monitor_cleanup_domain() if there are no pending CR-writes, otherwise it only frees emul_read_data * if there were pending CR-writes when monitor_cleanup_domain() was called, arch_vm_event.monitor will eventually be freed either after the CR-writes are committed (monitor_ctrlreg_write_data()) or when the domain/vcpu is destroyed * calls monitor_ctrlreg_write_data() even if the monitor subsystem is not initialized Signed-off-by: Corneliu ZUZU --- xen/arch/x86/hvm/emulate.c | 4 ++-- xen/arch/x86/hvm/hvm.c | 7 ++++++- xen/arch/x86/mm/p2m.c | 2 +- xen/arch/x86/monitor.c | 49 +++++++++++++++++++++++++++++++++++++++----- xen/include/asm-x86/domain.h | 3 +-- 5 files changed, 54 insertions(+), 11 deletions(-) diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c index 7a9c6af..e08d9c6 100644 --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -77,9 +77,9 @@ static int set_context_data(void *buffer, unsigned int size) if ( unlikely(monitor_domain_initialised(curr->domain)) ) { struct arch_vm_event_monitor *vem = curr->arch.vm_event->monitor; - unsigned int safe_size = min(size, vem->emul_read_data.size); + unsigned int safe_size = min(size, vem->emul_read_data->size); - memcpy(buffer, vem->emul_read_data.data, safe_size); + memcpy(buffer, vem->emul_read_data->data, safe_size); memset(buffer + safe_size, 0, size - safe_size); return X86EMUL_OKAY; } diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 1bfd9d4..a2568fd 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -488,9 +488,14 @@ void hvm_do_resume(struct vcpu *v) vem->emulate_flags = 0; } + } + /* + * Handle pending monitor CR-writes. Note that the monitor subsystem + * might be uninitialized. + */ + if ( unlikely(v->arch.vm_event && v->arch.vm_event->monitor) ) monitor_ctrlreg_write_data(v); - } /* Inject pending hw/sw trap */ if ( v->arch.hvm_vcpu.inject_trap.vector != -1 ) diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c index 92c0576..b280dc0 100644 --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -1645,7 +1645,7 @@ void p2m_mem_access_emulate_check(struct vcpu *v, vem->emulate_flags = violation ? rsp->flags : 0; if ( (rsp->flags & VM_EVENT_FLAG_SET_EMUL_READ_DATA) ) - vem->emul_read_data = rsp->data.emul_read_data; + *vem->emul_read_data = rsp->data.emul_read_data; } } diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c index c3123bc..05a2f0d 100644 --- a/xen/arch/x86/monitor.c +++ b/xen/arch/x86/monitor.c @@ -56,6 +56,7 @@ int monitor_init_domain(struct domain *d) { int rc = 0; struct vcpu *v; + struct arch_vm_event_monitor *vem; /* Already initialised? */ if ( unlikely(monitor_domain_initialised(d)) ) @@ -65,10 +66,22 @@ int monitor_init_domain(struct domain *d) for_each_vcpu(d, v) { ASSERT(v->arch.vm_event); - ASSERT(!v->arch.vm_event->monitor); - v->arch.vm_event->monitor = xzalloc(struct arch_vm_event_monitor); - if ( unlikely(!v->arch.vm_event->monitor) ) + if ( likely(!v->arch.vm_event->monitor) ) + { + v->arch.vm_event->monitor = xzalloc(struct arch_vm_event_monitor); + if ( unlikely(!v->arch.vm_event->monitor) ) + { + rc = -ENOMEM; + goto err; + } + } + + vem = v->arch.vm_event->monitor; + + ASSERT(!vem->emul_read_data); + vem->emul_read_data = xzalloc(struct vm_event_emul_read_data); + if ( unlikely(!vem->emul_read_data) ) { rc = -ENOMEM; goto err; @@ -100,6 +113,7 @@ err: void monitor_cleanup_domain(struct domain *d) { struct vcpu *v; + struct arch_vm_event_monitor *vem; xfree(d->arch.monitor.msr_bitmap); d->arch.monitor.msr_bitmap = NULL; @@ -109,8 +123,22 @@ void monitor_cleanup_domain(struct domain *d) if ( unlikely(!v->arch.vm_event || !v->arch.vm_event->monitor) ) continue; - xfree(v->arch.vm_event->monitor); - v->arch.vm_event->monitor = NULL; + vem = v->arch.vm_event->monitor; + xfree(vem->emul_read_data); + vem->emul_read_data = NULL; + + /* + * Only invalidate write_data if no CR-writes are pending or domain is + * dead (domain_kill(d) code-path). If CR-writes are pending, write_data + * will eventually be freed when those writes are committed (see + * monitor_ctrlreg_write_data() function). + */ + if ( likely(!vem->write_data.writes_pending) || + unlikely(DOMDYING_dead == d->is_dying) ) + { + xfree(vem); + v->arch.vm_event->monitor = NULL; + } } monitor_ctrlreg_disable_traps(d); @@ -192,6 +220,17 @@ void monitor_ctrlreg_write_data(struct vcpu *v) hvm_set_cr3(w->cr3, 0); w->do_write.cr3 = 0; } + + /* + * Was the monitor subsystem actually uninitialized + * and only waiting for pending CR-writes? + */ + if ( unlikely(!monitor_domain_initialised(v->domain)) ) + { + xfree(v->arch.vm_event->monitor); + xfree(v->arch.vm_event); + v->arch.vm_event = NULL; + } } static unsigned long *monitor_bitmap_for_msr(const struct domain *d, u32 *msr) diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h index 7663da2..e337cb3 100644 --- a/xen/include/asm-x86/domain.h +++ b/xen/include/asm-x86/domain.h @@ -9,7 +9,6 @@ #include #include #include -#include #include #define has_32bit_shinfo(d) ((d)->arch.has_32bit_shinfo) @@ -510,7 +509,7 @@ typedef enum __packed { */ struct arch_vm_event_monitor { uint32_t emulate_flags; - struct vm_event_emul_read_data emul_read_data; + struct vm_event_emul_read_data *emul_read_data; struct monitor_write_data write_data; };