Re: [Libvirt-cim] [PATCH 1/6] VSMS: Coverity cleanups

A new version of Coverity found a number of issues: parse_ip_address(): FORWARD_NULL - Benign issue regarding how 'tmp_ip' was compared against NULL for the IPv6 processing and then used blindly later when strdup()'ing into *ip. Rather than use NULL check, compare against return of 1 or more which indicates that something is there update_system_settings(): RESOURCE_LEAK - The 'uuid' value was being leaked if strdup()'d. Also rather than strdup()'g and strdup()'d value and risking failure, just assign the initially strdup()'d value and reinitialize uuid to NULL fv_vssd_to_domain(): USE_AFTER_FREE - The domain->os_info.fv.arch is free()'d only to be potentially strdup()'d after processing the 'cu_get_str_prop()' for "Arch". The complaint was that it was possible to not strdup() a new value and thus possible to pass a free()'d value to get_default_machine(). Passing a NULL is not an issue as that is checked. Additionally found by inspection, 'val' was not initialized to NULL, so the setting of os_info.fv.arch may not be what was expected. Also, after processing "Arch" it was not reinitialized to NULL so its contents could potentially have been saved in os_info.fv.machine. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/Virt_VirtualSystemManagementService.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index 5c7238f..b624d8c 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -464,7 +464,7 @@ static int fv_vssd_to_domain(CMPIInstance *inst, { int ret = 1; int retr; - const char *val; + const char *val = NULL; const char *domtype = NULL; const char *ostype = "hvm"; struct capabilities *capsinfo = NULL; @@ -494,6 +494,7 @@ static int fv_vssd_to_domain(CMPIInstance *inst, } free(domain->os_info.fv.arch); + domain->os_info.fv.arch = NULL; retr = cu_get_str_prop(inst, "Arch", &val); if (retr != CMPI_RC_OK) { if (capsinfo != NULL) { /* set default */ @@ -506,6 +507,8 @@ static int fv_vssd_to_domain(CMPIInstance *inst, domain->os_info.fv.arch = strdup(val); free(domain->os_info.fv.machine); + domain->os_info.fv.machine = NULL; + val = NULL; retr = cu_get_str_prop(inst, "Machine", &val); if (retr != CMPI_RC_OK) { if (capsinfo != NULL && domtype != NULL) { /* set default */ @@ -1415,7 +1418,7 @@ static int parse_ip_address(const char *id, if (strstr(id, "[") != NULL) { /* its an ipv6 address */ ret = sscanf(id, "%a[^]]]:%as", &tmp_ip, &tmp_port); - if (tmp_ip != NULL) { + if (ret >= 1) { tmp_ip = realloc(tmp_ip, strlen(tmp_ip) + 2); if (tmp_ip == NULL) { ret = 0; @@ -2798,7 +2801,8 @@ static CMPIStatus update_system_settings(const CMPIContext *context, } if ((dominfo->uuid == NULL) || (STREQ(dominfo->uuid, ""))) { - dominfo->uuid = strdup(uuid); + dominfo->uuid = uuid; + uuid = NULL;

} else if (!STREQ(uuid, dominfo->uuid)) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, @@ -2829,6 +2833,7 @@ static CMPIStatus update_system_settings(const CMPIContext *context, } out: + free(uuid); free(xml); virDomainFree(dom); virConnectClose(conn);