[Qemu-devel] [PULL 01/25] virtio: add checks for the size of the indirect table

Michael S. Tsirkin posted 25 patches 7 years ago
Only 20 patches received!
[Qemu-devel] [PULL 01/25] virtio: add checks for the size of the indirect table
Posted by Michael S. Tsirkin 7 years ago
From: Dima Stepanov <dimastep@yandex-team.ru>

The virtqueue_pop() and virtqueue_get_avail_bytes() routines can use the
INDIRECT table to get the data. It is possible to create a packet which
will lead to the assert message like:
  include/exec/memory.h:1995: void
  address_space_read_cached(MemoryRegionCache *, hwaddr, void *, int):
  Assertion `addr < cache->len && len <= cache->len - addr' failed.
  Aborted
To do it the first descriptor should have a link to the INDIRECT table
and set the size of it to 0. It doesn't look good that the guest should
be able to trigger the assert in qemu. Add additional check for the size
of the INDIRECT table, which should not be 0.

Signed-off-by: Dima Stepanov <dimastep@yandex-team.ru>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/virtio/virtio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 22bd1ac34e..a1ff647a66 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -646,7 +646,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
         vring_desc_read(vdev, &desc, desc_cache, i);
 
         if (desc.flags & VRING_DESC_F_INDIRECT) {
-            if (desc.len % sizeof(VRingDesc)) {
+            if (!desc.len || (desc.len % sizeof(VRingDesc))) {
                 virtio_error(vdev, "Invalid size for indirect buffer table");
                 goto err;
             }
@@ -902,7 +902,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
     desc_cache = &caches->desc;
     vring_desc_read(vdev, &desc, desc_cache, i);
     if (desc.flags & VRING_DESC_F_INDIRECT) {
-        if (desc.len % sizeof(VRingDesc)) {
+        if (!desc.len || (desc.len % sizeof(VRingDesc))) {
             virtio_error(vdev, "Invalid size for indirect buffer table");
             goto done;
         }
-- 
MST