From patchwork Fri Oct 31 05:30:35 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tiejun Chen X-Patchwork-Id: 5202311 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 2FD76C11AC for ; Fri, 31 Oct 2014 05:31:02 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 3AED620173 for ; Fri, 31 Oct 2014 05:31:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4495A2011D for ; Fri, 31 Oct 2014 05:31:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753002AbaJaFak (ORCPT ); Fri, 31 Oct 2014 01:30:40 -0400 Received: from mga09.intel.com ([134.134.136.24]:10215 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750961AbaJaFaj (ORCPT ); Fri, 31 Oct 2014 01:30:39 -0400 Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga102.jf.intel.com with ESMTP; 30 Oct 2014 22:29:12 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,292,1413270000"; d="scan'208";a="599533882" Received: from tiejunch-mobl.ccr.corp.intel.com (HELO [10.238.128.116]) ([10.238.128.116]) by orsmga001.jf.intel.com with ESMTP; 30 Oct 2014 22:30:36 -0700 Message-ID: <54531E7B.1040006@intel.com> Date: Fri, 31 Oct 2014 13:30:35 +0800 From: "Chen, Tiejun" User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Wanpeng Li , Paolo Bonzini CC: Gleb Natapov , Alexei Starovoitov , Takashi Iwai , kvm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] KVM: x86: fix access memslots w/o hold srcu read lock References: <1414730012-7278-1-git-send-email-wanpeng.li@linux.intel.com> In-Reply-To: <1414730012-7278-1-git-send-email-wanpeng.li@linux.intel.com> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, 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 On 2014/10/31 12:33, Wanpeng Li wrote: > The srcu read lock must be held while accessing memslots (e.g. > when using gfn_to_* functions), however, commit c24ae0dcd3e8 > ("kvm: x86: Unpin and remove kvm_arch->apic_access_page") call > gfn_to_page() in kvm_vcpu_reload_apic_access_page() w/o hold it > which leads to suspicious rcu_dereference_check() usage warning. > This patch fix it by holding srcu read lock when call gfn_to_page() > in kvm_vcpu_reload_apic_access_page() function. > > > [ INFO: suspicious RCU usage. ] > 3.18.0-rc2-test2+ #70 Not tainted > ------------------------------- > include/linux/kvm_host.h:474 suspicious rcu_dereference_check() usage! > > other info that might help us debug this: > > rcu_scheduler_active = 1, debug_locks = 0 > 1 lock held by qemu-system-x86/2371: > #0: (&vcpu->mutex){+.+...}, at: [] vcpu_load+0x20/0xd0 [kvm] > > stack backtrace: > CPU: 4 PID: 2371 Comm: qemu-system-x86 Not tainted 3.18.0-rc2-test2+ #70 > Hardware name: Dell Inc. OptiPlex 9010/0M9KCM, BIOS A12 01/10/2013 > 0000000000000001 ffff880209983ca8 ffffffff816f514f 0000000000000000 > ffff8802099b8990 ffff880209983cd8 ffffffff810bd687 00000000000fee00 > ffff880208a2c000 ffff880208a10000 ffff88020ef50040 ffff880209983d08 > Call Trace: > [] dump_stack+0x4e/0x71 > [] lockdep_rcu_suspicious+0xe7/0x120 > [] gfn_to_memslot+0xd5/0xe0 [kvm] > [] __gfn_to_pfn+0x33/0x60 [kvm] > [] gfn_to_page+0x25/0x90 [kvm] > [] kvm_vcpu_reload_apic_access_page+0x3c/0x80 [kvm] > [] vmx_vcpu_reset+0x20c/0x460 [kvm_intel] > [] kvm_vcpu_reset+0x15e/0x1b0 [kvm] > [] kvm_arch_vcpu_setup+0x2c/0x50 [kvm] > [] kvm_vm_ioctl+0x1d0/0x780 [kvm] > [] ? __lock_is_held+0x54/0x80 > [] do_vfs_ioctl+0x300/0x520 > [] ? __fget+0x5/0x250 > [] ? __fget_light+0x2a/0xe0 > [] SyS_ioctl+0x81/0xa0 > [] system_call_fastpath+0x16/0x1b > > Reported-by: Takashi Iwai > Reported-by: Alexei Starovoitov > Signed-off-by: Wanpeng Li > --- > arch/x86/kvm/x86.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 0033df3..2d97329 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6059,6 +6059,7 @@ static void kvm_vcpu_flush_tlb(struct kvm_vcpu *vcpu) > void kvm_vcpu_reload_apic_access_page(struct kvm_vcpu *vcpu) > { > struct page *page = NULL; > + int idx; > > if (!irqchip_in_kernel(vcpu->kvm)) > return; > @@ -6066,7 +6067,9 @@ void kvm_vcpu_reload_apic_access_page(struct kvm_vcpu *vcpu) > if (!kvm_x86_ops->set_apic_access_page_addr) > return; > > + idx = srcu_read_lock(&vcpu->kvm->srcu); There's another scenario that we already hold srcu before call kvm_vcpu_reload_apic_access_page(), __vcpu_run() | + vcpu->srcu_idx = srcu_read_lock(&kvm->srcu); + r = vcpu_enter_guest(vcpu); | + kvm_vcpu_reload_apic_access_page(vcpu); So according to backtrace I think we should fix as follows: kvm: x86: vmx: hold kvm->srcu while reload apic access page kvm_vcpu_reload_apic_access_page() needs to access memslots via gfn_to_page(), so its necessary to hold kvm->srcu. Signed-off-by: Tiejun Chen --- arch/x86/kvm/vmx.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index b25a588..9fa1f46 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -4442,6 +4442,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu) { struct vcpu_vmx *vmx = to_vmx(vcpu); struct msr_data apic_base_msr; + int idx; vmx->rmode.vm86_active = 0; @@ -4509,7 +4510,9 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu) vmcs_write32(TPR_THRESHOLD, 0); } + idx = srcu_read_lock(&vcpu->kvm->srcu); kvm_vcpu_reload_apic_access_page(vcpu); + srcu_read_unlock(&vcpu->kvm->srcu, idx); if (vmx_vm_has_apicv(vcpu->kvm)) memset(&vmx->pi_desc, 0, sizeof(struct pi_desc));