[PATCH v3 0/4] Fix errors raised by Coverity tool
From: "Eduardo Lima (Etrunko)" <eblima@br.ibm.com> Yet another series of patches fixing errors revealed by Coverity tool. It would be nice if we had a similar setup so we could run the tool on a regular basis. I started playing with clang a while ago. It also provides a static analyser, but I could not complete the setup by then. https://bugzilla.redhat.com/show_bug.cgi?id=750418 Changes from v2: - Revert leak change for libxkutil/xmlgen.c in patch 2 which was causing a double-free crash and correctly fix the leak problem. Changes from v1: - Fix compilation error in Patch 1 Best regards, Etrunko -- Eduardo de Barros Lima Software Engineer, Open Virtualization Linux Technology Center - IBM/Brazil eblima@br.ibm.com Eduardo Lima (Etrunko) (4): libxkutil: Fix possible NULL dereferences Fix possible memory leaks xml_parse_test: Fix invalid dereference Fix possible use of unitialized variables libxkutil/cs_util_instance.c | 5 +++++ libxkutil/device_parsing.c | 14 +++++++------- libxkutil/pool_parsing.c | 5 +++-- libxkutil/xml_parse_test.c | 3 +-- libxkutil/xmlgen.c | 8 +++++--- src/Virt_AppliedFilterList.c | 3 ++- src/Virt_Device.c | 20 ++++++++++++++++++++ src/Virt_DevicePool.c | 19 ++++++++++++++++++- src/Virt_SwitchService.c | 24 ++++++++++++++++++++---- src/Virt_VirtualSystemManagementService.c | 16 ++++++++-------- src/Virt_VirtualSystemSnapshotService.c | 2 +- 11 files changed, 90 insertions(+), 29 deletions(-) -- 1.7.7.5
From: "Eduardo Lima (Etrunko)" <eblima@br.ibm.com> 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@br.ibm.com> --- 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
"Eduardo Lima (Etrunko)" <eblima@linux.vnet.ibm.com> Sent by: libvirt-cim-bounces@redhat.com
01/18/2012 08:06 AM
Please respond to List for discussion and development of libvirt CIM <libvirt-cim@redhat.com>
To
libvirt-cim@redhat.com
cc
"Eduardo Lima \(Etrunko\)" <eblima@br.ibm.com>
Subject
[Libvirt-cim] [PATCH 1/4] libxkutil: Fix possible NULL dereferences
From: "Eduardo Lima (Etrunko)" <eblima@br.ibm.com>
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@br.ibm.com> --- 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,
+1 Sharad Mishra Open Virtualization Linux Technology Center IBM libvirt-cim-bounces@redhat.com wrote on 01/18/2012 08:06:09 AM: true);
-- 1.7.7.5
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
From: "Eduardo Lima (Etrunko)" <eblima@br.ibm.com> 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: RESOURCE_LEAK: pool_parsing.c:140: alloc_fn: Calling allocation function "realloc". pool_parsing.c:140: var_assign: Assigning: "tmp" = storage returned from "realloc(dev_paths, 8UL * (ct + 1U))". pool_parsing.c:145: var_assign: Assigning: "dev_paths" = "tmp". pool_parsing.c:149: leaked_storage: Variable "tmp" going out of scope leaks the storage it points to. pool_parsing.c:169: leaked_storage: Variable "dev_paths" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: xmlgen.c:891: alloc_fn: Calling allocation function "virt_device_dup". device_parsing.c:873: alloc_fn: Storage is returned from allocation function "calloc". device_parsing.c:873: var_assign: Assigning: "dev" = "calloc(1UL, 152UL)". device_parsing.c:928: return_alloc: Returning allocated memory "dev". xmlgen.c:891: var_assign: Assigning: "dev" = storage returned from "virt_device_dup(_dev)". xmlgen.c:952: leaked_storage: Variable "dev" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: device_parsing.c:354: alloc_fn: Calling allocation function "calloc". device_parsing.c:354: var_assign: Assigning: "vsi_dev" = storage returned from "calloc(1UL, 56UL)". device_parsing.c:385: noescape: Variable "vsi_dev" is not freed or pointed-to in function "memcpy". device_parsing.c:386: leaked_storage: Variable "vsi_dev" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: Virt_DevicePool.c:1077: alloc_arg: Calling allocation function "get_diskpool_config" on "pools". Virt_DevicePool.c:161: alloc_arg: "get_disk_parent" allocates memory that is stored into "pools". Virt_DevicePool.c:74: alloc_fn: Storage is returned from allocation function "realloc". Virt_DevicePool.c:74: var_assign: Assigning: "pools" = "realloc(pools, (count + 1) * 24UL)". Virt_DevicePool.c:86: var_assign: Assigning: "*_pools" = "pools". Virt_DevicePool.c:163: var_assign: Assigning: "*_pools" = "pools". Virt_DevicePool.c:1080: leaked_storage: Variable "pools" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: Virt_DevicePool.c:404: alloc_arg: Calling allocation function "get_diskpool_config" on "pools". Virt_DevicePool.c:161: alloc_arg: "get_disk_parent" allocates memory that is stored into "pools". Virt_DevicePool.c:74: alloc_fn: Storage is returned from allocation function "realloc". Virt_DevicePool.c:74: var_assign: Assigning: "pools" = "realloc(pools, (count + 1) * 24UL)". Virt_DevicePool.c:86: var_assign: Assigning: "*_pools" = "pools". Virt_DevicePool.c:163: var_assign: Assigning: "*_pools" = "pools". Virt_DevicePool.c:406: leaked_storage: Variable "pools" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: Virt_VirtualSystemManagementService.c:415: alloc_fn: Calling allocation function "realloc". Virt_VirtualSystemManagementService.c:415: var_assign: Assigning: "tmp_str_arr" = storage returned from "realloc(domain->os_info.fv.bootlist, bl_size * 8UL)". Virt_VirtualSystemManagementService.c:432: leaked_storage: Variable "tmp_str_arr" going out of scope leaks the storage it points to. Virt_VirtualSystemManagementService.c:440: leaked_storage: Variable "tmp_str_arr" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: Virt_VirtualSystemManagementService.c:2834: alloc_fn: Calling allocation function "malloc". Virt_VirtualSystemManagementService.c:2834: var_assign: Assigning: "__retval" = storage returned from "malloc(__len)". Virt_VirtualSystemManagementService.c:2834: noescape: Variable "__retval" is not freed or pointed-to in function "memcpy". Virt_VirtualSystemManagementService.c:2834: overwrite_var: Overwriting "__retval" in call "__retval = (char *)memcpy(__retval, "ResourceAllocationSettingDataCreatedIndication", __len)" leaks the storage that "__retval" points to. Virt_VirtualSystemManagementService.c:2834: var_assign: Assigning: "__retval" = storage returned from "memcpy(__retval, "ResourceAllocationSettingDataCreatedIndication", __len)". Virt_VirtualSystemManagementService.c:2834: var_assign: Assigning: "indication" = "__retval". Virt_VirtualSystemManagementService.c:2901: leaked_storage: Variable "indication" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: Virt_VirtualSystemManagementService.c:2837: alloc_fn: Calling allocation function "malloc". Virt_VirtualSystemManagementService.c:2837: var_assign: Assigning: "__retval" = storage returned from "malloc(__len)". Virt_VirtualSystemManagementService.c:2837: noescape: Variable "__retval" is not freed or pointed-to in function "memcpy". Virt_VirtualSystemManagementService.c:2837: overwrite_var: Overwriting "__retval" in call "__retval = (char *)memcpy(__retval, "ResourceAllocationSettingDataDeletedIndication", __len)" leaks the storage that "__retval" points to. Virt_VirtualSystemManagementService.c:2837: var_assign: Assigning: "__retval" = storage returned from "memcpy(__retval, "ResourceAllocationSettingDataDeletedIndication", __len)". Virt_VirtualSystemManagementService.c:2837: var_assign: Assigning: "indication" = "__retval". Virt_VirtualSystemManagementService.c:2901: leaked_storage: Variable "indication" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: Virt_VirtualSystemManagementService.c:2840: alloc_fn: Calling allocation function "malloc". Virt_VirtualSystemManagementService.c:2840: var_assign: Assigning: "__retval" = storage returned from "malloc(__len)". Virt_VirtualSystemManagementService.c:2840: noescape: Variable "__retval" is not freed or pointed-to in function "memcpy". Virt_VirtualSystemManagementService.c:2840: overwrite_var: Overwriting "__retval" in call "__retval = (char *)memcpy(__retval, "ResourceAllocationSettingDataModifiedIndication", __len)" leaks the storage that "__retval" points to. Virt_VirtualSystemManagementService.c:2840: var_assign: Assigning: "__retval" = storage returned from "memcpy(__retval, "ResourceAllocationSettingDataModifiedIndication", __len)". Virt_VirtualSystemManagementService.c:2840: var_assign: Assigning: "indication" = "__retval". Virt_VirtualSystemManagementService.c:2901: leaked_storage: Variable "indication" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: Virt_AppliedFilterList.c:199: alloc_arg: Calling allocation function "get_domain_list" on "doms". cs_util_instance.c:52: alloc_fn: Storage is returned from allocation function "calloc". cs_util_instance.c:52: var_assign: Assigning: "list" = "calloc(n_names + n_ids, 8UL)". cs_util_instance.c:107: var_assign: Assigning: "*_list" = "list". Virt_AppliedFilterList.c:251: leaked_storage: Variable "doms" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: misc_util.c:275: alloc_arg: Calling allocation function "get_domain_list" on "list". cs_util_instance.c:52: alloc_fn: Storage is returned from allocation function "calloc". cs_util_instance.c:52: var_assign: Assigning: "list" = "calloc(n_names + n_ids, 8UL)". cs_util_instance.c:107: var_assign: Assigning: "*_list" = "list". misc_util.c:277: leaked_storage: Variable "list" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: Virt_SwitchService.c:108: alloc_fn: Calling allocation function "popen". Virt_SwitchService.c:108: var_assign: Assigning: "stream" = storage returned from "popen(func, "r")". Virt_SwitchService.c:118: noescape: Variable "stream" is not freed or pointed-to in function "fgets". Virt_SwitchService.c:131: leaked_storage: Variable "stream" going out of scope leaks the storage it points to. Virt_SwitchService.c:142: leaked_storage: Variable "stream" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: Virt_SwitchService.c:123: alloc_fn: Calling allocation function "realloc". Virt_SwitchService.c:123: var_assign: Assigning: "tmp_list" = storage returned from "realloc(arr, (i + 1) * 8UL)". Virt_SwitchService.c:134: var_assign: Assigning: "arr" = "tmp_list". Virt_SwitchService.c:142: leaked_storage: Variable "arr" going out of scope leaks the storage it points to. Virt_SwitchService.c:142: leaked_storage: Variable "tmp_list" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: Virt_SwitchService.c:236: alloc_fn: Calling allocation function "run_command". Virt_SwitchService.c:123: alloc_fn: Storage is returned from allocation function "realloc". Virt_SwitchService.c:123: var_assign: Assigning: "tmp_list" = "realloc(arr, (i + 1) * 8UL)". Virt_SwitchService.c:134: var_assign: Assigning: "arr" = "tmp_list". Virt_SwitchService.c:151: return_alloc: Returning allocated memory "arr". Virt_SwitchService.c:236: var_assign: Assigning: "if_list" = storage returned from "run_command("/sbin/ifconfig -a | /bin/grep eth | /bin/awk \'{print$1}\'", &count, &s)". /builddir/build/BUILD/libvirt-cim-0.6.0/src/Virt_SwitchService.c:269: leaked_storage: Variable "if_list" going out of scope leaks the storage it points to. Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> --- libxkutil/cs_util_instance.c | 5 +++++ libxkutil/device_parsing.c | 1 + libxkutil/pool_parsing.c | 5 +++-- libxkutil/xmlgen.c | 4 +++- src/Virt_AppliedFilterList.c | 3 ++- src/Virt_DevicePool.c | 5 ++++- src/Virt_SwitchService.c | 24 ++++++++++++++++++++---- src/Virt_VirtualSystemManagementService.c | 14 +++++++------- 8 files changed, 45 insertions(+), 16 deletions(-) diff --git a/libxkutil/cs_util_instance.c b/libxkutil/cs_util_instance.c index d21f0ff..a383147 100644 --- a/libxkutil/cs_util_instance.c +++ b/libxkutil/cs_util_instance.c @@ -104,6 +104,11 @@ int get_domain_list(virConnectPtr conn, virDomainPtr **_list) free(names); free(ids); + if (idx == 0) { + free(list); + list = NULL; + } + *_list = list; return idx; diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c index b0eccfc..a1e8d6c 100644 --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -382,6 +382,7 @@ static int parse_vsi_device(xmlNode *dnode, struct net_device *vdevs) } memcpy(&(vdevs->vsi), vsi_dev, sizeof(*vsi_dev)); + free(vsi_dev); return 1; err: diff --git a/libxkutil/pool_parsing.c b/libxkutil/pool_parsing.c index f73b0fd..e41fc09 100644 --- a/libxkutil/pool_parsing.c +++ b/libxkutil/pool_parsing.c @@ -163,10 +163,11 @@ static int parse_disk_source(xmlNode *node, struct disk_pool *pool) pool->device_paths_ct = ct; pool->device_paths = dev_paths; + return 1; err: - - return 1; + free(dev_paths); + return 0; } char *get_disk_pool_type(uint16_t type) diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c index 96b4e96..9a2ada9 100644 --- a/libxkutil/xmlgen.c +++ b/libxkutil/xmlgen.c @@ -889,8 +889,10 @@ char *device_to_xml(struct virt_device *_dev) goto out; root = xmlNewNode(NULL, BAD_CAST "tmp"); - if (root == NULL) + if (root == NULL) { + cleanup_virt_devices(&dev, 1); goto out; + } switch (type) { case CIM_RES_TYPE_DISK: diff --git a/src/Virt_AppliedFilterList.c b/src/Virt_AppliedFilterList.c index 6567118..bc31c14 100644 --- a/src/Virt_AppliedFilterList.c +++ b/src/Virt_AppliedFilterList.c @@ -197,7 +197,7 @@ static CMPIStatus list_to_net( /* get domains */ dcount = get_domain_list(conn, &doms); - if (dcount < 0) { + if (dcount <= 0) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Failed to get domain list"); @@ -246,6 +246,7 @@ static CMPIStatus list_to_net( } out: + free(doms); virConnectClose(conn); return s; diff --git a/src/Virt_DevicePool.c b/src/Virt_DevicePool.c index ab0baa0..fe5573f 100644 --- a/src/Virt_DevicePool.c +++ b/src/Virt_DevicePool.c @@ -402,8 +402,10 @@ static char *_diskpool_member_of(virConnectPtr conn, char *pool = NULL; count = get_diskpool_config(conn, &pools); - if (count == 0) + if (count == 0) { + free(pools); return NULL; + } for (i = 0; i < count; i++) { if (_diskpool_is_member(conn, &pools[i], file)) { @@ -1091,6 +1093,7 @@ static CMPIStatus diskpool_instance(virConnectPtr conn, count = get_diskpool_config(conn, &pools); if ((id == NULL) && (count == 0)) { CU_DEBUG("No defined DiskPools"); + free(pools); return s; } diff --git a/src/Virt_SwitchService.c b/src/Virt_SwitchService.c index 0d57f54..7e59d38 100644 --- a/src/Virt_SwitchService.c +++ b/src/Virt_SwitchService.c @@ -128,20 +128,19 @@ static char **run_command(char *func, int *len, CMPIStatus *s) { cu_statusf(_BROKER, s, CMPI_RC_ERR_NOT_FOUND, "Failed to realloc"); - return NULL; + goto err; } arr = tmp_list; - string = calloc(len, sizeof(char)); + string = strndup(buff, len); if (string == NULL) { CU_DEBUG("Failed to allocate memory"); cu_statusf(_BROKER, s, CMPI_RC_ERR_NOT_FOUND, "Failed to calloc"); - return NULL; + goto err; } - strncpy(string, buff, len); arr[i] = string; i++; } @@ -149,6 +148,19 @@ static char **run_command(char *func, int *len, CMPIStatus *s) { pclose(stream); *len = i; return arr; + + err: + /* undo everything */ + if (i > 0) { + int count; + for (count = 0; count < i; count++) + free(arr[count]); + } + + free(arr); + pclose(stream); + return NULL; + } static CMPIStatus set_inst_properties(const CMPIBroker *broker, @@ -262,6 +274,10 @@ static CMPIStatus get_switchservice(const CMPIObjectPath *reference, CMSetProperty(inst, "IsVSISupported", (CMPIValue *)&vsi, CMPI_boolean); s.rc = CMPI_RC_OK; + for (i = 0; i < count; i++) + free(if_list[i]); + + free(if_list); out: virConnectClose(conn); *_inst = inst; diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index f8c5f24..3a0b423 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -429,6 +429,7 @@ static int bootord_vssd_to_domain(CMPIInstance *inst, if (CMIsNullValue(boot_elem)) { CU_DEBUG("Null BootDevice"); + free(tmp_str_arr); return 0; } @@ -437,6 +438,7 @@ static int bootord_vssd_to_domain(CMPIInstance *inst, if (str == NULL) { CU_DEBUG("Could not extract char pointer from " "CMPIArray"); + free(tmp_str_arr); return 0; } @@ -2766,13 +2768,11 @@ static CMPIStatus _update_resources_for(const CMPIContext *context, } if (func == &resource_add) { - indication = strdup(RASD_IND_CREATED); - } - else if (func == &resource_del) { - indication = strdup(RASD_IND_DELETED); - } - else { - indication = strdup(RASD_IND_MODIFIED); + indication = RASD_IND_CREATED; + } else if (func == &resource_del) { + indication = RASD_IND_DELETED; + } else { + indication = RASD_IND_MODIFIED; char *dummy_name = NULL; if (asprintf(&dummy_name, "%s/%s",dominfo->name, devid) == -1) { -- 1.7.7.5
+1 Sharad Mishra Open Virtualization Linux Technology Center IBM libvirt-cim-bounces@redhat.com wrote on 01/18/2012 08:06:10 AM:
"Eduardo Lima (Etrunko)" <eblima@linux.vnet.ibm.com> Sent by: libvirt-cim-bounces@redhat.com
01/18/2012 08:06 AM
Please respond to List for discussion and development of libvirt CIM <libvirt-cim@redhat.com>
To
libvirt-cim@redhat.com
cc
"Eduardo Lima \(Etrunko\)" <eblima@br.ibm.com>
Subject
[Libvirt-cim] [PATCH 2/4] Fix possible memory leaks
From: "Eduardo Lima (Etrunko)" <eblima@br.ibm.com>
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: RESOURCE_LEAK: pool_parsing.c:140: alloc_fn: Calling allocation function "realloc". pool_parsing.c:140: var_assign: Assigning: "tmp" = storage returned from "realloc(dev_paths, 8UL * (ct + 1U))". pool_parsing.c:145: var_assign: Assigning: "dev_paths" = "tmp". pool_parsing.c:149: leaked_storage: Variable "tmp" going out of scope leaks the storage it points to. pool_parsing.c:169: leaked_storage: Variable "dev_paths" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK: xmlgen.c:891: alloc_fn: Calling allocation function "virt_device_dup". device_parsing.c:873: alloc_fn: Storage is returned from allocation function "calloc". device_parsing.c:873: var_assign: Assigning: "dev" = "calloc(1UL, 152UL)". device_parsing.c:928: return_alloc: Returning allocated memory "dev". xmlgen.c:891: var_assign: Assigning: "dev" = storage returned from "virt_device_dup(_dev)". xmlgen.c:952: leaked_storage: Variable "dev" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK: device_parsing.c:354: alloc_fn: Calling allocation function "calloc". device_parsing.c:354: var_assign: Assigning: "vsi_dev" = storage returned from "calloc(1UL, 56UL)". device_parsing.c:385: noescape: Variable "vsi_dev" is not freed or pointed-to in function "memcpy". device_parsing.c:386: leaked_storage: Variable "vsi_dev" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK: Virt_DevicePool.c:1077: alloc_arg: Calling allocation function "get_diskpool_config" on "pools". Virt_DevicePool.c:161: alloc_arg: "get_disk_parent" allocates memory that is stored into "pools". Virt_DevicePool.c:74: alloc_fn: Storage is returned from allocation function "realloc". Virt_DevicePool.c:74: var_assign: Assigning: "pools" = "realloc (pools, (count + 1) * 24UL)". Virt_DevicePool.c:86: var_assign: Assigning: "*_pools" = "pools". Virt_DevicePool.c:163: var_assign: Assigning: "*_pools" = "pools". Virt_DevicePool.c:1080: leaked_storage: Variable "pools" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK: Virt_DevicePool.c:404: alloc_arg: Calling allocation function "get_diskpool_config" on "pools". Virt_DevicePool.c:161: alloc_arg: "get_disk_parent" allocates memory that is stored into "pools". Virt_DevicePool.c:74: alloc_fn: Storage is returned from allocation function "realloc". Virt_DevicePool.c:74: var_assign: Assigning: "pools" = "realloc (pools, (count + 1) * 24UL)". Virt_DevicePool.c:86: var_assign: Assigning: "*_pools" = "pools". Virt_DevicePool.c:163: var_assign: Assigning: "*_pools" = "pools". Virt_DevicePool.c:406: leaked_storage: Variable "pools" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK: Virt_VirtualSystemManagementService.c:415: alloc_fn: Calling allocation function "realloc". Virt_VirtualSystemManagementService.c:415: var_assign: Assigning: "tmp_str_arr" = storage returned from "realloc (domain->os_info.fv.bootlist, bl_size * 8UL)". Virt_VirtualSystemManagementService.c:432: leaked_storage: Variable "tmp_str_arr" going out of scope leaks the storage it points to. Virt_VirtualSystemManagementService.c:440: leaked_storage: Variable "tmp_str_arr" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK: Virt_VirtualSystemManagementService.c:2834: alloc_fn: Calling allocation function "malloc". Virt_VirtualSystemManagementService.c:2834: var_assign: Assigning: "__retval" = storage returned from "malloc(__len)". Virt_VirtualSystemManagementService.c:2834: noescape: Variable "__retval" is not freed or pointed-to in function "memcpy". Virt_VirtualSystemManagementService.c:2834: overwrite_var: Overwriting "__retval" in call "__retval = (char *)memcpy(__retval, "ResourceAllocationSettingDataCreatedIndication", __len)" leaks the storage that "__retval" points to. Virt_VirtualSystemManagementService.c:2834: var_assign: Assigning: "__retval" = storage returned from "memcpy(__retval, "ResourceAllocationSettingDataCreatedIndication", __len)". Virt_VirtualSystemManagementService.c:2834: var_assign: Assigning: "indication" = "__retval". Virt_VirtualSystemManagementService.c:2901: leaked_storage: Variable "indication" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK: Virt_VirtualSystemManagementService.c:2837: alloc_fn: Calling allocation function "malloc". Virt_VirtualSystemManagementService.c:2837: var_assign: Assigning: "__retval" = storage returned from "malloc(__len)". Virt_VirtualSystemManagementService.c:2837: noescape: Variable "__retval" is not freed or pointed-to in function "memcpy". Virt_VirtualSystemManagementService.c:2837: overwrite_var: Overwriting "__retval" in call "__retval = (char *)memcpy(__retval, "ResourceAllocationSettingDataDeletedIndication", __len)" leaks the storage that "__retval" points to. Virt_VirtualSystemManagementService.c:2837: var_assign: Assigning: "__retval" = storage returned from "memcpy(__retval, "ResourceAllocationSettingDataDeletedIndication", __len)". Virt_VirtualSystemManagementService.c:2837: var_assign: Assigning: "indication" = "__retval". Virt_VirtualSystemManagementService.c:2901: leaked_storage: Variable "indication" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK: Virt_VirtualSystemManagementService.c:2840: alloc_fn: Calling allocation function "malloc". Virt_VirtualSystemManagementService.c:2840: var_assign: Assigning: "__retval" = storage returned from "malloc(__len)". Virt_VirtualSystemManagementService.c:2840: noescape: Variable "__retval" is not freed or pointed-to in function "memcpy". Virt_VirtualSystemManagementService.c:2840: overwrite_var: Overwriting "__retval" in call "__retval = (char *)memcpy(__retval, "ResourceAllocationSettingDataModifiedIndication", __len)" leaks the storage that "__retval" points to. Virt_VirtualSystemManagementService.c:2840: var_assign: Assigning: "__retval" = storage returned from "memcpy(__retval, "ResourceAllocationSettingDataModifiedIndication", __len)". Virt_VirtualSystemManagementService.c:2840: var_assign: Assigning: "indication" = "__retval". Virt_VirtualSystemManagementService.c:2901: leaked_storage: Variable "indication" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK: Virt_AppliedFilterList.c:199: alloc_arg: Calling allocation function "get_domain_list" on "doms". cs_util_instance.c:52: alloc_fn: Storage is returned from allocation function "calloc". cs_util_instance.c:52: var_assign: Assigning: "list" = "calloc (n_names + n_ids, 8UL)". cs_util_instance.c:107: var_assign: Assigning: "*_list" = "list". Virt_AppliedFilterList.c:251: leaked_storage: Variable "doms" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK: misc_util.c:275: alloc_arg: Calling allocation function "get_domain_list" on "list". cs_util_instance.c:52: alloc_fn: Storage is returned from allocation function "calloc". cs_util_instance.c:52: var_assign: Assigning: "list" = "calloc (n_names + n_ids, 8UL)". cs_util_instance.c:107: var_assign: Assigning: "*_list" = "list". misc_util.c:277: leaked_storage: Variable "list" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK: Virt_SwitchService.c:108: alloc_fn: Calling allocation function "popen". Virt_SwitchService.c:108: var_assign: Assigning: "stream" = storage returned from "popen(func, "r")". Virt_SwitchService.c:118: noescape: Variable "stream" is not freed or pointed-to in function "fgets". Virt_SwitchService.c:131: leaked_storage: Variable "stream" going out of scope leaks the storage it points to. Virt_SwitchService.c:142: leaked_storage: Variable "stream" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK: Virt_SwitchService.c:123: alloc_fn: Calling allocation function "realloc". Virt_SwitchService.c:123: var_assign: Assigning: "tmp_list" = storage returned from "realloc(arr, (i + 1) * 8UL)". Virt_SwitchService.c:134: var_assign: Assigning: "arr" = "tmp_list". Virt_SwitchService.c:142: leaked_storage: Variable "arr" going out of scope leaks the storage it points to. Virt_SwitchService.c:142: leaked_storage: Variable "tmp_list" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK: Virt_SwitchService.c:236: alloc_fn: Calling allocation function "run_command". Virt_SwitchService.c:123: alloc_fn: Storage is returned from allocation function "realloc". Virt_SwitchService.c:123: var_assign: Assigning: "tmp_list" = "realloc(arr, (i + 1) * 8UL)". Virt_SwitchService.c:134: var_assign: Assigning: "arr" = "tmp_list". Virt_SwitchService.c:151: return_alloc: Returning allocated memory "arr". Virt_SwitchService.c:236: var_assign: Assigning: "if_list" = storage returned from "run_command("/sbin/ifconfig -a | /bin/grep eth | /bin/awk \'{print$1}\'", &count, &s)". /builddir/build/BUILD/libvirt-cim-0.6.0/src/Virt_SwitchService.c: 269: leaked_storage: Variable "if_list" going out of scope leaks the storage it points to.
Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> --- libxkutil/cs_util_instance.c | 5 +++++ libxkutil/device_parsing.c | 1 + libxkutil/pool_parsing.c | 5 +++-- libxkutil/xmlgen.c | 4 +++- src/Virt_AppliedFilterList.c | 3 ++- src/Virt_DevicePool.c | 5 ++++- src/Virt_SwitchService.c | 24 +++++++++++++++++++ +---- src/Virt_VirtualSystemManagementService.c | 14 +++++++------- 8 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/libxkutil/cs_util_instance.c b/libxkutil/cs_util_instance.c index d21f0ff..a383147 100644 --- a/libxkutil/cs_util_instance.c +++ b/libxkutil/cs_util_instance.c @@ -104,6 +104,11 @@ int get_domain_list(virConnectPtr conn, virDomainPtr **_list) free(names); free(ids);
+ if (idx == 0) { + free(list); + list = NULL; + } + *_list = list;
return idx; diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c index b0eccfc..a1e8d6c 100644 --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -382,6 +382,7 @@ static int parse_vsi_device(xmlNode *dnode, struct net_device *vdevs) }
memcpy(&(vdevs->vsi), vsi_dev, sizeof(*vsi_dev)); + free(vsi_dev); return 1;
err: diff --git a/libxkutil/pool_parsing.c b/libxkutil/pool_parsing.c index f73b0fd..e41fc09 100644 --- a/libxkutil/pool_parsing.c +++ b/libxkutil/pool_parsing.c @@ -163,10 +163,11 @@ static int parse_disk_source(xmlNode *node, struct disk_pool *pool)
pool->device_paths_ct = ct; pool->device_paths = dev_paths; + return 1;
err: - - return 1; + free(dev_paths); + return 0; }
char *get_disk_pool_type(uint16_t type) diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c index 96b4e96..9a2ada9 100644 --- a/libxkutil/xmlgen.c +++ b/libxkutil/xmlgen.c @@ -889,8 +889,10 @@ char *device_to_xml(struct virt_device *_dev) goto out;
root = xmlNewNode(NULL, BAD_CAST "tmp"); - if (root == NULL) + if (root == NULL) { + cleanup_virt_devices(&dev, 1); goto out; + }
switch (type) { case CIM_RES_TYPE_DISK: diff --git a/src/Virt_AppliedFilterList.c b/src/Virt_AppliedFilterList.c index 6567118..bc31c14 100644 --- a/src/Virt_AppliedFilterList.c +++ b/src/Virt_AppliedFilterList.c @@ -197,7 +197,7 @@ static CMPIStatus list_to_net(
/* get domains */ dcount = get_domain_list(conn, &doms); - if (dcount < 0) { + if (dcount <= 0) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Failed to get domain list"); @@ -246,6 +246,7 @@ static CMPIStatus list_to_net( }
out: + free(doms); virConnectClose(conn);
return s; diff --git a/src/Virt_DevicePool.c b/src/Virt_DevicePool.c index ab0baa0..fe5573f 100644 --- a/src/Virt_DevicePool.c +++ b/src/Virt_DevicePool.c @@ -402,8 +402,10 @@ static char *_diskpool_member_of(virConnectPtr conn, char *pool = NULL;
count = get_diskpool_config(conn, &pools); - if (count == 0) + if (count == 0) { + free(pools); return NULL; + }
for (i = 0; i < count; i++) { if (_diskpool_is_member(conn, &pools[i], file)) { @@ -1091,6 +1093,7 @@ static CMPIStatus diskpool_instance(virConnectPtr conn, count = get_diskpool_config(conn, &pools); if ((id == NULL) && (count == 0)) { CU_DEBUG("No defined DiskPools"); + free(pools); return s; }
diff --git a/src/Virt_SwitchService.c b/src/Virt_SwitchService.c index 0d57f54..7e59d38 100644 --- a/src/Virt_SwitchService.c +++ b/src/Virt_SwitchService.c @@ -128,20 +128,19 @@ static char **run_command(char *func, int *len, CMPIStatus *s) { cu_statusf(_BROKER, s, CMPI_RC_ERR_NOT_FOUND, "Failed to realloc"); - return NULL; + goto err; }
arr = tmp_list;
- string = calloc(len, sizeof(char)); + string = strndup(buff, len); if (string == NULL) { CU_DEBUG("Failed to allocate memory"); cu_statusf(_BROKER, s, CMPI_RC_ERR_NOT_FOUND, "Failed to calloc"); - return NULL; + goto err; } - strncpy(string, buff, len); arr[i] = string; i++; } @@ -149,6 +148,19 @@ static char **run_command(char *func, int *len, CMPIStatus *s) { pclose(stream); *len = i; return arr; + + err: + /* undo everything */ + if (i > 0) { + int count; + for (count = 0; count < i; count++) + free(arr[count]); + } + + free(arr); + pclose(stream); + return NULL; + }
static CMPIStatus set_inst_properties(const CMPIBroker *broker, @@ -262,6 +274,10 @@ static CMPIStatus get_switchservice(const CMPIObjectPath *reference, CMSetProperty(inst, "IsVSISupported", (CMPIValue *)&vsi, CMPI_boolean); s.rc = CMPI_RC_OK;
+ for (i = 0; i < count; i++) + free(if_list[i]); + + free(if_list); out: virConnectClose(conn); *_inst = inst; diff --git a/src/Virt_VirtualSystemManagementService.c b/src/ Virt_VirtualSystemManagementService.c index f8c5f24..3a0b423 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -429,6 +429,7 @@ static int bootord_vssd_to_domain(CMPIInstance *inst,
if (CMIsNullValue(boot_elem)) { CU_DEBUG("Null BootDevice"); + free(tmp_str_arr); return 0; }
@@ -437,6 +438,7 @@ static int bootord_vssd_to_domain(CMPIInstance *inst, if (str == NULL) { CU_DEBUG("Could not extract char pointer from " "CMPIArray"); + free(tmp_str_arr); return 0; }
@@ -2766,13 +2768,11 @@ static CMPIStatus _update_resources_for (const CMPIContext *context, }
if (func == &resource_add) { - indication = strdup(RASD_IND_CREATED); - } - else if (func == &resource_del) { - indication = strdup(RASD_IND_DELETED); - } - else { - indication = strdup(RASD_IND_MODIFIED); + indication = RASD_IND_CREATED; + } else if (func == &resource_del) { + indication = RASD_IND_DELETED; + } else { + indication = RASD_IND_MODIFIED; char *dummy_name = NULL;
if (asprintf(&dummy_name, "%s/%s",dominfo->name, devid) == -1) { -- 1.7.7.5
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
From: "Eduardo Lima (Etrunko)" <eblima@br.ibm.com> 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: USE_AFTER_FREE: xml_parse_test.c:239: freed_arg: "free" frees "xml". xml_parse_test.c:242: pass_freed_arg: Passing freed pointer "xml" as an argument to function "printf". Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> --- libxkutil/xml_parse_test.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/libxkutil/xml_parse_test.c b/libxkutil/xml_parse_test.c index 16ceefb..384593d 100644 --- a/libxkutil/xml_parse_test.c +++ b/libxkutil/xml_parse_test.c @@ -236,11 +236,10 @@ static int dominfo_from_file(const char *fname, struct domain **d) ret = get_dominfo_from_xml(xml, d); + printf("XML:\n%s", xml); free(xml); fclose(file); - printf("XML:\n%s", xml); - return ret; } -- 1.7.7.5
+1 Sharad Mishra Open Virtualization Linux Technology Center IBM libvirt-cim-bounces@redhat.com wrote on 01/18/2012 08:06:11 AM:
"Eduardo Lima (Etrunko)" <eblima@linux.vnet.ibm.com> Sent by: libvirt-cim-bounces@redhat.com
01/18/2012 08:06 AM
Please respond to List for discussion and development of libvirt CIM <libvirt-cim@redhat.com>
To
libvirt-cim@redhat.com
cc
"Eduardo Lima \(Etrunko\)" <eblima@br.ibm.com>
Subject
[Libvirt-cim] [PATCH 3/4] xml_parse_test: Fix invalid dereference
From: "Eduardo Lima (Etrunko)" <eblima@br.ibm.com>
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: USE_AFTER_FREE: xml_parse_test.c:239: freed_arg: "free" frees "xml". xml_parse_test.c:242: pass_freed_arg: Passing freed pointer "xml" as an argument to function "printf".
Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> --- libxkutil/xml_parse_test.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/libxkutil/xml_parse_test.c b/libxkutil/xml_parse_test.c index 16ceefb..384593d 100644 --- a/libxkutil/xml_parse_test.c +++ b/libxkutil/xml_parse_test.c @@ -236,11 +236,10 @@ static int dominfo_from_file(const char *fname, struct domain **d)
ret = get_dominfo_from_xml(xml, d);
+ printf("XML:\n%s", xml); free(xml); fclose(file);
- printf("XML:\n%s", xml); - return ret; }
-- 1.7.7.5
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
From: "Eduardo Lima (Etrunko)" <eblima@br.ibm.com> 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: UNINIT: Virt_VirtualSystemManagementService.c:2798: var_decl: Declaring variable "s" without initializer. Virt_VirtualSystemManagementService.c:2901: uninit_use: Using uninitialized value "s": field "s".msg is uninitialized. Error: UNINIT: Virt_VirtualSystemSnapshotService.c:393: var_decl: Declaring variable "s" without initializer. Virt_VirtualSystemSnapshotService.c:398: uninit_use_in_call: Using uninitialized value "s.rc" when calling "new_context". Virt_VirtualSystemSnapshotService.c:378: read_parm_fld: Reading a parameter field. Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> --- src/Virt_VirtualSystemManagementService.c | 2 +- src/Virt_VirtualSystemSnapshotService.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index 3a0b423..6f42c42 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -2732,7 +2732,7 @@ static CMPIStatus _update_resources_for(const CMPIContext *context, CMPIInstance *rasd, resmod_fn func) { - CMPIStatus s; + CMPIStatus s = {CMPI_RC_OK, NULL}; struct domain *dominfo = NULL; uint16_t type; char *xml = NULL; diff --git a/src/Virt_VirtualSystemSnapshotService.c b/src/Virt_VirtualSystemSnapshotService.c index 898fa57..aae628f 100644 --- a/src/Virt_VirtualSystemSnapshotService.c +++ b/src/Virt_VirtualSystemSnapshotService.c @@ -390,7 +390,7 @@ static CMPIStatus start_snapshot_job(const CMPIObjectPath *ref, CMPIArgs *argsout) { struct snap_context *ctx; - CMPIStatus s; + CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIObjectPath *job; CMPIObjectPath *vssd; CMPIInstance *inst; -- 1.7.7.5
+1 Sharad Mishra Open Virtualization Linux Technology Center IBM libvirt-cim-bounces@redhat.com wrote on 01/18/2012 08:06:12 AM:
"Eduardo Lima (Etrunko)" <eblima@linux.vnet.ibm.com> Sent by: libvirt-cim-bounces@redhat.com
01/18/2012 08:06 AM
Please respond to List for discussion and development of libvirt CIM <libvirt-cim@redhat.com>
To
libvirt-cim@redhat.com
cc
"Eduardo Lima \(Etrunko\)" <eblima@br.ibm.com>
Subject
[Libvirt-cim] [PATCH 4/4] Fix possible use of unitialized variables
From: "Eduardo Lima (Etrunko)" <eblima@br.ibm.com>
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: UNINIT: Virt_VirtualSystemManagementService.c:2798: var_decl: Declaring variable "s" without initializer. Virt_VirtualSystemManagementService.c:2901: uninit_use: Using uninitialized value "s": field "s".msg is uninitialized.
Error: UNINIT: Virt_VirtualSystemSnapshotService.c:393: var_decl: Declaring variable "s" without initializer. Virt_VirtualSystemSnapshotService.c:398: uninit_use_in_call: Using uninitialized value "s.rc" when calling "new_context". Virt_VirtualSystemSnapshotService.c:378: read_parm_fld: Reading a parameter field.
Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> --- src/Virt_VirtualSystemManagementService.c | 2 +- src/Virt_VirtualSystemSnapshotService.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/Virt_VirtualSystemManagementService.c b/src/ Virt_VirtualSystemManagementService.c index 3a0b423..6f42c42 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -2732,7 +2732,7 @@ static CMPIStatus _update_resources_for(const CMPIContext *context, CMPIInstance *rasd, resmod_fn func) { - CMPIStatus s; + CMPIStatus s = {CMPI_RC_OK, NULL}; struct domain *dominfo = NULL; uint16_t type; char *xml = NULL; diff --git a/src/Virt_VirtualSystemSnapshotService.c b/src/ Virt_VirtualSystemSnapshotService.c index 898fa57..aae628f 100644 --- a/src/Virt_VirtualSystemSnapshotService.c +++ b/src/Virt_VirtualSystemSnapshotService.c @@ -390,7 +390,7 @@ static CMPIStatus start_snapshot_job(const CMPIObjectPath *ref, CMPIArgs *argsout) { struct snap_context *ctx; - CMPIStatus s; + CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIObjectPath *job; CMPIObjectPath *vssd; CMPIInstance *inst; -- 1.7.7.5
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
+1 Sharad Mishra Open Virtualization Linux Technology Center IBM libvirt-cim-bounces@redhat.com wrote on 01/18/2012 08:06:08 AM:
"Eduardo Lima (Etrunko)" <eblima@linux.vnet.ibm.com> Sent by: libvirt-cim-bounces@redhat.com
01/18/2012 08:06 AM
Please respond to List for discussion and development of libvirt CIM <libvirt-cim@redhat.com>
To
libvirt-cim@redhat.com
cc
"Eduardo Lima \(Etrunko\)" <eblima@br.ibm.com>
Subject
[Libvirt-cim] [PATCH v3 0/4] Fix errors raised by Coverity tool
From: "Eduardo Lima (Etrunko)" <eblima@br.ibm.com>
Yet another series of patches fixing errors revealed by Coverity tool. It would be nice if we had a similar setup so we could run the tool on a regular basis. I started playing with clang a while ago. It also provides a static analyser, but I could not complete the setup by then.
https://bugzilla.redhat.com/show_bug.cgi?id=750418
Changes from v2: - Revert leak change for libxkutil/xmlgen.c in patch 2 which was causing a double-free crash and correctly fix the leak problem.
Changes from v1: - Fix compilation error in Patch 1
Best regards, Etrunko
-- Eduardo de Barros Lima Software Engineer, Open Virtualization Linux Technology Center - IBM/Brazil eblima@br.ibm.com
Eduardo Lima (Etrunko) (4): libxkutil: Fix possible NULL dereferences Fix possible memory leaks xml_parse_test: Fix invalid dereference Fix possible use of unitialized variables
libxkutil/cs_util_instance.c | 5 +++++ libxkutil/device_parsing.c | 14 +++++++------- libxkutil/pool_parsing.c | 5 +++-- libxkutil/xml_parse_test.c | 3 +-- libxkutil/xmlgen.c | 8 +++++--- src/Virt_AppliedFilterList.c | 3 ++- src/Virt_Device.c | 20 ++++++++++++++++++++ src/Virt_DevicePool.c | 19 ++++++++++++++++++- src/Virt_SwitchService.c | 24 +++++++++++++++++++ +---- src/Virt_VirtualSystemManagementService.c | 16 ++++++++-------- src/Virt_VirtualSystemSnapshotService.c | 2 +- 11 files changed, 90 insertions(+), 29 deletions(-)
-- 1.7.7.5
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
Thanks, it would be nice to have this upstream soon so we still have a chance to get it into RHEL 6.3. Best regards, Etrunko On 01/18/2012 06:46 PM, Sharad Mishra wrote:
+1
Sharad Mishra Open Virtualization Linux Technology Center IBM
libvirt-cim-bounces@redhat.com wrote on 01/18/2012 08:06:08 AM:
"Eduardo Lima (Etrunko)" <eblima@linux.vnet.ibm.com> Sent by: libvirt-cim-bounces@redhat.com
01/18/2012 08:06 AM
Please respond to List for discussion and development of libvirt CIM <libvirt-cim@redhat.com>
To
libvirt-cim@redhat.com
cc
"Eduardo Lima \(Etrunko\)" <eblima@br.ibm.com>
Subject
[Libvirt-cim] [PATCH v3 0/4] Fix errors raised by Coverity tool
From: "Eduardo Lima (Etrunko)" <eblima@br.ibm.com>
Yet another series of patches fixing errors revealed by Coverity tool. It would be nice if we had a similar setup so we could run the tool on a regular basis. I started playing with clang a while ago. It also provides a static analyser, but I could not complete the setup by then.
https://bugzilla.redhat.com/show_bug.cgi?id=750418
Changes from v2: - Revert leak change for libxkutil/xmlgen.c in patch 2 which was causing a double-free crash and correctly fix the leak problem.
Changes from v1: - Fix compilation error in Patch 1
Best regards, Etrunko
-- Eduardo de Barros Lima Software Engineer, Open Virtualization Linux Technology Center - IBM/Brazil eblima@br.ibm.com
Eduardo Lima (Etrunko) (4): libxkutil: Fix possible NULL dereferences Fix possible memory leaks xml_parse_test: Fix invalid dereference Fix possible use of unitialized variables
libxkutil/cs_util_instance.c | 5 +++++ libxkutil/device_parsing.c | 14 +++++++------- libxkutil/pool_parsing.c | 5 +++-- libxkutil/xml_parse_test.c | 3 +-- libxkutil/xmlgen.c | 8 +++++--- src/Virt_AppliedFilterList.c | 3 ++- src/Virt_Device.c | 20 ++++++++++++++++++++ src/Virt_DevicePool.c | 19 ++++++++++++++++++- src/Virt_SwitchService.c | 24 +++++++++++++++++++ +---- src/Virt_VirtualSystemManagementService.c | 16 ++++++++-------- src/Virt_VirtualSystemSnapshotService.c | 2 +- 11 files changed, 90 insertions(+), 29 deletions(-)
-- 1.7.7.5
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Eduardo de Barros Lima Software Engineer, Open Virtualization Linux Technology Center - IBM/Brazil eblima@br.ibm.com
participants (2)
-
Eduardo Lima (Etrunko) -
Sharad Mishra