When QEMU restores pending virtio requests from a migration stream, it
calls qemu_get_virtqueue_element(), which calls virtqueue_map() and
virtqueue_map_iovec() to re-map the saved DMA fragment addresses into
the destination address space. If the addresses in the migration stream
are corrupted, and as a result dma_memory_map() returns NULL or a
shorter-than-expected length, virtqueue_map_iovec() calls exit(1),
terminating the destination QEMU process instead of failing the
migration cleanly.
Convert virtqueue_map_iovec() and virtqueue_map() from void to bool.
On failure, virtqueue_map_iovec() unmaps any entries it has already
mapped and returns false; virtqueue_map() similarly cleans up the in_sg
entries if out_sg mapping fails. qemu_get_virtqueue_element() now
checks the return value, frees the element and returns NULL on failure,
allowing the migration restore path to propagate a clean error. The
callers in virtio-blk, virtio-serial-bus and virtio-scsi that invoke
qemu_get_virtqueue_element() during load are updated to check for NULL
and return an error code.
Fixes: 3b3b062821 ("virtio: slim down allocation of VirtQueueElements")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3888
Reviewed-by: Michael S. Tsirkin <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
---
include/hw/virtio/virtio.h | 2 +-
hw/block/virtio-blk.c | 4 +++
hw/char/virtio-serial-bus.c | 4 +++
hw/scsi/virtio-scsi.c | 4 +++
hw/virtio/virtio.c | 49 +++++++++++++++++++++++++++----------
5 files changed, 49 insertions(+), 14 deletions(-)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index c99cb19d88..27c5fe3a6b 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -320,7 +320,7 @@ bool virtqueue_rewind(VirtQueue *vq, unsigned int num);
void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
unsigned int len, unsigned int idx);
-void virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem);
+bool virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem);
void *virtqueue_pop(VirtQueue *vq, size_t sz);
unsigned int virtqueue_drop_all(VirtQueue *vq);
void *qemu_get_virtqueue_element(VirtIODevice *vdev, QEMUFile *f, size_t sz);
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 6b92066aff..cb6a276a82 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) {
+ error_report("Failed to restore virtio-blk request");
+ 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 83a033ce85..81db0bdc91 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -764,6 +764,10 @@ static int fetch_active_ports_list(QEMUFile *f,
port->elem =
qemu_get_virtqueue_element(vdev, f, sizeof(VirtQueueElement));
+ if (!port->elem) {
+ error_report("Failed to restore virtio-serial element");
+ 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 bf64d1231a..132f833226 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) {
+ error_report("Failed to restore virtio-scsi request");
+ 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 daa5607338..34f1df260b 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1680,36 +1680,56 @@ static void virtqueue_undo_map_desc(AddressSpace *as,
}
}
-static void virtqueue_map_iovec(VirtIODevice *vdev, struct iovec *sg,
+static bool virtqueue_map_iovec(VirtIODevice *vdev, struct iovec *sg,
hwaddr *addr, unsigned int num_sg,
bool is_write)
{
unsigned int i;
hwaddr len;
+ DMADirection dir = is_write ? DMA_DIRECTION_FROM_DEVICE :
+ DMA_DIRECTION_TO_DEVICE;
for (i = 0; i < num_sg; i++) {
len = sg[i].iov_len;
- sg[i].iov_base = dma_memory_map(vdev->dma_as,
- addr[i], &len, is_write ?
- DMA_DIRECTION_FROM_DEVICE :
- DMA_DIRECTION_TO_DEVICE,
- MEMTXATTRS_UNSPECIFIED);
+ sg[i].iov_base = dma_memory_map(vdev->dma_as, addr[i], &len,
+ dir, MEMTXATTRS_UNSPECIFIED);
if (!sg[i].iov_base) {
error_report("virtio: error trying to map MMIO memory");
- exit(1);
+ goto err_undo_map;
}
if (len != sg[i].iov_len) {
error_report("virtio: unexpected memory split");
- exit(1);
+ dma_memory_unmap(vdev->dma_as, sg[i].iov_base, len, dir, 0);
+ goto err_undo_map;
}
}
+ return true;
+
+err_undo_map:
+ while (i-- > 0) {
+ dma_memory_unmap(vdev->dma_as, sg[i].iov_base, sg[i].iov_len,
+ dir, 0);
+ }
+ return false;
}
-void virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem)
+bool virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem)
{
- virtqueue_map_iovec(vdev, elem->in_sg, elem->in_addr, elem->in_num, true);
- virtqueue_map_iovec(vdev, elem->out_sg, elem->out_addr, elem->out_num,
- false);
+ if (!virtqueue_map_iovec(vdev, elem->in_sg, elem->in_addr,
+ elem->in_num, true)) {
+ return false;
+ }
+ if (!virtqueue_map_iovec(vdev, elem->out_sg, elem->out_addr,
+ elem->out_num, false)) {
+ unsigned int i;
+ for (i = 0; i < elem->in_num; i++) {
+ dma_memory_unmap(vdev->dma_as, elem->in_sg[i].iov_base,
+ elem->in_sg[i].iov_len,
+ DMA_DIRECTION_FROM_DEVICE, 0);
+ }
+ return false;
+ }
+ return true;
}
static void *virtqueue_alloc_element(size_t sz, unsigned out_num, unsigned
in_num)
@@ -2206,7 +2226,10 @@ void *qemu_get_virtqueue_element(VirtIODevice *vdev,
QEMUFile *f, size_t sz)
qemu_get_be32s(f, &elem->ndescs);
}
- virtqueue_map(vdev, elem);
+ if (!virtqueue_map(vdev, elem)) {
+ g_free(elem);
+ return NULL;
+ }
return elem;
}
--
MST
