qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] Allow mismatched virtio config-len


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH] Allow mismatched virtio config-len
Date: Fri, 27 Jun 2014 13:29:54 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0

Il 27/06/2014 10:34, Dr. David Alan Gilbert (git) ha scritto:
From: "Dr. David Alan Gilbert" <address@hidden>

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 <address@hidden>
---
 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);






reply via email to

[Prev in Thread] Current Thread [Next in Thread]