Cleanup and sanity-checking of OOB accesses and pointer-to-local issues (Coverity...

diff --git a/linux_module/iface-file.c b/linux_module/iface-file.c
index 92ed9ff..67a4e73 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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);