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: 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(a)br.ibm.com&gt; --- 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..7fff4d1 100644 --- a/libxkutil/xmlgen.c +++ b/libxkutil/xmlgen.c @@ -928,7 +928,6 @@ char *device_to_xml(struct virt_device *_dev) dominfo->dev_input = dev; break; default: - cleanup_virt_devices(&dev, 1); goto out; } @@ -942,6 +941,9 @@ char *device_to_xml(struct virt_device *_dev) out: CU_DEBUG("Created Device XML:\n%s\n", xml); + if (dev != NULL) + cleanup_virt_devices(&dev, 1); + cleanup_dominfo(&dominfo); xmlFreeNode(root); 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

On 01/18/2012 02:54 AM, Sharad Mishra wrote: [snip] You're right, it will end up in a double-free. Thanks for the spot. Will send the patch with the correct fix once again. Best regards, Etrunko -- Eduardo de Barros Lima Software Engineer, Open Virtualization Linux Technology Center - IBM/Brazil eblima(a)br.ibm.com

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: 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(a)br.ibm.com&gt; --- 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