From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Check calls to qemu_get_virtqueue_element for NULL and pass
up the chain.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/block/virtio-blk.c       | 4 ++++
 hw/char/virtio-serial-bus.c | 4 ++++
 hw/scsi/virtio-scsi.c       | 4 ++++
 3 files changed, 12 insertions(+)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 50b5c869e3..324c6b2b27 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -888,6 +888,10 @@ static int virtio_blk_load_device(VirtIODevice *vdev, QEMUFile *f,
         }
 
         req = qemu_get_virtqueue_element(vdev, f, sizeof(VirtIOBlockReq));
+        if (!req) {
+            error_report("%s: Bad vq element %u", __func__, vq_idx);
+            return -EINVAL;
+        }
         virtio_blk_init_request(s, virtio_get_queue(vdev, vq_idx), req);
         req->next = s->rq;
         s->rq = req;
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index d2dd8ab502..e99dc9bf59 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -781,6 +781,10 @@ static int fetch_active_ports_list(QEMUFile *f,
 
             port->elem =
                 qemu_get_virtqueue_element(vdev, f, sizeof(VirtQueueElement));
+            if (!port->elem) {
+                error_report("%s: Bad vq element", __func__);
+                return -EINVAL;
+            }
 
             /*
              *  Port was throttled on source machine.  Let's
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 3aa99717e2..6301af76ad 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -207,6 +207,10 @@ static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
     assert(n < vs->conf.num_queues);
     req = qemu_get_virtqueue_element(vdev, f,
                                      sizeof(VirtIOSCSIReq) + vs->cdb_size);
+    if (!req) {
+        error_report("%s: Bad vq element", __func__);
+        return NULL;
+    }
     virtio_scsi_init_req(s, vs->cmd_vqs[n], req);
 
     if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
-- 
2.17.1

On Mon, 16 Jul 2018 18:37:41 +0100
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Check calls to qemu_get_virtqueue_element for NULL and pass
> up the chain.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/block/virtio-blk.c       | 4 ++++
>  hw/char/virtio-serial-bus.c | 4 ++++
>  hw/scsi/virtio-scsi.c       | 4 ++++
>  3 files changed, 12 insertions(+)
> 
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 50b5c869e3..324c6b2b27 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -888,6 +888,10 @@ static int virtio_blk_load_device(VirtIODevice *vdev, QEMUFile *f,
>          }
>  
>          req = qemu_get_virtqueue_element(vdev, f, sizeof(VirtIOBlockReq));
> +        if (!req) {
> +            error_report("%s: Bad vq element %u", __func__, vq_idx);

Minor nit: vq_idx is the virtqueue index, and this message makes it
look like it is the 'bad vq element'... either add 'vq index', or drop
it completely from the error message?

> +            return -EINVAL;
> +        }
>          virtio_blk_init_request(s, virtio_get_queue(vdev, vq_idx), req);
>          req->next = s->rq;
>          s->rq = req;

Anyway,
Reviewed-by: Cornelia Huck <cohuck@redhat.com>

On Mon, Jul 16, 2018 at 06:37:41PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Check calls to qemu_get_virtqueue_element for NULL and pass
> up the chain.

What happens to the device state that has been partially deserialized
(e.g. virtio-blk's s->rq linked list)?

It's not clear to me that simply returning NULL is enough to put QEMU
into a sane state without memory leaks or crashes if we decide to retry.