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: [email protected] Fixes: 6bdc21c050a2 ("virtio: fix up max size checks") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3802 Signed-off-by: Laurent Vivier <[email protected]> --- hw/block/virtio-blk.c | 4 ++++ hw/char/virtio-serial-bus.c | 3 +++ hw/scsi/scsi-bus.c | 4 ++++ hw/scsi/virtio-scsi.c | 4 ++++ hw/virtio/virtio.c | 11 ++++------- 5 files changed, 19 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 80f1b308aa53..4ed8840b2efe 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/scsi-bus.c b/hw/scsi/scsi-bus.c index dccb2f25b2af..96c4844374c5 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -1915,6 +1915,10 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size, req->retry = (sbyte == 1); if (bus->info->load_request) { req->hba_private = bus->info->load_request(f, req); + if (!req->hba_private) { + scsi_req_unref(req); + return -EINVAL; + } } if (req->ops->load_request) { req->ops->load_request(f, req); 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); 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
