From: Peter Dinda Date: Mon, 31 Aug 2015 23:18:28 +0000 (-0500) Subject: Cleanup and sanity-checking of OOB accesses and pointer-to-local issues (Coverity... X-Git-Url: http://v3vee.org/palacios/gitweb/gitweb.cgi?p=palacios.git;a=commitdiff_plain;h=d775bbfa668ce9968bacc0e4257cf86e5ab88e90 Cleanup and sanity-checking of OOB accesses and pointer-to-local issues (Coverity static analysis) --- diff --git a/linux_module/iface-file.c b/linux_module/iface-file.c index 92ed9ff..67a4e73 100644 --- a/linux_module/iface-file.c +++ b/linux_module/iface-file.c @@ -119,6 +119,9 @@ static int palacios_file_mkdir(const char * pathname, unsigned short perms, int /* It only exists to provide version compatibility */ struct path tmp_path; #endif +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,41) + struct nameidata nd; +#endif struct path * path_ptr = NULL; struct dentry * dentry; @@ -133,8 +136,6 @@ static int palacios_file_mkdir(const char * pathname, unsigned short perms, int /* Before Linux 3.1 this was somewhat more difficult */ #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,41) { - struct nameidata nd; - // I'm not 100% sure about the version here, but it was around this time that the API changed #if LINUX_VERSION_CODE > KERNEL_VERSION(2,6,38) ret = kern_path_parent(pathname, &nd); diff --git a/linux_module/iface-host-pci-hw.h b/linux_module/iface-host-pci-hw.h index 0439b51..66e6f45 100644 --- a/linux_module/iface-host-pci-hw.h +++ b/linux_module/iface-host-pci-hw.h @@ -77,8 +77,10 @@ static int setup_hw_pci_dev(struct host_pci_device * host_dev) { if (flags & IORESOURCE_IO) { bar->type = PT_BAR_IO; } else if (flags & IORESOURCE_MEM) { - if (flags & IORESOURCE_MEM_64) { - struct v3_host_pci_bar * hi_bar = &(v3_dev->bars[i + 1]); + if ((flags & IORESOURCE_MEM_64)) { + // this should never happen with i==5, but it + // is technically an OOB access without the modulo + struct v3_host_pci_bar * hi_bar = &(v3_dev->bars[(i + 1) % 6]); bar->type = PT_BAR_MEM64_LO; diff --git a/linux_module/main.c b/linux_module/main.c index 12fa2fe..310c5a1 100644 --- a/linux_module/main.c +++ b/linux_module/main.c @@ -162,7 +162,7 @@ out_err: unsigned long vm_idx = arg; struct v3_guest * guest; - if (vm_idx > MAX_VMS) { + if (vm_idx >= MAX_VMS) { ERROR("Invalid VM index: %ld\n", vm_idx); return -1; } diff --git a/palacios/src/devices/8259a.c b/palacios/src/devices/8259a.c index 6609a5c..25d6fa1 100644 --- a/palacios/src/devices/8259a.c +++ b/palacios/src/devices/8259a.c @@ -162,7 +162,7 @@ struct pic_internal { struct { int (*ack)(struct guest_info * core, uint32_t irq, void * private_data); void * private_data; - } irq_ack_cbs[15]; + } irq_ack_cbs[16]; void * router_handle; diff --git a/palacios/src/devices/cga.c b/palacios/src/devices/cga.c index 36bec51..7652b04 100644 --- a/palacios/src/devices/cga.c +++ b/palacios/src/devices/cga.c @@ -1083,7 +1083,8 @@ int v3_cons_get_fb_text(struct video_internal * state, uint8_t * dst, uint_t off len1 = min_uint(length, state->activefb_len - framebuf_offset); len2 = length - len1; if (len1 > 0) memcpy(dst, framebuf + framebuf_offset, len1); - if (len2 > 0) memcpy(dst + len1, framebuf, len2); + // the following is tagged as OOB access to dst but appears to be fine + if (len2 > 0) memcpy(dst + len1, framebuf, len2); return 0; } diff --git a/palacios/src/devices/ide.c b/palacios/src/devices/ide.c index bc42ea8..e9a90a8 100644 --- a/palacios/src/devices/ide.c +++ b/palacios/src/devices/ide.c @@ -299,7 +299,12 @@ static inline int get_channel_index(ushort_t port) { static inline struct ide_channel * get_selected_channel(struct ide_internal * ide, ushort_t port) { int channel_idx = get_channel_index(port); - return &(ide->channels[channel_idx]); + if (channel_idx >= 0) { + return &(ide->channels[channel_idx]); + } else { + PrintError(VM_NONE,VCORE_NONE,"ide: Cannot Determine Selected Channel\n"); + return 0; + } } static inline struct ide_drive * get_selected_drive(struct ide_channel * channel) { diff --git a/palacios/src/devices/io_apic.c b/palacios/src/devices/io_apic.c index 247abe6..dec1e12 100644 --- a/palacios/src/devices/io_apic.c +++ b/palacios/src/devices/io_apic.c @@ -190,7 +190,7 @@ static int ioapic_read(struct guest_info * core, addr_t guest_addr, void * dst, uint_t redir_index = (ioapic->index_reg - IOAPIC_REDIR_BASE_REG) >> 1; uint_t hi_val = (ioapic->index_reg - IOAPIC_REDIR_BASE_REG) & 1; - if (redir_index > 0x3f) { + if (redir_index > 23) { PrintError(core->vm_info, core, "ioapic %u: Invalid redirection table entry %x\n", ioapic->ioapic_id.id, (uint32_t)redir_index); return -1; } @@ -242,7 +242,7 @@ static int ioapic_write(struct guest_info * core, addr_t guest_addr, void * src, PrintDebug(core->vm_info, core, "ioapic %u: Writing value 0x%x to redirection entry %u (%s)\n", ioapic->ioapic_id.id, op_val, redir_index, hi_val ? "hi" : "low"); - if (redir_index > 0x3f) { + if (redir_index > 23) { PrintError(core->vm_info, core, "ioapic %u: Invalid redirection table entry %x\n", ioapic->ioapic_id.id, (uint32_t)redir_index); return -1; } @@ -279,7 +279,7 @@ static int ioapic_raise_irq(struct v3_vm_info * vm, void * private_data, struct irq_num = 2; } - if (irq_num > 24) { + if (irq_num >= 24) { PrintDebug(vm, VCORE_NONE, "ioapic %u: IRQ out of range of IO APIC\n", ioapic->ioapic_id.id); return -1; } diff --git a/palacios/src/devices/lnx_virtio_nic.c b/palacios/src/devices/lnx_virtio_nic.c index 636d8a5..ad0b9e9 100644 --- a/palacios/src/devices/lnx_virtio_nic.c +++ b/palacios/src/devices/lnx_virtio_nic.c @@ -574,7 +574,7 @@ static int virtio_io_read(struct guest_info *core, virtio->pci_dev, 0); break; - case VIRTIO_NET_CONFIG ... VIRTIO_NET_CONFIG + ETH_ALEN: + case VIRTIO_NET_CONFIG ... VIRTIO_NET_CONFIG + ETH_ALEN - 1: *(uint8_t *)dst = virtio->net_cfg.mac[port_idx-VIRTIO_NET_CONFIG]; break; diff --git a/palacios/src/devices/ne2k.c b/palacios/src/devices/ne2k.c index 413ec24..f94b966 100644 --- a/palacios/src/devices/ne2k.c +++ b/palacios/src/devices/ne2k.c @@ -391,6 +391,7 @@ static int ne2k_rxbuf_full(struct ne2k_registers * regs) { // This needs to be completely redone... static int rx_one_pkt(struct ne2k_state * nic_state, const uchar_t * pkt, uint32_t length) { struct ne2k_registers * regs = (struct ne2k_registers *)&(nic_state->context); + uchar_t buf[MIN_BUF_SIZE]; uchar_t * p; uint32_t total_len; uint32_t next; @@ -414,8 +415,6 @@ static int rx_one_pkt(struct ne2k_state * nic_state, const uchar_t * pkt, uint3 //packet too small, expand it if (length < MIN_BUF_SIZE) { - uchar_t buf[MIN_BUF_SIZE]; - memcpy(buf, pkt, length); memset(buf + length, 0, MIN_BUF_SIZE - length); pkt = buf; diff --git a/palacios/src/devices/telnet_cons.c b/palacios/src/devices/telnet_cons.c index be40ba4..5f82993 100644 --- a/palacios/src/devices/telnet_cons.c +++ b/palacios/src/devices/telnet_cons.c @@ -217,7 +217,7 @@ static const uint8_t bg_color_map[] = { static int send_update(struct cons_state * state, uint8_t x, uint8_t y, uint8_t attrib, uint8_t val) { uint8_t fg_color = fg_color_map[(attrib & 0x0f) % 16]; uint8_t bg_color = bg_color_map[(attrib & 0xf0) % 16]; - uint8_t buf[32]; + uint8_t buf[64]; int ret = 0; int i = 0; @@ -264,7 +264,7 @@ static int send_update(struct cons_state * state, uint8_t x, uint8_t y, uint8_ uint64_t start, end; rdtscll(start); - ret = send_all(state->client_fd, buf, 32); + ret = send_all(state->client_fd, buf, i); rdtscll(end); PrintDebug(VM_NONE, VCORE_NONE, "Sendall latency=%d cycles\n", (uint32_t)(end - start)); diff --git a/palacios/src/palacios/vmx.c b/palacios/src/palacios/vmx.c index 544b848..80a52a3 100644 --- a/palacios/src/palacios/vmx.c +++ b/palacios/src/palacios/vmx.c @@ -1138,6 +1138,8 @@ int v3_vmx_enter(struct guest_info * info) { if (info->shdw_pg_mode == NESTED_PAGING) { check_vmcs_read(VMCS_GUEST_PHYS_ADDR, &(exit_info.ept_fault_addr)); + } else { + exit_info.ept_fault_addr = 0; } //PrintDebug(info->vm_info, info, "VMX Exit taken, id-qual: %u-%lu\n", exit_info.exit_reason, exit_info.exit_qual);