Re: [PATCH] virtio-iommu: Do not process commands with bad size

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Apr 04, 2024 at 02:45:05PM +0200, Zheyu Ma wrote:
> The device should not handle the commands which have bad request/reply
> size, it should just report the error instead of raising an assertation.
> 
> Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>

Seems to cause CI failures:
https://gitlab.com/mstredhat/qemu/-/jobs/7004894370


41/399 /riscv32/virt/generic-pcihost/pci-bus-generic/pci-bus/virtio-iommu-pci/virtio-iommu/virtio-iommu-tests/attach_detach - ERROR:../tests/qtest/libqos/virtio.c:230:qvirtio_wait_used_elem: assertion failed: (g_get_monotonic_time() - start_time <= timeout_us) FAIL          
▶  41/399                                                                     ERROR         
 41/399 qemu:qtest+qtest-riscv32 / qtest-riscv32/qos-test                     ERROR           76.80s   killed by signal 6 SIGABRT
>>> MALLOC_PERTURB_=235 QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon QTEST_QEMU_BINARY=./qemu-system-riscv32 G_TEST_DBUS_DAEMON=/builds/mstredhat/qemu/tests/dbus-vmstate-daemon.sh QTEST_QEMU_IMG=./qemu-img PYTHON=/builds/mstredhat/qemu/build/pyvenv/bin/python3 /builds/mstredhat/qemu/build/tests/qtest/qos-test --tap -k




> ---
>  hw/virtio/virtio-iommu.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 1326c6ec41..3a7cdfe777 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -770,8 +770,8 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
>              return;
>          }
>  
> -        if (iov_size(elem->in_sg, elem->in_num) < sizeof(tail) ||
> -            iov_size(elem->out_sg, elem->out_num) < sizeof(head)) {
> +        if (iov_size(elem->in_sg, elem->in_num) != sizeof(tail) ||
> +            iov_size(elem->out_sg, elem->out_num) != sizeof(head)) {
>              virtio_error(vdev, "virtio-iommu bad head/tail size");
>              virtqueue_detach_element(vq, elem, 0);
>              g_free(elem);
> @@ -818,8 +818,6 @@ 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);
> -        assert(sz == output_size);
> -
>          virtqueue_push(vq, elem, sz);
>          virtio_notify(vdev, vq);
>          g_free(elem);
> @@ -852,7 +850,7 @@ static void virtio_iommu_report_fault(VirtIOIOMMU *viommu, uint8_t reason,
>          return;
>      }
>  
> -    if (iov_size(elem->in_sg, elem->in_num) < sizeof(fault)) {
> +    if (iov_size(elem->in_sg, elem->in_num) != sizeof(fault)) {
>          virtio_error(vdev, "error buffer of wrong size");
>          virtqueue_detach_element(vq, elem, 0);
>          g_free(elem);
> @@ -861,8 +859,6 @@ static void virtio_iommu_report_fault(VirtIOIOMMU *viommu, uint8_t reason,
>  
>      sz = iov_from_buf(elem->in_sg, elem->in_num, 0,
>                        &fault, sizeof(fault));
> -    assert(sz == sizeof(fault));
> -
>      trace_virtio_iommu_report_fault(reason, flags, endpoint, address);
>      virtqueue_push(vq, elem, sz);
>      virtio_notify(vdev, vq);
> -- 
> 2.34.1