From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Auger Subject: Re: [PATCH 4/5] arm/arm64: KVM: Don't allow creating VCPUs after vgic_initialized Date: Wed, 10 Dec 2014 13:35:08 +0100 Message-ID: <54883DFC.4060001@linaro.org> References: <1418139844-27892-1-git-send-email-christoffer.dall@linaro.org> <1418139844-27892-5-git-send-email-christoffer.dall@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Christoffer Dall , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Return-path: Received: from mail-wi0-f170.google.com ([209.85.212.170]:33018 "EHLO mail-wi0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752932AbaLJMgb (ORCPT ); Wed, 10 Dec 2014 07:36:31 -0500 Received: by mail-wi0-f170.google.com with SMTP id bs8so13147325wib.5 for ; Wed, 10 Dec 2014 04:36:30 -0800 (PST) In-Reply-To: <1418139844-27892-5-git-send-email-christoffer.dall@linaro.org> Sender: kvm-owner@vger.kernel.org List-ID: On 12/09/2014 04:44 PM, Christoffer Dall wrote: > When the vgic initializes its internal state it does so based on the > number of VCPUs available at the time. If we allow KVM to create more > VCPUs after the VGIC has been initialized, we are likely to error out in > unfortunate ways later, perform buffer overflows etc. > > Cc: Eric Auger > Signed-off-by: Christoffer Dall > --- > This replaces Eric Auger's previous patch > (https://lists.cs.columbia.edu/pipermail/kvmarm/2014-December/012646.html), > because it fits better with testing to include it in this series and I > realized that we need to add a check against irqchip_in_kernel() as > well. > > arch/arm/kvm/arm.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c > index a9d005f..d4da244 100644 > --- a/arch/arm/kvm/arm.c > +++ b/arch/arm/kvm/arm.c > @@ -213,6 +213,11 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id) > int err; > struct kvm_vcpu *vcpu; > > + if (irqchip_in_kernel(kvm) && vgic_initialized(kvm)) { Reviewed-by: Eric Auger a question about that irqchip_in_kernel(kvm): kvm->arch.vgic.in_kernel is set in kvm_vgic_create but nobody resets it, especially in destroy, am i wrong? if the vgic is initialized shouldn't it be also created? Shouldn't we test irqchip_in_kernel in vgic_init instead? Also in case we need irqchip_in_kernel(kvm) here we might need it also in kvm_vgic_inject_irq because dist->lock is grabbed in vgic_update_irq_pending. Eric > + err = -EBUSY; > + goto out; > + } > + > vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL); > if (!vcpu) { > err = -ENOMEM; >