* Re: [Qemu-devel] [PATCH for-1.4 v2] target-i386: kvm: prevent buffer overflow if -cpu foo, [x]level is too big
[not found] <1359373766-19201-1-git-send-email-imammedo@redhat.com>
@ 2013-01-28 13:37 ` Andreas Färber
0 siblings, 0 replies; only message in thread
From: Andreas Färber @ 2013-01-28 13:37 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, mtosatti, lersek, kvm@vger.kernel.org list,
Gleb Natapov
Am 28.01.2013 12:49, schrieb Igor Mammedov:
> Stack corruption may occur if too big 'level' or 'xlevel' values passed
> on command line with KVM enabled, due to limited size of cpuid_data
> in kvm_arch_init_vcpu().
>
> reproduces with:
> qemu -enable-kvm -cpu qemu64,level=4294967295
> or
> qemu -enable-kvm -cpu qemu64,xlevel=4294967295
>
> Check if there is space in cpuid_data before passing it to cpu_x86_cpuid()
> or abort() if there is not space.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Andreas Färber <afaerber@suse.de>
CC'ing Gleb and KVM list.
Andreas
> ---
> * v2:
> * use macro instead of const int max_cpuid_entries to fix build breakage
> in C99 mode. Suggested-By: Laszlo Ersek <lersek@redhat.com>
> * compare with array index instead of address of the last element
> Sugested-By: Marcelo Tosatti <mtosatti@redhat.com>
>
> ---
> target-i386/kvm.c | 25 ++++++++++++++++++++++++-
> 1 files changed, 24 insertions(+), 1 deletions(-)
>
> 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)
> }
> }
>
> +#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 = X86_CPU(cs);
> CPUX86State *env = &cpu->env;
> @@ -502,6 +503,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
> cpu_x86_cpuid(env, 0, 0, &limit, &unused, &unused, &unused);
>
> for (i = 0; i <= limit; i++) {
> + if (cpuid_i == KVM_MAX_CPUID_ENTRIES) {
> + fprintf(stderr, "unsupported level value: 0x%x\n", limit);
> + abort();
> + }
> c = &cpuid_data.entries[cpuid_i++];
>
> switch (i) {
> @@ -516,6 +521,11 @@ int kvm_arch_init_vcpu(CPUState *cs)
> times = c->eax & 0xff;
>
> for (j = 1; j < times; ++j) {
> + if (cpuid_i == KVM_MAX_CPUID_ENTRIES) {
> + fprintf(stderr, "cpuid_data is full, no space for "
> + "cpuid(eax:2):eax & 0xf = 0x%x\n", times);
> + abort();
> + }
> c = &cpuid_data.entries[cpuid_i++];
> c->function = i;
> c->flags = KVM_CPUID_FLAG_STATEFUL_FUNC;
> @@ -544,6 +554,11 @@ int kvm_arch_init_vcpu(CPUState *cs)
> if (i == 0xd && c->eax == 0) {
> continue;
> }
> + if (cpuid_i == KVM_MAX_CPUID_ENTRIES) {
> + fprintf(stderr, "cpuid_data is full, no space for "
> + "cpuid(eax:0x%x,ecx:0x%x)\n", i, j);
> + abort();
> + }
> c = &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, &unused);
>
> for (i = 0x80000000; i <= limit; i++) {
> + if (cpuid_i == KVM_MAX_CPUID_ENTRIES) {
> + fprintf(stderr, "unsupported xlevel value: 0x%x\n", limit);
> + abort();
> + }
> c = &cpuid_data.entries[cpuid_i++];
>
> c->function = i;
> @@ -569,6 +588,10 @@ int kvm_arch_init_vcpu(CPUState *cs)
> cpu_x86_cpuid(env, 0xC0000000, 0, &limit, &unused, &unused, &unused);
>
> for (i = 0xC0000000; i <= limit; i++) {
> + if (cpuid_i == KVM_MAX_CPUID_ENTRIES) {
> + fprintf(stderr, "unsupported xlevel2 value: 0x%x\n", limit);
> + abort();
> + }
> c = &cpuid_data.entries[cpuid_i++];
>
> c->function = i;
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] only message in thread