[Qemu-devel] [PATCH v2] [PATCH] Allow mismatched virtio config-len

[Qemu-devel] [PATCH v2] [PATCH] Allow mismatched virtio config-len

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@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 wce addition in virtio-blk.

Allow mismatched config-lengths:
   *) If the version on the wire is shorter then fine
   *) If the version on the wire is longer, load what we have space
      for and skip the rest.

(This is mst@redhat.com's rework of what I originally posted)

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/virtio/virtio.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index a3082d5..c1d538c 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -926,12 +926,18 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
         return -1;
     }
     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;
+
+    /*
+     * There are cases where the incoming config can be bigger or smaller
+     * than what we have; so load what we have space for, and skip
+     * any excess that's in the stream.
+     */
+    qemu_get_buffer(f, vdev->config, MIN(config_len, vdev->config_len));
+
+    while (config_len > vdev->config_len) {
+        qemu_get_byte(f);
+        config_len--;
     }
-    qemu_get_buffer(f, vdev->config, vdev->config_len);
 
     num = qemu_get_be32(f);
 
-- 
1.9.3

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

[Michael S. Tsirkin]

On Fri, Jun 27, 2014 at 08:02:48PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@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 wce addition in virtio-blk.
> 
> Allow mismatched config-lengths:
>    *) If the version on the wire is shorter then fine
>    *) If the version on the wire is longer, load what we have space
>       for and skip the rest.
> 
> (This is mst@redhat.com's rework of what I originally posted)
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Applied, thanks!

> ---
>  hw/virtio/virtio.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index a3082d5..c1d538c 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -926,12 +926,18 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
>          return -1;
>      }
>      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;
> +
> +    /*
> +     * There are cases where the incoming config can be bigger or smaller
> +     * than what we have; so load what we have space for, and skip
> +     * any excess that's in the stream.
> +     */
> +    qemu_get_buffer(f, vdev->config, MIN(config_len, vdev->config_len));
> +
> +    while (config_len > vdev->config_len) {
> +        qemu_get_byte(f);
> +        config_len--;
>      }
> -    qemu_get_buffer(f, vdev->config, vdev->config_len);
>  
>      num = qemu_get_be32(f);
>  
> -- 
> 1.9.3