Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752888AbcDTPpF (ORCPT ); Wed, 20 Apr 2016 11:45:05 -0400 Received: from e06smtp07.uk.ibm.com ([195.75.94.103]:41456 "EHLO e06smtp07.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751520AbcDTPpC (ORCPT ); Wed, 20 Apr 2016 11:45:02 -0400 X-IBM-Helo: d06dlp02.portsmouth.uk.ibm.com X-IBM-MailFrom: gkurz@linux.vnet.ibm.com X-IBM-RcptTo: kvm@vger.kernel.org;linux-kernel@vger.kernel.org Subject: [PATCH v3] KVM: remove buggy vcpu id check on vcpu creation From: Greg Kurz To: Paolo Bonzini , james.hogan@imgtec.com, mingo@redhat.com Cc: linux-mips@linux-mips.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, qemu-ppc@nongnu.org, Cornelia Huck , Paul Mackerras , David Gibson Date: Wed, 20 Apr 2016 17:44:54 +0200 Message-ID: <146116689259.20666.15860134511726195550.stgit@bahia.huguette.org> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16042015-0029-0000-0000-000013CC26B6 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2594 Lines: 78 Commit 338c7dbadd26 ("KVM: Improve create VCPU parameter (CVE-2013-4587)") introduced a check to prevent potential kernel memory corruption in case the vcpu id is too great. Unfortunately this check assumes vcpu ids grow in sequence with a common difference of 1, which is wrong: archs are free to use vcpu id as they fit. For example, QEMU originated vcpu ids for PowerPC cpus running in boot3s_hv mode, can grow with a common difference of 2, 4 or 8: if KVM_MAX_VCPUS is 1024, guests may be limited down to 128 vcpus on POWER8. This means the check does not belong here and should be moved to some arch specific function: kvm_arch_vcpu_create() looks like a good candidate. ARM and s390 already have such a check. I could not spot any path in the PowerPC or common KVM code where a vcpu id is used as described in the above commit: I believe PowerPC can live without this check. In the end, this patch simply moves the check to MIPS and x86. Signed-off-by: Greg Kurz --- v3: use ERR_PTR() arch/mips/kvm/mips.c | 7 ++++++- arch/x86/kvm/x86.c | 3 +++ virt/kvm/kvm_main.c | 3 --- 3 files changed, 9 insertions(+), 4 deletions(-) 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 = kzalloc(sizeof(struct kvm_vcpu), GFP_KERNEL); if (!vcpu) { err = -ENOMEM; goto out; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 9b7798c7b210..7738202edcce 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -7358,6 +7358,9 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, { struct kvm_vcpu *vcpu; + if (id >= KVM_MAX_VCPUS) + return ERR_PTR(-EINVAL); + if (check_tsc_unstable() && atomic_read(&kvm->online_vcpus) != 0) printk_once(KERN_WARNING "kvm: SMP vm created on host with unstable TSC; " diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 4fd482fb9260..6b6cca3cb488 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -2272,9 +2272,6 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id) int r; struct kvm_vcpu *vcpu; - if (id >= KVM_MAX_VCPUS) - return -EINVAL; - vcpu = kvm_arch_vcpu_create(kvm, id); if (IS_ERR(vcpu)) return PTR_ERR(vcpu);