virtio: guard against negative vq notifies
The virtio_queue_notify() function checks that the virtqueue number is less than the maximum number of virtqueues. A signed comparison is used but the virtqueue number could be negative if a buggy or malicious guest is run. This results in memory accesses outside of the virtqueue array. It is risky doing input validation in common code instead of at the guest<->host boundary. Note that virtio_queue_set_addr(), virtio_queue_get_addr(), virtio_queue_get_num(), and many other virtio functions do *not* validate the virtqueue number argument. Instead of fixing the comparison in virtio_queue_notify(), move the comparison to the virtio bindings (just like VIRTIO_PCI_QUEUE_SEL) where we have a uint32_t value and can avoid ever calling into common virtio code if the virtqueue number is invalid. Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
This commit is contained in:
		
							parent
							
								
									e75ccf2c03
								
							
						
					
					
						commit
						7157e2e23e
					
				| 
						 | 
				
			
			@ -146,7 +146,9 @@ static void syborg_virtio_writel(void *opaque, target_phys_addr_t offset,
 | 
			
		|||
            vdev->queue_sel = value;
 | 
			
		||||
        break;
 | 
			
		||||
    case SYBORG_VIRTIO_QUEUE_NOTIFY:
 | 
			
		||||
        virtio_queue_notify(vdev, value);
 | 
			
		||||
        if (value < VIRTIO_PCI_QUEUE_MAX) {
 | 
			
		||||
            virtio_queue_notify(vdev, value);
 | 
			
		||||
        }
 | 
			
		||||
        break;
 | 
			
		||||
    case SYBORG_VIRTIO_STATUS:
 | 
			
		||||
        virtio_set_status(vdev, value & 0xFF);
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -348,7 +348,9 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
 | 
			
		|||
            vdev->queue_sel = val;
 | 
			
		||||
        break;
 | 
			
		||||
    case VIRTIO_PCI_QUEUE_NOTIFY:
 | 
			
		||||
        virtio_queue_notify(vdev, val);
 | 
			
		||||
        if (val < VIRTIO_PCI_QUEUE_MAX) {
 | 
			
		||||
            virtio_queue_notify(vdev, val);
 | 
			
		||||
        }
 | 
			
		||||
        break;
 | 
			
		||||
    case VIRTIO_PCI_STATUS:
 | 
			
		||||
        if (!(val & VIRTIO_CONFIG_S_DRIVER_OK)) {
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -585,9 +585,7 @@ void virtio_queue_notify_vq(VirtQueue *vq)
 | 
			
		|||
 | 
			
		||||
void virtio_queue_notify(VirtIODevice *vdev, int n)
 | 
			
		||||
{
 | 
			
		||||
    if (n < VIRTIO_PCI_QUEUE_MAX) {
 | 
			
		||||
        virtio_queue_notify_vq(&vdev->vq[n]);
 | 
			
		||||
    }
 | 
			
		||||
    virtio_queue_notify_vq(&vdev->vq[n]);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
uint16_t virtio_queue_vector(VirtIODevice *vdev, int n)
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in New Issue