Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752832AbcDUOVN (ORCPT ); Thu, 21 Apr 2016 10:21:13 -0400 Received: from e06smtp15.uk.ibm.com ([195.75.94.111]:48279 "EHLO e06smtp15.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751697AbcDUOVK (ORCPT ); Thu, 21 Apr 2016 10:21:10 -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 v4 2/2] KVM: move vcpu id checking to archs From: Greg Kurz To: Paolo Bonzini , james.hogan@imgtec.com, mingo@redhat.com Cc: linux-mips@linux-mips.org, kvm@vger.kernel.org, rkrcmar@redhat.com, linux-kernel@vger.kernel.org, David Hildenbrand , qemu-ppc@nongnu.org, Cornelia Huck , Paul Mackerras , David Gibson Date: Thu, 21 Apr 2016 16:20:53 +0200 Message-ID: <146124811255.32509.17679765789502091772.stgit@bahia.huguette.org> In-Reply-To: <146124809455.32509.15232948272580716135.stgit@bahia.huguette.org> References: <146124809455.32509.15232948272580716135.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: 16042114-0021-0000-0000-0000339D25E1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3938 Lines: 104 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. While here, we also update the documentation to dissociate vcpu ids from the maximum number of vcpus per virtual machine. Acked-by: James Hogan Acked-by: Cornelia Huck Signed-off-by: Greg Kurz --- v4: - updated subject for more clarity on what the patch does - added James's and Connie's A-b tags - updated documentation Documentation/virtual/kvm/api.txt | 7 +++---- arch/mips/kvm/mips.c | 7 ++++++- arch/x86/kvm/x86.c | 3 +++ virt/kvm/kvm_main.c | 3 --- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index 4d0542c5206b..486a1d783b82 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -199,11 +199,10 @@ Type: vm ioctl Parameters: vcpu id (apic id on x86) Returns: vcpu fd on success, -1 on error -This API adds a vcpu to a virtual machine. The vcpu id is a small integer -in the range [0, max_vcpus). +This API adds a vcpu to a virtual machine. The vcpu id is a positive integer. -The recommended max_vcpus value can be retrieved using the KVM_CAP_NR_VCPUS of -the KVM_CHECK_EXTENSION ioctl() at run-time. +The recommended maximum number of vcpus (max_vcpus) can be retrieved using the +KVM_CAP_NR_VCPUS of the KVM_CHECK_EXTENSION ioctl() at run-time. The maximum possible value for max_vcpus can be retrieved using the KVM_CAP_MAX_VCPUS of the KVM_CHECK_EXTENSION ioctl() at run-time. 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);