virtio_load() reads config_len from the migration stream into a local
int32_t variable and then, if the incoming length exceeds the device's
own config size, skips the surplus one byte at a time in a loop. Two
defects in this loop allow a malicious or truncated migration stream to
cause a prolonged CPU spin on the destination QEMU.
First, the local variable is int32_t while vdev->config_len is size_t.
The C standard converts the signed operand to the unsigned type for the
comparison, so any value with the high bit set becomes a very large
size_t that exceeds the device config length. The loop then counts the
signed counter down through all negative values and back around,
effectively running for up to 2^32 iterations.
Second, once the file stream reaches EOF, qemu_get_byte() returns 0
without advancing the read position. Without a check for file errors,
the loop spins until the counter reaches the device config length. An
attacker who sends a large config_len and then closes the stream can
hold the destination QEMU busy for attacker-controlled time.
Change the local config_len to uint32_t so that the comparison with
size_t is unsigned and values with the high bit set are treated as
large-but-finite skip counts. In addition, check qemu_file_get_error()
after each byte read and return failure immediately when the stream
signals EOF or an I/O error.
Fixes: 2f5732e964 ("Allow mismatched virtio config-len")
Cc: Dr. David Alan Gilbert <[email protected]>
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3891
Reviewed-by: Michael S. Tsirkin <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>
---
hw/virtio/virtio.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 34f1df260b..848fe8539e 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3584,6 +3584,9 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int
version_id)
return -1;
}
qemu_get_byte(f);
+ if (qemu_file_get_error(f)) {
+ return -1;
+ }
config_len--;
}
--
MST
