[libvirt] [PATCH 2/2] esx_vi: fix possible segfault

[Pavel Hrdina]

On 01/21/2015 06:47 PM, Peter Krempa wrote:
> On Wed, Jan 21, 2015 at 18:09:28 +0100, Pavel Hrdina wrote:
>> Clang found possible dereference of NULL pointer which is right.
>> Function 'esxVI_LookupTaskInfoByTask' should find a task info. The issue
>> is that we could return 0 and leave 'taksInfo' pointer NULL because if
>> there is no match we simply end the search loop end set 'result' to 0.
>> Every caller count on the fact that if the return value is 0 than it's
>> safe to dereference 'taskInfo'. We should return 0 only in case we found
>> something and the '*taskInfo' is not NULL.
>>
>> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
>> ---
>>   src/esx/esx_vi.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
>> index a87f2c0..752d32a 100644
>> --- a/src/esx/esx_vi.c
>> +++ b/src/esx/esx_vi.c
>> @@ -3298,7 +3298,8 @@ esxVI_LookupTaskInfoByTask(esxVI_Context *ctx,
>>           }
>>       }
>>
>> -    result = 0;
>> +    if (*taskInfo)
>> +        result = 0;
>
> I think a more obvious fix would be to move setting of result to zero
> into the condition that breaks the loop. In that case, result gets set
> to 0 only if the entry was found.
>
>>
>>    cleanup:
>>       esxVI_String_Free(&propertyNameList);
>
> ACK with or without that change. I think that this wasn't discovered due
> to the fact that the correct entry is always in the list and is always
> first as we haven't seen reports for spamming the warning.
>
> Peter
>

Pushed with the update that you've proposed, thanks.

Pavel