Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751676AbcDTSbt (ORCPT ); Wed, 20 Apr 2016 14:31:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34768 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750808AbcDTSbs (ORCPT ); Wed, 20 Apr 2016 14:31:48 -0400 Date: Wed, 20 Apr 2016 20:31:42 +0200 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Greg Kurz Cc: James Hogan , Paolo Bonzini , mingo@redhat.com, linux-mips@linux-mips.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, qemu-ppc@nongnu.org, Cornelia Huck , Paul Mackerras , David Gibson Subject: Re: [PATCH v3] KVM: remove buggy vcpu id check on vcpu creation Message-ID: <20160420183142.GA7202@potion> References: <146116689259.20666.15860134511726195550.stgit@bahia.huguette.org> <20160420170209.GA11071@potion> <20160420170924.GA7859@jhogan-linux.le.imgtec.org> <20160420172706.GA4044@potion> <20160420195329.23c8982c@bahia.huguette.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20160420195329.23c8982c@bahia.huguette.org> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1443 Lines: 39 2016-04-20 19:53+0200, Greg Kurz: > On Wed, 20 Apr 2016 19:27:06 +0200 > Radim Krčmář wrote: >> 2016-04-20 18:09+0100, James Hogan: >> > On Wed, Apr 20, 2016 at 07:02:10PM +0200, Radim Krčmář wrote: >> >> 2016-04-20 17:44+0200, Greg Kurz: >> >> > diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c >> >> > index 70ef1a43c114..0278ea146db5 100644 >> >> > --- a/arch/mips/kvm/mips.c >> >> > +++ b/arch/mips/kvm/mips.c >> >> > @@ -248,9 +248,14 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id) >> >> > int err, size, offset; >> >> > void *gebase; >> >> > int i; >> >> > + struct kvm_vcpu *vcpu; >> >> > >> >> > - struct kvm_vcpu *vcpu = kzalloc(sizeof(struct kvm_vcpu), GFP_KERNEL); >> >> > + if (id >= KVM_MAX_VCPUS) { >> >> > + err = -EINVAL; >> >> > + goto out; >> >> >> >> 'vcpu' looks undefined at this point, so kfree in 'out:' may bug. >> > >> > Thats out_free_cpu I think? >> >> My bad, it is. Thank you! >> > > I kept the goto based construct because it was done this way for kzalloc(). > but I agree that 'return ERR_PTR(-EINVAL)' may look more explicit. > > Worth a v4 ? No, it is consistent with kzalloc fault handling this way. I was going to queue it, but found an issue with kvm_get_vcpu_by_id() that would allow the guest to create multiple VCPUs with the same id, which led to an unfortunate discourse on KVM API. (Please see a new thread.)