[PATCH] VSMS: fv_vssd_to_domain() resolve Coverity error

Coverity discovered that the free(domain->os_info.fv.arch) and then usage later on during get_default_machine() and get_default_emulator() calls could result in using free()'d memory. If the 'cu_get_str_prop() failed or capsinfo == NULL, then the fv.arch wouldn't necessarily be strdup()'d. Passing a NULL os_info.fv_arch into the get*() API's is fine since they'll call findDomainInfo() which can handle a NULL arch value. Also added an initialization of val just to be safe. I don't think it's necessary though. --- NOTE: I found this during a Coverity run applying the endianness patches. For some reason Coverity "woke up" and saw this even though it hasn't found this issue in a couple months of runs since the changes to this module were made. See commit id '117dabb9'. src/Virt_VirtualSystemManagementService.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index d51f230..9f8b5b9 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 */ -- 1.8.3.1

On 11/12/2013 06:29 PM, John Ferlan wrote:
Coverity discovered that the free(domain->os_info.fv.arch) and then usage later on during get_default_machine() and get_default_emulator() calls could result in using free()'d memory.
If the 'cu_get_str_prop() failed or capsinfo == NULL, then the fv.arch wouldn't necessarily be strdup()'d.
Passing a NULL os_info.fv_arch into the get*() API's is fine since they'll call findDomainInfo() which can handle a NULL arch value.
Also added an initialization of val just to be safe. I don't think it's necessary though.
--- NOTE:
I found this during a Coverity run applying the endianness patches. For some reason Coverity "woke up" and saw this even though it hasn't found this issue in a couple months of runs since the changes to this module were made. See commit id '117dabb9'.
src/Virt_VirtualSystemManagementService.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
I'm going to revisit this as part of a larger set of Coverity patches. I've updated to Coverity 7.0 and there's 27 "issues" (although some in the same module). Once they are pushed maybe we can go with another release. John

On 11/13/2013 12:29 AM, John Ferlan wrote:
Coverity discovered that the free(domain->os_info.fv.arch) and then usage later on during get_default_machine() and get_default_emulator() calls could result in using free()'d memory.
If the 'cu_get_str_prop() failed or capsinfo == NULL, then the fv.arch wouldn't necessarily be strdup()'d.
Passing a NULL os_info.fv_arch into the get*() API's is fine since they'll call findDomainInfo() which can handle a NULL arch value.
Also added an initialization of val just to be safe. I don't think it's necessary though.
--- NOTE:
I found this during a Coverity run applying the endianness patches. For some reason Coverity "woke up" and saw this even though it hasn't found this issue in a couple months of runs since the changes to this module were made. See commit id '117dabb9'.
src/Virt_VirtualSystemManagementService.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index d51f230..9f8b5b9 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 */
ACK -- 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
participants (2)
-
Boris Fiuczynski
-
John Ferlan