From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= Subject: Re: [Qemu-devel] [PATCH for-1.4 v2] target-i386: kvm: prevent buffer overflow if -cpu foo, [x]level is too big Date: Mon, 28 Jan 2013 14:37:14 +0100 Message-ID: <51067F0A.7000309@suse.de> References: <1359373766-19201-1-git-send-email-imammedo@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: qemu-devel@nongnu.org, mtosatti@redhat.com, lersek@redhat.com, "kvm@vger.kernel.org list" , Gleb Natapov To: Igor Mammedov Return-path: Received: from cantor2.suse.de ([195.135.220.15]:33575 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754287Ab3A1NhY (ORCPT ); Mon, 28 Jan 2013 08:37:24 -0500 In-Reply-To: <1359373766-19201-1-git-send-email-imammedo@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Am 28.01.2013 12:49, schrieb Igor Mammedov: > Stack corruption may occur if too big 'level' or 'xlevel' values pass= ed > on command line with KVM enabled, due to limited size of cpuid_data > in kvm_arch_init_vcpu(). >=20 > reproduces with: > qemu -enable-kvm -cpu qemu64,level=3D4294967295 > or > qemu -enable-kvm -cpu qemu64,xlevel=3D4294967295 >=20 > Check if there is space in cpuid_data before passing it to cpu_x86_cp= uid() > or abort() if there is not space. >=20 > Signed-off-by: Igor Mammedov Reviewed-by: Andreas F=E4rber CC'ing Gleb and KVM list. Andreas > --- > * v2: > * use macro instead of const int max_cpuid_entries to fix build b= reakage > in C99 mode. Suggested-By: Laszlo Ersek > * compare with array index instead of address of the last element > Sugested-By: Marcelo Tosatti >=20 > --- > target-i386/kvm.c | 25 ++++++++++++++++++++++++- > 1 files changed, 24 insertions(+), 1 deletions(-) >=20 > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > index 3acff40..4ecb728 100644 > --- a/target-i386/kvm.c > +++ b/target-i386/kvm.c > @@ -411,11 +411,12 @@ static void cpu_update_state(void *opaque, int = running, RunState state) > } > } > =20 > +#define KVM_MAX_CPUID_ENTRIES 100 > int kvm_arch_init_vcpu(CPUState *cs) > { > struct { > struct kvm_cpuid2 cpuid; > - struct kvm_cpuid_entry2 entries[100]; > + struct kvm_cpuid_entry2 entries[KVM_MAX_CPUID_ENTRIES]; > } QEMU_PACKED cpuid_data; > X86CPU *cpu =3D X86_CPU(cs); > CPUX86State *env =3D &cpu->env; > @@ -502,6 +503,10 @@ int kvm_arch_init_vcpu(CPUState *cs) > cpu_x86_cpuid(env, 0, 0, &limit, &unused, &unused, &unused); > =20 > for (i =3D 0; i <=3D limit; i++) { > + if (cpuid_i =3D=3D KVM_MAX_CPUID_ENTRIES) { > + fprintf(stderr, "unsupported level value: 0x%x\n", limit= ); > + abort(); > + } > c =3D &cpuid_data.entries[cpuid_i++]; > =20 > switch (i) { > @@ -516,6 +521,11 @@ int kvm_arch_init_vcpu(CPUState *cs) > times =3D c->eax & 0xff; > =20 > for (j =3D 1; j < times; ++j) { > + if (cpuid_i =3D=3D KVM_MAX_CPUID_ENTRIES) { > + fprintf(stderr, "cpuid_data is full, no space fo= r " > + "cpuid(eax:2):eax & 0xf =3D 0x%x\n", tim= es); > + abort(); > + } > c =3D &cpuid_data.entries[cpuid_i++]; > c->function =3D i; > c->flags =3D KVM_CPUID_FLAG_STATEFUL_FUNC; > @@ -544,6 +554,11 @@ int kvm_arch_init_vcpu(CPUState *cs) > if (i =3D=3D 0xd && c->eax =3D=3D 0) { > continue; > } > + if (cpuid_i =3D=3D KVM_MAX_CPUID_ENTRIES) { > + fprintf(stderr, "cpuid_data is full, no space fo= r " > + "cpuid(eax:0x%x,ecx:0x%x)\n", i, j); > + abort(); > + } > c =3D &cpuid_data.entries[cpuid_i++]; > } > break; > @@ -557,6 +572,10 @@ int kvm_arch_init_vcpu(CPUState *cs) > cpu_x86_cpuid(env, 0x80000000, 0, &limit, &unused, &unused, &unu= sed); > =20 > for (i =3D 0x80000000; i <=3D limit; i++) { > + if (cpuid_i =3D=3D KVM_MAX_CPUID_ENTRIES) { > + fprintf(stderr, "unsupported xlevel value: 0x%x\n", limi= t); > + abort(); > + } > c =3D &cpuid_data.entries[cpuid_i++]; > =20 > c->function =3D i; > @@ -569,6 +588,10 @@ int kvm_arch_init_vcpu(CPUState *cs) > cpu_x86_cpuid(env, 0xC0000000, 0, &limit, &unused, &unused, = &unused); > =20 > for (i =3D 0xC0000000; i <=3D limit; i++) { > + if (cpuid_i =3D=3D KVM_MAX_CPUID_ENTRIES) { > + fprintf(stderr, "unsupported xlevel2 value: 0x%x\n",= limit); > + abort(); > + } > c =3D &cpuid_data.entries[cpuid_i++]; > =20 > c->function =3D i; >=20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrn= berg