Reviewed-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>
On Sat, Oct 11, 2025 at 7:24 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> This helps to simplify failure paths of vhost_virtqueue_start()
> a lot. We also need to zero-out pointers on unmap, to not try
> to unmap invalid pointers.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> hw/virtio/vhost.c | 41 +++++++++++++++++++++--------------------
> 1 file changed, 21 insertions(+), 20 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 8031c74e7b..6fec193d5f 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -472,14 +472,20 @@ static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
> }
> }
>
> -static void vhost_memory_unmap(struct vhost_dev *dev, void *buffer,
> +static void vhost_memory_unmap(struct vhost_dev *dev, void **buffer,
> hwaddr len, int is_write,
> hwaddr access_len)
> {
> + if (!*buffer) {
> + return;
> + }
> +
> if (!vhost_dev_has_iommu(dev)) {
> - address_space_unmap(dev->vdev->dma_as, buffer, len, is_write,
> + address_space_unmap(dev->vdev->dma_as, *buffer, len, is_write,
> access_len);
> }
> +
> + *buffer = NULL;
> }
>
> static int vhost_verify_ring_part_mapping(void *ring_hva,
> @@ -1315,33 +1321,33 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
> vq->desc = vhost_memory_map(dev, a, l, false);
> if (!vq->desc) {
> r = -ENOMEM;
> - goto fail_alloc_desc;
> + goto fail;
> }
> vq->avail_size = l = virtio_queue_get_avail_size(vdev, idx);
> vq->avail_phys = a = virtio_queue_get_avail_addr(vdev, idx);
> vq->avail = vhost_memory_map(dev, a, l, false);
> if (!vq->avail) {
> r = -ENOMEM;
> - goto fail_alloc_avail;
> + goto fail;
> }
> vq->used_size = l = virtio_queue_get_used_size(vdev, idx);
> vq->used_phys = a = virtio_queue_get_used_addr(vdev, idx);
> vq->used = vhost_memory_map(dev, a, l, true);
> if (!vq->used) {
> r = -ENOMEM;
> - goto fail_alloc_used;
> + goto fail;
> }
>
> r = vhost_virtqueue_set_addr(dev, vq, vhost_vq_index, dev->log_enabled);
> if (r < 0) {
> - goto fail_alloc;
> + goto fail;
> }
>
> file.fd = event_notifier_get_fd(virtio_queue_get_host_notifier(vvq));
> r = dev->vhost_ops->vhost_set_vring_kick(dev, &file);
> if (r) {
> VHOST_OPS_DEBUG(r, "vhost_set_vring_kick failed");
> - goto fail_kick;
> + goto fail;
> }
>
> /* Clear and discard previous events if any. */
> @@ -1361,24 +1367,19 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
> file.fd = -1;
> r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
> if (r) {
> - goto fail_vector;
> + goto fail;
> }
> }
>
> return 0;
>
> -fail_vector:
> -fail_kick:
> -fail_alloc:
> - vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
> +fail:
> + vhost_memory_unmap(dev, &vq->used, virtio_queue_get_used_size(vdev, idx),
> 0, 0);
> -fail_alloc_used:
> - vhost_memory_unmap(dev, vq->avail, virtio_queue_get_avail_size(vdev, idx),
> + vhost_memory_unmap(dev, &vq->avail, virtio_queue_get_avail_size(vdev, idx),
> 0, 0);
> -fail_alloc_avail:
> - vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
> + vhost_memory_unmap(dev, &vq->desc, virtio_queue_get_desc_size(vdev, idx),
> 0, 0);
> -fail_alloc_desc:
> return r;
> }
>
> @@ -1425,11 +1426,11 @@ static int do_vhost_virtqueue_stop(struct vhost_dev *dev,
> vhost_vq_index);
> }
>
> - vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
> + 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),
> + 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),
> + vhost_memory_unmap(dev, &vq->desc, virtio_queue_get_desc_size(vdev, idx),
> 0, virtio_queue_get_desc_size(vdev, idx));
> return r;
> }
> --
> 2.48.1
>