[libvirt] [PATCH 3/7] qemu: Remove possibility of NULL dereference

[John Ferlan]

On 10/10/2016 12:19 PM, Peter Krempa wrote:
> On Mon, Oct 10, 2016 at 11:42:14 -0400, John Ferlan wrote:
>> If qemubinCaps is NULL, then calling virQEMUCapsGetMachineTypesCaps and
>       ^^^^^
> This symbol name refers to the name in the caller. The function uses a
> different name.
> 
>> dereferencing to get the nmachineTypes will cause a core. Rework the code
> 
> It will cause a crash, not a core. A core dump is caused by the crash
> and can be turned off.
> 

Ok - semantics...

>> slightly to avoid the issue and return immediately if !qemubinCaps or
>> !nmachineTypes
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  src/qemu/qemu_capabilities.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>> index da8f3d1..ee3e50f 100644
>> --- a/src/qemu/qemu_capabilities.c
>> +++ b/src/qemu/qemu_capabilities.c
>> @@ -2405,10 +2405,13 @@ int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps,
>>      size_t i;
>>  
>>      *machines = NULL;
>> +    *nmachines = 0;
>> +
>> +    if (!qemuCaps || !qemuCaps->nmachineTypes)
>> +        return 0;
>>      *nmachines = qemuCaps->nmachineTypes;
>>  
>> -    if (*nmachines &&
>> -        VIR_ALLOC_N(*machines, qemuCaps->nmachineTypes) < 0)
>> +    if (VIR_ALLOC_N(*machines, qemuCaps->nmachineTypes) < 0)
>>          goto error;
> 
> This is a false positive, the caller checks that 'binary' is set and
> does not call this function at all. Coverity probably doesn't see it as
> it depends on the state of 'binary' rather than 'qemubinCaps' in the
> caller.

Hmm... true. Coverity generally has an issue with similar conditions
where there's two arguments that are related and only one is checked
while the other is used. This one's not so obvious. Interesting that the
subsequent call wasn't flagged, but that's perhaps because it's preceded
by "kvmbin &&".

In any case, I've dropped this one and tagged it in my personal coverity
branch as a false positive.

John