diff mbox

[v2,1/4] virtio: fix up max size checks

Message ID 1484772931-16272-2-git-send-email-mst@redhat.com
State New
Headers show

Commit Message

Michael S. Tsirkin Jan. 18, 2017, 8:55 p.m. UTC 
Coverity reports that ARRAY_SIZE(elem->out_sg) (and all the others too)
is wrong because elem->out_sg is a pointer.

However, the check is not in the right place and the max_size argument
of virtqueue_map_iovec can be removed.  The check on in_num/out_num
should be moved to qemu_get_virtqueue_element instead, before the call
to virtqueue_alloc_element.

Cc: qemu-stable@nongnu.org
Reported-by: Paolo Bonzini <pbonzini@redhat.com>
Fixes: 3724650db07057333879484c8bc7d900b5c1bf8e ("virtio: introduce virtqueue_alloc_element")
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio/virtio.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

Comments

Cornelia Huck Jan. 19, 2017, 9:46 a.m. UTC | #1 
On Wed, 18 Jan 2017 22:55:40 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> Coverity reports that ARRAY_SIZE(elem->out_sg) (and all the others too)
> is wrong because elem->out_sg is a pointer.
> 
> However, the check is not in the right place and the max_size argument
> of virtqueue_map_iovec can be removed.  The check on in_num/out_num
> should be moved to qemu_get_virtqueue_element instead, before the call
> to virtqueue_alloc_element.
> 
> Cc: qemu-stable@nongnu.org
> Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> Fixes: 3724650db07057333879484c8bc7d900b5c1bf8e ("virtio: introduce virtqueue_alloc_element")
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/virtio/virtio.c | 33 +++++++++++++--------------------
>  1 file changed, 13 insertions(+), 20 deletions(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
diff mbox

Patch

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 34065c7..6e34f05 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -599,23 +599,11 @@  static void virtqueue_undo_map_desc(unsigned int out_num, unsigned int in_num,
 
 static void virtqueue_map_iovec(VirtIODevice *vdev, struct iovec *sg,
                                 hwaddr *addr, unsigned int *num_sg,
-                                unsigned int max_size, int is_write)
+                                int is_write)
 {
     unsigned int i;
     hwaddr len;
 
-    /* Note: this function MUST validate input, some callers
-     * are passing in num_sg values received over the network.
-     */
-    /* TODO: teach all callers that this can fail, and return failure instead
-     * of asserting here.
-     * When we do, we might be able to re-enable NDEBUG below.
-     */
-#ifdef NDEBUG
-#error building with NDEBUG is not supported
-#endif
-    assert(*num_sg <= max_size);
-
     for (i = 0; i < *num_sg; i++) {
         len = sg[i].iov_len;
         sg[i].iov_base = dma_memory_map(vdev->dma_as,
@@ -635,13 +623,8 @@  static void virtqueue_map_iovec(VirtIODevice *vdev, struct iovec *sg,
 
 void virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem)
 {
-    virtqueue_map_iovec(vdev, elem->in_sg, elem->in_addr, &elem->in_num,
-                        MIN(ARRAY_SIZE(elem->in_sg), ARRAY_SIZE(elem->in_addr)),
-                        1);
-    virtqueue_map_iovec(vdev, elem->out_sg, elem->out_addr, &elem->out_num,
-                        MIN(ARRAY_SIZE(elem->out_sg),
-                        ARRAY_SIZE(elem->out_addr)),
-                        0);
+    virtqueue_map_iovec(vdev, elem->in_sg, elem->in_addr, &elem->in_num, 1);
+    virtqueue_map_iovec(vdev, elem->out_sg, elem->out_addr, &elem->out_num, 0);
 }
 
 static void *virtqueue_alloc_element(size_t sz, unsigned out_num, unsigned in_num)
@@ -840,6 +823,16 @@  void *qemu_get_virtqueue_element(VirtIODevice *vdev, QEMUFile *f, size_t sz)
 
     qemu_get_buffer(f, (uint8_t *)&data, sizeof(VirtQueueElementOld));
 
+    /* TODO: teach all callers that this can fail, and return failure instead
+     * of asserting here.
+     * When we do, we might be able to re-enable NDEBUG below.
+     */
+#ifdef NDEBUG
+#error building with NDEBUG is not supported
+#endif
+    assert(ARRAY_SIZE(data.in_addr) >= data.in_num);
+    assert(ARRAY_SIZE(data.out_addr) >= data.out_num);
+
     elem = virtqueue_alloc_element(sz, data.out_num, data.in_num);
     elem->index = data.index;