[PATCH 0/2] Address Coverity issues
During a scan of the Red Hat libvirt-cim sources there were 3 issues discovered that weren't already address by upstream patches. See https://bugzilla.redhat.com/show_bug.cgi?id=885104 These patches address them... I'd like to get these in and then cut a release so that it can be included in RHEL7.0 pools which are set to close at the end of July. I ran these changes through cimtest with no new regression. John Ferlan (2): CSI: Address two Coverity isssues Resolve Coverity complaint src/Virt_ComputerSystemIndication.c | 14 ++++++++------ src/Virt_VSMigrationService.c | 2 ++ 2 files changed, 10 insertions(+), 6 deletions(-) -- 1.8.1.4
Found by a scan done on the Red Hat libvirt-cim code, but still valid for the upstream code. Error: CHECKED_RETURN (CWE-252): libvirt-cim-0.6.1/src/Virt_ComputerSystemIndication.c:507: switch: Switch case value "0" libvirt-cim-0.6.1/src/Virt_ComputerSystemIndication.c:508: switch_case: Reached case "0" libvirt-cim-0.6.1/src/Virt_ComputerSystemIndication.c:509: cond_false: Condition "detail == VIR_DOMAIN_EVENT_DEFINED_ADDED", taking false branch libvirt-cim-0.6.1/src/Virt_ComputerSystemIndication.c:513: else_branch: Reached else branch libvirt-cim-0.6.1/src/Virt_ComputerSystemIndication.c:513: cond_true: Condition "detail == VIR_DOMAIN_EVENT_DEFINED_UPDATED", taking true branch libvirt-cim-0.6.1/src/Virt_ComputerSystemIndication.c:518: break: Breaking from switch libvirt-cim-0.6.1/src/Virt_ComputerSystemIndication.c:529: switch_end: Reached end of switch libvirt-cim-0.6.1/src/Virt_ComputerSystemIndication.c:531: cond_true: Condition "cs_event != CS_CREATED", taking true branch libvirt-cim-0.6.1/src/Virt_ComputerSystemIndication.c:533: check_return: Calling function "virDomainGetUUIDString(virDomainPtr, char *)" without checking return value (as is done elsewhere 4 out of 5 times). libvirt-cim-0.6.1/libxkutil/infostore.c:287: example_checked: "virDomainGetUUIDString(dom, uuid)" has its value checked in "virDomainGetUUIDString(dom, uuid) != 0". libvirt-cim-0.6.1/src/Virt_ComputerSystem.c:76: example_assign: Assigning: "ret" = return value from "virDomainGetUUIDString(dom, uuid)". libvirt-cim-0.6.1/src/Virt_ComputerSystem.c:77: example_checked: "ret" has its value checked in "ret != 0". libvirt-cim-0.6.1/src/Virt_ComputerSystemIndication.c:143: example_assign: Assigning: "rc" = return value from "virDomainGetUUIDString(dom_ptr, dom->uuid)". libvirt-cim-0.6.1/src/Virt_ComputerSystemIndication.c:144: example_checked: "rc" has its value checked in "rc == -1". libvirt-cim-0.6.1/src/Virt_VSMigrationService.c:847: example_checked: "virDomainGetUUIDString(dom, uuid)" has its value checked in "virDomainGetUUIDString(dom, uuid) != 0". libvirt-cim-0.6.1/src/Virt_ComputerSystemIndication.c:533: unchecked_value: No check of the return value of "virDomainGetUUIDString(dom, &uuid[0])". Simple case of return value not being checked. In this instance the UUID would not have been filled in and the ensuing code to 'list_find' would probably fail resulting in 'dom_xml' being returned as NULL and a failure in the dom_xml == NULL check. 2. Error: DEADCODE (CWE-561): libvirt-cim-0.6.1/src/Virt_ComputerSystemIndication.c:553: dead_error_condition: The condition "event == 0" cannot be true. libvirt-cim-0.6.1/src/Virt_ComputerSystemIndication.c:553: dead_error_line: Execution cannot reach this expression "detail == 0" inside statement "if (event == 0 && detail ==...". In 'csi_domain_event_cb', the else if condition could never be reached so I just moved it inside the existing if condition. --- src/Virt_ComputerSystemIndication.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Virt_ComputerSystemIndication.c b/src/Virt_ComputerSystemIndication.c index a4d30ab..20c60bc 100644 --- a/src/Virt_ComputerSystemIndication.c +++ b/src/Virt_ComputerSystemIndication.c @@ -1269,7 +1269,10 @@ static void csi_domain_event_cb(virConnectPtr conn, if (cs_event != CS_CREATED) { char uuid[VIR_UUID_STRING_BUFLEN] = {0}; - virDomainGetUUIDString(dom, &uuid[0]); + if (virDomainGetUUIDString(dom, &uuid[0]) == -1) { + CU_DEBUG("Failed to get domain UUID"); + goto end; + } pthread_mutex_lock(&lifecycle_mutex); dom_xml = list_find(thread->dom_list, uuid); pthread_mutex_unlock(&lifecycle_mutex); @@ -1294,12 +1297,11 @@ static void csi_domain_event_cb(virConnectPtr conn, free(dom_xml->name); free(dom_xml->xml); csi_dom_xml_set(dom_xml, dom, &s); + } else if (detail == VIR_DOMAIN_EVENT_UNDEFINED_REMOVED) { + pthread_mutex_lock(&lifecycle_mutex); + list_remove(thread->dom_list, dom_xml); + pthread_mutex_unlock(&lifecycle_mutex); } - } else if (event == VIR_DOMAIN_EVENT_DEFINED && - detail == VIR_DOMAIN_EVENT_UNDEFINED_REMOVED) { - pthread_mutex_lock(&lifecycle_mutex); - list_remove(thread->dom_list, dom_xml); - pthread_mutex_unlock(&lifecycle_mutex); } end: -- 1.8.1.4
Found by a scan done on the Red Hat libvirt-cim code, but still valid for the upstream code. Error: SECURE_TEMP (CWE-377): [#def23] libvirt-cim-0.6.1/src/Virt_VSMigrationService.c:504: cond_true: Condition "__retval != NULL", taking true branch libvirt-cim-0.6.1/src/Virt_VSMigrationService.c:507: cond_false: Condition "filename == NULL", taking false branch libvirt-cim-0.6.1/src/Virt_VSMigrationService.c:510: if_end: End of if statement libvirt-cim-0.6.1/src/Virt_VSMigrationService.c:512: secure_temp: Calling "mkstemp(char *)" without securely setting umask first. Resolve by adding umask code --- src/Virt_VSMigrationService.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Virt_VSMigrationService.c b/src/Virt_VSMigrationService.c index f48d56b..78f9e05 100644 --- a/src/Virt_VSMigrationService.c +++ b/src/Virt_VSMigrationService.c @@ -569,6 +569,7 @@ static char *write_params(CMPIArray *array) { int i; int fd; + mode_t cur_umask = umask(S_IRWXO|S_IRWXG); char *filename = strdup("/tmp/libvirtcim_mig.XXXXXX"); FILE *file = NULL; @@ -578,6 +579,7 @@ static char *write_params(CMPIArray *array) } fd = mkstemp(filename); + umask(cur_umask); if (fd < 0) { CU_DEBUG("Unable to get temporary file: %s", strerror(errno)); free(filename); -- 1.8.1.4
On Fri, Jul 19, 2013 at 02:28:56PM -0400, John Ferlan wrote:
During a scan of the Red Hat libvirt-cim sources there were 3 issues discovered that weren't already address by upstream patches. See
https://bugzilla.redhat.com/show_bug.cgi?id=885104
These patches address them... I'd like to get these in and then cut a release so that it can be included in RHEL7.0 pools which are set to close at the end of July.
I ran these changes through cimtest with no new regression.
John Ferlan (2): CSI: Address two Coverity isssues Resolve Coverity complaint
src/Virt_ComputerSystemIndication.c | 14 ++++++++------ src/Virt_VSMigrationService.c | 2 ++ 2 files changed, 10 insertions(+), 6 deletions(-)
Okay, from an eye review both patches looks fine, so applied and pushed ! thanks ! Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veillard@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (2)
-
Daniel Veillard -
John Ferlan