From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH for 1.7] kvm: Fix uninitialized cpuid_data Date: Thu, 7 Nov 2013 13:15:16 +0200 Message-ID: <20131107111516.GB7513@redhat.com> References: <1383773727-29327-1-git-send-email-sw@weilnetz.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: qemu-devel , Marcelo Tosatti , Paolo Bonzini , kvm@vger.kernel.org To: Stefan Weil Return-path: Received: from mx1.redhat.com ([209.132.183.28]:35363 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753801Ab3KGLPU (ORCPT ); Thu, 7 Nov 2013 06:15:20 -0500 Content-Disposition: inline In-Reply-To: <1383773727-29327-1-git-send-email-sw@weilnetz.de> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Nov 06, 2013 at 10:35:27PM +0100, Stefan Weil wrote: > This error was reported by valgrind when running qemu-system-x86_64 > with kvm: > > ==975== Conditional jump or move depends on uninitialised value(s) > ==975== at 0x521C38: cpuid_find_entry (kvm.c:176) > ==975== by 0x5235BA: kvm_arch_init_vcpu (kvm.c:686) > ==975== by 0x4D5175: kvm_init_vcpu (kvm-all.c:267) > ==975== by 0x45035B: qemu_kvm_cpu_thread_fn (cpus.c:858) > ==975== by 0xD361E0D: start_thread (pthread_create.c:311) > ==975== by 0xD65E9EC: clone (clone.S:113) > ==975== Uninitialised value was created by a stack allocation > ==975== at 0x5226E4: kvm_arch_init_vcpu (kvm.c:446) > > Instead of adding more memset calls for parts of cpuid_data, the existing > calls were removed and cpuid_data is now initialized completely in one > call. > > Signed-off-by: Stefan Weil Applied, thanks. > --- > > I did not check whether older versions also need this fix. > > Stefan W. > > target-i386/kvm.c | 9 ++------- > 1 file changed, 2 insertions(+), 7 deletions(-) > > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > index 749aa09..3ffd3e0 100644 > --- a/target-i386/kvm.c > +++ b/target-i386/kvm.c > @@ -456,11 +456,12 @@ int kvm_arch_init_vcpu(CPUState *cs) > uint32_t signature[3]; > int r; > > + memset(&cpuid_data, 0, sizeof(cpuid_data)); > + > cpuid_i = 0; > > /* Paravirtualization CPUIDs */ > c = &cpuid_data.entries[cpuid_i++]; > - memset(c, 0, sizeof(*c)); > c->function = KVM_CPUID_SIGNATURE; > if (!hyperv_enabled(cpu)) { > memcpy(signature, "KVMKVMKVM\0\0\0", 12); > @@ -474,7 +475,6 @@ int kvm_arch_init_vcpu(CPUState *cs) > c->edx = signature[2]; > > c = &cpuid_data.entries[cpuid_i++]; > - memset(c, 0, sizeof(*c)); > c->function = KVM_CPUID_FEATURES; > c->eax = env->features[FEAT_KVM]; > > @@ -483,13 +483,11 @@ int kvm_arch_init_vcpu(CPUState *cs) > c->eax = signature[0]; > > c = &cpuid_data.entries[cpuid_i++]; > - memset(c, 0, sizeof(*c)); > c->function = HYPERV_CPUID_VERSION; > c->eax = 0x00001bbc; > c->ebx = 0x00060001; > > c = &cpuid_data.entries[cpuid_i++]; > - memset(c, 0, sizeof(*c)); > c->function = HYPERV_CPUID_FEATURES; > if (cpu->hyperv_relaxed_timing) { > c->eax |= HV_X64_MSR_HYPERCALL_AVAILABLE; > @@ -500,7 +498,6 @@ int kvm_arch_init_vcpu(CPUState *cs) > } > > c = &cpuid_data.entries[cpuid_i++]; > - memset(c, 0, sizeof(*c)); > c->function = HYPERV_CPUID_ENLIGHTMENT_INFO; > if (cpu->hyperv_relaxed_timing) { > c->eax |= HV_X64_RELAXED_TIMING_RECOMMENDED; > @@ -511,13 +508,11 @@ int kvm_arch_init_vcpu(CPUState *cs) > c->ebx = cpu->hyperv_spinlock_attempts; > > c = &cpuid_data.entries[cpuid_i++]; > - memset(c, 0, sizeof(*c)); > c->function = HYPERV_CPUID_IMPLEMENT_LIMITS; > c->eax = 0x40; > c->ebx = 0x40; > > c = &cpuid_data.entries[cpuid_i++]; > - memset(c, 0, sizeof(*c)); > c->function = KVM_CPUID_SIGNATURE_NEXT; > memcpy(signature, "KVMKVMKVM\0\0\0", 12); > c->eax = 0; > -- > 1.7.10.4 -- Gleb.