On 01/22/2014 08:30 PM, John Ferlan wrote:
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 <[email protected]>
---
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;
I am getting a compile error here and below for the free of uuid.
error: assignment discards 'const' qualifier from pointer target type
[-Werror]
error: passing argument 1 of 'free' discards 'const' qualifier from
pointer target type [-Werror]
Removing the const in the declaration works... for me.
} 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);
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
_______________________________________________
Libvirt-cim mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvirt-cim
