From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] kvm-s390: fix potential array overrun in intercept handling Date: Thu, 21 Jan 2010 13:00:19 +0200 Message-ID: <4B5833C3.8070908@redhat.com> References: <201001211156.03669.borntraeger@de.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm@vger.kernel.org, Martin Schwidefsky , Heiko Carstens , cotte@de.ibm.com To: Christian Borntraeger Return-path: Received: from mx1.redhat.com ([209.132.183.28]:18910 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754620Ab0AULAY (ORCPT ); Thu, 21 Jan 2010 06:00:24 -0500 In-Reply-To: <201001211156.03669.borntraeger@de.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: On 01/21/2010 12:56 PM, Christian Borntraeger wrote: > Avi, Marcelo, > > kvm_handle_sie_intercept uses a jump table to get the intercept handler > for a SIE intercept. Static code analysis revealed a potential problem: > the intercept_funcs jump table was defined to contain (0x48>> 2) entries, > but we only checked for code> 0x48 which would cause an off-by-one > array overflow if code == 0x48. > > Since the table is only populated up to (0x28>> 2), we can reduce the > jump table size while fixing the off-by-one. > > > > -static const intercept_handler_t intercept_funcs[0x48>> 2] = { > +static const intercept_handler_t intercept_funcs[(0x28>> 2) + 1] = { > [0x00>> 2] = handle_noop, > [0x04>> 2] = handle_instruction, > [0x08>> 2] = handle_prog, > [0x0C>> 2] = handle_instruction_and_prog, > [0x10>> 2] = handle_noop, > [0x14>> 2] = handle_noop, > [0x1C>> 2] = kvm_s390_handle_wait, > [0x20>> 2] = handle_validity, > [0x28>> 2] = handle_stop, > }; > You can define the array without a size to let the compiler figure out the minimum size. > > int kvm_handle_sie_intercept(struct kvm_vcpu *vcpu) > { > intercept_handler_t func; > u8 code = vcpu->arch.sie_block->icptcode; > > - if (code& 3 || code> 0x48) > + if (code& 3 || code> 0x28) > return -ENOTSUPP; > And here, check against ARRAY_SIZE() instead of a magic number. -- error compiling committee.c: too many arguments to function