[Libvirt-cim] [PATCH 2/4] Fix possible memory leaks
Sharad Mishra
snmishra at us.ibm.com
Wed Jan 18 04:54:53 UTC 2012
libvirt-cim-bounces at redhat.com wrote on 01/13/2012 10:53:20 AM:
> "Eduardo Lima (Etrunko)" <eblima at linux.vnet.ibm.com>
> Sent by: libvirt-cim-bounces at redhat.com
>
> 01/13/12 10:53 AM
>
> Please respond to
> List for discussion and development of libvirt CIM
<libvirt-cim at redhat.com>
>
> To
>
> libvirt-cim at redhat.com
>
> cc
>
> "Eduardo Lima \(Etrunko\)" <eblima at br.ibm.com>
>
> Subject
>
> [Libvirt-cim] [PATCH 2/4] Fix possible memory leaks
>
> From: "Eduardo Lima (Etrunko)" <eblima at 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 at 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..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);
This change is causing FilterList -> 03_create.py to dump core. There are
few others that are segfaulting.
-Sharad
> 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
>
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim
>
More information about the Libvirt-cim
mailing list