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

[Michael S. Tsirkin]

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>

This fails test for cross-i686-tci


https://gitlab.com/mstredhat/qemu/-/jobs/6578222837

57/258 ERROR:../tests/qtest/libqos/virtio.c:230:qvirtio_wait_used_elem: 
assertion failed: (g_get_monotonic_time() - start_time <= timeout_us) ERROR     
    
 57/258 qemu:qtest+qtest-i386 / qtest-i386/qos-test                        
ERROR           56.69s   killed by signal 6 SIGABRT
>>> QTEST_QEMU_BINARY=./qemu-system-i386 
>>> PYTHON=/builds/mstredhat/qemu/build/pyvenv/bin/python3 MALLOC_PERTURB_=112 
>>> G_TEST_DBUS_DAEMON=/builds/mstredhat/qemu/tests/dbus-vmstate-daemon.sh 
>>> /builds/mstredhat/qemu/build/tests/qtest/qos-test --tap -k
――――――――――――――――――――――――――――――――――――― ✀  ―――――――――――――――――――――――――――――――――――――
stderr:
Vhost user backend fails to broadcast fake RARP
qemu-system-i386: -chardev 
socket,id=chr-reconnect,path=/tmp/vhost-test-80QXL2/reconnect.sock,server=on: 
info: QEMU waiting for connection on: 
disconnected:unix:/tmp/vhost-test-80QXL2/reconnect.sock,server=on
qemu-system-i386: Failed to set msg fds.
qemu-system-i386: vhost VQ 0 ring restore failed: -22: Invalid argument (22)
qemu-system-i386: Failed to set msg fds.
qemu-system-i386: vhost VQ 1 ring restore failed: -22: Invalid argument (22)
qemu-system-i386: Failed to set msg fds.
qemu-system-i386: vhost VQ 0 ring restore failed: -22: Invalid argument (22)
qemu-system-i386: Failed to set msg fds.
qemu-system-i386: vhost VQ 1 ring restore failed: -22: Invalid argument (22)
qemu-system-i386: -chardev 
socket,id=chr-connect-fail,path=/tmp/vhost-test-CZMWL2/connect-fail.sock,server=on:
 info: QEMU waiting for connection on: 
disconnected:unix:/tmp/vhost-test-CZMWL2/connect-fail.sock,server=on
qemu-system-i386: -netdev 
vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: Failed to read msg 
header. Read 0 instead of 12. Original request 1.
qemu-system-i386: -netdev 
vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: vhost_backend_init 
failed: Protocol error
qemu-system-i386: -netdev 
vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: failed to init 
vhost_net for queue 0
qemu-system-i386: -netdev 
vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: info: QEMU waiting 
for connection on: 
disconnected:unix:/tmp/vhost-test-CZMWL2/connect-fail.sock,server=on
qemu-system-i386: Failed to write msg. Wrote -1 instead of 20.
qemu-system-i386: vhost VQ 0 ring restore failed: -22: Invalid argument (22)
qemu-system-i386: Failed to set msg fds.
qemu-system-i386: vhost VQ 1 ring restore failed: -22: Invalid argument (22)
qemu-system-i386: -chardev 
socket,id=chr-flags-mismatch,path=/tmp/vhost-test-CBHDM2/flags-mismatch.sock,server=on:
 info: QEMU waiting for connection on: 
disconnected:unix:/tmp/vhost-test-CBHDM2/flags-mismatch.sock,server=on
qemu-system-i386: Failed to write msg. Wrote -1 instead of 84.
qemu-system-i386: vhost_set_mem_table failed: Invalid argument (22)
qemu-system-i386: unable to start vhost net: 22: falling back on userspace 
virtio
vhost lacks feature mask 0x40000000 for backend
qemu-system-i386: failed to init vhost_net for queue 0
qemu-system-i386: Failed to set msg fds.
qemu-system-i386: vhost VQ 0 ring restore failed: -22: Invalid argument (22)
qemu-system-i386: Failed to set msg fds.
qemu-system-i386: vhost VQ 1 ring restore failed: -22: Invalid argument (22)
qemu-system-i386: Failed to write msg. Wrote -1 instead of 20.
qemu-system-i386: vhost VQ 0 ring restore failed: -22: Invalid argument (22)
qemu-system-i386: Failed to set msg fds.
qemu-system-i386: vhost VQ 1 ring restore failed: -22: Invalid argument (22)
qemu-system-i386: Failed to set msg fds.
qemu-system-i386: vhost VQ 2 ring restore failed: -22: Invalid argument (22)
qemu-system-i386: Failed to set msg fds.
qemu-system-i386: vhost VQ 3 ring restore failed: -22: Invalid argument (22)
qemu-system-i386: Failed to set msg fds.
qemu-system-i386: vhost VQ 0 ring restore failed: -22: Invalid argument (22)
qemu-system-i386: Failed to set msg fds.
qemu-system-i386: vhost VQ 1 ring restore failed: -22: Invalid argument (22)
qemu-system-i386: Failed to set msg fds.
qemu-system-i386: vhost VQ 0 ring restore failed: -22: Invalid argument (22)
qemu-system-i386: Failed to set msg fds.
qemu-system-i386: vhost VQ 1 ring restore failed: -22: Invalid argument (22)
qemu-system-i386: virtio-iommu bad head/tail size
**
ERROR:../tests/qtest/libqos/virtio.c:230:qvirtio_wait_used_elem: assertion 
failed: (g_get_monotonic_time() - start_time <= timeout_us)
(test program exited with status code -6)
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――


So we hit the error where we did not previously hit the assert.

Dropped for now. Pls figure it out and resubmit.

> ---
>  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