num_default tracks the allocation size of used_elems, set by
virtio_add_queue(). Migrating it via the ringsize subsection is
wrong: a migration stream (malicious or simply from a different
configuration) can inflate num_default so that
virtio_queue_set_num() accepts oversized values, leading to OOB
access on the used_elems array.
It is not even migrated consistently: a configuration with a
smaller num_default could thinkably migrate and work but in the
common case of num == num_default the value is not actually sent.
Stop migrating num_default: make virtio_ringsize_needed() return
false so the subsection is never sent, and use VMSTATE_UNUSED to
consume the field from old streams without applying it. The
destination keeps its local num_default from virtio_add_queue(),
which matches the actual allocation.
Also validate vring.num against num_default when loading the core
virtio state, rejecting streams that supply a queue size larger
than the locally allocated maximum.
Fixes: 46c5d0823d ("virtio: ring sizes vs. reset")
Fixes: 50e5ae4dc3 ("migration/virtio: Remove simple .get/.put use")
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/virtio/virtio.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 94ddbfd09f..437c001a04 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2817,14 +2817,6 @@ static bool virtio_packed_virtqueue_needed(void *opaque)
static bool virtio_ringsize_needed(void *opaque)
{
- VirtIODevice *vdev = opaque;
- int i;
-
- for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
- if (vdev->vq[i].vring.num != vdev->vq[i].vring.num_default) {
- return true;
- }
- }
return false;
}
@@ -2913,7 +2905,7 @@ static const VMStateDescription vmstate_ringsize = {
.version_id = 1,
.minimum_version_id = 1,
.fields = (const VMStateField[]) {
- VMSTATE_UINT32(vring.num_default, struct VirtQueue),
+ VMSTATE_UNUSED(sizeof(uint32_t)),
VMSTATE_END_OF_LIST()
}
};
@@ -3593,6 +3585,12 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
for (i = 0; i < num; i++) {
vdev->vq[i].vring.num = qemu_get_be32(f);
+ if (vdev->vq[i].vring.num > vdev->vq[i].vring.num_default) {
+ error_report("VQ %d vring.num %u exceeds allocated max %u",
+ i, vdev->vq[i].vring.num,
+ vdev->vq[i].vring.num_default);
+ return -1;
+ }
if (k->has_variable_vring_alignment) {
vdev->vq[i].vring.align = qemu_get_be32(f);
}
--
MST