Fuzzing found that on some error paths vhost_memory_unmap() is called twice or
for NULL address. Let's reset pointers after unmap and ingnore unmap for NULL.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
---
 hw/virtio/vhost.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 7b03efccec..4e5d5f2ea4 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -335,7 +335,7 @@ static void vhost_memory_unmap(struct vhost_dev *dev, void *buffer,
                                hwaddr len, int is_write,
                                hwaddr access_len)
 {
-    if (!vhost_dev_has_iommu(dev)) {
+    if (buffer && !vhost_dev_has_iommu(dev)) {
         cpu_physical_memory_unmap(buffer, len, is_write, access_len);
     }
 }
@@ -1191,6 +1191,7 @@ fail_alloc_avail:
     vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
                        0, 0);
 fail_alloc_desc:
+    vq->used = vq->avail = vq->desc = NULL;
     return r;
 }
 
@@ -1238,6 +1239,7 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
                        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));
+    vq->used = vq->avail = vq->desc = NULL;
 }
 
 static void vhost_eventfd_add(MemoryListener *listener,

On 10/2/22 12:46, Konstantin Khlebnikov wrote:
> Fuzzing found that on some error paths vhost_memory_unmap() is called twice or
> for NULL address. Let's reset pointers after unmap and ingnore unmap for NULL.
> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
> ---
>   hw/virtio/vhost.c |    4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 7b03efccec..4e5d5f2ea4 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -335,7 +335,7 @@ static void vhost_memory_unmap(struct vhost_dev *dev, void *buffer,
>                                  hwaddr len, int is_write,
>                                  hwaddr access_len)
>   {
> -    if (!vhost_dev_has_iommu(dev)) {
> +    if (buffer && !vhost_dev_has_iommu(dev)) {

Shouldn't we simply add an "assert(buffer);" check here instead?

>           cpu_physical_memory_unmap(buffer, len, is_write, access_len);
>       }
>   }
> @@ -1191,6 +1191,7 @@ fail_alloc_avail:
>       vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
>                          0, 0);
>   fail_alloc_desc:
> +    vq->used = vq->avail = vq->desc = NULL;
>       return r;
>   }
>   
> @@ -1238,6 +1239,7 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
>                          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));
> +    vq->used = vq->avail = vq->desc = NULL;
>   }

This part is OK.