diff options
| author | Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> | 2011-05-08 22:29:07 +0100 |
|---|---|---|
| committer | Michael S. Tsirkin <mst@redhat.com> | 2011-06-12 10:33:38 +0300 |
| commit | 7157e2e23e89adcd436caeab31fdd6b47eded377 (patch) | |
| tree | 5012c5e003f5d66032ef7892240ad8c9a9abe5eb /hw | |
| parent | e75ccf2c033fb0503d6cb7ddd0fd1dfa0aa4fc16 (diff) | |
| download | hqemu-7157e2e23e89adcd436caeab31fdd6b47eded377.zip hqemu-7157e2e23e89adcd436caeab31fdd6b47eded377.tar.gz | |
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>
Diffstat (limited to 'hw')
| -rw-r--r-- | hw/syborg_virtio.c | 4 | ||||
| -rw-r--r-- | hw/virtio-pci.c | 4 | ||||
| -rw-r--r-- | hw/virtio.c | 4 |
3 files changed, 7 insertions, 5 deletions
diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c index 2f3e6da..00c7be8 100644 --- a/hw/syborg_virtio.c +++ b/hw/syborg_virtio.c @@ -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); diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 270e2c7..b86c441 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -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)) { diff --git a/hw/virtio.c b/hw/virtio.c index 6e8814c..a651860 100644 --- a/hw/virtio.c +++ b/hw/virtio.c @@ -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) |
