Re: [Qemu-devel] [PATCH v6 4/5] virtio: validate config_len on load

[Juan Quintela]

Juan Quintela  wrote:
> "Michael S. Tsirkin"  wrote:
>> Malformed input can have config_len in migration stream
>> exceed the array size allocated on destination, the
>> result will be heap overflow.
>>
>> To fix, that config_len matches on both sides.
>>
>> CVE-2014-0182
>>
>> Reported-by: "Dr. David Alan Gilbert" 
>> Signed-off-by: Michael S. Tsirkin 
>> ---
>>  hw/virtio/virtio.c | 8 +++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>> index 3bad71e..0d5d368 100644
>> --- a/hw/virtio/virtio.c
>> +++ b/hw/virtio/virtio.c
>> @@ -898,6 +898,7 @@ int virtio_set_features(VirtIODevice *vdev, uint32_t val)
>>  int virtio_load(VirtIODevice *vdev, QEMUFile *f)
>>  {
>>  int i, ret;
>> +int32_t config_len;
>
> Has a warning.
>
> /mnt/kvm/qemu/next/hw/virtio/virtio.c: In function ‘virtio_load’:
> /mnt/kvm/qemu/next/hw/virtio/virtio.c:931:22: error: format ‘%i’
> expects argument of type ‘int’, but argument 2 has type ‘size_t’
> [-Werror=format=]
>   config_len, vdev->config_len);
>   ^
>
> changing config_len to size_t.

After discussing with michael, left it as int32_t


>
>>  uint32_t num;
>>  uint32_t features;
>>  uint32_t supported_features;
>> @@ -924,7 +925,12 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
>>   features, supported_features);
>>  return -1;
>>  }
>> -vdev->config_len = qemu_get_be32(f);
>> +config_len = qemu_get_be32(f);
>> +if (config_len != vdev->config_len) {
>> +error_report("Unexpected config length 0x%x. Expected 0x%x",
>
> and this to:
>
> s/%x/%zx/

and use here "%ix    %zx"

Later, Juan.

> Later, Juan.
>
>
>> + config_len, vdev->config_len);
>> +return -1;
>> +}
>>  qemu_get_buffer(f, vdev->config, vdev->config_len);
>>  
>>  num = qemu_get_be32(f);

Re: [Qemu-devel] [PATCH v6 4/5] virtio: validate config_len on load

[Juan Quintela]

"Michael S. Tsirkin"  wrote:
> Malformed input can have config_len in migration stream
> exceed the array size allocated on destination, the
> result will be heap overflow.
>
> To fix, that config_len matches on both sides.
>
> CVE-2014-0182
>
> Reported-by: "Dr. David Alan Gilbert" 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  hw/virtio/virtio.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 3bad71e..0d5d368 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -898,6 +898,7 @@ int virtio_set_features(VirtIODevice *vdev, uint32_t val)
>  int virtio_load(VirtIODevice *vdev, QEMUFile *f)
>  {
>  int i, ret;
> +int32_t config_len;

Has a warning.

/mnt/kvm/qemu/next/hw/virtio/virtio.c: In function ‘virtio_load’:
/mnt/kvm/qemu/next/hw/virtio/virtio.c:931:22: error: format ‘%i’ expects 
argument of type ‘int’, but argument 2 has type ‘size_t’ [-Werror=format=]
  config_len, vdev->config_len);
  ^

changing config_len to size_t.

>  uint32_t num;
>  uint32_t features;
>  uint32_t supported_features;
> @@ -924,7 +925,12 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
>   features, supported_features);
>  return -1;
>  }
> -vdev->config_len = qemu_get_be32(f);
> +config_len = qemu_get_be32(f);
> +if (config_len != vdev->config_len) {
> +error_report("Unexpected config length 0x%x. Expected 0x%x",

and this to:

s/%x/%zx/

Later, Juan.


> + config_len, vdev->config_len);
> +return -1;
> +}
>  qemu_get_buffer(f, vdev->config, vdev->config_len);
>  
>  num = qemu_get_be32(f);

[Qemu-devel] [PATCH v6 4/5] virtio: validate config_len on load

[Michael S. Tsirkin]

Malformed input can have config_len in migration stream
exceed the array size allocated on destination, the
result will be heap overflow.

To fix, that config_len matches on both sides.

CVE-2014-0182

Reported-by: "Dr. David Alan Gilbert" 
Signed-off-by: Michael S. Tsirkin 
---
 hw/virtio/virtio.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 3bad71e..0d5d368 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -898,6 +898,7 @@ int virtio_set_features(VirtIODevice *vdev, uint32_t val)
 int virtio_load(VirtIODevice *vdev, QEMUFile *f)
 {
 int i, ret;
+int32_t config_len;
 uint32_t num;
 uint32_t features;
 uint32_t supported_features;
@@ -924,7 +925,12 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
  features, supported_features);
 return -1;
 }
-vdev->config_len = qemu_get_be32(f);
+config_len = qemu_get_be32(f);
+if (config_len != vdev->config_len) {
+error_report("Unexpected config length 0x%x. Expected 0x%x",
+ config_len, vdev->config_len);
+return -1;
+}
 qemu_get_buffer(f, vdev->config, vdev->config_len);
 
 num = qemu_get_be32(f);
-- 
MST