From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Xu Subject: Re: [PATCH v1 01/22] KVM: x86: race between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP Date: Wed, 15 Mar 2017 14:24:42 +0800 Message-ID: <20170315062442.GI12964@pxdev.xzpeter.org> References: <20170314133450.13259-1-david@redhat.com> <20170314133450.13259-2-david@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Cc: kvm@vger.kernel.org, Paolo Bonzini , rkrcmar@redhat.com To: David Hildenbrand Return-path: Received: from mx1.redhat.com ([209.132.183.28]:51400 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751108AbdCOGYs (ORCPT ); Wed, 15 Mar 2017 02:24:48 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 950BB15561 for ; Wed, 15 Mar 2017 06:24:48 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20170314133450.13259-2-david@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, Mar 14, 2017 at 02:34:29PM +0100, David Hildenbrand wrote: > Avoid races between KVM_SET_GSI_ROUTING and KVM_CREATE_IRQCHIP by taking > the kvm->lock when setting up routes. > > If KVM_CREATE_IRQCHIP fails, KVM_SET_GSI_ROUTING could have already set > up routes pointing at pic/ioapic, being silently removed already. > > Signed-off-by: David Hildenbrand > --- > virt/kvm/kvm_main.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index a17d787..ad0f8b2 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -3079,8 +3079,11 @@ static long kvm_vm_ioctl(struct file *filp, > routing.nr * sizeof(*entries))) > goto out_free_irq_routing; > } > + /* avoid races with KVM_CREATE_IRQCHIP on x86 */ > + mutex_lock(&kvm->lock); > r = kvm_set_irq_routing(kvm, entries, routing.nr, > routing.flags); > + mutex_unlock(&kvm->lock); > out_free_irq_routing: > vfree(entries); > break; > -- > 2.9.3 > Out of my curiousity: do we have a use case that these two operations might collapse (KVM_CREATE_IRQCHIP and KVM_SET_GSI_ROUTING)? Or is this patch only for the sake of security? Another thing to mention is that, I guess adding this lock will benefit KVM_CAP_SPLIT_IRQCHIP as well, maybe nice to mention it too in the commit message. No worth a repost for this single reason though. Thanks, -- peterx