From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36048) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WwrEX-0008CM-M8 for qemu-devel@nongnu.org; Tue, 17 Jun 2014 07:07:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WwrEQ-0001PL-Cg for qemu-devel@nongnu.org; Tue, 17 Jun 2014 07:07:21 -0400 Message-ID: <53A02160.5060000@suse.de> Date: Tue, 17 Jun 2014 13:07:12 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1402988887-30418-1-git-send-email-Bharat.Bhushan@freescale.com> <1402988887-30418-4-git-send-email-Bharat.Bhushan@freescale.com> <539FF926.4020300@suse.de> <53A00F35.6000909@suse.de> <53A01BD9.50207@suse.de> <53A02064.8040804@suse.de> <1e9a6fa3679d415f9c108f3818fdbab3@DM2PR03MB574.namprd03.prod.outlook.com> In-Reply-To: <1e9a6fa3679d415f9c108f3818fdbab3@DM2PR03MB574.namprd03.prod.outlook.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/3 v2] ppc debug: Add debug stub support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Bharat.Bhushan@freescale.com" , "qemu-ppc@nongnu.org" , "qemu-devel@nongnu.org" On 17.06.14 13:05, Bharat.Bhushan@freescale.com wrote: > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@suse.de] >> Sent: Tuesday, June 17, 2014 4:33 PM >> To: Bhushan Bharat-R65777; qemu-ppc@nongnu.org; qemu-devel@nongnu.org >> Subject: Re: [PATCH 3/3 v2] ppc debug: Add debug stub support >> >> >> On 17.06.14 13:01, Bharat.Bhushan@freescale.com wrote: >>>>>>>>> +static void kvm_arch_e500_update_guest_debug(CPUState *cs, >>>>>>>>> + struct >>>>>>>>> +kvm_guest_debug >>>>>>>>> +*dbg) { >>>>>>>>> + int n; >>>>>>>>> + >>>>>>>>> + if (nb_hw_breakpoint + nb_hw_watchpoint > 0) { >>>>>>>>> + dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_HW_BP; >>>>>>>>> + memset(dbg->arch.bp, 0, sizeof(dbg->arch.bp)); >>>>>>>>> + for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; >>>>>>>>> + n++) { >>>>>>>> Boundary check against dbg->arch.bp missing. >>>>>>> Did not get, what you mean by " dbg->arch.bp missing" ? >>>>>> dbg->arch.bp is an array of a certain size. If nb_hw_breakpoint + >>>>>> nb_hw_watchpoint > ARRAY_SIZE(dbg->arch.bp) we might overwrite >>>>>> memory we don't want to overwrite. >>>>> Actually this will never overflow here because nb_hw_breakpoint and >>>> nb_hw_watchpoint overflow in taken care in in hw_insert_breakpoint(). >>>>> Do you thing that to be double safe we can add a check? >>>> We only check against an overflow of hw_breakpoint[], not dbg->arch.bp. >>>> What if nb_hw_breakpoint becomes 17? >>> nb_hw_breakpoint can never be more than max_hw_breakpoint, how >> nb_hw_breakpoint can be 17 ? >> >> Someone comes along and bumps up max_hw_breakpoint to 17? > You mean some buggy code in qemu does this? I mean the next person that comes along and touches this code might not realize that dbg->arch.bp[] is an array of 16 and by the time I review that code I might have forgotten as well :) Alex