Cleanup and sanity-checking of divide-by-zero and floating point use bugs (Coverity...

Note that varargs use within Palacios itself is problematic
as the varargs implementation used handles floating point as well
as integer.  This means that an innocuous use of a va_list
results in FP register reads.

diff --git a/palacios/include/palacios/vmm.h b/palacios/include/palacios/vmm.h
index 108da4a..ce75c64 100644 (file)
--- a/palacios/include/palacios/vmm.h
+++ b/palacios/include/palacios/vmm.h
@@ -325,7 +325,6 @@ v3_cpu_mode_t v3_get_host_cpu_mode();
 
 void v3_yield(struct guest_info * info, int usec);
 void v3_yield_cond(struct guest_info * info, int usec);
-void v3_print_cond(const char * fmt, ...);
 
 void v3_interrupt_cpu(struct v3_vm_info * vm, int logical_cpu, int vector);
 

diff --git a/palacios/include/palacios/vmm_xml.h b/palacios/include/palacios/vmm_xml.h
index 16a93cb..79fe2ae 100644 (file)
--- a/palacios/include/palacios/vmm_xml.h
+++ b/palacios/include/palacios/vmm_xml.h
@@ -78,14 +78,6 @@ const char *v3_xml_attr(struct v3_xml * xml, const char * attr);
 
 
 
-// Traverses the v3_xml sturcture to retrieve a specific subtag. Takes a
-// variable length list of tag names and indexes. The argument list must be
-// terminated by either an index of -1 or an empty string tag name. Example: 
-// title = v3_xml_get(library, "shelf", 0, "book", 2, "title", -1);
-// This retrieves the title of the 3rd book on the 1st shelf of library.
-// Returns NULL if not found.
-struct v3_xml * v3_xml_get(struct v3_xml * xml, ...);
-
 
 // frees the memory allocated for an v3_xml structure
 void v3_xml_free(struct v3_xml * xml);

diff --git a/palacios/src/devices/8254.c b/palacios/src/devices/8254.c
index 7394224..69fe0e7 100644 (file)
--- a/palacios/src/devices/8254.c
+++ b/palacios/src/devices/8254.c
@@ -294,7 +294,7 @@ static void pit_update_timer(struct guest_info * info, ullong_t cpu_cycles, ullo
            //   (void *)(addr_t)state->pit_reload);
 
            // How do we check for a one shot....
-           if (state->pit_reload == 0) {
+           if (reload_val == 0) {
                reload_val = 1;
            }
 
@@ -302,10 +302,10 @@ static void pit_update_timer(struct guest_info * info, ullong_t cpu_cycles, ullo
 
       
 #ifdef __V3_64BIT__
-           cpu_cycles = tmp_cycles % state->pit_reload;
-           tmp_cycles = tmp_cycles / state->pit_reload;
+           cpu_cycles = tmp_cycles % reload_val;
+           tmp_cycles = tmp_cycles / reload_val;
 #else
-           cpu_cycles = do_divll(tmp_cycles, state->pit_reload);
+           cpu_cycles = do_divll(tmp_cycles, reload_val);
 #endif
        
            oscillations += tmp_cycles;
@@ -871,7 +871,7 @@ static int pit_init(struct v3_vm_info * vm, v3_cfg_tree_t * cfg) {
 
     // Get cpu frequency and calculate the global pit oscilattor counter/cycle
 
-    do_divll(reload_val, OSC_HZ);
+    do_divll(reload_val, OSC_HZ);   // this is a floor, but will be >=1 for any machine faster than 1.2 MHz
     pit_state->pit_counter = reload_val;
     pit_state->pit_reload = reload_val;
 

diff --git a/palacios/src/extensions/ext_sched_edf.c b/palacios/src/extensions/ext_sched_edf.c
index 7b7c108..498c00f 100644 (file)
--- a/palacios/src/extensions/ext_sched_edf.c
+++ b/palacios/src/extensions/ext_sched_edf.c
@@ -451,6 +451,9 @@ activate_core(struct vm_core_edf_sched * core, struct vm_edf_rq *runqueue){
 }
 
 
+#define CEIL_DIV(x,y) (((x)/(y)) + !!((x)%(y)))
+#define MAX(x,y) ((x)>(y) ? (x) : (y))
+
 /*
  * edf_sched_core_init: Initializes per core data structure and
  * calls activate function.
@@ -516,6 +519,7 @@ edf_sched_core_init(struct guest_info * info){
 
             if(slice){
                 core_edf->slice = atoi(slice);
+               core_edf->slice = MAX(MIN_SLICE,core_edf->slice);
             }
            else{
                core_edf->slice = MIN_SLICE;
@@ -523,10 +527,13 @@ edf_sched_core_init(struct guest_info * info){
 
            if(period){
                core_edf->period = atoi(period);
+               core_edf->period = MAX(MIN_PERIOD,core_edf->period);
             }
            else{
                core_edf->period = (core_edf->slice * cpu_khz * tdf)/speed_khz;
-               core_edf->period += 0.3*(100*core_edf->slice/core_edf->period); // Give faster vcores a little more bigger periods.
+               // WTF is this floating point doing here?!
+               // core_edf->period += 0.3*(100*core_edf->slice/core_edf->period); // Give faster vcores a little more bigger periods.
+               core_edf->period += CEIL_DIV(100*core_edf->slice/core_edf->period,3); // Give faster vcores a little more bigger periods.
             }
 
            PrintDebug(info->vm_info,info,"EDF_SCHED. Vcore %d, Pcore %d, cpu_khz %u, Period %llu Speed %d, Utilization %d, tdf %d %llu \n",

diff --git a/palacios/src/palacios/vmm.c b/palacios/src/palacios/vmm.c
index 89b0173..b0e973f 100644 (file)
--- a/palacios/src/palacios/vmm.c
+++ b/palacios/src/palacios/vmm.c
@@ -1226,18 +1226,6 @@ v3_cpu_mode_t v3_get_host_cpu_mode() {
 
 #endif 
 
-void v3_print_cond(const char * fmt, ...) {
-    if (v3_dbg_enable == 1) {
-       char buf[2048];
-       va_list ap;
-
-       va_start(ap, fmt);
-       vsnprintf(buf, 2048, fmt, ap);
-       va_end(ap);
-
-       V3_Print(VM_NONE, VCORE_NONE,"%s", buf);
-    }    
-}
 
 
 

diff --git a/palacios/src/palacios/vmm_config.c b/palacios/src/palacios/vmm_config.c
index e4c9b4f..420b316 100644 (file)
--- a/palacios/src/palacios/vmm_config.c
+++ b/palacios/src/palacios/vmm_config.c
@@ -386,6 +386,10 @@ static int pre_config_vm(struct v3_vm_info * vm, v3_cfg_tree_t * vm_cfg) {
 
    if (schedule_hz_str) {
        sched_hz = atoi(schedule_hz_str);
+       if (sched_hz==0) { 
+           PrintError(vm,VCORE_NONE,"Cannot set sched Hz to 0\n");
+           return -1;
+       }
     }
 
     PrintDebug(VM_NONE, VCORE_NONE, "CPU_KHZ = %d, schedule_freq=%p\n", V3_CPU_KHZ(), 

diff --git a/palacios/src/palacios/vmm_scheduler.c b/palacios/src/palacios/vmm_scheduler.c
index 07600bf..506429b 100644 (file)
--- a/palacios/src/palacios/vmm_scheduler.c
+++ b/palacios/src/palacios/vmm_scheduler.c
@@ -204,6 +204,10 @@ int host_sched_vm_init(struct v3_vm_info *vm)
 
     if (schedule_hz_str) {
        sched_hz = atoi(schedule_hz_str);
+       if (sched_hz==0) { 
+           PrintError(vm, VCORE_NONE,"Cannot set Sched Hz to 0\n");
+           return -1;
+       }
     }
 
     PrintDebug(vm, VCORE_NONE,"CPU_KHZ = %d, schedule_freq=%p\n", V3_CPU_KHZ(),

diff --git a/palacios/src/palacios/vmm_xml.c b/palacios/src/palacios/vmm_xml.c
index c2213ca..f3cf7e3 100644 (file)
--- a/palacios/src/palacios/vmm_xml.c
+++ b/palacios/src/palacios/vmm_xml.c
@@ -54,7 +54,6 @@ struct v3_xml_root {       // additional data for the root tag
     char *tmp_start;              // start of work area
     char *tmp_end;              // end of work area
     short standalone;     // non-zero if <?xml standalone="yes"?>
-    char err[V3_XML_ERRL]; // error string
 };
 
 static char * empty_attrib_list[] = { NULL }; // empty, null terminated array of strings
@@ -80,11 +79,9 @@ static void * tmp_realloc(void * old_ptr, size_t old_size, size_t new_size) {
 }
 
 // set an error string and return root
-static void v3_xml_err(struct v3_xml_root * root, char * xml_str, const char * err, ...) {
-    va_list ap;
+static void v3_xml_err(struct v3_xml_root * root, char * xml_str, const char * err, const char *arg) {
     int line = 1;
     char * tmp;
-    char fmt[V3_XML_ERRL];
     
     for (tmp = root->tmp_start; tmp < xml_str; tmp++) {
        if (*tmp == '\n') {
@@ -92,14 +89,8 @@ static void v3_xml_err(struct v3_xml_root * root, char * xml_str, const char * e
        }
     }
 
-    snprintf(fmt, V3_XML_ERRL, "[error near line %d]: %s", line, err);
-
-    va_start(ap, err);
-    vsnprintf(root->err, V3_XML_ERRL, fmt, ap);
-    va_end(ap);
-
-    PrintError(VM_NONE, VCORE_NONE, "XML Error: %s\n", root->err);
-
+    PrintError(VM_NONE, VCORE_NONE, "XML Error: [error near line %d]: %s (%s)", line, err ,arg ? arg : "");
+    
     // free memory
     v3_xml_free(&(root->xml));
 
@@ -152,33 +143,7 @@ const char * v3_xml_attr(struct v3_xml * xml, const char * attr) {
     return NULL; // found default
 }
 
-// same as v3_xml_get but takes an already initialized va_list
-static struct v3_xml * v3_xml_vget(struct v3_xml * xml, va_list ap) {
-    char * name = va_arg(ap, char *);
-    int idx = -1;
-
-    if ((name != NULL) && (*name != 0)) {
-        idx = va_arg(ap, int);    
-        xml = v3_xml_child(xml, name);
-    }
-    return (idx < 0) ? xml : v3_xml_vget(v3_xml_idx(xml, idx), ap);
-}
 
-// Traverses the xml tree to retrieve a specific subtag. Takes a variable
-// length list of tag names and indexes. The argument list must be terminated
-// by either an index of -1 or an empty string tag name. Example: 
-// title = v3_xml_get(library, "shelf", 0, "book", 2, "title", -1);
-// This retrieves the title of the 3rd book on the 1st shelf of library.
-// Returns NULL if not found.
-struct v3_xml * v3_xml_get(struct v3_xml * xml, ...) {
-    va_list ap;
-    struct v3_xml * r;
-
-    va_start(ap, xml);
-    r = v3_xml_vget(xml, ap);
-    va_end(ap);
-    return r;
-}
 
 
 // sets a flag for the given tag and returns the tag
@@ -330,7 +295,7 @@ static int v3_xml_close_tag(struct v3_xml_root * root, char * name, char * s) {
     if ( (root->cur == NULL) || 
         (root->cur->name == NULL) || 
         (strcasecmp(name, root->cur->name))) {
-       v3_xml_err(root, s, "unexpected closing tag </%s>", name);
+       v3_xml_err(root, s, "unexpected closing tag", name);
        return -1;
     }
 
@@ -380,7 +345,6 @@ static struct v3_xml * v3_xml_new(const char * name) {
     root->xml.name = (char *)name;
     root->cur = &root->xml;
     root->xml.txt = "";
-    memset(root->err, 0, V3_XML_ERRL);
 
 
     return &root->xml;
@@ -523,7 +487,7 @@ static struct v3_xml * parse_str(char * buf, size_t len) {
     root->str_ptr = buf;
 
     if (len == 0) {
-       v3_xml_err(root, NULL, "Empty XML String\n");
+       v3_xml_err(root, NULL, "Empty XML String", 0);
        return NULL;
     }
 
@@ -539,7 +503,7 @@ static struct v3_xml * parse_str(char * buf, size_t len) {
     }
 
     if (*buf == '\0') {
-       v3_xml_err(root, buf, "root tag missing");
+       v3_xml_err(root, buf, "root tag missing", 0);
        return NULL;
     }
 
@@ -551,7 +515,7 @@ static struct v3_xml * parse_str(char * buf, size_t len) {
            // new tag
 
             if (root->cur == NULL) {
-                v3_xml_err(root, tag_ptr, "markup outside of root element");
+                v3_xml_err(root, tag_ptr, "markup outside of root element", 0);
                return NULL;
            }
 
@@ -642,8 +606,10 @@ static struct v3_xml * parse_str(char * buf, size_t len) {
                            // null terminate attribute val
                            *(buf++) = '\0';
                        } else {
+                           char err_buf[2] = {quote_char,0};
+
                             v3_xml_free_attr(attr);
-                            v3_xml_err(root, tag_ptr, "missing %c", quote_char);
+                            v3_xml_err(root, tag_ptr, "missing quote char", err_buf);
                            return NULL;
                         }
 
@@ -666,7 +632,7 @@ static struct v3_xml * parse_str(char * buf, size_t len) {
                     if (attr_idx > 0) {
                        v3_xml_free_attr(attr);
                    }
-                   v3_xml_err(root, tag_ptr, "missing >");
+                   v3_xml_err(root, tag_ptr, "missing >", 0);
                    return NULL;
                 }
                 v3_xml_open_tag(root, tag_ptr, attr);
@@ -681,7 +647,7 @@ static struct v3_xml * parse_str(char * buf, size_t len) {
                 if (attr_idx > 0) {
                    v3_xml_free_attr(attr);
                }
-               v3_xml_err(root, tag_ptr, "missing >"); 
+               v3_xml_err(root, tag_ptr, "missing >", 0); 
                return NULL;
             }
         } else if (*buf == '/') { 
@@ -691,7 +657,7 @@ static struct v3_xml * parse_str(char * buf, size_t len) {
             
            quote_char = *buf;
            if ((*buf == '\0') && (last_char != '>')) {
-               v3_xml_err(root, tag_ptr, "missing >");
+               v3_xml_err(root, tag_ptr, "missing >", 0);
                return NULL;
             }
 
@@ -710,7 +676,7 @@ static struct v3_xml * parse_str(char * buf, size_t len) {
             if ( ((buf = strstr(buf + 3, "--")) == 0) || 
                 ((*(buf += 2) != '>') && (*buf)) ||
                 ((!*buf) && (last_char != '>'))) {
-               v3_xml_err(root, tag_ptr, "unclosed <!--");
+               v3_xml_err(root, tag_ptr, "unclosed <!--", 0);
                return NULL;
            }
         } else if (! strncmp(buf, "![CDATA[", 8)) { 
@@ -718,11 +684,11 @@ static struct v3_xml * parse_str(char * buf, size_t len) {
             if ((buf = strstr(buf, "]]>"))) {
                 v3_xml_char_content(root, tag_ptr + 8, (buf += 2) - tag_ptr - 10, 'c');
            } else {
-               v3_xml_err(root, tag_ptr, "unclosed <![CDATA[");
+               v3_xml_err(root, tag_ptr, "unclosed <![CDATA[", 0);
                return NULL;
            }
        } else {
-           v3_xml_err(root, tag_ptr, "unexpected <");
+           v3_xml_err(root, tag_ptr, "unexpected <",0);
            return NULL;
         }
 
@@ -757,10 +723,10 @@ static struct v3_xml * parse_str(char * buf, size_t len) {
     if (root->cur == NULL) {
        return &root->xml;
     } else if (root->cur->name == NULL) {
-       v3_xml_err(root, tag_ptr, "root tag missing");
+       v3_xml_err(root, tag_ptr, "root tag missing",0);
        return NULL;
     } else {
-       v3_xml_err(root, tag_ptr, "unclosed tag <%s>", root->cur->name);
+       v3_xml_err(root, tag_ptr, "unclosed tag", root->cur->name);
        return NULL;
     }
 }