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

2014-05-05 Thread Juan Quintela 
Michael S. Tsirkin m...@redhat.com 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 dgilb...@redhat.com
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
  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);

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

2014-05-05 Thread Juan Quintela 
Juan Quintela quint...@redhat.com wrote:
 Michael S. Tsirkin m...@redhat.com 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 dgilb...@redhat.com
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
  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);