Skip to content

Commit

Permalink
virtio: fix up max size checks
Browse files Browse the repository at this point in the history
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: 3724650 ("virtio: introduce virtqueue_alloc_element")
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
  • Loading branch information
mstsirkin committed Jan 19, 2017
1 parent 9568700 commit 6bdc21c
Showing 1 changed file with 13 additions and 20 deletions.
33 changes: 13 additions & 20 deletions hw/virtio/virtio.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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;

Expand Down

0 comments on commit 6bdc21c

Please sign in to comment.