Cleanup and sanity-checking of unintentional integer overflow, unsigned/zero comparis...

There are a fair number of uses of tainted data (e.g., from user or file) -
I left most of these alone for now.

diff --git a/palacios/src/devices/ata.h b/palacios/src/devices/ata.h
index d79e012..c6ad4b4 100644 (file)
--- a/palacios/src/devices/ata.h
+++ b/palacios/src/devices/ata.h
@@ -215,7 +215,7 @@ static int ata_get_lba_and_size(struct ide_internal * ide, struct ide_channel *
            // we are in CHS mode....
            
            *lba = 
-               (drive->cylinder * drive->num_heads + 
+               ((uint64_t)drive->cylinder * drive->num_heads + 
                 channel->drive_head.head_num) * drive->num_sectors +
                // sector number is 1 based
                (drive->sector_num - 1);

diff --git a/palacios/src/devices/curses_cons.c b/palacios/src/devices/curses_cons.c
index 8824447..5272789 100644 (file)
--- a/palacios/src/devices/curses_cons.c
+++ b/palacios/src/devices/curses_cons.c
@@ -66,8 +66,6 @@ static int cursor_update(uint_t x, uint_t y, void *private_data)
     PrintDebug(VM_NONE, VCORE_NONE, "cursor_update(%d, %d, %p)\n", x, y, private_data);
 
     /* avoid out-of-range coordinates */
-    if (x < 0) x = 0;
-    if (y < 0) y = 0;
     if (x >= state->cols) x = state->cols - 1;
     if (y >= state->rows) y = state->rows - 1;
     offset = (x + y * state->cols) * BYTES_PER_COL;

diff --git a/palacios/src/devices/lnx_virtio_nic.c b/palacios/src/devices/lnx_virtio_nic.c
index ad0b9e9..b22a5be 100644 (file)
--- a/palacios/src/devices/lnx_virtio_nic.c
+++ b/palacios/src/devices/lnx_virtio_nic.c
@@ -612,7 +612,8 @@ static int virtio_rx(uint8_t * buf, uint32_t size, void * private_data) {
     if (q->cur_avail_idx != q->avail->index){
        uint16_t buf_idx;
        struct vring_desc * buf_desc;
-       uint32_t hdr_len, len;
+       uint32_t hdr_len;
+       int len;
        uint32_t offset = 0;
 
        hdr_len = (virtio->mergeable_rx_bufs)?

diff --git a/palacios/src/devices/paragraph.c b/palacios/src/devices/paragraph.c
index 36e44d8..973c580 100644 (file)
--- a/palacios/src/devices/paragraph.c
+++ b/palacios/src/devices/paragraph.c
@@ -247,7 +247,7 @@ static int register_dev(struct paragraph_state *state)
     target_size = state->mem_size;
     break;
   case GCONS_DIRECT:
-    target_size = state->target_spec.height*state->target_spec.width*state->target_spec.bytes_per_pixel;
+      target_size = (uint64_t)state->target_spec.height*state->target_spec.width*state->target_spec.bytes_per_pixel;
     break;
   default:
     PrintError(state->vm, VCORE_NONE, "paragraph: Unknown mode\n");
@@ -344,7 +344,7 @@ static int render_callback(v3_graphics_console_t cons,
     PrintDebug(state->vm, VCORE_NONE, "paragraph: render callback GCONS_MEM\n");
 
     void *fb = v3_graphics_console_get_frame_buffer_data_rw(state->host_cons,&(state->target_spec));
-    uint64_t target_size = state->target_spec.height*state->target_spec.width*state->target_spec.bytes_per_pixel;
+    uint64_t target_size = (uint64_t)state->target_spec.height*state->target_spec.width*state->target_spec.bytes_per_pixel;
     
     // must be smaller than the memory we have allocated
     target_size = target_size<state->mem_size ? target_size : state->mem_size;

diff --git a/palacios/src/devices/tmpdisk.c b/palacios/src/devices/tmpdisk.c
index eb49a64..9a4371c 100644 (file)
--- a/palacios/src/devices/tmpdisk.c
+++ b/palacios/src/devices/tmpdisk.c
@@ -101,7 +101,7 @@ static int blk_init(struct v3_vm_info * vm, v3_cfg_tree_t * cfg) {
     struct blk_state * blk = NULL;
     v3_cfg_tree_t * frontend_cfg = v3_cfg_subtree(cfg, "frontend");
     char * dev_id = v3_cfg_val(cfg, "ID");
-    uint64_t capacity = atoi(v3_cfg_val(cfg, "size")) * 1024 * 1024;
+    uint64_t capacity = atoi(v3_cfg_val(cfg, "size")) * 1024ULL * 1024ULL;
     
     if (!frontend_cfg) {
        PrintError(vm, VCORE_NONE, "Frontend Configuration not present\n");

diff --git a/palacios/src/palacios/vmm_cpu_mapper.c b/palacios/src/palacios/vmm_cpu_mapper.c
index 2c7fe89..449e2e7 100644 (file)
--- a/palacios/src/palacios/vmm_cpu_mapper.c
+++ b/palacios/src/palacios/vmm_cpu_mapper.c
@@ -194,6 +194,7 @@ int default_mapper_admit(struct v3_vm_info *vm, unsigned int cpu_mask){
        if (specified_cpu != NULL) {
            core_idx = atoi(specified_cpu);
 
+           // unsigned comparison with 0
            if (core_idx < 0) {
                PrintError(vm, VCORE_NONE, "Target CPU out of bounds (%d) \n", core_idx);
            }

diff --git a/palacios/src/palacios/vmm_hvm.c b/palacios/src/palacios/vmm_hvm.c
index 43a8672..26d3298 100644 (file)
--- a/palacios/src/palacios/vmm_hvm.c
+++ b/palacios/src/palacios/vmm_hvm.c
@@ -563,7 +563,7 @@ uint32_t v3_get_hvm_hrt_cores(struct v3_vm_info *vm)
 int v3_is_hvm_ros_mem_gpa(struct v3_vm_info *vm, addr_t gpa)
 {
     if (vm->hvm_state.is_hvm) { 
-       return gpa>=0 && gpa<vm->hvm_state.first_hrt_gpa;
+       return gpa<vm->hvm_state.first_hrt_gpa;
     } else {
        return 1;
     }

diff --git a/palacios/src/palacios/vmm_time.c b/palacios/src/palacios/vmm_time.c
index dff562b..a028b2f 100644 (file)
--- a/palacios/src/palacios/vmm_time.c
+++ b/palacios/src/palacios/vmm_time.c
@@ -499,7 +499,7 @@ void v3_init_time_core(struct guest_info * info) {
        (time_state->clock_ratio_num != 1) ||
        (info->vm_info->time_state.td_num != 1) || 
        (info->vm_info->time_state.td_denom != 1)) { 
-       if (time_state->flags | VM_TIME_TSC_PASSTHROUGH) {
+       if (time_state->flags & VM_TIME_TSC_PASSTHROUGH) {
            PrintError(info->vm_info, info, "WARNING: Cannot use reqested passthrough TSC with clock or time modification also requested.\n");
            time_state->flags &= ~VM_TIME_TSC_PASSTHROUGH;
        }

diff --git a/palacios/src/palacios/vmx_msr.c b/palacios/src/palacios/vmx_msr.c
index ea20374..18bb3ee 100644 (file)
--- a/palacios/src/palacios/vmx_msr.c
+++ b/palacios/src/palacios/vmx_msr.c
@@ -31,6 +31,7 @@
 
 static int get_bitmap_index(uint_t msr)
 {
+    // unsigned comparison with 0 here for clarity
     if( (msr >= LOW_MSR_START) && msr <= LOW_MSR_END) {
         return LOW_MSR_INDEX + msr;
     } else if (( msr >= HIGH_MSR_START ) && (msr <= HIGH_MSR_END)) {