[PATCH v2] virtio-iommu: add error check before assert

Manos Pitsidianakis posted 1 patch 4 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240613-fuzz-2359-fix-v2-manos.pitsidianakis@linaro.org
Maintainers: Eric Auger <eric.auger@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
hw/virtio/virtio-iommu.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
[PATCH v2] virtio-iommu: add error check before assert
Posted by Manos Pitsidianakis 4 months, 2 weeks ago
A fuzzer case discovered by Zheyu Ma causes an assert failure.

Add a check before the assert, and respond with an error before moving
on to the next queue element.

To reproduce the failure:

cat << EOF | \
qemu-system-x86_64 \
-display none -machine accel=qtest -m 512M -machine q35 -nodefaults \
-device virtio-iommu -qtest stdio
outl 0xcf8 0x80000804
outw 0xcfc 0x06
outl 0xcf8 0x80000820
outl 0xcfc 0xe0004000
write 0x10000e 0x1 0x01
write 0xe0004020 0x4 0x00001000
write 0xe0004028 0x4 0x00101000
write 0xe000401c 0x1 0x01
write 0x106000 0x1 0x05
write 0x100001 0x1 0x60
write 0x100002 0x1 0x10
write 0x100009 0x1 0x04
write 0x10000c 0x1 0x01
write 0x100018 0x1 0x04
write 0x10001c 0x1 0x02
write 0x101003 0x1 0x01
write 0xe0007001 0x1 0x00
EOF

Reported-by: Zheyu Ma <zheyuma97@gmail.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2359
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
Range-diff against v1:
1:  a665c6e73d ! 1:  8e50c1b00e virtio-iommu: add error check before assert
    @@ Commit message
     
      ## hw/virtio/virtio-iommu.c ##
     @@ hw/virtio/virtio-iommu.c: static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
    +         iov = elem->out_sg;
    +         sz = iov_to_buf(iov, iov_cnt, 0, &head, sizeof(head));
    +         if (unlikely(sz != sizeof(head))) {
    ++            qemu_log_mask(LOG_GUEST_ERROR,
    ++                          "%s: read %zu bytes from command head"
    ++                          "but expected %zu\n", __func__, sz, sizeof(head));
    +             tail.status = VIRTIO_IOMMU_S_DEVERR;
    +             goto out;
    +         }
    +@@ hw/virtio/virtio-iommu.c: static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
      out:
              sz = iov_from_buf(elem->in_sg, elem->in_num, 0,
                                buf ? buf : &tail, output_size);
     +        if (unlikely(sz != output_size)) {
    ++            qemu_log_mask(LOG_GUEST_ERROR,
    ++                          "%s: wrote %zu bytes to command response"
    ++                          "but response size is %zu\n",
    ++                          __func__, sz, output_size);
     +            tail.status = VIRTIO_IOMMU_S_DEVERR;
    -+            /* We checked that tail can fit earlier */
    ++            /*
    ++             * We checked that sizeof(tail) can fit to elem->in_sg at the
    ++             * beginning of the loop
    ++             */
     +            output_size = sizeof(tail);
     +            g_free(buf);
     +            buf = NULL;

 hw/virtio/virtio-iommu.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 1326c6ec41..9d801fb180 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -782,6 +782,9 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
         iov = elem->out_sg;
         sz = iov_to_buf(iov, iov_cnt, 0, &head, sizeof(head));
         if (unlikely(sz != sizeof(head))) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: read %zu bytes from command head"
+                          "but expected %zu\n", __func__, sz, sizeof(head));
             tail.status = VIRTIO_IOMMU_S_DEVERR;
             goto out;
         }
@@ -818,6 +821,25 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
 out:
         sz = iov_from_buf(elem->in_sg, elem->in_num, 0,
                           buf ? buf : &tail, output_size);
+        if (unlikely(sz != output_size)) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "%s: wrote %zu bytes to command response"
+                          "but response size is %zu\n",
+                          __func__, sz, output_size);
+            tail.status = VIRTIO_IOMMU_S_DEVERR;
+            /*
+             * We checked that sizeof(tail) can fit to elem->in_sg at the
+             * beginning of the loop
+             */
+            output_size = sizeof(tail);
+            g_free(buf);
+            buf = NULL;
+            sz = iov_from_buf(elem->in_sg,
+                              elem->in_num,
+                              0,
+                              &tail,
+                              output_size);
+        }
         assert(sz == output_size);
 
         virtqueue_push(vq, elem, sz);

base-commit: f3e8cc47de2bc537d4991e883a85208e4e1c0f98
-- 
γαῖα πυρί μιχθήτω