Re: [PATCH] hw/virtio: return NULL from qemu_get_virtqueue_element() on invalid state

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Jul 09, 2026 at 12:06:44PM +0200, Laurent Vivier wrote:
> qemu_get_virtqueue_element() uses assert() to check that the in_num
> and out_num fields deserialized from the migration stream do not
> exceed VIRTQUEUE_MAX_SIZE. A crafted migration stream can set these
> fields to invalid values, hitting the assertion and aborting the
> destination QEMU process.
> 
> Replace the assertions with a bounds check that returns NULL on
> failure. Update all callers (virtio-serial-bus, virtio-blk,
> virtio-scsi) to handle the NULL return and fail the migration
> gracefully.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 6bdc21c050a2 ("virtio: fix up max size checks")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3802
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  hw/block/virtio-blk.c       |  4 ++++
>  hw/char/virtio-serial-bus.c |  3 +++
>  hw/scsi/virtio-scsi.c       |  4 ++++
>  hw/virtio/virtio.c          | 11 ++++-------
>  4 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 6b92066aff4c..42c0f2553f18 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -1384,6 +1384,10 @@ static int virtio_blk_load_device(VirtIODevice *vdev, QEMUFile *f,
>          }
>  
>          req = qemu_get_virtqueue_element(vdev, f, sizeof(VirtIOBlockReq));
> +        if (!req) {
> +            return -EINVAL;
> +        }
> +
>          virtio_blk_init_request(s, virtio_get_queue(vdev, vq_idx), req);
>  
>          WITH_QEMU_LOCK_GUARD(&s->rq_lock) {
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index c1973f0248fc..879df158608a 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -763,6 +763,9 @@ static int fetch_active_ports_list(QEMUFile *f,
>  
>              port->elem =
>                  qemu_get_virtqueue_element(vdev, f, sizeof(VirtQueueElement));
> +            if (!port->elem) {
> +                return -EINVAL;
> +            }
>  
>              /*
>               *  Port was throttled on source machine.  Let's
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 6c7376801190..8dd0b88a30c4 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -274,6 +274,10 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
>      assert(n < vs->conf.num_queues);


I mean just above this we have this assert.


If you are really want to be nicer about handling
malformed input, pls do this systematically, not just
at random places for no apparent reason.


>      req = qemu_get_virtqueue_element(vdev, f,
>                                       sizeof(VirtIOSCSIReq) + vs->cdb_size);
> +    if (!req) {
> +        return NULL;
> +    }
> +
>      virtio_scsi_init_req(s, vs->cmd_vqs[n], req);
>  
>      if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index f4d86a365530..7bccfdde33e7 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2167,13 +2167,10 @@ 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.
> -     * This is just one thing (there are probably more) that must be
> -     * fixed before we can allow NDEBUG compilation.
> -     */
> -    assert(ARRAY_SIZE(data.in_addr) >= data.in_num);
> -    assert(ARRAY_SIZE(data.out_addr) >= data.out_num);
> +    if (data.in_num > ARRAY_SIZE(data.in_addr) ||
> +        data.out_num > ARRAY_SIZE(data.out_addr)) {
> +        return NULL;
> +    }
>  
>      elem = virtqueue_alloc_element(sz, data.out_num, data.in_num);
>      elem->index = data.index;
> -- 
> 2.54.0