[Libvirt-cim] [PATCH 1/4] libxkutil: Fix possible NULL dereferences

From: "Eduardo Lima (Etrunko)" <eblima(a)br.ibm.com&gt; As revealed by recent Coverity scan report provided by Red Hat: https://bugzilla.redhat.com/show_bug.cgi?id=750418 https://bugzilla.redhat.com/attachment.cgi?id=552325 Error: FORWARD_NULL: xmlgen.c:100: var_compare_op: Comparing "dev->device" to null implies that "dev->device" might be null. xmlgen.c:115: var_deref_model: Passing null variable "(char *)dev->device" to function "__coverity_strcmp", which dereferences it. Error: FORWARD_NULL: device_parsing.c:615: var_compare_op: Comparing "gdev->type" to null implies that "gdev->type" might be null. device_parsing.c:677: var_deref_model: Passing null variable "gdev->type" to function "cleanup_graphics_device", which dereferences it. device_parsing.c:126: deref_parm_in_call: Function "strcasecmp" dereferences parameter "dev->type". (The dereference is assumed on the basis of the 'nonnull' parameter attribute.) Error: NULL_RETURNS: Virt_DevicePool.c:805: returned_null: Function "get_typed_instance" returns null (checked 37 out of 44 times). misc_util.c:348: null_assign: Assigning: "inst" = NULL. misc_util.c:369: return_null_var: Returning "inst", which is null. Virt_DevicePool.c:805: var_assigned: Assigning: "inst" = null return value from "get_typed_instance". Virt_DevicePool.c:810: dereference: Dereferencing a pointer that might be null "inst" when calling "mempool_set_total". Virt_DevicePool.c:686: deref_parm: Directly dereferencing parameter "inst". Error: NULL_RETURNS: Virt_DevicePool.c:837: returned_null: Function "get_typed_instance" returns null (checked 37 out of 44 times). misc_util.c:348: null_assign: Assigning: "inst" = NULL. misc_util.c:369: return_null_var: Returning "inst", which is null. Virt_DevicePool.c:837: var_assigned: Assigning: "inst" = null return value from "get_typed_instance". Virt_DevicePool.c:842: dereference: Dereferencing a pointer that might be null "inst" when calling "procpool_set_total". Virt_DevicePool.c:743: deref_parm: Directly dereferencing parameter "inst". Error: NULL_RETURNS: Virt_Device.c:219: returned_null: Function "get_typed_instance" returns null (checked 37 out of 44 times). misc_util.c:348: null_assign: Assigning: "inst" = NULL. misc_util.c:369: return_null_var: Returning "inst", which is null. Virt_Device.c:219: var_assigned: Assigning: "inst" = null return value from "get_typed_instance". Virt_Device.c:224: dereference: Dereferencing a pointer that might be null "inst" when calling "graphics_set_attr". Virt_Device.c:202: deref_parm: Directly dereferencing parameter "instance". Error: NULL_RETURNS: Virt_Device.c:133: returned_null: Function "get_typed_instance" returns null (checked 37 out of 44 times). misc_util.c:348: null_assign: Assigning: "inst" = NULL. misc_util.c:369: return_null_var: Returning "inst", which is null. Virt_Device.c:133: var_assigned: Assigning: "inst" = null return value from "get_typed_instance". Virt_Device.c:138: dereference: Dereferencing a pointer that might be null "inst" when calling "disk_set_name". Virt_Device.c:117: deref_parm: Directly dereferencing parameter "instance". Error: NULL_RETURNS: Virt_Device.c:175: returned_null: Function "get_typed_instance" returns null (checked 37 out of 44 times). misc_util.c:348: null_assign: Assigning: "inst" = NULL. misc_util.c:369: return_null_var: Returning "inst", which is null. Virt_Device.c:175: var_assigned: Assigning: "inst" = null return value from "get_typed_instance". Virt_Device.c:180: dereference: Dereferencing a pointer that might be null "inst" when calling "mem_set_size". Virt_Device.c:156: deref_parm: Directly dereferencing parameter "instance". Error: NULL_RETURNS: Virt_Device.c:100: returned_null: Function "get_typed_instance" returns null (checked 37 out of 44 times). misc_util.c:348: null_assign: Assigning: "inst" = NULL. misc_util.c:369: return_null_var: Returning "inst", which is null. Virt_Device.c:100: var_assigned: Assigning: "inst" = null return value from "get_typed_instance". Virt_Device.c:105: dereference: Dereferencing a pointer that might be null "inst" when calling "net_set_type". Virt_Device.c:61: deref_parm: Directly dereferencing parameter "instance". Signed-off-byr Eduardo Lima (Etrunko) <eblima(a)br.ibm.com&gt; --- libxkutil/device_parsing.c | 13 ++++++------- libxkutil/xmlgen.c | 4 ++-- src/Virt_Device.c | 20 ++++++++++++++++++++ src/Virt_DevicePool.c | 14 ++++++++++++++ 4 files changed, 42 insertions(+), 9 deletions(-) diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c index 7eaa63e..b0eccfc 100644 --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -120,15 +120,14 @@ static void cleanup_sdl_device(struct graphics_device *dev) static void cleanup_graphics_device(struct graphics_device *dev) { - if (dev == NULL) + if (dev == NULL || dev->type == NULL) return; - if (STREQC(dev->type, "sdl")) { - cleanup_sdl_device(dev); - } - else { - cleanup_vnc_device(dev); - } + if (STREQC(dev->type, "sdl")) + cleanup_sdl_device(dev); + else + cleanup_vnc_device(dev); + free(dev->type); } diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c index d73ffd0..96b4e96 100644 --- a/libxkutil/xmlgen.c +++ b/libxkutil/xmlgen.c @@ -112,8 +112,8 @@ static const char *disk_file_xml(xmlNodePtr root, struct disk_device *dev) xmlNewProp(tmp, BAD_CAST "cache", BAD_CAST dev->cache); } - if ((XSTREQ(dev->device, "cdrom")) && - (XSTREQ(dev->source, ""))) { + if (dev->device != NULL && XSTREQ(dev->device, "cdrom") && + XSTREQ(dev->source, "")) { /* This is the situation that user defined a cdrom device without disk in it, so skip generating a line saying "source", for that xml defination for libvirt should not have this defined in this diff --git a/src/Virt_Device.c b/src/Virt_Device.c index fd11370..abe3d6f 100644 --- a/src/Virt_Device.c +++ b/src/Virt_Device.c @@ -102,6 +102,11 @@ static CMPIInstance *net_instance(const CMPIBroker *broker, "NetworkPort", ns); + if (inst == NULL) { + CU_DEBUG("Failed to get instance for NetworkPort"); + return NULL; + } + if (!net_set_type(inst, dev)) return NULL; @@ -135,6 +140,11 @@ static CMPIInstance *disk_instance(const CMPIBroker *broker, "LogicalDisk", ns); + if (inst == NULL) { + CU_DEBUG("Failed to get instance for LogicalDisk"); + return NULL; + } + if (!disk_set_name(inst, dev)) return NULL; @@ -177,6 +187,11 @@ static CMPIInstance *mem_instance(const CMPIBroker *broker, "Memory", ns); + if (inst == NULL) { + CU_DEBUG("Failed to get instance for Memory"); + return NULL; + } + if (!mem_set_size(inst, dev)) return NULL; @@ -221,6 +236,11 @@ static CMPIInstance *graphics_instance(const CMPIBroker *broker, "DisplayController", ns); + if (inst == NULL) { + CU_DEBUG("Failed to get instance for DisplayController"); + return NULL; + } + if (!graphics_set_attr(inst, dev)) return NULL; diff --git a/src/Virt_DevicePool.c b/src/Virt_DevicePool.c index a41a378..ab0baa0 100644 --- a/src/Virt_DevicePool.c +++ b/src/Virt_DevicePool.c @@ -807,6 +807,13 @@ static CMPIStatus mempool_instance(virConnectPtr conn, "MemoryPool", ns); + if (inst == NULL) { + cu_statusf(broker, &s, + CMPI_RC_ERR_FAILED, + "Failed to get instance for MemoryPool"); + return s; + } + mempool_set_total(inst, conn); mempool_set_consumed(inst, conn); @@ -839,6 +846,13 @@ static CMPIStatus procpool_instance(virConnectPtr conn, "ProcessorPool", ns); + if (inst == NULL) { + cu_statusf(broker, &s, + CMPI_RC_ERR_FAILED, + "Failed to get instance for ProcessorPool"); + return s; + } + procpool_set_total(inst, conn); set_params(inst, CIM_RES_TYPE_PROC, id, "Processors", NULL, true); -- 1.7.7.5