include/hw/virtio/virtio.h | 2 +- hw/virtio/virtio.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
virtio_add_queue()'s queue_size argument is a signed int. Coverity is
unhappy when that type is used as an argument to g_new0():
*** CID 1664271: Error handling issues (NEGATIVE_RETURNS)
/builds/qemu-project/qemu/hw/virtio/virtio.c: 2595 in virtio_add_queue()
2589 }
2590
2591 vdev->vq[i].vring.num = queue_size;
2592 vdev->vq[i].vring.num_default = queue_size;
2593 vdev->vq[i].vring.align = VIRTIO_PCI_VRING_ALIGN;
2594 vdev->vq[i].handle_output = handle_output;
>>> CID 1664271: Error handling issues (NEGATIVE_RETURNS)
>>> "__n" is passed to a parameter that cannot be negative.
2595 vdev->vq[i].used_elems = g_new0(VirtQueueElement, queue_size);
The vdev->vq[i].vring.num and num_default fields are already declared as
unsigned int, so change the virtio_add_queue() argument's type for
consistency.
A note on consistency: the VIRTIO specification defines queue size as an
unsigned 16-bit value. QEMU's device models variously use uint16_t,
uint32_t, and other unsigned types for queue size qdev properties.
virtio_add_queue() limits queue size to the much smaller
VIRTQUEUE_MAX_SIZE (1024) constant, so the different widths don't really
matter.
I have checked that all callers of virtio_add_queue() pass an unsigned
queue size.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/hw/virtio/virtio.h | 2 +-
hw/virtio/virtio.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index c99cb19d886..ff7f837fb92 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -302,7 +302,7 @@ void virtio_device_set_child_bus_name(VirtIODevice *vdev, char *bus_name);
typedef void (*VirtIOHandleOutput)(VirtIODevice *, VirtQueue *);
-VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
+VirtQueue *virtio_add_queue(VirtIODevice *vdev, unsigned int queue_size,
VirtIOHandleOutput handle_output);
void virtio_del_queue(VirtIODevice *vdev, int n);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index daa5607338c..e1210f024c6 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2564,7 +2564,7 @@ void virtio_queue_set_vector(VirtIODevice *vdev, int n, uint16_t vector)
}
}
-VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
+VirtQueue *virtio_add_queue(VirtIODevice *vdev, unsigned int queue_size,
VirtIOHandleOutput handle_output)
{
int i;
--
2.55.0
On Thu, 30 Jul 2026 at 16:36, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> virtio_add_queue()'s queue_size argument is a signed int. Coverity is
> unhappy when that type is used as an argument to g_new0():
>
> *** CID 1664271: Error handling issues (NEGATIVE_RETURNS)
> /builds/qemu-project/qemu/hw/virtio/virtio.c: 2595 in virtio_add_queue()
> 2589 }
> 2590
> 2591 vdev->vq[i].vring.num = queue_size;
> 2592 vdev->vq[i].vring.num_default = queue_size;
> 2593 vdev->vq[i].vring.align = VIRTIO_PCI_VRING_ALIGN;
> 2594 vdev->vq[i].handle_output = handle_output;
> >>> CID 1664271: Error handling issues (NEGATIVE_RETURNS)
> >>> "__n" is passed to a parameter that cannot be negative.
> 2595 vdev->vq[i].used_elems = g_new0(VirtQueueElement, queue_size);
Though note that the reason Coverity thinks this value can be negative
is not because the function might be passed a negative value, but
because it incorrectly thinks that
int override = object_property_get_int(OBJECT(qbus->parent),
VIRTIO_QUEUE_SIZE_OVERRIDE,
&error_abort);
can return a negative number. (It can't because of the error_abort
and because the property is a UINT16 one.)
It might be a good idea to enforce the VIRTQUEUE_MAX_SIZE bounds
check on the queue size we get from the override property. Since
I think that can be set by the end-user (though probably it won't
be in practice) ideally we would not abort() on a bad property
value, but that's awkward to do at this point...
thanks
-- PMM
On Thu, Jul 30, 2026 at 12:02 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Thu, 30 Jul 2026 at 16:36, Stefan Hajnoczi <stefanha@redhat.com> wrote: > > > > virtio_add_queue()'s queue_size argument is a signed int. Coverity is > > unhappy when that type is used as an argument to g_new0(): > > > > *** CID 1664271: Error handling issues (NEGATIVE_RETURNS) > > /builds/qemu-project/qemu/hw/virtio/virtio.c: 2595 in virtio_add_queue() > > 2589 } > > 2590 > > 2591 vdev->vq[i].vring.num = queue_size; > > 2592 vdev->vq[i].vring.num_default = queue_size; > > 2593 vdev->vq[i].vring.align = VIRTIO_PCI_VRING_ALIGN; > > 2594 vdev->vq[i].handle_output = handle_output; > > >>> CID 1664271: Error handling issues (NEGATIVE_RETURNS) > > >>> "__n" is passed to a parameter that cannot be negative. > > 2595 vdev->vq[i].used_elems = g_new0(VirtQueueElement, queue_size); > > Though note that the reason Coverity thinks this value can be negative > is not because the function might be passed a negative value, but > because it incorrectly thinks that > int override = object_property_get_int(OBJECT(qbus->parent), > VIRTIO_QUEUE_SIZE_OVERRIDE, > &error_abort); > > can return a negative number. (It can't because of the error_abort > and because the property is a UINT16 one.) > > It might be a good idea to enforce the VIRTQUEUE_MAX_SIZE bounds > check on the queue size we get from the override property. Since > I think that can be set by the end-user (though probably it won't > be in practice) ideally we would not abort() on a bad property > value, but that's awkward to do at this point... I'll send a v2 that addresses VIRTIO_QUEUE_SIZE_OVERRIDE. Stefan
© 2016 - 2026 Red Hat, Inc.