Re: [PATCH v2 4/5] vhost: check vring address before calling unmap

[Raphael Norwitz]

On Thu, Apr 30, 2020 at 9:50 AM Dima Stepanov <address@hidden> wrote:
>
> Since disconnect can happen at any time during initialization not all
> vring buffers (for instance used vring) can be intialized successfully.
> If the buffer was not initialized then vhost_memory_unmap call will lead
> to SIGSEGV. Add checks for the vring address value before calling unmap.
> Also add assert() in the vhost_memory_unmap() routine.
>
> Signed-off-by: Dima Stepanov <address@hidden>

Reviewed-by: Raphael Norwitz <address@hidden>

> ---
>  hw/virtio/vhost.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index ddbdc53..3ee50c4 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -314,6 +314,8 @@ static void vhost_memory_unmap(struct vhost_dev *dev, 
> void *buffer,
>                                 hwaddr len, int is_write,
>                                 hwaddr access_len)
>  {
> +    assert(buffer);
> +
>      if (!vhost_dev_has_iommu(dev)) {
>          cpu_physical_memory_unmap(buffer, len, is_write, access_len);
>      }
> @@ -1132,12 +1134,25 @@ static void vhost_virtqueue_stop(struct vhost_dev 
> *dev,
>                                                  vhost_vq_index);
>      }
>
> -    vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
> -                       1, virtio_queue_get_used_size(vdev, idx));
> -    vhost_memory_unmap(dev, vq->avail, virtio_queue_get_avail_size(vdev, 
> idx),
> -                       0, virtio_queue_get_avail_size(vdev, idx));
> -    vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
> -                       0, virtio_queue_get_desc_size(vdev, idx));
> +    /*
> +     * Since the vhost-user disconnect can happen during initialization
> +     * check if vring was initialized, before making unmap.
> +     */
> +    if (vq->used) {
> +        vhost_memory_unmap(dev, vq->used,
> +                           virtio_queue_get_used_size(vdev, idx),
> +                           1, virtio_queue_get_used_size(vdev, idx));
> +    }
> +    if (vq->avail) {
> +        vhost_memory_unmap(dev, vq->avail,
> +                           virtio_queue_get_avail_size(vdev, idx),
> +                           0, virtio_queue_get_avail_size(vdev, idx));
> +    }
> +    if (vq->desc) {
> +        vhost_memory_unmap(dev, vq->desc,
> +                           virtio_queue_get_desc_size(vdev, idx),
> +                           0, virtio_queue_get_desc_size(vdev, idx));
> +    }
>  }
>
>  static void vhost_eventfd_add(MemoryListener *listener,
> --
> 2.7.4
>
>