| Message ID | 164449358353.2210343.10106072767435805975.stgit@dynamic-vpn.dhcp.yndx.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vhost: fix repeated memory unmap in error paths | expand |
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.
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,
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(-)