1:28 p.m.
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
1:28 p.m.
New subject: [PATCH 1/2] CSI: Address two Coverity isssues
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
1:28 p.m.
New subject: [PATCH 2/2] Resolve Coverity complaint
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
2:29 a.m.
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(a)redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
4167
days inactive
4173
days old
3 comments
2 participants
participants (2)
-
Daniel Veillard -
John Ferlan