Il 27/06/2014 10:34, Dr. David Alan Gilbert (git) ha scritto:
From: "Dr. David Alan Gilbert" <dgilb...@redhat.com>
Commit 'virtio: validate config_len on load' restricted config_len
loaded from the wire to match the config_len that the device had.
Unfortunately, there are cases where this isn't true, the one
we found it on was the wqe addition in virtio-blk.
Indeed, the alternative here is to break migration.
As a follow up, it would be nice to let the bus detect whether the
config_len change is valid or not.
For virtio-mmio and s390, mst said that config length must always match
(luckily, these machines aren't versioned so they are not affected by
the wce change).
For virtio-pci, it is okay as long as the old_length +
VIRTIO_PCI_REGION_SIZE(vdev) and new_length +
VIRTIO_PCI_REGION_SIZE(vdev) do not cross a power of two.
Paolo
Allow mismatched config-lengths:
*) If the version on the wire is shorter then ensure that the
remainder is 0xff filled (as virtio_config_read does on
out of range reads)
*) If the version on the wire is longer, load what we have space
for and skip the rest.
Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
---
hw/virtio/virtio.c | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index a3082d5..2b11142 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -927,11 +927,33 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
}
config_len = qemu_get_be32(f);
if (config_len != vdev->config_len) {
- error_report("Unexpected config length 0x%x. Expected 0x%zx",
- config_len, vdev->config_len);
- return -1;
+ /*
+ * Unfortunately the reality is that there are cases where we
+ * see mismatched config lengths, so we have to deal with them
+ * rather than rejecting them.
+ */
+
+ if (config_len < vdev->config_len) {
+ /* This is normal in some devices when they add a new option */
+ memset(vdev->config, 0xff, vdev->config_len);
+ qemu_get_buffer(f, vdev->config, config_len);
+ } else {
+ int32_t diff;
+ /* config_len > vdev->config_len
+ * This is rarer, but is here to allow us to fix the case above
+ */
+ qemu_get_buffer(f, vdev->config, vdev->config_len);
+ /*
+ * Even though we expect the diff to be small, we can't use
+ * qemu_file_skip because it's not safe for a large skip.
+ */
+ for (diff = config_len - vdev->config_len; diff > 0; diff--) {
+ qemu_get_byte(f);
+ }
+ }
+ } else {
+ qemu_get_buffer(f, vdev->config, vdev->config_len);
}
- qemu_get_buffer(f, vdev->config, vdev->config_len);
num = qemu_get_be32(f);