[PATCH v2] tests/qtest/fuzz/virtio_net_fuzz.c: fix virtio_net_fuzz_multi

Dmitry Frolov posted 1 patch 6 months, 1 week ago
tests/qtest/fuzz/virtio_net_fuzz.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
[PATCH v2] tests/qtest/fuzz/virtio_net_fuzz.c: fix virtio_net_fuzz_multi
Posted by Dmitry Frolov 6 months, 1 week ago
The main loop is executed during flush_events(), where virtio error may occur.
This behavior is legit and should not produce any crash report.
But the test is waiting on used descriptors w/o a check, and, in case of error
fails with message: "assertion timer != NULL failed".
Thus, any invalid input data produces a meaningless crash report.
Debuging the problem, I found that in case of virtio error in the main loop,
dev->bus->get_status(dev) is 0 in most cases.
In rare cases VIRTIO_CONFIG_S_NEEDS_RESET bit is set.
So, checking only for VIRTIO_CONFIG_S_NEEDS_RESET bit is not enough.

Also, the second qvirtqueue_add() call with corresponding comment are redundant.

v1: https://patchew.org/QEMU/20240523102813.396750-2-frolov@swemel.ru/
v2: modified error-check & clean-up

Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
---
 tests/qtest/fuzz/virtio_net_fuzz.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/qtest/fuzz/virtio_net_fuzz.c b/tests/qtest/fuzz/virtio_net_fuzz.c
index e239875e3b..f62d2b9478 100644
--- a/tests/qtest/fuzz/virtio_net_fuzz.c
+++ b/tests/qtest/fuzz/virtio_net_fuzz.c
@@ -65,22 +65,21 @@ static void virtio_net_fuzz_multi(QTestState *s,
         } else {
             vqa.rx = 0;
             uint64_t req_addr = guest_alloc(t_alloc, vqa.length);
-            /*
-             * If checking used ring, ensure that the fuzzer doesn't trigger
-             * trivial asserion failure on zero-zied buffer
-             */
             qtest_memwrite(s, req_addr, Data, vqa.length);
 
-
             free_head = qvirtqueue_add(s, q, req_addr, vqa.length,
                     vqa.write, vqa.next);
-            qvirtqueue_add(s, q, req_addr, vqa.length, vqa.write , vqa.next);
             qvirtqueue_kick(s, dev, q, free_head);
         }
 
         /* Run the main loop */
         qtest_clock_step(s, 100);
         flush_events(s);
+        /* Input led to a virtio_error */
+        if (dev->bus->get_status(dev) & VIRTIO_CONFIG_S_NEEDS_RESET ||
+          !(dev->bus->get_status(dev) & VIRTIO_CONFIG_S_DRIVER_OK)) {
+            return;
+        }
 
         /* Wait on used descriptors */
         if (check_used && !vqa.rx) {
@@ -92,10 +91,6 @@ static void virtio_net_fuzz_multi(QTestState *s,
              */
             while (!vqa.rx && q != net_if->queues[QVIRTIO_RX_VQ]) {
                 uint32_t got_desc_idx;
-                /* Input led to a virtio_error */
-                if (dev->bus->get_status(dev) & VIRTIO_CONFIG_S_NEEDS_RESET) {
-                    break;
-                }
                 if (dev->bus->get_queue_isr_status(dev, q) &&
                         qvirtqueue_get_buf(s, q, &got_desc_idx, NULL)) {
                     g_assert_cmpint(got_desc_idx, ==, free_head);
@@ -107,6 +102,11 @@ static void virtio_net_fuzz_multi(QTestState *s,
                 /* Run the main loop */
                 qtest_clock_step(s, 100);
                 flush_events(s);
+                /* Input led to a virtio_error */
+                if (dev->bus->get_status(dev) & VIRTIO_CONFIG_S_NEEDS_RESET ||
+                  !(dev->bus->get_status(dev) & VIRTIO_CONFIG_S_DRIVER_OK)) {
+                    return;
+                }
             }
         }
         Data += vqa.length;
-- 
2.43.0