Re: [Qemu-devel] [PATCH] kvm-all: Use 'tmpcpu' instead of 'cpu' in sub-looping to avoid 'cpu' be NULL

[Chen Gang]

On 07/20/2014 03:29 PM, Jan Kiszka wrote:
> On 2014-07-19 03:21, Chen Gang wrote:
>> If kvm_arch_remove_sw_breakpoint() in CPU_FOREACH() always be fail, it
>> will let 'cpu' NULL. And the next kvm_arch_remove_sw_breakpoint() in
>> QTAILQ_FOREACH_SAFE() will get NULL parameter for 'cpu'.
>>
>> And kvm_arch_remove_sw_breakpoint() can assumes 'cpu' must never be NULL,
>> so need define additional temporary variable for 'cpu' to avoid the case.
>>
>>
>> Signed-off-by: Chen Gang <address@hidden>
>> ---
>>  kvm-all.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/kvm-all.c b/kvm-all.c
>> index 3ae30ee..1402f4f 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -2077,12 +2077,13 @@ void kvm_remove_all_breakpoints(CPUState *cpu)
>>  {
>>      struct kvm_sw_breakpoint *bp, *next;
>>      KVMState *s = cpu->kvm_state;
>> +    CPUState *tmpcpu;
>>  
>>      QTAILQ_FOREACH_SAFE(bp, &s->kvm_sw_breakpoints, entry, next) {
>>          if (kvm_arch_remove_sw_breakpoint(cpu, bp) != 0) {
>>              /* Try harder to find a CPU that currently sees the breakpoint. 
>> */
>> -            CPU_FOREACH(cpu) {
>> -                if (kvm_arch_remove_sw_breakpoint(cpu, bp) == 0) {
>> +            CPU_FOREACH(tmpcpu) {
>> +                if (kvm_arch_remove_sw_breakpoint(tmpcpu, bp) == 0) {
>>                      break;
>>                  }
>>              }
>>
> 
> Good catch. To make it clear in the changelog: The actual issue is that
> we misuse "cpu" as an iteration variable while its original value is
> still in use. That cpu can eventually become NULL this way is one result.
> 

OK, thanks. If necessary, I shall send patch v2 for additional comments.
(if really necessary to send, please let me know)


Thanks.
-- 
Chen Gang

Open share and attitude like air water and life which God blessed