Re: [Qemu-devel] [PATCH 3/3 v2] ppc debug: Add debug stub support

[Alexander Graf <agraf@suse.de>]

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