From: Peter Dinda Date: Tue, 1 Sep 2015 21:12:15 +0000 (-0500) Subject: Cleanup and sanity-checking of unintentional integer overflow, unsigned/zero comparis... X-Git-Url: http://v3vee.org/palacios/gitweb/gitweb.cgi?p=palacios.git;a=commitdiff_plain;h=198151f1d58834ff7889389007232a3d250f51f1 Cleanup and sanity-checking of unintentional integer overflow, unsigned/zero comparisons, untrusted or tainted data tracking, weird operator choice (Coverity static analysis) 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 --- 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 --- 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 --- 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 --- 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_sizemem_size ? target_size : state->mem_size; diff --git a/palacios/src/devices/tmpdisk.c b/palacios/src/devices/tmpdisk.c index eb49a64..9a4371c 100644 --- 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 --- 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 --- 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 && gpahvm_state.first_hrt_gpa; + return gpahvm_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 --- 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 --- 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)) {