From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [patch RFC] kvm, cpuid: silence a buffer overflow warning Date: Thu, 20 Feb 2014 14:07:42 +0100 Message-ID: <5305FE1E.6060902@redhat.com> References: <20140220123419.GA10110@elgon.mountain> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, kvm@vger.kernel.org, kernel-janitors@vger.kernel.org To: Dan Carpenter , Gleb Natapov Return-path: In-Reply-To: <20140220123419.GA10110@elgon.mountain> Sender: kernel-janitors-owner@vger.kernel.org List-Id: kvm.vger.kernel.org Il 20/02/2014 13:34, Dan Carpenter ha scritto: > This seems like a harmless off by one overflow if "i" is the last > element in the vcpu->arch.cpuid_entries[] array. > > Signed-off-by: Dan Carpenter > --- > Not tested. I always wonder if it's worth fixing these or if it's worth > reporting them? Either of those seem like a lot of work for something > harmless. Could it oops if cpuid_nent is INT_MAX? If so, it's not entirely harmless. In this case I'd rather take the occasion to cleanup the code like this (compile-tested): diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 9fed5bedaad6..2fd6e7169936 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -656,18 +656,19 @@ out: static int move_to_next_stateful_cpuid_entry(struct kvm_vcpu *vcpu, int i) { struct kvm_cpuid_entry2 *e = &vcpu->arch.cpuid_entries[i]; - int j, nent = vcpu->arch.cpuid_nent; + struct kvm_cpuid_entry2 *ej; + int j = i, nent = vcpu->arch.cpuid_nent; e->flags &= ~KVM_CPUID_FLAG_STATE_READ_NEXT; + /* when no next entry is found, the current entry[i] is reselected */ - for (j = i + 1; ; j = (j + 1) % nent) { - struct kvm_cpuid_entry2 *ej = &vcpu->arch.cpuid_entries[j]; - if (ej->function == e->function) { - ej->flags |= KVM_CPUID_FLAG_STATE_READ_NEXT; - return j; - } - } - return 0; /* silence gcc, even though control never reaches here */ + do { + j = (j + 1) % nent; + ej = &vcpu->arch.cpuid_entries[j]; + } while (ej->function != e->function); + + ej->flags |= KVM_CPUID_FLAG_STATE_READ_NEXT; + return j; } /* find an entry with matching function, matching index (if needed), and that What do you think? Paolo