[PATCH for-7.0] virtio-iommu: use-after-free fix

* [PATCH for-7.0] virtio-iommu: use-after-free fix
@ 2022-04-07  9:51 Michael S. Tsirkin
  2022-04-07 10:03 ` Peter Maydell
  0 siblings, 1 reply; 4+ messages in thread
From: Michael S. Tsirkin @ 2022-04-07  9:51 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell; +Cc: Eric Auger, Wentao Liang

From: Wentao Liang <Wentao_Liang_g@163.com>

A potential Use-after-free was reported in virtio_iommu_handle_command
when using virtio-iommu:

> I find a potential Use-after-free in QEMU 6.2.0, which is in
> virtio_iommu_handle_command() (./hw/virtio/virtio-iommu.c).
>
>
> Specifically, in the loop body, the variable 'buf' allocated at line 639 can be
> freed by g_free() at line 659. However, if the execution path enters the loop
> body again and the if branch takes true at line 616, the control will directly
> jump to 'out' at line 651. At this time, 'buf' is a freed pointer, which is not
> assigned with an allocated memory but used at line 653. As a result, a UAF bug
> is triggered.
>
>
>
> 599     for (;;) {
> ...
> 615         sz = iov_to_buf(iov, iov_cnt, 0, &head, sizeof(head));
> 616         if (unlikely(sz != sizeof(head))) {
> 617             tail.status = VIRTIO_IOMMU_S_DEVERR;
> 618             goto out;
> 619         }
> ...
> 639             buf = g_malloc0(output_size);
> ...
> 651 out:
> 652         sz = iov_from_buf(elem->in_sg, elem->in_num, 0,
> 653                           buf ? buf : &tail, output_size);
> ...
> 659         g_free(buf);
>
> We can fix it by set ‘buf‘ to NULL after freeing it:
>
>
> 651 out:
> 652         sz = iov_from_buf(elem->in_sg, elem->in_num, 0,
> 653                           buf ? buf : &tail, output_size);
> ...
> 659         g_free(buf);
> +++ buf = NULL;
> 660     }

Fix as suggested by the reporter.

Signed-off-by: Wentao Liang <Wentao_Liang_g@163.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Message-ID: <20220406040445-mutt-send-email-mst@kernel.org>
---
 hw/virtio/virtio-iommu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 239fe97b12..2b1d21edd1 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -683,6 +683,7 @@ out:
         virtio_notify(vdev, vq);
         g_free(elem);
         g_free(buf);
+        buf = NULL;
     }
 }
 
-- 
MST



^ permalink raw reply related	[flat|nested] 4+ messages in thread