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

[Peter Krempa]

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.

> 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.

As of such I disagree with this fix. The only acceptable way is to make
the caller better introspectable. This fix obscures what is actually
happening.

Peter




-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161010/e5b1d8e3/attachment-0001.sig>