From: Peter Dinda <pdinda@northwestern.edu>
Date: Tue, 1 Sep 2015 16:47:14 +0000 (-0500)
Subject: Cleanup and sanity-checking of before/after null-check and copy+paste errors (Coverit... 
X-Git-Url: http://v3vee.org/palacios/gitweb/gitweb.cgi?p=palacios.git;a=commitdiff_plain;h=88a3605446744969abe6f193a7bc20e62d5aa555

Cleanup and sanity-checking of before/after null-check and copy+paste errors (Coverity static analysis)
---

diff --git a/linux_module/iface-keyed-stream.c b/linux_module/iface-keyed-stream.c
index 365a9c7..a6fb960 100644
--- a/linux_module/iface-keyed-stream.c
+++ b/linux_module/iface-keyed-stream.c
@@ -2780,7 +2780,7 @@ static sint64_t write_key_net(v3_keyed_stream_t stream, v3_keyed_stream_key_t ke
 	    ERROR("Could not send tag length in write_key_net\n");
 	    return -1;
 	} 
-	if (send_msg(nks->ns,tag,taglen)!=len) { 
+	if (send_msg(nks->ns,tag,taglen)!=taglen) { 
 	    ERROR("Could not send tag in write_key_net\n");
 	    return -1;
 	}
diff --git a/palacios/src/devices/apic.c b/palacios/src/devices/apic.c
index e44c9b0..a325006 100644
--- a/palacios/src/devices/apic.c
+++ b/palacios/src/devices/apic.c
@@ -894,6 +894,11 @@ static int deliver_ipi(struct apic_state * src_apic,
 	    }
 
 
+	    if (!src_apic) { 
+		PrintError(VM_NONE, VCORE_NONE, "Attempting to INIT from somewhere other than an APIC...  Ignoring\n");
+		break;
+	    }
+
 	    if (dst_apic->ipi_state != INIT_ST) { 
 		v3_raise_barrier(dst_core->vm_info, src_apic->core);
 		dst_core->core_run_state = CORE_STOPPED;
diff --git a/palacios/src/palacios/vmm_config.c b/palacios/src/palacios/vmm_config.c
index 9e73d11..e4c9b4f 100644
--- a/palacios/src/palacios/vmm_config.c
+++ b/palacios/src/palacios/vmm_config.c
@@ -640,7 +640,7 @@ struct v3_vm_info * v3_config_guest(void * cfg_blob, void * priv_data) {
     vm = allocate_guest(num_cores);    
 
     if (!vm) {
-	PrintError(VM_NONE, VCORE_NONE, "Could not allocate %d core guest\n", vm->num_cores);
+	PrintError(VM_NONE, VCORE_NONE, "Could not allocate %d core guest\n", num_cores);
 	return NULL;
     }
 
diff --git a/palacios/src/palacios/vmm_extensions.c b/palacios/src/palacios/vmm_extensions.c
index e9f4a71..af2c0e2 100644
--- a/palacios/src/palacios/vmm_extensions.c
+++ b/palacios/src/palacios/vmm_extensions.c
@@ -58,6 +58,12 @@ int V3_init_extensions() {
     ext_table = v3_create_htable(0, ext_hash_fn, ext_eq_fn);
 
     while (tmp_ext != __stop__v3_extensions) {
+
+        if ((*tmp_ext) && (*tmp_ext)->init && ((*tmp_ext)->init() != 0)) {
+	    PrintError(VM_NONE, VCORE_NONE, "Could not initialize extension (%s)\n", (*tmp_ext)->name);
+	    return -1;
+	} 
+
         V3_Print(VM_NONE, VCORE_NONE, "Registering Extension (%s)\n", (*tmp_ext)->name);
 
 	if (v3_htable_search(ext_table, (addr_t)((*tmp_ext)->name))) {
@@ -70,11 +76,6 @@ int V3_init_extensions() {
 	    return -1;
 	}
 
-        if ((*tmp_ext) && (*tmp_ext)->init && ((*tmp_ext)->init() != 0)) {
-	    PrintError(VM_NONE, VCORE_NONE, "Could not initialize extension (%s)\n", (*tmp_ext)->name);
-	    return -1;
-	} 
-
 	tmp_ext = &(__start__v3_extensions[++i]);
     }
 
@@ -115,22 +116,24 @@ int v3_deinit_ext_manager(struct v3_vm_info * vm)  {
     list_for_each_entry_safe(ext, tmp, &(ext_state->extensions), node) {
         
 	V3_Print(vm, VCORE_NONE, "Cleaning up Extension (%s)\n", ext->impl->name);
-	if ((ext->impl) && (ext->impl->vm_deinit)) {
-	    if (ext->impl->vm_deinit(vm, ext->priv_data) == -1) {
-		PrintError(vm, VCORE_NONE, "Error cleaning up extension (%s)\n", ext->impl->name);
-		return -1;
+	if (ext->impl) { 
+	    if (ext->impl->vm_deinit) {
+		if (ext->impl->vm_deinit(vm, ext->priv_data) == -1) {
+		    PrintError(vm, VCORE_NONE, "Error cleaning up extension (%s)\n", ext->impl->name);
+		    return -1;
+		}
 	    }
-	}
-
-        if (ext->impl->on_exit)
-            list_del(&ext->exit_node);
-
-        if (ext->impl->on_entry)
-            list_del(&ext->entry_node);
-
-        list_del(&ext->node);
-        V3_Free(ext);
 
+	    if (ext->impl->on_exit)
+		list_del(&ext->exit_node);
+	    
+	    if (ext->impl->on_entry)
+		list_del(&ext->entry_node);
+	}
+	    
+	list_del(&ext->node);
+	V3_Free(ext);
+	    
     }
 
     return 0;
diff --git a/palacios/src/palacios/vmm_paging.c b/palacios/src/palacios/vmm_paging.c
index c44f461..85c0874 100644
--- a/palacios/src/palacios/vmm_paging.c
+++ b/palacios/src/palacios/vmm_paging.c
@@ -357,7 +357,7 @@ static int check_pt_32_cb(struct guest_info * info, page_type_t type, addr_t vad
 	    return -1;
     }
 
-    if (chk_data->access_status != PT_ACCESS_OK) {
+    if (*(chk_data->access_status) != PT_ACCESS_OK) {
 	return 1;
     }
 
@@ -386,7 +386,7 @@ static int check_pt_32pae_cb(struct guest_info * info, page_type_t type, addr_t
 	    return -1;
     }
 
-    if (chk_data->access_status != PT_ACCESS_OK) {
+    if (*(chk_data->access_status) != PT_ACCESS_OK) {
 	return 1;
     }
 
@@ -419,7 +419,7 @@ static int check_pt_64_cb(struct guest_info * info, page_type_t type, addr_t vad
 	    return -1;
     }
 
-    if (chk_data->access_status != PT_ACCESS_OK) {
+    if (*(chk_data->access_status) != PT_ACCESS_OK) {
 	return 1;
     }
 
diff --git a/palacios/src/palacios/vmm_rbtree.c b/palacios/src/palacios/vmm_rbtree.c
index 3b15bcf..ff63faa 100644
--- a/palacios/src/palacios/vmm_rbtree.c
+++ b/palacios/src/palacios/vmm_rbtree.c
@@ -287,7 +287,10 @@ void v3_rb_erase(struct rb_node *node, struct rb_root *root)
 	root->rb_node = child;
 
  color:
-    if (color == RB_BLACK)
+    // parent check here is because a possible
+    // codepath here has parent=null, which would
+    // be a disaster in kernel
+    if (color == RB_BLACK && parent) 
 	__rb_erase_color(child, parent, root);
 }
 
diff --git a/palacios/src/palacios/vmm_time.c b/palacios/src/palacios/vmm_time.c
index a88b978..dff562b 100644
--- a/palacios/src/palacios/vmm_time.c
+++ b/palacios/src/palacios/vmm_time.c
@@ -346,7 +346,7 @@ handle_time_configuration(struct v3_vm_info * vm, v3_cfg_tree_t *cfg) {
 	} else {
 	    PrintDebug(vm, VCORE_NONE,"VM time slaved to host TSC.\n");
 	}
-    }
+    } 	
 
     // Should we make a separate TSC device that handles this sort of thing?
     tsc = v3_cfg_val(cfg, "tsc");
@@ -356,7 +356,7 @@ handle_time_configuration(struct v3_vm_info * vm, v3_cfg_tree_t *cfg) {
 		PrintError(vm, VCORE_NONE, "WARNING: Guest TSC set to passthrough host TSC, but guest time not slaved to host time.");
 	    }
 	    vm->time_state.flags |= V3_TIME_TSC_PASSTHROUGH;
-	} else if (strcasecmp(source, "guest") != 0) {
+	} else if (!source || (strcasecmp(source, "guest") != 0)) {
 	    PrintError(vm, VCORE_NONE, "ERROR: Unknown TSC configuration in time configuration.\n");
 	}
     }
diff --git a/palacios/src/palacios/vmm_xml.c b/palacios/src/palacios/vmm_xml.c
index 755ed59..c2213ca 100644
--- a/palacios/src/palacios/vmm_xml.c
+++ b/palacios/src/palacios/vmm_xml.c
@@ -516,6 +516,10 @@ static struct v3_xml * parse_str(char * buf, size_t len) {
     char ** attr; 
     int attr_idx;
 
+    if (!buf) {
+	return NULL;
+    }
+
     root->str_ptr = buf;
 
     if (len == 0) {