Re: [PATCH v2 2/2] virtio: use Error for queue size validation in virtio_add_queue()

On Thu, 30 Jul 2026 at 21:59, Stefan Hajnoczi <[email protected]> wrote:
>
> Coverity is unhappy with the code path where the x-override-queue-size
> property value is passed to g_new0() since it is a signed int rather
> than an unsigned int:
>
>       *** 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);
>
> Introduce an Error **errp argument to virtio_add_queue() and set it when
> the queue_size argument or the x-override-queue-size property value are
> invalid.
>
> At the moment none of the callers propagate the Error object. Instead
> they are all modified to pass &error_abort so that the error message is
> printed and the program terminates (it also terminated before). Further
> work, especially in new device emulation code, could actually propagate
> the Error object but is left for the future.



> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index bf64d1231a8..4045414cd2f 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -1317,10 +1317,13 @@ void virtio_scsi_common_realize(DeviceState *dev,
>      s->sense_size = VIRTIO_SCSI_SENSE_DEFAULT_SIZE;
>      s->cdb_size = VIRTIO_SCSI_CDB_DEFAULT_SIZE;
>
> -    s->ctrl_vq = virtio_add_queue(vdev, s->conf.virtqueue_size, ctrl);
> -    s->event_vq = virtio_add_queue(vdev, s->conf.virtqueue_size, evt);
> +    s->ctrl_vq = virtio_add_queue(vdev, s->conf.virtqueue_size, ctrl,
> +                                  &error_abort);
> +    s->event_vq = virtio_add_queue(vdev, s->conf.virtqueue_size, evt,
> +                                   &error_abort);

The virtqueue_size here is user-settable via the device property,
and the realize function only checks that it is at least 2, not
for any upper bound. So this one at least ought ideally to be
propagating the error up. Still, this is no worse than the
previous situation, where we abort() inside virtio_add_queue().

thanks
-- PMM