| Message ID | 20240404124505.2108743-1-zheyuma97@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | virtio-iommu: Do not process commands with bad size | expand |
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
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);
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> --- hw/virtio/virtio-iommu.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)