[PATCH 00/10] Retire Fork-Based Fuzzing

[PATCH 00/10] Retire Fork-Based Fuzzing

[Alexander Bulekov]

Hello,
This series removes fork-based fuzzing.
How does fork-based fuzzing work?
 * A single parent process initializes QEMU
 * We identify the devices we wish to fuzz (fuzzer-dependent)
 * Use QTest to PCI enumerate the devices
 * After that we start a fork-server which forks the process and executes
   fuzzer inputs inside the disposable children.

In a normal fuzzing process, everything happens in a single process.

Pros of fork-based fuzzing:
 * We only need to do common configuration once (e.g. PCI enumeration).
 * Fork provides a strong guarantee that fuzzer inputs will not interfere with
   each-other
 * The fuzzing process can continue even after a child-process crashes
 * We can apply our-own timers to child-processes to exit slow inputs, early

Cons of fork-based fuzzing:
 * Fork-based fuzzing is not supported by libfuzzer. We had to build our own
   fork-server and rely on tricks using linker-scripts and shared-memory to
   support fuzzing. ( https://physics.bu.edu/~alxndr/libfuzzer-forkserver/ )
 * Fork-based fuzzing is currently the main blocker preventing us from enabling
   other fuzzers such as AFL++ on OSS-Fuzz
 * Fork-based fuzzing may be a reason why coverage-builds are failing on
   OSS-Fuzz. Coverage is an important fuzzing metric which would allow us to
   find parts of the code that are not well-covered.
 * Fork-based fuzzing has high overhead. fork() is an expensive system-call,
   especially for processes running ASAN (with large/complex) VMA layouts.
 * Fork prevents us from effectively fuzzing devices that rely on
   threads (e.g. qxl).

These patches remove fork-based fuzzing and replace it with reboot-based
fuzzing for most cases. Misc notes about this change:
 * libfuzzer appears to be no longer in active development. As such, the
   current implementation of fork-based fuzzing (while having some nice
   advantages) is likely to hold us back in the future. If these changes
   are approved and appear to run successfully on OSS-Fuzz, we should be
   able to easily experiment with other fuzzing engines (AFL++).
 * Some device do not completely reset their state. This can lead to
   non-reproducible crashes. However, in my local tests, most crashes
   were reproducible. OSS-Fuzz shouldn't send us reports unless it can
   consistently reproduce a crash.
 * In theory, the corpus-format should not change, so the existing
   corpus-inputs on OSS-Fuzz will transfer to the new reset()-able
   fuzzers.
 * Each fuzzing process will now exit after a single crash is found. To
   continue the fuzzing process, use libfuzzer flags such as -jobs=-1
 * We no long control input-timeouts (those are handled by libfuzzer).
   Since timeouts on oss-fuzz can be many seconds long, I added a limit
   on the number of DMA bytes written.
 

Alexander Bulekov (10):
  hw/sparse-mem: clear memory on reset
  fuzz: add fuzz_reboot API
  fuzz/generic-fuzz: use reboots instead of forks to reset state
  fuzz/generic-fuzz: add a limit on DMA bytes written
  fuzz/virtio-scsi: remove fork-based fuzzer
  fuzz/virtio-net: remove fork-based fuzzer
  fuzz/virtio-blk: remove fork-based fuzzer
  fuzz/i440fx: remove fork-based fuzzer
  fuzz: remove fork-fuzzing scaffolding
  docs/fuzz: remove mentions of fork-based fuzzing

 docs/devel/fuzzing.rst              |  22 +-----
 hw/mem/sparse-mem.c                 |  13 +++-
 meson.build                         |   4 -
 tests/qtest/fuzz/fork_fuzz.c        |  41 ----------
 tests/qtest/fuzz/fork_fuzz.h        |  23 ------
 tests/qtest/fuzz/fork_fuzz.ld       |  56 --------------
 tests/qtest/fuzz/fuzz.c             |   6 ++
 tests/qtest/fuzz/fuzz.h             |   2 +-
 tests/qtest/fuzz/generic_fuzz.c     | 111 +++++++---------------------
 tests/qtest/fuzz/i440fx_fuzz.c      |  27 +------
 tests/qtest/fuzz/meson.build        |   6 +-
 tests/qtest/fuzz/virtio_blk_fuzz.c  |  51 ++-----------
 tests/qtest/fuzz/virtio_net_fuzz.c  |  54 ++------------
 tests/qtest/fuzz/virtio_scsi_fuzz.c |  51 ++-----------
 14 files changed, 72 insertions(+), 395 deletions(-)
 delete mode 100644 tests/qtest/fuzz/fork_fuzz.c
 delete mode 100644 tests/qtest/fuzz/fork_fuzz.h
 delete mode 100644 tests/qtest/fuzz/fork_fuzz.ld

-- 
2.39.0

[PATCH 01/10] hw/sparse-mem: clear memory on reset

[Alexander Bulekov]

We use sparse-mem for fuzzing. For long-running fuzzing processes, we
eventually end up with many allocated sparse-mem pages. To avoid this,
clear the allocated pages on system-reset.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 hw/mem/sparse-mem.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/mem/sparse-mem.c b/hw/mem/sparse-mem.c
index e6640eb8e7..72f038d47d 100644
--- a/hw/mem/sparse-mem.c
+++ b/hw/mem/sparse-mem.c
@@ -77,6 +77,13 @@ static void sparse_mem_write(void *opaque, hwaddr addr, uint64_t v,
 
 }
 
+static void sparse_mem_enter_reset(Object *obj, ResetType type)
+{
+    SparseMemState *s = SPARSE_MEM(obj);
+    g_hash_table_remove_all(s->mapped);
+    return;
+}
+
 static const MemoryRegionOps sparse_mem_ops = {
     .read = sparse_mem_read,
     .write = sparse_mem_write,
@@ -123,7 +130,8 @@ static void sparse_mem_realize(DeviceState *dev, Error **errp)
 
     assert(s->baseaddr + s->length > s->baseaddr);
 
-    s->mapped = g_hash_table_new(NULL, NULL);
+    s->mapped = g_hash_table_new_full(NULL, NULL, NULL,
+                                      (GDestroyNotify)g_free);
     memory_region_init_io(&s->mmio, OBJECT(s), &sparse_mem_ops, s,
                           "sparse-mem", s->length);
     sysbus_init_mmio(sbd, &s->mmio);
@@ -131,12 +139,15 @@ static void sparse_mem_realize(DeviceState *dev, Error **errp)
 
 static void sparse_mem_class_init(ObjectClass *klass, void *data)
 {
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     device_class_set_props(dc, sparse_mem_properties);
 
     dc->desc = "Sparse Memory Device";
     dc->realize = sparse_mem_realize;
+
+    rc->phases.enter = sparse_mem_enter_reset;
 }
 
 static const TypeInfo sparse_mem_types[] = {
-- 
2.39.0

[PATCH 02/10] fuzz: add fuzz_reboot API

[Alexander Bulekov]

As we are converting most fuzzers to rely on reboots to reset state,
introduce an API to make sure reboots are invoked in a consistent
manner.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 tests/qtest/fuzz/fuzz.c | 6 ++++++
 tests/qtest/fuzz/fuzz.h | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
index eb7520544b..c2d07a4c7e 100644
--- a/tests/qtest/fuzz/fuzz.c
+++ b/tests/qtest/fuzz/fuzz.c
@@ -51,6 +51,12 @@ void flush_events(QTestState *s)
     }
 }
 
+void fuzz_reboot(QTestState *s)
+{
+    qemu_system_reset(SHUTDOWN_CAUSE_GUEST_RESET);
+    main_loop_wait(true);
+}
+
 static QTestState *qtest_setup(void)
 {
     qtest_server_set_send_handler(&qtest_client_inproc_recv, &fuzz_qts);
diff --git a/tests/qtest/fuzz/fuzz.h b/tests/qtest/fuzz/fuzz.h
index 327c1c5a55..69e2b3877f 100644
--- a/tests/qtest/fuzz/fuzz.h
+++ b/tests/qtest/fuzz/fuzz.h
@@ -103,7 +103,7 @@ typedef struct FuzzTarget {
 } FuzzTarget;
 
 void flush_events(QTestState *);
-void reboot(QTestState *);
+void fuzz_reboot(QTestState *);
 
 /* Use the QTest ASCII protocol or call address_space API directly?*/
 void fuzz_qtest_set_serialize(bool option);
-- 
2.39.0

[PATCH 03/10] fuzz/generic-fuzz: use reboots instead of forks to reset state

[Alexander Bulekov]

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 tests/qtest/fuzz/generic_fuzz.c | 106 +++++++-------------------------
 1 file changed, 23 insertions(+), 83 deletions(-)

diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
index 7326f6840b..c2e5642150 100644
--- a/tests/qtest/fuzz/generic_fuzz.c
+++ b/tests/qtest/fuzz/generic_fuzz.c
@@ -18,7 +18,6 @@
 #include "tests/qtest/libqtest.h"
 #include "tests/qtest/libqos/pci-pc.h"
 #include "fuzz.h"
-#include "fork_fuzz.h"
 #include "string.h"
 #include "exec/memory.h"
 #include "exec/ramblock.h"
@@ -29,6 +28,8 @@
 #include "generic_fuzz_configs.h"
 #include "hw/mem/sparse-mem.h"
 
+static void pci_enum(gpointer pcidev, gpointer bus);
+
 /*
  * SEPARATOR is used to separate "operations" in the fuzz input
  */
@@ -589,30 +590,6 @@ static void op_disable_pci(QTestState *s, const unsigned char *data, size_t len)
     pci_disabled = true;
 }
 
-static void handle_timeout(int sig)
-{
-    if (qtest_log_enabled) {
-        fprintf(stderr, "[Timeout]\n");
-        fflush(stderr);
-    }
-
-    /*
-     * If there is a crash, libfuzzer/ASAN forks a child to run an
-     * "llvm-symbolizer" process for printing out a pretty stacktrace. It
-     * communicates with this child using a pipe.  If we timeout+Exit, while
-     * libfuzzer is still communicating with the llvm-symbolizer child, we will
-     * be left with an orphan llvm-symbolizer process. Sometimes, this appears
-     * to lead to a deadlock in the forkserver. Use waitpid to check if there
-     * are any waitable children. If so, exit out of the signal-handler, and
-     * let libfuzzer finish communicating with the child, and exit, on its own.
-     */
-    if (waitpid(-1, NULL, WNOHANG) == 0) {
-        return;
-    }
-
-    _Exit(0);
-}
-
 /*
  * Here, we interpret random bytes from the fuzzer, as a sequence of commands.
  * Some commands can be variable-width, so we use a separator, SEPARATOR, to
@@ -669,64 +646,34 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
     size_t cmd_len;
     uint8_t op;
 
-    if (fork() == 0) {
-        struct sigaction sact;
-        struct itimerval timer;
-        sigset_t set;
-        /*
-         * Sometimes the fuzzer will find inputs that take quite a long time to
-         * process. Often times, these inputs do not result in new coverage.
-         * Even if these inputs might be interesting, they can slow down the
-         * fuzzer, overall. Set a timeout for each command to avoid hurting
-         * performance, too much
-         */
-        if (timeout) {
-
-            sigemptyset(&sact.sa_mask);
-            sact.sa_flags   = SA_NODEFER;
-            sact.sa_handler = handle_timeout;
-            sigaction(SIGALRM, &sact, NULL);
-
-            sigemptyset(&set);
-            sigaddset(&set, SIGALRM);
-            pthread_sigmask(SIG_UNBLOCK, &set, NULL);
-
-            memset(&timer, 0, sizeof(timer));
-            timer.it_value.tv_sec = timeout / USEC_IN_SEC;
-            timer.it_value.tv_usec = timeout % USEC_IN_SEC;
-        }
+    op_clear_dma_patterns(s, NULL, 0);
+    pci_disabled = false;
 
-        op_clear_dma_patterns(s, NULL, 0);
-        pci_disabled = false;
+    QPCIBus *pcibus = qpci_new_pc(s, NULL);
+    g_ptr_array_foreach(fuzzable_pci_devices, pci_enum, pcibus);
+    qpci_free_pc(pcibus);
 
-        while (cmd && Size) {
-            /* Reset the timeout, each time we run a new command */
-            if (timeout) {
-                setitimer(ITIMER_REAL, &timer, NULL);
-            }
+    while (cmd && Size) {
+        /* Reset the timeout, each time we run a new command */
 
-            /* Get the length until the next command or end of input */
-            nextcmd = memmem(cmd, Size, SEPARATOR, strlen(SEPARATOR));
-            cmd_len = nextcmd ? nextcmd - cmd : Size;
+        /* Get the length until the next command or end of input */
+        nextcmd = memmem(cmd, Size, SEPARATOR, strlen(SEPARATOR));
+        cmd_len = nextcmd ? nextcmd - cmd : Size;
 
-            if (cmd_len > 0) {
-                /* Interpret the first byte of the command as an opcode */
-                op = *cmd % (sizeof(ops) / sizeof((ops)[0]));
-                ops[op](s, cmd + 1, cmd_len - 1);
+        if (cmd_len > 0) {
+            /* Interpret the first byte of the command as an opcode */
+            op = *cmd % (sizeof(ops) / sizeof((ops)[0]));
+            ops[op](s, cmd + 1, cmd_len - 1);
 
-                /* Run the main loop */
-                flush_events(s);
-            }
-            /* Advance to the next command */
-            cmd = nextcmd ? nextcmd + sizeof(SEPARATOR) - 1 : nextcmd;
-            Size = Size - (cmd_len + sizeof(SEPARATOR) - 1);
-            g_array_set_size(dma_regions, 0);
+            /* Run the main loop */
+            flush_events(s);
         }
-        _Exit(0);
-    } else {
-        flush_events(s);
-        wait(0);
+        /* Advance to the next command */
+        cmd = nextcmd ? nextcmd + sizeof(SEPARATOR) - 1 : nextcmd;
+        Size = Size - (cmd_len + sizeof(SEPARATOR) - 1);
+        g_array_set_size(dma_regions, 0);
     }
+    fuzz_reboot(s);
 }
 
 static void usage(void)
@@ -825,7 +772,6 @@ static void generic_pre_fuzz(QTestState *s)
 {
     GHashTableIter iter;
     MemoryRegion *mr;
-    QPCIBus *pcibus;
     char **result;
     GString *name_pattern;
 
@@ -883,12 +829,6 @@ static void generic_pre_fuzz(QTestState *s)
         printf("No fuzzable memory regions found...\n");
         exit(1);
     }
-
-    pcibus = qpci_new_pc(s, NULL);
-    g_ptr_array_foreach(fuzzable_pci_devices, pci_enum, pcibus);
-    qpci_free_pc(pcibus);
-
-    counter_shm_init();
 }
 
 /*
-- 
2.39.0

[PATCH 04/10] fuzz/generic-fuzz: add a limit on DMA bytes written

[Alexander Bulekov]

As we have repplaced fork-based fuzzing, with reboots - we can no longer
use a timeout+exit() to avoid slow inputs. Libfuzzer has its own timer
that it uses to catch slow inputs, however these timeouts are usually
seconds-minutes long: more than enough to bog-down the fuzzing process.
However, I found that slow inputs often attempt to fill overly large DMA
requests. Thus, we can mitigate most timeouts by setting a cap on the
total number of DMA bytes written by an input.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 tests/qtest/fuzz/generic_fuzz.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
index c2e5642150..eab92cbc23 100644
--- a/tests/qtest/fuzz/generic_fuzz.c
+++ b/tests/qtest/fuzz/generic_fuzz.c
@@ -52,6 +52,7 @@ enum cmds {
 #define USEC_IN_SEC 1000000000
 
 #define MAX_DMA_FILL_SIZE 0x10000
+#define MAX_TOTAL_DMA_SIZE 0x10000000
 
 #define PCI_HOST_BRIDGE_CFG 0xcf8
 #define PCI_HOST_BRIDGE_DATA 0xcfc
@@ -64,6 +65,7 @@ typedef struct {
 static useconds_t timeout = DEFAULT_TIMEOUT_US;
 
 static bool qtest_log_enabled;
+size_t dma_bytes_written;
 
 MemoryRegion *sparse_mem_mr;
 
@@ -197,6 +199,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr)
      */
     if (dma_patterns->len == 0
         || len == 0
+        || dma_bytes_written > MAX_TOTAL_DMA_SIZE
         || (mr != current_machine->ram && mr != sparse_mem_mr)) {
         return;
     }
@@ -269,6 +272,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr)
                 fflush(stderr);
             }
             qtest_memwrite(qts_global, addr, buf, l);
+            dma_bytes_written += l;
         }
         len -= l;
         buf += l;
@@ -648,6 +652,7 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
 
     op_clear_dma_patterns(s, NULL, 0);
     pci_disabled = false;
+    dma_bytes_written = 0;
 
     QPCIBus *pcibus = qpci_new_pc(s, NULL);
     g_ptr_array_foreach(fuzzable_pci_devices, pci_enum, pcibus);
-- 
2.39.0

[PATCH 05/10] fuzz/virtio-scsi: remove fork-based fuzzer

[Alexander Bulekov]

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 tests/qtest/fuzz/virtio_scsi_fuzz.c | 51 ++++-------------------------
 1 file changed, 7 insertions(+), 44 deletions(-)

diff --git a/tests/qtest/fuzz/virtio_scsi_fuzz.c b/tests/qtest/fuzz/virtio_scsi_fuzz.c
index b3220ef6cb..8b26e951ae 100644
--- a/tests/qtest/fuzz/virtio_scsi_fuzz.c
+++ b/tests/qtest/fuzz/virtio_scsi_fuzz.c
@@ -20,7 +20,6 @@
 #include "standard-headers/linux/virtio_pci.h"
 #include "standard-headers/linux/virtio_scsi.h"
 #include "fuzz.h"
-#include "fork_fuzz.h"
 #include "qos_fuzz.h"
 
 #define PCI_SLOT                0x02
@@ -132,48 +131,24 @@ static void virtio_scsi_fuzz(QTestState *s, QVirtioSCSIQueues* queues,
     }
 }
 
-static void virtio_scsi_fork_fuzz(QTestState *s,
-        const unsigned char *Data, size_t Size)
-{
-    QVirtioSCSI *scsi = fuzz_qos_obj;
-    static QVirtioSCSIQueues *queues;
-    if (!queues) {
-        queues = qvirtio_scsi_init(scsi->vdev, 0);
-    }
-    if (fork() == 0) {
-        virtio_scsi_fuzz(s, queues, Data, Size);
-        flush_events(s);
-        _Exit(0);
-    } else {
-        flush_events(s);
-        wait(NULL);
-    }
-}
-
 static void virtio_scsi_with_flag_fuzz(QTestState *s,
         const unsigned char *Data, size_t Size)
 {
     QVirtioSCSI *scsi = fuzz_qos_obj;
     static QVirtioSCSIQueues *queues;
 
-    if (fork() == 0) {
-        if (Size >= sizeof(uint64_t)) {
-            queues = qvirtio_scsi_init(scsi->vdev, *(uint64_t *)Data);
-            virtio_scsi_fuzz(s, queues,
-                             Data + sizeof(uint64_t), Size - sizeof(uint64_t));
-            flush_events(s);
-        }
-        _Exit(0);
-    } else {
+    if (Size >= sizeof(uint64_t)) {
+        queues = qvirtio_scsi_init(scsi->vdev, *(uint64_t *)Data);
+        virtio_scsi_fuzz(s, queues,
+                Data + sizeof(uint64_t), Size - sizeof(uint64_t));
         flush_events(s);
-        wait(NULL);
     }
+    fuzz_reboot(s);
 }
 
 static void virtio_scsi_pre_fuzz(QTestState *s)
 {
     qos_init_path(s);
-    counter_shm_init();
 }
 
 static void *virtio_scsi_test_setup(GString *cmd_line, void *arg)
@@ -189,22 +164,10 @@ static void *virtio_scsi_test_setup(GString *cmd_line, void *arg)
 
 static void register_virtio_scsi_fuzz_targets(void)
 {
-    fuzz_add_qos_target(&(FuzzTarget){
-                .name = "virtio-scsi-fuzz",
-                .description = "Fuzz the virtio-scsi virtual queues, forking "
-                                "for each fuzz run",
-                .pre_vm_init = &counter_shm_init,
-                .pre_fuzz = &virtio_scsi_pre_fuzz,
-                .fuzz = virtio_scsi_fork_fuzz,},
-                "virtio-scsi",
-                &(QOSGraphTestOptions){.before = virtio_scsi_test_setup}
-                );
-
     fuzz_add_qos_target(&(FuzzTarget){
                 .name = "virtio-scsi-flags-fuzz",
-                .description = "Fuzz the virtio-scsi virtual queues, forking "
-                "for each fuzz run (also fuzzes the virtio flags)",
-                .pre_vm_init = &counter_shm_init,
+                .description = "Fuzz the virtio-scsi virtual queues. "
+                "Also fuzzes the virtio flags",
                 .pre_fuzz = &virtio_scsi_pre_fuzz,
                 .fuzz = virtio_scsi_with_flag_fuzz,},
                 "virtio-scsi",
-- 
2.39.0

[PATCH 06/10] fuzz/virtio-net: remove fork-based fuzzer

[Alexander Bulekov]

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 tests/qtest/fuzz/virtio_net_fuzz.c | 54 +++---------------------------
 1 file changed, 5 insertions(+), 49 deletions(-)

diff --git a/tests/qtest/fuzz/virtio_net_fuzz.c b/tests/qtest/fuzz/virtio_net_fuzz.c
index c2c15f07f0..d245ee66a1 100644
--- a/tests/qtest/fuzz/virtio_net_fuzz.c
+++ b/tests/qtest/fuzz/virtio_net_fuzz.c
@@ -16,7 +16,6 @@
 #include "tests/qtest/libqtest.h"
 #include "tests/qtest/libqos/virtio-net.h"
 #include "fuzz.h"
-#include "fork_fuzz.h"
 #include "qos_fuzz.h"
 
 
@@ -115,36 +114,18 @@ static void virtio_net_fuzz_multi(QTestState *s,
     }
 }
 
-static void virtio_net_fork_fuzz(QTestState *s,
-        const unsigned char *Data, size_t Size)
-{
-    if (fork() == 0) {
-        virtio_net_fuzz_multi(s, Data, Size, false);
-        flush_events(s);
-        _Exit(0);
-    } else {
-        flush_events(s);
-        wait(NULL);
-    }
-}
 
-static void virtio_net_fork_fuzz_check_used(QTestState *s,
+static void virtio_net_fuzz_check_used(QTestState *s,
         const unsigned char *Data, size_t Size)
 {
-    if (fork() == 0) {
-        virtio_net_fuzz_multi(s, Data, Size, true);
-        flush_events(s);
-        _Exit(0);
-    } else {
-        flush_events(s);
-        wait(NULL);
-    }
+    virtio_net_fuzz_multi(s, Data, Size, true);
+    flush_events(s);
+    fuzz_reboot(s);
 }
 
 static void virtio_net_pre_fuzz(QTestState *s)
 {
     qos_init_path(s);
-    counter_shm_init();
 }
 
 static void *virtio_net_test_setup_socket(GString *cmd_line, void *arg)
@@ -158,23 +139,8 @@ static void *virtio_net_test_setup_socket(GString *cmd_line, void *arg)
     return arg;
 }
 
-static void *virtio_net_test_setup_user(GString *cmd_line, void *arg)
-{
-    g_string_append_printf(cmd_line, " -netdev user,id=hs0 ");
-    return arg;
-}
-
 static void register_virtio_net_fuzz_targets(void)
 {
-    fuzz_add_qos_target(&(FuzzTarget){
-            .name = "virtio-net-socket",
-            .description = "Fuzz the virtio-net virtual queues. Fuzz incoming "
-            "traffic using the socket backend",
-            .pre_fuzz = &virtio_net_pre_fuzz,
-            .fuzz = virtio_net_fork_fuzz,},
-            "virtio-net",
-            &(QOSGraphTestOptions){.before = virtio_net_test_setup_socket}
-            );
 
     fuzz_add_qos_target(&(FuzzTarget){
             .name = "virtio-net-socket-check-used",
@@ -182,20 +148,10 @@ static void register_virtio_net_fuzz_targets(void)
             "descriptors to be used. Timeout may indicate improperly handled "
             "input",
             .pre_fuzz = &virtio_net_pre_fuzz,
-            .fuzz = virtio_net_fork_fuzz_check_used,},
+            .fuzz = virtio_net_fuzz_check_used,},
             "virtio-net",
             &(QOSGraphTestOptions){.before = virtio_net_test_setup_socket}
             );
-    fuzz_add_qos_target(&(FuzzTarget){
-            .name = "virtio-net-slirp",
-            .description = "Fuzz the virtio-net virtual queues with the slirp "
-            " backend. Warning: May result in network traffic emitted from the "
-            " process. Run in an isolated network environment.",
-            .pre_fuzz = &virtio_net_pre_fuzz,
-            .fuzz = virtio_net_fork_fuzz,},
-            "virtio-net",
-            &(QOSGraphTestOptions){.before = virtio_net_test_setup_user}
-            );
 }
 
 fuzz_target_init(register_virtio_net_fuzz_targets);
-- 
2.39.0

[PATCH 07/10] fuzz/virtio-blk: remove fork-based fuzzer

[Alexander Bulekov]

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 tests/qtest/fuzz/virtio_blk_fuzz.c | 51 ++++--------------------------
 1 file changed, 7 insertions(+), 44 deletions(-)

diff --git a/tests/qtest/fuzz/virtio_blk_fuzz.c b/tests/qtest/fuzz/virtio_blk_fuzz.c
index a9fb9ecf6c..82575a11d9 100644
--- a/tests/qtest/fuzz/virtio_blk_fuzz.c
+++ b/tests/qtest/fuzz/virtio_blk_fuzz.c
@@ -19,7 +19,6 @@
 #include "standard-headers/linux/virtio_pci.h"
 #include "standard-headers/linux/virtio_blk.h"
 #include "fuzz.h"
-#include "fork_fuzz.h"
 #include "qos_fuzz.h"
 
 #define TEST_IMAGE_SIZE         (64 * 1024 * 1024)
@@ -128,48 +127,24 @@ static void virtio_blk_fuzz(QTestState *s, QVirtioBlkQueues* queues,
     }
 }
 
-static void virtio_blk_fork_fuzz(QTestState *s,
-        const unsigned char *Data, size_t Size)
-{
-    QVirtioBlk *blk = fuzz_qos_obj;
-    static QVirtioBlkQueues *queues;
-    if (!queues) {
-        queues = qvirtio_blk_init(blk->vdev, 0);
-    }
-    if (fork() == 0) {
-        virtio_blk_fuzz(s, queues, Data, Size);
-        flush_events(s);
-        _Exit(0);
-    } else {
-        flush_events(s);
-        wait(NULL);
-    }
-}
-
 static void virtio_blk_with_flag_fuzz(QTestState *s,
         const unsigned char *Data, size_t Size)
 {
     QVirtioBlk *blk = fuzz_qos_obj;
     static QVirtioBlkQueues *queues;
 
-    if (fork() == 0) {
-        if (Size >= sizeof(uint64_t)) {
-            queues = qvirtio_blk_init(blk->vdev, *(uint64_t *)Data);
-            virtio_blk_fuzz(s, queues,
-                             Data + sizeof(uint64_t), Size - sizeof(uint64_t));
-            flush_events(s);
-        }
-        _Exit(0);
-    } else {
+    if (Size >= sizeof(uint64_t)) {
+        queues = qvirtio_blk_init(blk->vdev, *(uint64_t *)Data);
+        virtio_blk_fuzz(s, queues,
+                Data + sizeof(uint64_t), Size - sizeof(uint64_t));
         flush_events(s);
-        wait(NULL);
     }
+    fuzz_reboot(s);
 }
 
 static void virtio_blk_pre_fuzz(QTestState *s)
 {
     qos_init_path(s);
-    counter_shm_init();
 }
 
 static void drive_destroy(void *path)
@@ -208,22 +183,10 @@ static void *virtio_blk_test_setup(GString *cmd_line, void *arg)
 
 static void register_virtio_blk_fuzz_targets(void)
 {
-    fuzz_add_qos_target(&(FuzzTarget){
-                .name = "virtio-blk-fuzz",
-                .description = "Fuzz the virtio-blk virtual queues, forking "
-                                "for each fuzz run",
-                .pre_vm_init = &counter_shm_init,
-                .pre_fuzz = &virtio_blk_pre_fuzz,
-                .fuzz = virtio_blk_fork_fuzz,},
-                "virtio-blk",
-                &(QOSGraphTestOptions){.before = virtio_blk_test_setup}
-                );
-
     fuzz_add_qos_target(&(FuzzTarget){
                 .name = "virtio-blk-flags-fuzz",
-                .description = "Fuzz the virtio-blk virtual queues, forking "
-                "for each fuzz run (also fuzzes the virtio flags)",
-                .pre_vm_init = &counter_shm_init,
+                .description = "Fuzz the virtio-blk virtual queues. "
+                "Also fuzzes the virtio flags)",
                 .pre_fuzz = &virtio_blk_pre_fuzz,
                 .fuzz = virtio_blk_with_flag_fuzz,},
                 "virtio-blk",
-- 
2.39.0

[PATCH 08/10] fuzz/i440fx: remove fork-based fuzzer

[Alexander Bulekov]

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 tests/qtest/fuzz/i440fx_fuzz.c | 27 +--------------------------
 1 file changed, 1 insertion(+), 26 deletions(-)

diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c
index b17fc725df..5d6a703481 100644
--- a/tests/qtest/fuzz/i440fx_fuzz.c
+++ b/tests/qtest/fuzz/i440fx_fuzz.c
@@ -18,7 +18,6 @@
 #include "tests/qtest/libqos/pci-pc.h"
 #include "fuzz.h"
 #include "qos_fuzz.h"
-#include "fork_fuzz.h"
 
 
 #define I440FX_PCI_HOST_BRIDGE_CFG 0xcf8
@@ -89,6 +88,7 @@ static void i440fx_fuzz_qtest(QTestState *s,
                               size_t Size)
 {
     ioport_fuzz_qtest(s, Data, Size);
+    fuzz_reboot(s);
 }
 
 static void pciconfig_fuzz_qos(QTestState *s, QPCIBus *bus,
@@ -145,17 +145,6 @@ static void i440fx_fuzz_qos(QTestState *s,
     pciconfig_fuzz_qos(s, bus, Data, Size);
 }
 
-static void i440fx_fuzz_qos_fork(QTestState *s,
-        const unsigned char *Data, size_t Size) {
-    if (fork() == 0) {
-        i440fx_fuzz_qos(s, Data, Size);
-        _Exit(0);
-    } else {
-        flush_events(s);
-        wait(NULL);
-    }
-}
-
 static const char *i440fx_qtest_argv = TARGET_NAME " -machine accel=qtest"
                                        " -m 0 -display none";
 static GString *i440fx_argv(FuzzTarget *t)
@@ -163,10 +152,6 @@ static GString *i440fx_argv(FuzzTarget *t)
     return g_string_new(i440fx_qtest_argv);
 }
 
-static void fork_init(void)
-{
-    counter_shm_init();
-}
 
 static void register_pci_fuzz_targets(void)
 {
@@ -178,16 +163,6 @@ static void register_pci_fuzz_targets(void)
                 .get_init_cmdline = i440fx_argv,
                 .fuzz = i440fx_fuzz_qtest});
 
-    /* Uses libqos and forks to prevent state leakage */
-    fuzz_add_qos_target(&(FuzzTarget){
-                .name = "i440fx-qos-fork-fuzz",
-                .description = "Fuzz the i440fx using raw qtest commands and "
-                               "rebooting after each run",
-                .pre_vm_init = &fork_init,
-                .fuzz = i440fx_fuzz_qos_fork,},
-                "i440FX-pcihost",
-                &(QOSGraphTestOptions){}
-                );
 
     /*
      * Uses libqos. Doesn't do anything to reset state. Note that if we were to
-- 
2.39.0

[PATCH 09/10] fuzz: remove fork-fuzzing scaffolding

[Alexander Bulekov]

Fork-fuzzing provides a few pros, but our implementation prevents us
from using fuzzers other than libFuzzer, and may be causing issues such
as coverage-failure builds on OSS-Fuzz. It is not a great long-term
solution as it depends on internal implementation details of libFuzzer
(which is no longer in active development). Remove it in favor of other
methods of resetting state between inputs.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 meson.build                   |  4 ---
 tests/qtest/fuzz/fork_fuzz.c  | 41 -------------------------
 tests/qtest/fuzz/fork_fuzz.h  | 23 --------------
 tests/qtest/fuzz/fork_fuzz.ld | 56 -----------------------------------
 tests/qtest/fuzz/meson.build  |  6 ++--
 5 files changed, 3 insertions(+), 127 deletions(-)
 delete mode 100644 tests/qtest/fuzz/fork_fuzz.c
 delete mode 100644 tests/qtest/fuzz/fork_fuzz.h
 delete mode 100644 tests/qtest/fuzz/fork_fuzz.ld

diff --git a/meson.build b/meson.build
index 6d3b665629..8be27c2408 100644
--- a/meson.build
+++ b/meson.build
@@ -215,10 +215,6 @@ endif
 # Specify linker-script with add_project_link_arguments so that it is not placed
 # within a linker --start-group/--end-group pair
 if get_option('fuzzing')
-  add_project_link_arguments(['-Wl,-T,',
-                              (meson.current_source_dir() / 'tests/qtest/fuzz/fork_fuzz.ld')],
-                             native: false, language: all_languages)
-
   # Specify a filter to only instrument code that is directly related to
   # virtual-devices.
   configure_file(output: 'instrumentation-filter',
diff --git a/tests/qtest/fuzz/fork_fuzz.c b/tests/qtest/fuzz/fork_fuzz.c
deleted file mode 100644
index 6ffb2a7937..0000000000
--- a/tests/qtest/fuzz/fork_fuzz.c
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * Fork-based fuzzing helpers
- *
- * Copyright Red Hat Inc., 2019
- *
- * Authors:
- *  Alexander Bulekov   <alxndr@bu.edu>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-
-#include "qemu/osdep.h"
-#include "fork_fuzz.h"
-
-
-void counter_shm_init(void)
-{
-    /* Copy what's in the counter region to a temporary buffer.. */
-    void *copy = malloc(&__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
-    memcpy(copy,
-           &__FUZZ_COUNTERS_START,
-           &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
-
-    /* Map a shared region over the counter region */
-    if (mmap(&__FUZZ_COUNTERS_START,
-             &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START,
-             PROT_READ | PROT_WRITE, MAP_SHARED | MAP_FIXED | MAP_ANONYMOUS,
-             0, 0) == MAP_FAILED) {
-        perror("Error: ");
-        exit(1);
-    }
-
-    /* Copy the original data back to the counter-region */
-    memcpy(&__FUZZ_COUNTERS_START, copy,
-           &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
-    free(copy);
-}
-
-
diff --git a/tests/qtest/fuzz/fork_fuzz.h b/tests/qtest/fuzz/fork_fuzz.h
deleted file mode 100644
index 9ecb8b58ef..0000000000
--- a/tests/qtest/fuzz/fork_fuzz.h
+++ /dev/null
@@ -1,23 +0,0 @@
-/*
- * Fork-based fuzzing helpers
- *
- * Copyright Red Hat Inc., 2019
- *
- * Authors:
- *  Alexander Bulekov   <alxndr@bu.edu>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- *
- */
-
-#ifndef FORK_FUZZ_H
-#define FORK_FUZZ_H
-
-extern uint8_t __FUZZ_COUNTERS_START;
-extern uint8_t __FUZZ_COUNTERS_END;
-
-void counter_shm_init(void);
-
-#endif
-
diff --git a/tests/qtest/fuzz/fork_fuzz.ld b/tests/qtest/fuzz/fork_fuzz.ld
deleted file mode 100644
index cfb88b7fdb..0000000000
--- a/tests/qtest/fuzz/fork_fuzz.ld
+++ /dev/null
@@ -1,56 +0,0 @@
-/*
- * We adjust linker script modification to place all of the stuff that needs to
- * persist across fuzzing runs into a contiguous section of memory. Then, it is
- * easy to re-map the counter-related memory as shared.
- */
-
-SECTIONS
-{
-  .data.fuzz_start : ALIGN(4K)
-  {
-      __FUZZ_COUNTERS_START = .;
-      __start___sancov_cntrs = .;
-      *(_*sancov_cntrs);
-      __stop___sancov_cntrs = .;
-
-      /* Lowest stack counter */
-      *(__sancov_lowest_stack);
-  }
-}
-INSERT AFTER .data;
-
-SECTIONS
-{
-  .data.fuzz_ordered :
-  {
-      /*
-       * Coverage counters. They're not necessary for fuzzing, but are useful
-       * for analyzing the fuzzing performance
-       */
-      __start___llvm_prf_cnts = .;
-      *(*llvm_prf_cnts);
-      __stop___llvm_prf_cnts = .;
-
-      /* Internal Libfuzzer TracePC object which contains the ValueProfileMap */
-      FuzzerTracePC*(.bss*);
-      /*
-       * In case the above line fails, explicitly specify the (mangled) name of
-       * the object we care about
-       */
-       *(.bss._ZN6fuzzer3TPCE);
-  }
-}
-INSERT AFTER .data.fuzz_start;
-
-SECTIONS
-{
-  .data.fuzz_end : ALIGN(4K)
-  {
-      __FUZZ_COUNTERS_END = .;
-  }
-}
-/*
- * Don't overwrite the SECTIONS in the default linker script. Instead insert the
- * above into the default script
- */
-INSERT AFTER .data.fuzz_ordered;
diff --git a/tests/qtest/fuzz/meson.build b/tests/qtest/fuzz/meson.build
index 189901d4a2..4d10b47b8f 100644
--- a/tests/qtest/fuzz/meson.build
+++ b/tests/qtest/fuzz/meson.build
@@ -2,7 +2,7 @@ if not get_option('fuzzing')
   subdir_done()
 endif
 
-specific_fuzz_ss.add(files('fuzz.c', 'fork_fuzz.c', 'qos_fuzz.c',
+specific_fuzz_ss.add(files('fuzz.c', 'qos_fuzz.c',
                            'qtest_wrappers.c'), qos)
 
 # Targets
@@ -12,7 +12,7 @@ specific_fuzz_ss.add(when: 'CONFIG_VIRTIO_SCSI', if_true: files('virtio_scsi_fuz
 specific_fuzz_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio_blk_fuzz.c'))
 specific_fuzz_ss.add(files('generic_fuzz.c'))
 
-fork_fuzz = declare_dependency(
+fuzz_ld = declare_dependency(
   link_args: fuzz_exe_ldflags +
              ['-Wl,-wrap,qtest_inb',
               '-Wl,-wrap,qtest_inw',
@@ -35,4 +35,4 @@ fork_fuzz = declare_dependency(
               '-Wl,-wrap,qtest_memset']
 )
 
-specific_fuzz_ss.add(fork_fuzz)
+specific_fuzz_ss.add(fuzz_ld)
-- 
2.39.0

[PATCH 10/10] docs/fuzz: remove mentions of fork-based fuzzing

[Alexander Bulekov]

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 docs/devel/fuzzing.rst | 22 ++--------------------
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/docs/devel/fuzzing.rst b/docs/devel/fuzzing.rst
index 715330c856..3bfcb33fc4 100644
--- a/docs/devel/fuzzing.rst
+++ b/docs/devel/fuzzing.rst
@@ -19,11 +19,6 @@ responsibility to ensure that state is reset between fuzzing-runs.
 Building the fuzzers
 --------------------
 
-*NOTE*: If possible, build a 32-bit binary. When forking, the 32-bit fuzzer is
-much faster, since the page-map has a smaller size. This is due to the fact that
-AddressSanitizer maps ~20TB of memory, as part of its detection. This results
-in a large page-map, and a much slower ``fork()``.
-
 To build the fuzzers, install a recent version of clang:
 Configure with (substitute the clang binaries with the version you installed).
 Here, enable-sanitizers, is optional but it allows us to reliably detect bugs
@@ -296,10 +291,9 @@ input. It is also responsible for manually calling ``main_loop_wait`` to ensure
 that bottom halves are executed and any cleanup required before the next input.
 
 Since the same process is reused for many fuzzing runs, QEMU state needs to
-be reset at the end of each run. There are currently two implemented
-options for resetting state:
+be reset at the end of each run. For example, this can be done by rebooting the
+VM, after each run.
 
-- Reboot the guest between runs.
   - *Pros*: Straightforward and fast for simple fuzz targets.
 
   - *Cons*: Depending on the device, does not reset all device state. If the
@@ -308,15 +302,3 @@ options for resetting state:
     reboot.
 
   - *Example target*: ``i440fx-qtest-reboot-fuzz``
-
-- Run each test case in a separate forked process and copy the coverage
-   information back to the parent. This is fairly similar to AFL's "deferred"
-   fork-server mode [3]
-
-  - *Pros*: Relatively fast. Devices only need to be initialized once. No need to
-    do slow reboots or vmloads.
-
-  - *Cons*: Not officially supported by libfuzzer. Does not work well for
-     devices that rely on dedicated threads.
-
-  - *Example target*: ``virtio-net-fork-fuzz``
-- 
2.39.0

Re: [PATCH 00/10] Retire Fork-Based Fuzzing

[Philippe Mathieu-Daudé]

On 5/2/23 05:29, Alexander Bulekov wrote:

>   * Some device do not completely reset their state. This can lead to
>     non-reproducible crashes. However, in my local tests, most crashes
>     were reproducible. OSS-Fuzz shouldn't send us reports unless it can
>     consistently reproduce a crash.

These devices are buggy, hard/cold reset should be reproducible.

>   * In theory, the corpus-format should not change, so the existing
>     corpus-inputs on OSS-Fuzz will transfer to the new reset()-able
>     fuzzers.

Re: [PATCH 01/10] hw/sparse-mem: clear memory on reset

[Philippe Mathieu-Daudé]

On 5/2/23 05:29, Alexander Bulekov wrote:
> We use sparse-mem for fuzzing. For long-running fuzzing processes, we
> eventually end up with many allocated sparse-mem pages. To avoid this,
> clear the allocated pages on system-reset.
> 
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>   hw/mem/sparse-mem.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 04/10] fuzz/generic-fuzz: add a limit on DMA bytes written

[Philippe Mathieu-Daudé]

On 5/2/23 05:29, Alexander Bulekov wrote:
> As we have repplaced fork-based fuzzing, with reboots - we can no longer

Typo "replaced".

> use a timeout+exit() to avoid slow inputs. Libfuzzer has its own timer
> that it uses to catch slow inputs, however these timeouts are usually
> seconds-minutes long: more than enough to bog-down the fuzzing process.
> However, I found that slow inputs often attempt to fill overly large DMA
> requests. Thus, we can mitigate most timeouts by setting a cap on the
> total number of DMA bytes written by an input.
> 
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>   tests/qtest/fuzz/generic_fuzz.c | 5 +++++
>   1 file changed, 5 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 02/10] fuzz: add fuzz_reboot API

[Philippe Mathieu-Daudé]

On 5/2/23 05:29, Alexander Bulekov wrote:
> As we are converting most fuzzers to rely on reboots to reset state,
> introduce an API to make sure reboots are invoked in a consistent
> manner.
> 
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>   tests/qtest/fuzz/fuzz.c | 6 ++++++
>   tests/qtest/fuzz/fuzz.h | 2 +-
>   2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
> index eb7520544b..c2d07a4c7e 100644
> --- a/tests/qtest/fuzz/fuzz.c
> +++ b/tests/qtest/fuzz/fuzz.c
> @@ -51,6 +51,12 @@ void flush_events(QTestState *s)
>       }
>   }
>   
> +void fuzz_reboot(QTestState *s)

"reboot" sounds like guest software triggered.
IIUC from the fuzzer PoV this is more a "power-cycle" right?

> +{
> +    qemu_system_reset(SHUTDOWN_CAUSE_GUEST_RESET);

Is SHUTDOWN_CAUSE_HOST_QMP_SYSTEM_RESET more appropriate?

> +    main_loop_wait(true);
> +}

Re: [PATCH 00/10] Retire Fork-Based Fuzzing

[Alexander Bulekov]

On 230205 1139, Philippe Mathieu-Daudé wrote:
> On 5/2/23 05:29, Alexander Bulekov wrote:
> 
> >   * Some device do not completely reset their state. This can lead to
> >     non-reproducible crashes. However, in my local tests, most crashes
> >     were reproducible. OSS-Fuzz shouldn't send us reports unless it can
> >     consistently reproduce a crash.
> 
> These devices are buggy, hard/cold reset should be reproducible.

Agreed. However I don't think the fuzzer is tailored to report these
types of bugs. OSS-Fuzz will just see that some crashes/inputs are not
reproducible. I have been thinking about ways to make the fuzzer report
incomplete VMStateDescriptions. Maybe something similar can be done for
reboots.
-Alex

> 
> >   * In theory, the corpus-format should not change, so the existing
> >     corpus-inputs on OSS-Fuzz will transfer to the new reset()-able
> >     fuzzers.
> 

Re: [PATCH 00/10] Retire Fork-Based Fuzzing

[Alexander Bulekov]

ping

On 230204 2329, Alexander Bulekov wrote:
> Hello,
> This series removes fork-based fuzzing.
> How does fork-based fuzzing work?
>  * A single parent process initializes QEMU
>  * We identify the devices we wish to fuzz (fuzzer-dependent)
>  * Use QTest to PCI enumerate the devices
>  * After that we start a fork-server which forks the process and executes
>    fuzzer inputs inside the disposable children.
> 
> In a normal fuzzing process, everything happens in a single process.
> 
> Pros of fork-based fuzzing:
>  * We only need to do common configuration once (e.g. PCI enumeration).
>  * Fork provides a strong guarantee that fuzzer inputs will not interfere with
>    each-other
>  * The fuzzing process can continue even after a child-process crashes
>  * We can apply our-own timers to child-processes to exit slow inputs, early
> 
> Cons of fork-based fuzzing:
>  * Fork-based fuzzing is not supported by libfuzzer. We had to build our own
>    fork-server and rely on tricks using linker-scripts and shared-memory to
>    support fuzzing. ( https://physics.bu.edu/~alxndr/libfuzzer-forkserver/ )
>  * Fork-based fuzzing is currently the main blocker preventing us from enabling
>    other fuzzers such as AFL++ on OSS-Fuzz
>  * Fork-based fuzzing may be a reason why coverage-builds are failing on
>    OSS-Fuzz. Coverage is an important fuzzing metric which would allow us to
>    find parts of the code that are not well-covered.
>  * Fork-based fuzzing has high overhead. fork() is an expensive system-call,
>    especially for processes running ASAN (with large/complex) VMA layouts.
>  * Fork prevents us from effectively fuzzing devices that rely on
>    threads (e.g. qxl).
> 
> These patches remove fork-based fuzzing and replace it with reboot-based
> fuzzing for most cases. Misc notes about this change:
>  * libfuzzer appears to be no longer in active development. As such, the
>    current implementation of fork-based fuzzing (while having some nice
>    advantages) is likely to hold us back in the future. If these changes
>    are approved and appear to run successfully on OSS-Fuzz, we should be
>    able to easily experiment with other fuzzing engines (AFL++).
>  * Some device do not completely reset their state. This can lead to
>    non-reproducible crashes. However, in my local tests, most crashes
>    were reproducible. OSS-Fuzz shouldn't send us reports unless it can
>    consistently reproduce a crash.
>  * In theory, the corpus-format should not change, so the existing
>    corpus-inputs on OSS-Fuzz will transfer to the new reset()-able
>    fuzzers.
>  * Each fuzzing process will now exit after a single crash is found. To
>    continue the fuzzing process, use libfuzzer flags such as -jobs=-1
>  * We no long control input-timeouts (those are handled by libfuzzer).
>    Since timeouts on oss-fuzz can be many seconds long, I added a limit
>    on the number of DMA bytes written.
>  
> 
> Alexander Bulekov (10):
>   hw/sparse-mem: clear memory on reset
>   fuzz: add fuzz_reboot API
>   fuzz/generic-fuzz: use reboots instead of forks to reset state
>   fuzz/generic-fuzz: add a limit on DMA bytes written
>   fuzz/virtio-scsi: remove fork-based fuzzer
>   fuzz/virtio-net: remove fork-based fuzzer
>   fuzz/virtio-blk: remove fork-based fuzzer
>   fuzz/i440fx: remove fork-based fuzzer
>   fuzz: remove fork-fuzzing scaffolding
>   docs/fuzz: remove mentions of fork-based fuzzing
> 
>  docs/devel/fuzzing.rst              |  22 +-----
>  hw/mem/sparse-mem.c                 |  13 +++-
>  meson.build                         |   4 -
>  tests/qtest/fuzz/fork_fuzz.c        |  41 ----------
>  tests/qtest/fuzz/fork_fuzz.h        |  23 ------
>  tests/qtest/fuzz/fork_fuzz.ld       |  56 --------------
>  tests/qtest/fuzz/fuzz.c             |   6 ++
>  tests/qtest/fuzz/fuzz.h             |   2 +-
>  tests/qtest/fuzz/generic_fuzz.c     | 111 +++++++---------------------
>  tests/qtest/fuzz/i440fx_fuzz.c      |  27 +------
>  tests/qtest/fuzz/meson.build        |   6 +-
>  tests/qtest/fuzz/virtio_blk_fuzz.c  |  51 ++-----------
>  tests/qtest/fuzz/virtio_net_fuzz.c  |  54 ++------------
>  tests/qtest/fuzz/virtio_scsi_fuzz.c |  51 ++-----------
>  14 files changed, 72 insertions(+), 395 deletions(-)
>  delete mode 100644 tests/qtest/fuzz/fork_fuzz.c
>  delete mode 100644 tests/qtest/fuzz/fork_fuzz.h
>  delete mode 100644 tests/qtest/fuzz/fork_fuzz.ld
> 
> -- 
> 2.39.0
> 

Re: [PATCH 01/10] hw/sparse-mem: clear memory on reset

[Darren Kenny]

On Sunday, 2023-02-05 at 11:40:55 +01, Philippe Mathieu-Daudé wrote:
> On 5/2/23 05:29, Alexander Bulekov wrote:
>> We use sparse-mem for fuzzing. For long-running fuzzing processes, we
>> eventually end up with many allocated sparse-mem pages. To avoid this,
>> clear the allocated pages on system-reset.
>> 
>> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
>> ---
>>   hw/mem/sparse-mem.c | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.

Re: [PATCH 02/10] fuzz: add fuzz_reboot API

[Darren Kenny]

On Sunday, 2023-02-05 at 11:50:52 +01, Philippe Mathieu-Daudé wrote:
> On 5/2/23 05:29, Alexander Bulekov wrote:
>> As we are converting most fuzzers to rely on reboots to reset state,
>> introduce an API to make sure reboots are invoked in a consistent
>> manner.
>> 
>> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
>> ---
>>   tests/qtest/fuzz/fuzz.c | 6 ++++++
>>   tests/qtest/fuzz/fuzz.h | 2 +-
>>   2 files changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
>> index eb7520544b..c2d07a4c7e 100644
>> --- a/tests/qtest/fuzz/fuzz.c
>> +++ b/tests/qtest/fuzz/fuzz.c
>> @@ -51,6 +51,12 @@ void flush_events(QTestState *s)
>>       }
>>   }
>>   
>> +void fuzz_reboot(QTestState *s)
>
> "reboot" sounds like guest software triggered.
> IIUC from the fuzzer PoV this is more a "power-cycle" right?

I think that 'fuzz_reset()' or 'fuzz_reset_state()' would make sense,
the primary purpose is to reset the fuzzing back to a known state, as
said in the commit message.

While right now it is a reboot, it may not always be, or could require
something more.

Thanks,

Darren.

Re: [PATCH 03/10] fuzz/generic-fuzz: use reboots instead of forks to reset state

[Darren Kenny]

Hi Alex,

On Saturday, 2023-02-04 at 23:29:44 -05, Alexander Bulekov wrote:
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  tests/qtest/fuzz/generic_fuzz.c | 106 +++++++-------------------------
>  1 file changed, 23 insertions(+), 83 deletions(-)
>
> diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
> index 7326f6840b..c2e5642150 100644
> --- a/tests/qtest/fuzz/generic_fuzz.c
> +++ b/tests/qtest/fuzz/generic_fuzz.c
> @@ -18,7 +18,6 @@
>  #include "tests/qtest/libqtest.h"
>  #include "tests/qtest/libqos/pci-pc.h"
>  #include "fuzz.h"
> -#include "fork_fuzz.h"
>  #include "string.h"
>  #include "exec/memory.h"
>  #include "exec/ramblock.h"
> @@ -29,6 +28,8 @@
>  #include "generic_fuzz_configs.h"
>  #include "hw/mem/sparse-mem.h"
>  
> +static void pci_enum(gpointer pcidev, gpointer bus);
> +
>  /*
>   * SEPARATOR is used to separate "operations" in the fuzz input
>   */
> @@ -589,30 +590,6 @@ static void op_disable_pci(QTestState *s, const unsigned char *data, size_t len)
>      pci_disabled = true;
>  }
>  
> -static void handle_timeout(int sig)
> -{
> -    if (qtest_log_enabled) {
> -        fprintf(stderr, "[Timeout]\n");
> -        fflush(stderr);
> -    }
> -
> -    /*
> -     * If there is a crash, libfuzzer/ASAN forks a child to run an
> -     * "llvm-symbolizer" process for printing out a pretty stacktrace. It
> -     * communicates with this child using a pipe.  If we timeout+Exit, while
> -     * libfuzzer is still communicating with the llvm-symbolizer child, we will
> -     * be left with an orphan llvm-symbolizer process. Sometimes, this appears
> -     * to lead to a deadlock in the forkserver. Use waitpid to check if there
> -     * are any waitable children. If so, exit out of the signal-handler, and
> -     * let libfuzzer finish communicating with the child, and exit, on its own.
> -     */
> -    if (waitpid(-1, NULL, WNOHANG) == 0) {
> -        return;
> -    }
> -
> -    _Exit(0);
> -}
> -
>  /*
>

I'm presuming that the timeout is being left to the fuzz orchestrator
now, rather than us managing it directly in our own way?

>   * Here, we interpret random bytes from the fuzzer, as a sequence of commands.
>   * Some commands can be variable-width, so we use a separator, SEPARATOR, to
> @@ -669,64 +646,34 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
>      size_t cmd_len;
>      uint8_t op;
>  
> -    if (fork() == 0) {
> -        struct sigaction sact;
> -        struct itimerval timer;
> -        sigset_t set;
> -        /*
> -         * Sometimes the fuzzer will find inputs that take quite a long time to
> -         * process. Often times, these inputs do not result in new coverage.
> -         * Even if these inputs might be interesting, they can slow down the
> -         * fuzzer, overall. Set a timeout for each command to avoid hurting
> -         * performance, too much
> -         */
> -        if (timeout) {
> -
> -            sigemptyset(&sact.sa_mask);
> -            sact.sa_flags   = SA_NODEFER;
> -            sact.sa_handler = handle_timeout;
> -            sigaction(SIGALRM, &sact, NULL);
> -
> -            sigemptyset(&set);
> -            sigaddset(&set, SIGALRM);
> -            pthread_sigmask(SIG_UNBLOCK, &set, NULL);
> -
> -            memset(&timer, 0, sizeof(timer));
> -            timer.it_value.tv_sec = timeout / USEC_IN_SEC;
> -            timer.it_value.tv_usec = timeout % USEC_IN_SEC;
> -        }
> +    op_clear_dma_patterns(s, NULL, 0);
> +    pci_disabled = false;
>  
> -        op_clear_dma_patterns(s, NULL, 0);
> -        pci_disabled = false;
> +    QPCIBus *pcibus = qpci_new_pc(s, NULL);
> +    g_ptr_array_foreach(fuzzable_pci_devices, pci_enum, pcibus);
> +    qpci_free_pc(pcibus);
>  
> -        while (cmd && Size) {
> -            /* Reset the timeout, each time we run a new command */
> -            if (timeout) {
> -                setitimer(ITIMER_REAL, &timer, NULL);
> -            }
> +    while (cmd && Size) {
> +        /* Reset the timeout, each time we run a new command */
>  
> -            /* Get the length until the next command or end of input */
> -            nextcmd = memmem(cmd, Size, SEPARATOR, strlen(SEPARATOR));
> -            cmd_len = nextcmd ? nextcmd - cmd : Size;
> +        /* Get the length until the next command or end of input */
> +        nextcmd = memmem(cmd, Size, SEPARATOR, strlen(SEPARATOR));
> +        cmd_len = nextcmd ? nextcmd - cmd : Size;
>  
> -            if (cmd_len > 0) {
> -                /* Interpret the first byte of the command as an opcode */
> -                op = *cmd % (sizeof(ops) / sizeof((ops)[0]));
> -                ops[op](s, cmd + 1, cmd_len - 1);
> +        if (cmd_len > 0) {
> +            /* Interpret the first byte of the command as an opcode */
> +            op = *cmd % (sizeof(ops) / sizeof((ops)[0]));
> +            ops[op](s, cmd + 1, cmd_len - 1);
>  
> -                /* Run the main loop */
> -                flush_events(s);
> -            }
> -            /* Advance to the next command */
> -            cmd = nextcmd ? nextcmd + sizeof(SEPARATOR) - 1 : nextcmd;
> -            Size = Size - (cmd_len + sizeof(SEPARATOR) - 1);
> -            g_array_set_size(dma_regions, 0);
> +            /* Run the main loop */
> +            flush_events(s);
>          }
> -        _Exit(0);
> -    } else {
> -        flush_events(s);
> -        wait(0);
> +        /* Advance to the next command */
> +        cmd = nextcmd ? nextcmd + sizeof(SEPARATOR) - 1 : nextcmd;
> +        Size = Size - (cmd_len + sizeof(SEPARATOR) - 1);
> +        g_array_set_size(dma_regions, 0);
>      }
> +    fuzz_reboot(s);
>

Guess this should be changed too if the declared function is too.

These are only nits, so:

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.

Re: [PATCH 04/10] fuzz/generic-fuzz: add a limit on DMA bytes written

[Darren Kenny]

Hi Alex,

On Saturday, 2023-02-04 at 23:29:45 -05, Alexander Bulekov wrote:
> As we have repplaced fork-based fuzzing, with reboots - we can no longer
> use a timeout+exit() to avoid slow inputs. Libfuzzer has its own timer
> that it uses to catch slow inputs, however these timeouts are usually
> seconds-minutes long: more than enough to bog-down the fuzzing process.
> However, I found that slow inputs often attempt to fill overly large DMA
> requests. Thus, we can mitigate most timeouts by setting a cap on the
> total number of DMA bytes written by an input.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  tests/qtest/fuzz/generic_fuzz.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
> index c2e5642150..eab92cbc23 100644
> --- a/tests/qtest/fuzz/generic_fuzz.c
> +++ b/tests/qtest/fuzz/generic_fuzz.c
> @@ -52,6 +52,7 @@ enum cmds {
>  #define USEC_IN_SEC 1000000000
>  
>  #define MAX_DMA_FILL_SIZE 0x10000
> +#define MAX_TOTAL_DMA_SIZE 0x10000000
>  
>  #define PCI_HOST_BRIDGE_CFG 0xcf8
>  #define PCI_HOST_BRIDGE_DATA 0xcfc
> @@ -64,6 +65,7 @@ typedef struct {
>  static useconds_t timeout = DEFAULT_TIMEOUT_US;
>  
>  static bool qtest_log_enabled;
> +size_t dma_bytes_written;
>  
>  MemoryRegion *sparse_mem_mr;
>  
> @@ -197,6 +199,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr)
>       */
>      if (dma_patterns->len == 0
>          || len == 0
> +        || dma_bytes_written > MAX_TOTAL_DMA_SIZE

NIT: Just wondering if you should check dma_bytes_written + l as opposed
     to dma_bytes_written? It's probably not important enough given it's
     just an artificial limit, but thought I'd ask.

>          || (mr != current_machine->ram && mr != sparse_mem_mr)) {
>          return;
>      }
> @@ -269,6 +272,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr)
>                  fflush(stderr);
>              }
>              qtest_memwrite(qts_global, addr, buf, l);
> +            dma_bytes_written += l;
>          }
>          len -= l;
>          buf += l;
> @@ -648,6 +652,7 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
>  
>      op_clear_dma_patterns(s, NULL, 0);
>      pci_disabled = false;
> +    dma_bytes_written = 0;
>  
>      QPCIBus *pcibus = qpci_new_pc(s, NULL);
>      g_ptr_array_foreach(fuzzable_pci_devices, pci_enum, pcibus);
> -- 
> 2.39.0

While this will still consume the existing corpus, is it likely to
cause these existing corpus to be trimmed?

Otherwise, the changes look good:

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.

Re: [PATCH 05/10] fuzz/virtio-scsi: remove fork-based fuzzer

[Darren Kenny]

On Saturday, 2023-02-04 at 23:29:46 -05, Alexander Bulekov wrote:
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.

> ---
>  tests/qtest/fuzz/virtio_scsi_fuzz.c | 51 ++++-------------------------
>  1 file changed, 7 insertions(+), 44 deletions(-)
>
> diff --git a/tests/qtest/fuzz/virtio_scsi_fuzz.c b/tests/qtest/fuzz/virtio_scsi_fuzz.c
> index b3220ef6cb..8b26e951ae 100644
> --- a/tests/qtest/fuzz/virtio_scsi_fuzz.c
> +++ b/tests/qtest/fuzz/virtio_scsi_fuzz.c
> @@ -20,7 +20,6 @@
>  #include "standard-headers/linux/virtio_pci.h"
>  #include "standard-headers/linux/virtio_scsi.h"
>  #include "fuzz.h"
> -#include "fork_fuzz.h"
>  #include "qos_fuzz.h"
>  
>  #define PCI_SLOT                0x02
> @@ -132,48 +131,24 @@ static void virtio_scsi_fuzz(QTestState *s, QVirtioSCSIQueues* queues,
>      }
>  }
>  
> -static void virtio_scsi_fork_fuzz(QTestState *s,
> -        const unsigned char *Data, size_t Size)
> -{
> -    QVirtioSCSI *scsi = fuzz_qos_obj;
> -    static QVirtioSCSIQueues *queues;
> -    if (!queues) {
> -        queues = qvirtio_scsi_init(scsi->vdev, 0);
> -    }
> -    if (fork() == 0) {
> -        virtio_scsi_fuzz(s, queues, Data, Size);
> -        flush_events(s);
> -        _Exit(0);
> -    } else {
> -        flush_events(s);
> -        wait(NULL);
> -    }
> -}
> -
>  static void virtio_scsi_with_flag_fuzz(QTestState *s,
>          const unsigned char *Data, size_t Size)
>  {
>      QVirtioSCSI *scsi = fuzz_qos_obj;
>      static QVirtioSCSIQueues *queues;
>  
> -    if (fork() == 0) {
> -        if (Size >= sizeof(uint64_t)) {
> -            queues = qvirtio_scsi_init(scsi->vdev, *(uint64_t *)Data);
> -            virtio_scsi_fuzz(s, queues,
> -                             Data + sizeof(uint64_t), Size - sizeof(uint64_t));
> -            flush_events(s);
> -        }
> -        _Exit(0);
> -    } else {
> +    if (Size >= sizeof(uint64_t)) {
> +        queues = qvirtio_scsi_init(scsi->vdev, *(uint64_t *)Data);
> +        virtio_scsi_fuzz(s, queues,
> +                Data + sizeof(uint64_t), Size - sizeof(uint64_t));
>          flush_events(s);
> -        wait(NULL);
>      }
> +    fuzz_reboot(s);
>  }
>  
>  static void virtio_scsi_pre_fuzz(QTestState *s)
>  {
>      qos_init_path(s);
> -    counter_shm_init();
>  }
>  
>  static void *virtio_scsi_test_setup(GString *cmd_line, void *arg)
> @@ -189,22 +164,10 @@ static void *virtio_scsi_test_setup(GString *cmd_line, void *arg)
>  
>  static void register_virtio_scsi_fuzz_targets(void)
>  {
> -    fuzz_add_qos_target(&(FuzzTarget){
> -                .name = "virtio-scsi-fuzz",
> -                .description = "Fuzz the virtio-scsi virtual queues, forking "
> -                                "for each fuzz run",
> -                .pre_vm_init = &counter_shm_init,
> -                .pre_fuzz = &virtio_scsi_pre_fuzz,
> -                .fuzz = virtio_scsi_fork_fuzz,},
> -                "virtio-scsi",
> -                &(QOSGraphTestOptions){.before = virtio_scsi_test_setup}
> -                );
> -
>      fuzz_add_qos_target(&(FuzzTarget){
>                  .name = "virtio-scsi-flags-fuzz",
> -                .description = "Fuzz the virtio-scsi virtual queues, forking "
> -                "for each fuzz run (also fuzzes the virtio flags)",
> -                .pre_vm_init = &counter_shm_init,
> +                .description = "Fuzz the virtio-scsi virtual queues. "
> +                "Also fuzzes the virtio flags",
>                  .pre_fuzz = &virtio_scsi_pre_fuzz,
>                  .fuzz = virtio_scsi_with_flag_fuzz,},
>                  "virtio-scsi",
> -- 
> 2.39.0

Re: [PATCH 06/10] fuzz/virtio-net: remove fork-based fuzzer

[Darren Kenny]

On Saturday, 2023-02-04 at 23:29:47 -05, Alexander Bulekov wrote:
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.

> ---
>  tests/qtest/fuzz/virtio_net_fuzz.c | 54 +++---------------------------
>  1 file changed, 5 insertions(+), 49 deletions(-)
>
> diff --git a/tests/qtest/fuzz/virtio_net_fuzz.c b/tests/qtest/fuzz/virtio_net_fuzz.c
> index c2c15f07f0..d245ee66a1 100644
> --- a/tests/qtest/fuzz/virtio_net_fuzz.c
> +++ b/tests/qtest/fuzz/virtio_net_fuzz.c
> @@ -16,7 +16,6 @@
>  #include "tests/qtest/libqtest.h"
>  #include "tests/qtest/libqos/virtio-net.h"
>  #include "fuzz.h"
> -#include "fork_fuzz.h"
>  #include "qos_fuzz.h"
>  
>  
> @@ -115,36 +114,18 @@ static void virtio_net_fuzz_multi(QTestState *s,
>      }
>  }
>  
> -static void virtio_net_fork_fuzz(QTestState *s,
> -        const unsigned char *Data, size_t Size)
> -{
> -    if (fork() == 0) {
> -        virtio_net_fuzz_multi(s, Data, Size, false);
> -        flush_events(s);
> -        _Exit(0);
> -    } else {
> -        flush_events(s);
> -        wait(NULL);
> -    }
> -}
>  
> -static void virtio_net_fork_fuzz_check_used(QTestState *s,
> +static void virtio_net_fuzz_check_used(QTestState *s,
>          const unsigned char *Data, size_t Size)
>  {
> -    if (fork() == 0) {
> -        virtio_net_fuzz_multi(s, Data, Size, true);
> -        flush_events(s);
> -        _Exit(0);
> -    } else {
> -        flush_events(s);
> -        wait(NULL);
> -    }
> +    virtio_net_fuzz_multi(s, Data, Size, true);
> +    flush_events(s);
> +    fuzz_reboot(s);
>  }
>  
>  static void virtio_net_pre_fuzz(QTestState *s)
>  {
>      qos_init_path(s);
> -    counter_shm_init();
>  }
>  
>  static void *virtio_net_test_setup_socket(GString *cmd_line, void *arg)
> @@ -158,23 +139,8 @@ static void *virtio_net_test_setup_socket(GString *cmd_line, void *arg)
>      return arg;
>  }
>  
> -static void *virtio_net_test_setup_user(GString *cmd_line, void *arg)
> -{
> -    g_string_append_printf(cmd_line, " -netdev user,id=hs0 ");
> -    return arg;
> -}
> -
>  static void register_virtio_net_fuzz_targets(void)
>  {
> -    fuzz_add_qos_target(&(FuzzTarget){
> -            .name = "virtio-net-socket",
> -            .description = "Fuzz the virtio-net virtual queues. Fuzz incoming "
> -            "traffic using the socket backend",
> -            .pre_fuzz = &virtio_net_pre_fuzz,
> -            .fuzz = virtio_net_fork_fuzz,},
> -            "virtio-net",
> -            &(QOSGraphTestOptions){.before = virtio_net_test_setup_socket}
> -            );
>  
>      fuzz_add_qos_target(&(FuzzTarget){
>              .name = "virtio-net-socket-check-used",
> @@ -182,20 +148,10 @@ static void register_virtio_net_fuzz_targets(void)
>              "descriptors to be used. Timeout may indicate improperly handled "
>              "input",
>              .pre_fuzz = &virtio_net_pre_fuzz,
> -            .fuzz = virtio_net_fork_fuzz_check_used,},
> +            .fuzz = virtio_net_fuzz_check_used,},
>              "virtio-net",
>              &(QOSGraphTestOptions){.before = virtio_net_test_setup_socket}
>              );
> -    fuzz_add_qos_target(&(FuzzTarget){
> -            .name = "virtio-net-slirp",
> -            .description = "Fuzz the virtio-net virtual queues with the slirp "
> -            " backend. Warning: May result in network traffic emitted from the "
> -            " process. Run in an isolated network environment.",
> -            .pre_fuzz = &virtio_net_pre_fuzz,
> -            .fuzz = virtio_net_fork_fuzz,},
> -            "virtio-net",
> -            &(QOSGraphTestOptions){.before = virtio_net_test_setup_user}
> -            );
>  }
>  
>  fuzz_target_init(register_virtio_net_fuzz_targets);
> -- 
> 2.39.0

Re: [PATCH 07/10] fuzz/virtio-blk: remove fork-based fuzzer

[Darren Kenny]

On Saturday, 2023-02-04 at 23:29:48 -05, Alexander Bulekov wrote:
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.

> ---
>  tests/qtest/fuzz/virtio_blk_fuzz.c | 51 ++++--------------------------
>  1 file changed, 7 insertions(+), 44 deletions(-)
>
> diff --git a/tests/qtest/fuzz/virtio_blk_fuzz.c b/tests/qtest/fuzz/virtio_blk_fuzz.c
> index a9fb9ecf6c..82575a11d9 100644
> --- a/tests/qtest/fuzz/virtio_blk_fuzz.c
> +++ b/tests/qtest/fuzz/virtio_blk_fuzz.c
> @@ -19,7 +19,6 @@
>  #include "standard-headers/linux/virtio_pci.h"
>  #include "standard-headers/linux/virtio_blk.h"
>  #include "fuzz.h"
> -#include "fork_fuzz.h"
>  #include "qos_fuzz.h"
>  
>  #define TEST_IMAGE_SIZE         (64 * 1024 * 1024)
> @@ -128,48 +127,24 @@ static void virtio_blk_fuzz(QTestState *s, QVirtioBlkQueues* queues,
>      }
>  }
>  
> -static void virtio_blk_fork_fuzz(QTestState *s,
> -        const unsigned char *Data, size_t Size)
> -{
> -    QVirtioBlk *blk = fuzz_qos_obj;
> -    static QVirtioBlkQueues *queues;
> -    if (!queues) {
> -        queues = qvirtio_blk_init(blk->vdev, 0);
> -    }
> -    if (fork() == 0) {
> -        virtio_blk_fuzz(s, queues, Data, Size);
> -        flush_events(s);
> -        _Exit(0);
> -    } else {
> -        flush_events(s);
> -        wait(NULL);
> -    }
> -}
> -
>  static void virtio_blk_with_flag_fuzz(QTestState *s,
>          const unsigned char *Data, size_t Size)
>  {
>      QVirtioBlk *blk = fuzz_qos_obj;
>      static QVirtioBlkQueues *queues;
>  
> -    if (fork() == 0) {
> -        if (Size >= sizeof(uint64_t)) {
> -            queues = qvirtio_blk_init(blk->vdev, *(uint64_t *)Data);
> -            virtio_blk_fuzz(s, queues,
> -                             Data + sizeof(uint64_t), Size - sizeof(uint64_t));
> -            flush_events(s);
> -        }
> -        _Exit(0);
> -    } else {
> +    if (Size >= sizeof(uint64_t)) {
> +        queues = qvirtio_blk_init(blk->vdev, *(uint64_t *)Data);
> +        virtio_blk_fuzz(s, queues,
> +                Data + sizeof(uint64_t), Size - sizeof(uint64_t));
>          flush_events(s);
> -        wait(NULL);
>      }
> +    fuzz_reboot(s);
>  }
>  
>  static void virtio_blk_pre_fuzz(QTestState *s)
>  {
>      qos_init_path(s);
> -    counter_shm_init();
>  }
>  
>  static void drive_destroy(void *path)
> @@ -208,22 +183,10 @@ static void *virtio_blk_test_setup(GString *cmd_line, void *arg)
>  
>  static void register_virtio_blk_fuzz_targets(void)
>  {
> -    fuzz_add_qos_target(&(FuzzTarget){
> -                .name = "virtio-blk-fuzz",
> -                .description = "Fuzz the virtio-blk virtual queues, forking "
> -                                "for each fuzz run",
> -                .pre_vm_init = &counter_shm_init,
> -                .pre_fuzz = &virtio_blk_pre_fuzz,
> -                .fuzz = virtio_blk_fork_fuzz,},
> -                "virtio-blk",
> -                &(QOSGraphTestOptions){.before = virtio_blk_test_setup}
> -                );
> -
>      fuzz_add_qos_target(&(FuzzTarget){
>                  .name = "virtio-blk-flags-fuzz",
> -                .description = "Fuzz the virtio-blk virtual queues, forking "
> -                "for each fuzz run (also fuzzes the virtio flags)",
> -                .pre_vm_init = &counter_shm_init,
> +                .description = "Fuzz the virtio-blk virtual queues. "
> +                "Also fuzzes the virtio flags)",
>                  .pre_fuzz = &virtio_blk_pre_fuzz,
>                  .fuzz = virtio_blk_with_flag_fuzz,},
>                  "virtio-blk",
> -- 
> 2.39.0

Re: [PATCH 08/10] fuzz/i440fx: remove fork-based fuzzer

[Darren Kenny]

On Saturday, 2023-02-04 at 23:29:49 -05, Alexander Bulekov wrote:
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.

> ---
>  tests/qtest/fuzz/i440fx_fuzz.c | 27 +--------------------------
>  1 file changed, 1 insertion(+), 26 deletions(-)
>
> diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c
> index b17fc725df..5d6a703481 100644
> --- a/tests/qtest/fuzz/i440fx_fuzz.c
> +++ b/tests/qtest/fuzz/i440fx_fuzz.c
> @@ -18,7 +18,6 @@
>  #include "tests/qtest/libqos/pci-pc.h"
>  #include "fuzz.h"
>  #include "qos_fuzz.h"
> -#include "fork_fuzz.h"
>  
>  
>  #define I440FX_PCI_HOST_BRIDGE_CFG 0xcf8
> @@ -89,6 +88,7 @@ static void i440fx_fuzz_qtest(QTestState *s,
>                                size_t Size)
>  {
>      ioport_fuzz_qtest(s, Data, Size);
> +    fuzz_reboot(s);
>  }
>  
>  static void pciconfig_fuzz_qos(QTestState *s, QPCIBus *bus,
> @@ -145,17 +145,6 @@ static void i440fx_fuzz_qos(QTestState *s,
>      pciconfig_fuzz_qos(s, bus, Data, Size);
>  }
>  
> -static void i440fx_fuzz_qos_fork(QTestState *s,
> -        const unsigned char *Data, size_t Size) {
> -    if (fork() == 0) {
> -        i440fx_fuzz_qos(s, Data, Size);
> -        _Exit(0);
> -    } else {
> -        flush_events(s);
> -        wait(NULL);
> -    }
> -}
> -
>  static const char *i440fx_qtest_argv = TARGET_NAME " -machine accel=qtest"
>                                         " -m 0 -display none";
>  static GString *i440fx_argv(FuzzTarget *t)
> @@ -163,10 +152,6 @@ static GString *i440fx_argv(FuzzTarget *t)
>      return g_string_new(i440fx_qtest_argv);
>  }
>  
> -static void fork_init(void)
> -{
> -    counter_shm_init();
> -}
>  
>  static void register_pci_fuzz_targets(void)
>  {
> @@ -178,16 +163,6 @@ static void register_pci_fuzz_targets(void)
>                  .get_init_cmdline = i440fx_argv,
>                  .fuzz = i440fx_fuzz_qtest});
>  
> -    /* Uses libqos and forks to prevent state leakage */
> -    fuzz_add_qos_target(&(FuzzTarget){
> -                .name = "i440fx-qos-fork-fuzz",
> -                .description = "Fuzz the i440fx using raw qtest commands and "
> -                               "rebooting after each run",
> -                .pre_vm_init = &fork_init,
> -                .fuzz = i440fx_fuzz_qos_fork,},
> -                "i440FX-pcihost",
> -                &(QOSGraphTestOptions){}
> -                );
>  
>      /*
>       * Uses libqos. Doesn't do anything to reset state. Note that if we were to
> -- 
> 2.39.0

Re: [PATCH 09/10] fuzz: remove fork-fuzzing scaffolding

[Darren Kenny]

On Saturday, 2023-02-04 at 23:29:50 -05, Alexander Bulekov wrote:
> Fork-fuzzing provides a few pros, but our implementation prevents us
> from using fuzzers other than libFuzzer, and may be causing issues such
> as coverage-failure builds on OSS-Fuzz. It is not a great long-term
> solution as it depends on internal implementation details of libFuzzer
> (which is no longer in active development). Remove it in favor of other
> methods of resetting state between inputs.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.

> ---
>  meson.build                   |  4 ---
>  tests/qtest/fuzz/fork_fuzz.c  | 41 -------------------------
>  tests/qtest/fuzz/fork_fuzz.h  | 23 --------------
>  tests/qtest/fuzz/fork_fuzz.ld | 56 -----------------------------------
>  tests/qtest/fuzz/meson.build  |  6 ++--
>  5 files changed, 3 insertions(+), 127 deletions(-)
>  delete mode 100644 tests/qtest/fuzz/fork_fuzz.c
>  delete mode 100644 tests/qtest/fuzz/fork_fuzz.h
>  delete mode 100644 tests/qtest/fuzz/fork_fuzz.ld
>
> diff --git a/meson.build b/meson.build
> index 6d3b665629..8be27c2408 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -215,10 +215,6 @@ endif
>  # Specify linker-script with add_project_link_arguments so that it is not placed
>  # within a linker --start-group/--end-group pair
>  if get_option('fuzzing')
> -  add_project_link_arguments(['-Wl,-T,',
> -                              (meson.current_source_dir() / 'tests/qtest/fuzz/fork_fuzz.ld')],
> -                             native: false, language: all_languages)
> -
>    # Specify a filter to only instrument code that is directly related to
>    # virtual-devices.
>    configure_file(output: 'instrumentation-filter',
> diff --git a/tests/qtest/fuzz/fork_fuzz.c b/tests/qtest/fuzz/fork_fuzz.c
> deleted file mode 100644
> index 6ffb2a7937..0000000000
> --- a/tests/qtest/fuzz/fork_fuzz.c
> +++ /dev/null
> @@ -1,41 +0,0 @@
> -/*
> - * Fork-based fuzzing helpers
> - *
> - * Copyright Red Hat Inc., 2019
> - *
> - * Authors:
> - *  Alexander Bulekov   <alxndr@bu.edu>
> - *
> - * This work is licensed under the terms of the GNU GPL, version 2 or later.
> - * See the COPYING file in the top-level directory.
> - *
> - */
> -
> -#include "qemu/osdep.h"
> -#include "fork_fuzz.h"
> -
> -
> -void counter_shm_init(void)
> -{
> -    /* Copy what's in the counter region to a temporary buffer.. */
> -    void *copy = malloc(&__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
> -    memcpy(copy,
> -           &__FUZZ_COUNTERS_START,
> -           &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
> -
> -    /* Map a shared region over the counter region */
> -    if (mmap(&__FUZZ_COUNTERS_START,
> -             &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START,
> -             PROT_READ | PROT_WRITE, MAP_SHARED | MAP_FIXED | MAP_ANONYMOUS,
> -             0, 0) == MAP_FAILED) {
> -        perror("Error: ");
> -        exit(1);
> -    }
> -
> -    /* Copy the original data back to the counter-region */
> -    memcpy(&__FUZZ_COUNTERS_START, copy,
> -           &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
> -    free(copy);
> -}
> -
> -
> diff --git a/tests/qtest/fuzz/fork_fuzz.h b/tests/qtest/fuzz/fork_fuzz.h
> deleted file mode 100644
> index 9ecb8b58ef..0000000000
> --- a/tests/qtest/fuzz/fork_fuzz.h
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/*
> - * Fork-based fuzzing helpers
> - *
> - * Copyright Red Hat Inc., 2019
> - *
> - * Authors:
> - *  Alexander Bulekov   <alxndr@bu.edu>
> - *
> - * This work is licensed under the terms of the GNU GPL, version 2 or later.
> - * See the COPYING file in the top-level directory.
> - *
> - */
> -
> -#ifndef FORK_FUZZ_H
> -#define FORK_FUZZ_H
> -
> -extern uint8_t __FUZZ_COUNTERS_START;
> -extern uint8_t __FUZZ_COUNTERS_END;
> -
> -void counter_shm_init(void);
> -
> -#endif
> -
> diff --git a/tests/qtest/fuzz/fork_fuzz.ld b/tests/qtest/fuzz/fork_fuzz.ld
> deleted file mode 100644
> index cfb88b7fdb..0000000000
> --- a/tests/qtest/fuzz/fork_fuzz.ld
> +++ /dev/null
> @@ -1,56 +0,0 @@
> -/*
> - * We adjust linker script modification to place all of the stuff that needs to
> - * persist across fuzzing runs into a contiguous section of memory. Then, it is
> - * easy to re-map the counter-related memory as shared.
> - */
> -
> -SECTIONS
> -{
> -  .data.fuzz_start : ALIGN(4K)
> -  {
> -      __FUZZ_COUNTERS_START = .;
> -      __start___sancov_cntrs = .;
> -      *(_*sancov_cntrs);
> -      __stop___sancov_cntrs = .;
> -
> -      /* Lowest stack counter */
> -      *(__sancov_lowest_stack);
> -  }
> -}
> -INSERT AFTER .data;
> -
> -SECTIONS
> -{
> -  .data.fuzz_ordered :
> -  {
> -      /*
> -       * Coverage counters. They're not necessary for fuzzing, but are useful
> -       * for analyzing the fuzzing performance
> -       */
> -      __start___llvm_prf_cnts = .;
> -      *(*llvm_prf_cnts);
> -      __stop___llvm_prf_cnts = .;
> -
> -      /* Internal Libfuzzer TracePC object which contains the ValueProfileMap */
> -      FuzzerTracePC*(.bss*);
> -      /*
> -       * In case the above line fails, explicitly specify the (mangled) name of
> -       * the object we care about
> -       */
> -       *(.bss._ZN6fuzzer3TPCE);
> -  }
> -}
> -INSERT AFTER .data.fuzz_start;
> -
> -SECTIONS
> -{
> -  .data.fuzz_end : ALIGN(4K)
> -  {
> -      __FUZZ_COUNTERS_END = .;
> -  }
> -}
> -/*
> - * Don't overwrite the SECTIONS in the default linker script. Instead insert the
> - * above into the default script
> - */
> -INSERT AFTER .data.fuzz_ordered;
> diff --git a/tests/qtest/fuzz/meson.build b/tests/qtest/fuzz/meson.build
> index 189901d4a2..4d10b47b8f 100644
> --- a/tests/qtest/fuzz/meson.build
> +++ b/tests/qtest/fuzz/meson.build
> @@ -2,7 +2,7 @@ if not get_option('fuzzing')
>    subdir_done()
>  endif
>  
> -specific_fuzz_ss.add(files('fuzz.c', 'fork_fuzz.c', 'qos_fuzz.c',
> +specific_fuzz_ss.add(files('fuzz.c', 'qos_fuzz.c',
>                             'qtest_wrappers.c'), qos)
>  
>  # Targets
> @@ -12,7 +12,7 @@ specific_fuzz_ss.add(when: 'CONFIG_VIRTIO_SCSI', if_true: files('virtio_scsi_fuz
>  specific_fuzz_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio_blk_fuzz.c'))
>  specific_fuzz_ss.add(files('generic_fuzz.c'))
>  
> -fork_fuzz = declare_dependency(
> +fuzz_ld = declare_dependency(
>    link_args: fuzz_exe_ldflags +
>               ['-Wl,-wrap,qtest_inb',
>                '-Wl,-wrap,qtest_inw',
> @@ -35,4 +35,4 @@ fork_fuzz = declare_dependency(
>                '-Wl,-wrap,qtest_memset']
>  )
>  
> -specific_fuzz_ss.add(fork_fuzz)
> +specific_fuzz_ss.add(fuzz_ld)
> -- 
> 2.39.0

Re: [PATCH 10/10] docs/fuzz: remove mentions of fork-based fuzzing

[Darren Kenny]

On Saturday, 2023-02-04 at 23:29:51 -05, Alexander Bulekov wrote:
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

Reviewed-by: Darren Kenny <darren.kenny@oracle.com>

Thanks,

Darren.

> ---
>  docs/devel/fuzzing.rst | 22 ++--------------------
>  1 file changed, 2 insertions(+), 20 deletions(-)
>
> diff --git a/docs/devel/fuzzing.rst b/docs/devel/fuzzing.rst
> index 715330c856..3bfcb33fc4 100644
> --- a/docs/devel/fuzzing.rst
> +++ b/docs/devel/fuzzing.rst
> @@ -19,11 +19,6 @@ responsibility to ensure that state is reset between fuzzing-runs.
>  Building the fuzzers
>  --------------------
>  
> -*NOTE*: If possible, build a 32-bit binary. When forking, the 32-bit fuzzer is
> -much faster, since the page-map has a smaller size. This is due to the fact that
> -AddressSanitizer maps ~20TB of memory, as part of its detection. This results
> -in a large page-map, and a much slower ``fork()``.
> -
>  To build the fuzzers, install a recent version of clang:
>  Configure with (substitute the clang binaries with the version you installed).
>  Here, enable-sanitizers, is optional but it allows us to reliably detect bugs
> @@ -296,10 +291,9 @@ input. It is also responsible for manually calling ``main_loop_wait`` to ensure
>  that bottom halves are executed and any cleanup required before the next input.
>  
>  Since the same process is reused for many fuzzing runs, QEMU state needs to
> -be reset at the end of each run. There are currently two implemented
> -options for resetting state:
> +be reset at the end of each run. For example, this can be done by rebooting the
> +VM, after each run.
>  
> -- Reboot the guest between runs.
>    - *Pros*: Straightforward and fast for simple fuzz targets.
>  
>    - *Cons*: Depending on the device, does not reset all device state. If the
> @@ -308,15 +302,3 @@ options for resetting state:
>      reboot.
>  
>    - *Example target*: ``i440fx-qtest-reboot-fuzz``
> -
> -- Run each test case in a separate forked process and copy the coverage
> -   information back to the parent. This is fairly similar to AFL's "deferred"
> -   fork-server mode [3]
> -
> -  - *Pros*: Relatively fast. Devices only need to be initialized once. No need to
> -    do slow reboots or vmloads.
> -
> -  - *Cons*: Not officially supported by libfuzzer. Does not work well for
> -     devices that rely on dedicated threads.
> -
> -  - *Example target*: ``virtio-net-fork-fuzz``
> -- 
> 2.39.0

Re: [PATCH 00/10] Retire Fork-Based Fuzzing

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 4660 bytes --]

On Sat, Feb 04, 2023 at 11:29:41PM -0500, Alexander Bulekov wrote:
> Hello,
> This series removes fork-based fuzzing.
> How does fork-based fuzzing work?
>  * A single parent process initializes QEMU
>  * We identify the devices we wish to fuzz (fuzzer-dependent)
>  * Use QTest to PCI enumerate the devices
>  * After that we start a fork-server which forks the process and executes
>    fuzzer inputs inside the disposable children.
> 
> In a normal fuzzing process, everything happens in a single process.
> 
> Pros of fork-based fuzzing:
>  * We only need to do common configuration once (e.g. PCI enumeration).
>  * Fork provides a strong guarantee that fuzzer inputs will not interfere with
>    each-other
>  * The fuzzing process can continue even after a child-process crashes
>  * We can apply our-own timers to child-processes to exit slow inputs, early
> 
> Cons of fork-based fuzzing:
>  * Fork-based fuzzing is not supported by libfuzzer. We had to build our own
>    fork-server and rely on tricks using linker-scripts and shared-memory to
>    support fuzzing. ( https://physics.bu.edu/~alxndr/libfuzzer-forkserver/ )
>  * Fork-based fuzzing is currently the main blocker preventing us from enabling
>    other fuzzers such as AFL++ on OSS-Fuzz
>  * Fork-based fuzzing may be a reason why coverage-builds are failing on
>    OSS-Fuzz. Coverage is an important fuzzing metric which would allow us to
>    find parts of the code that are not well-covered.
>  * Fork-based fuzzing has high overhead. fork() is an expensive system-call,
>    especially for processes running ASAN (with large/complex) VMA layouts.
>  * Fork prevents us from effectively fuzzing devices that rely on
>    threads (e.g. qxl).
> 
> These patches remove fork-based fuzzing and replace it with reboot-based
> fuzzing for most cases. Misc notes about this change:
>  * libfuzzer appears to be no longer in active development. As such, the
>    current implementation of fork-based fuzzing (while having some nice
>    advantages) is likely to hold us back in the future. If these changes
>    are approved and appear to run successfully on OSS-Fuzz, we should be
>    able to easily experiment with other fuzzing engines (AFL++).
>  * Some device do not completely reset their state. This can lead to
>    non-reproducible crashes. However, in my local tests, most crashes
>    were reproducible. OSS-Fuzz shouldn't send us reports unless it can
>    consistently reproduce a crash.
>  * In theory, the corpus-format should not change, so the existing
>    corpus-inputs on OSS-Fuzz will transfer to the new reset()-able
>    fuzzers.
>  * Each fuzzing process will now exit after a single crash is found. To
>    continue the fuzzing process, use libfuzzer flags such as -jobs=-1
>  * We no long control input-timeouts (those are handled by libfuzzer).
>    Since timeouts on oss-fuzz can be many seconds long, I added a limit
>    on the number of DMA bytes written.
>  
> 
> Alexander Bulekov (10):
>   hw/sparse-mem: clear memory on reset
>   fuzz: add fuzz_reboot API
>   fuzz/generic-fuzz: use reboots instead of forks to reset state
>   fuzz/generic-fuzz: add a limit on DMA bytes written
>   fuzz/virtio-scsi: remove fork-based fuzzer
>   fuzz/virtio-net: remove fork-based fuzzer
>   fuzz/virtio-blk: remove fork-based fuzzer
>   fuzz/i440fx: remove fork-based fuzzer
>   fuzz: remove fork-fuzzing scaffolding
>   docs/fuzz: remove mentions of fork-based fuzzing
> 
>  docs/devel/fuzzing.rst              |  22 +-----
>  hw/mem/sparse-mem.c                 |  13 +++-
>  meson.build                         |   4 -
>  tests/qtest/fuzz/fork_fuzz.c        |  41 ----------
>  tests/qtest/fuzz/fork_fuzz.h        |  23 ------
>  tests/qtest/fuzz/fork_fuzz.ld       |  56 --------------
>  tests/qtest/fuzz/fuzz.c             |   6 ++
>  tests/qtest/fuzz/fuzz.h             |   2 +-
>  tests/qtest/fuzz/generic_fuzz.c     | 111 +++++++---------------------
>  tests/qtest/fuzz/i440fx_fuzz.c      |  27 +------
>  tests/qtest/fuzz/meson.build        |   6 +-
>  tests/qtest/fuzz/virtio_blk_fuzz.c  |  51 ++-----------
>  tests/qtest/fuzz/virtio_net_fuzz.c  |  54 ++------------
>  tests/qtest/fuzz/virtio_scsi_fuzz.c |  51 ++-----------
>  14 files changed, 72 insertions(+), 395 deletions(-)
>  delete mode 100644 tests/qtest/fuzz/fork_fuzz.c
>  delete mode 100644 tests/qtest/fuzz/fork_fuzz.h
>  delete mode 100644 tests/qtest/fuzz/fork_fuzz.ld
> 
> -- 
> 2.39.0
> 

Whose tree should this go through? Laurent's qtest tree?

Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 00/10] Retire Fork-Based Fuzzing

[Philippe Mathieu-Daudé]

On 14/2/23 16:38, Stefan Hajnoczi wrote:
> On Sat, Feb 04, 2023 at 11:29:41PM -0500, Alexander Bulekov wrote:
>> Hello,
>> This series removes fork-based fuzzing.
>> How does fork-based fuzzing work?
>>   * A single parent process initializes QEMU
>>   * We identify the devices we wish to fuzz (fuzzer-dependent)
>>   * Use QTest to PCI enumerate the devices
>>   * After that we start a fork-server which forks the process and executes
>>     fuzzer inputs inside the disposable children.
>>
>> In a normal fuzzing process, everything happens in a single process.
>>
>> Pros of fork-based fuzzing:
>>   * We only need to do common configuration once (e.g. PCI enumeration).
>>   * Fork provides a strong guarantee that fuzzer inputs will not interfere with
>>     each-other
>>   * The fuzzing process can continue even after a child-process crashes
>>   * We can apply our-own timers to child-processes to exit slow inputs, early
>>
>> Cons of fork-based fuzzing:
>>   * Fork-based fuzzing is not supported by libfuzzer. We had to build our own
>>     fork-server and rely on tricks using linker-scripts and shared-memory to
>>     support fuzzing. ( https://physics.bu.edu/~alxndr/libfuzzer-forkserver/ )
>>   * Fork-based fuzzing is currently the main blocker preventing us from enabling
>>     other fuzzers such as AFL++ on OSS-Fuzz
>>   * Fork-based fuzzing may be a reason why coverage-builds are failing on
>>     OSS-Fuzz. Coverage is an important fuzzing metric which would allow us to
>>     find parts of the code that are not well-covered.
>>   * Fork-based fuzzing has high overhead. fork() is an expensive system-call,
>>     especially for processes running ASAN (with large/complex) VMA layouts.
>>   * Fork prevents us from effectively fuzzing devices that rely on
>>     threads (e.g. qxl).
>>
>> These patches remove fork-based fuzzing and replace it with reboot-based
>> fuzzing for most cases. Misc notes about this change:
>>   * libfuzzer appears to be no longer in active development. As such, the
>>     current implementation of fork-based fuzzing (while having some nice
>>     advantages) is likely to hold us back in the future. If these changes
>>     are approved and appear to run successfully on OSS-Fuzz, we should be
>>     able to easily experiment with other fuzzing engines (AFL++).
>>   * Some device do not completely reset their state. This can lead to
>>     non-reproducible crashes. However, in my local tests, most crashes
>>     were reproducible. OSS-Fuzz shouldn't send us reports unless it can
>>     consistently reproduce a crash.
>>   * In theory, the corpus-format should not change, so the existing
>>     corpus-inputs on OSS-Fuzz will transfer to the new reset()-able
>>     fuzzers.
>>   * Each fuzzing process will now exit after a single crash is found. To
>>     continue the fuzzing process, use libfuzzer flags such as -jobs=-1
>>   * We no long control input-timeouts (those are handled by libfuzzer).
>>     Since timeouts on oss-fuzz can be many seconds long, I added a limit
>>     on the number of DMA bytes written.
>>   
>>
>> Alexander Bulekov (10):
>>    hw/sparse-mem: clear memory on reset
>>    fuzz: add fuzz_reboot API
>>    fuzz/generic-fuzz: use reboots instead of forks to reset state
>>    fuzz/generic-fuzz: add a limit on DMA bytes written
>>    fuzz/virtio-scsi: remove fork-based fuzzer
>>    fuzz/virtio-net: remove fork-based fuzzer
>>    fuzz/virtio-blk: remove fork-based fuzzer
>>    fuzz/i440fx: remove fork-based fuzzer
>>    fuzz: remove fork-fuzzing scaffolding
>>    docs/fuzz: remove mentions of fork-based fuzzing
>>
>>   docs/devel/fuzzing.rst              |  22 +-----
>>   hw/mem/sparse-mem.c                 |  13 +++-
>>   meson.build                         |   4 -
>>   tests/qtest/fuzz/fork_fuzz.c        |  41 ----------
>>   tests/qtest/fuzz/fork_fuzz.h        |  23 ------
>>   tests/qtest/fuzz/fork_fuzz.ld       |  56 --------------
>>   tests/qtest/fuzz/fuzz.c             |   6 ++
>>   tests/qtest/fuzz/fuzz.h             |   2 +-
>>   tests/qtest/fuzz/generic_fuzz.c     | 111 +++++++---------------------
>>   tests/qtest/fuzz/i440fx_fuzz.c      |  27 +------
>>   tests/qtest/fuzz/meson.build        |   6 +-
>>   tests/qtest/fuzz/virtio_blk_fuzz.c  |  51 ++-----------
>>   tests/qtest/fuzz/virtio_net_fuzz.c  |  54 ++------------
>>   tests/qtest/fuzz/virtio_scsi_fuzz.c |  51 ++-----------
>>   14 files changed, 72 insertions(+), 395 deletions(-)
>>   delete mode 100644 tests/qtest/fuzz/fork_fuzz.c
>>   delete mode 100644 tests/qtest/fuzz/fork_fuzz.h
>>   delete mode 100644 tests/qtest/fuzz/fork_fuzz.ld
>>
>> -- 
>> 2.39.0
>>
> 
> Whose tree should this go through? Laurent's qtest tree?

Do you mean Thomas?

$ git shortlog -cs tests/qtest/fuzz | sort -rn
     32  Thomas Huth
     26  Paolo Bonzini
     19  Stefan Hajnoczi
      6  Markus Armbruster
      5  Alexander Bulekov
      4  Marc-André Lureau
      3  Peter Maydell
      2  Laurent Vivier
      1  Michael S. Tsirkin
      1  Gerd Hoffmann

In doubt, cc'ing both :)

> Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [PATCH 00/10] Retire Fork-Based Fuzzing

[Laurent Vivier]

On 2/14/23 17:08, Philippe Mathieu-Daudé wrote:
> On 14/2/23 16:38, Stefan Hajnoczi wrote:
>> On Sat, Feb 04, 2023 at 11:29:41PM -0500, Alexander Bulekov wrote:
>>> Hello,
>>> This series removes fork-based fuzzing.
>>> How does fork-based fuzzing work?
>>>   * A single parent process initializes QEMU
>>>   * We identify the devices we wish to fuzz (fuzzer-dependent)
>>>   * Use QTest to PCI enumerate the devices
>>>   * After that we start a fork-server which forks the process and executes
>>>     fuzzer inputs inside the disposable children.
>>>
>>> In a normal fuzzing process, everything happens in a single process.
>>>
>>> Pros of fork-based fuzzing:
>>>   * We only need to do common configuration once (e.g. PCI enumeration).
>>>   * Fork provides a strong guarantee that fuzzer inputs will not interfere with
>>>     each-other
>>>   * The fuzzing process can continue even after a child-process crashes
>>>   * We can apply our-own timers to child-processes to exit slow inputs, early
>>>
>>> Cons of fork-based fuzzing:
>>>   * Fork-based fuzzing is not supported by libfuzzer. We had to build our own
>>>     fork-server and rely on tricks using linker-scripts and shared-memory to
>>>     support fuzzing. ( https://physics.bu.edu/~alxndr/libfuzzer-forkserver/ )
>>>   * Fork-based fuzzing is currently the main blocker preventing us from enabling
>>>     other fuzzers such as AFL++ on OSS-Fuzz
>>>   * Fork-based fuzzing may be a reason why coverage-builds are failing on
>>>     OSS-Fuzz. Coverage is an important fuzzing metric which would allow us to
>>>     find parts of the code that are not well-covered.
>>>   * Fork-based fuzzing has high overhead. fork() is an expensive system-call,
>>>     especially for processes running ASAN (with large/complex) VMA layouts.
>>>   * Fork prevents us from effectively fuzzing devices that rely on
>>>     threads (e.g. qxl).
>>>
>>> These patches remove fork-based fuzzing and replace it with reboot-based
>>> fuzzing for most cases. Misc notes about this change:
>>>   * libfuzzer appears to be no longer in active development. As such, the
>>>     current implementation of fork-based fuzzing (while having some nice
>>>     advantages) is likely to hold us back in the future. If these changes
>>>     are approved and appear to run successfully on OSS-Fuzz, we should be
>>>     able to easily experiment with other fuzzing engines (AFL++).
>>>   * Some device do not completely reset their state. This can lead to
>>>     non-reproducible crashes. However, in my local tests, most crashes
>>>     were reproducible. OSS-Fuzz shouldn't send us reports unless it can
>>>     consistently reproduce a crash.
>>>   * In theory, the corpus-format should not change, so the existing
>>>     corpus-inputs on OSS-Fuzz will transfer to the new reset()-able
>>>     fuzzers.
>>>   * Each fuzzing process will now exit after a single crash is found. To
>>>     continue the fuzzing process, use libfuzzer flags such as -jobs=-1
>>>   * We no long control input-timeouts (those are handled by libfuzzer).
>>>     Since timeouts on oss-fuzz can be many seconds long, I added a limit
>>>     on the number of DMA bytes written.
>>>
>>> Alexander Bulekov (10):
>>>    hw/sparse-mem: clear memory on reset
>>>    fuzz: add fuzz_reboot API
>>>    fuzz/generic-fuzz: use reboots instead of forks to reset state
>>>    fuzz/generic-fuzz: add a limit on DMA bytes written
>>>    fuzz/virtio-scsi: remove fork-based fuzzer
>>>    fuzz/virtio-net: remove fork-based fuzzer
>>>    fuzz/virtio-blk: remove fork-based fuzzer
>>>    fuzz/i440fx: remove fork-based fuzzer
>>>    fuzz: remove fork-fuzzing scaffolding
>>>    docs/fuzz: remove mentions of fork-based fuzzing
>>>
>>>   docs/devel/fuzzing.rst              |  22 +-----
>>>   hw/mem/sparse-mem.c                 |  13 +++-
>>>   meson.build                         |   4 -
>>>   tests/qtest/fuzz/fork_fuzz.c        |  41 ----------
>>>   tests/qtest/fuzz/fork_fuzz.h        |  23 ------
>>>   tests/qtest/fuzz/fork_fuzz.ld       |  56 --------------
>>>   tests/qtest/fuzz/fuzz.c             |   6 ++
>>>   tests/qtest/fuzz/fuzz.h             |   2 +-
>>>   tests/qtest/fuzz/generic_fuzz.c     | 111 +++++++---------------------
>>>   tests/qtest/fuzz/i440fx_fuzz.c      |  27 +------
>>>   tests/qtest/fuzz/meson.build        |   6 +-
>>>   tests/qtest/fuzz/virtio_blk_fuzz.c  |  51 ++-----------
>>>   tests/qtest/fuzz/virtio_net_fuzz.c  |  54 ++------------
>>>   tests/qtest/fuzz/virtio_scsi_fuzz.c |  51 ++-----------
>>>   14 files changed, 72 insertions(+), 395 deletions(-)
>>>   delete mode 100644 tests/qtest/fuzz/fork_fuzz.c
>>>   delete mode 100644 tests/qtest/fuzz/fork_fuzz.h
>>>   delete mode 100644 tests/qtest/fuzz/fork_fuzz.ld
>>>
>>> -- 
>>> 2.39.0
>>>
>>
>> Whose tree should this go through? Laurent's qtest tree?
> 
> Do you mean Thomas?
> 
> $ git shortlog -cs tests/qtest/fuzz | sort -rn
>      32  Thomas Huth
>      26  Paolo Bonzini
>      19  Stefan Hajnoczi
>       6  Markus Armbruster
>       5  Alexander Bulekov
>       4  Marc-André Lureau
>       3  Peter Maydell
>       2  Laurent Vivier
>       1  Michael S. Tsirkin
>       1  Gerd Hoffmann
> 
> In doubt, cc'ing both :)

Yes, Thomas is the real maintainer.

> 
>> Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
> 

Re: [PATCH 00/10] Retire Fork-Based Fuzzing

[Stefan Hajnoczi]

On Tue, 14 Feb 2023 at 12:59, Laurent Vivier <lvivier@redhat.com> wrote:
>
> On 2/14/23 17:08, Philippe Mathieu-Daudé wrote:
> > On 14/2/23 16:38, Stefan Hajnoczi wrote:
> >> On Sat, Feb 04, 2023 at 11:29:41PM -0500, Alexander Bulekov wrote:
> >>> Hello,
> >>> This series removes fork-based fuzzing.
> >>> How does fork-based fuzzing work?
> >>>   * A single parent process initializes QEMU
> >>>   * We identify the devices we wish to fuzz (fuzzer-dependent)
> >>>   * Use QTest to PCI enumerate the devices
> >>>   * After that we start a fork-server which forks the process and executes
> >>>     fuzzer inputs inside the disposable children.
> >>>
> >>> In a normal fuzzing process, everything happens in a single process.
> >>>
> >>> Pros of fork-based fuzzing:
> >>>   * We only need to do common configuration once (e.g. PCI enumeration).
> >>>   * Fork provides a strong guarantee that fuzzer inputs will not interfere with
> >>>     each-other
> >>>   * The fuzzing process can continue even after a child-process crashes
> >>>   * We can apply our-own timers to child-processes to exit slow inputs, early
> >>>
> >>> Cons of fork-based fuzzing:
> >>>   * Fork-based fuzzing is not supported by libfuzzer. We had to build our own
> >>>     fork-server and rely on tricks using linker-scripts and shared-memory to
> >>>     support fuzzing. ( https://physics.bu.edu/~alxndr/libfuzzer-forkserver/ )
> >>>   * Fork-based fuzzing is currently the main blocker preventing us from enabling
> >>>     other fuzzers such as AFL++ on OSS-Fuzz
> >>>   * Fork-based fuzzing may be a reason why coverage-builds are failing on
> >>>     OSS-Fuzz. Coverage is an important fuzzing metric which would allow us to
> >>>     find parts of the code that are not well-covered.
> >>>   * Fork-based fuzzing has high overhead. fork() is an expensive system-call,
> >>>     especially for processes running ASAN (with large/complex) VMA layouts.
> >>>   * Fork prevents us from effectively fuzzing devices that rely on
> >>>     threads (e.g. qxl).
> >>>
> >>> These patches remove fork-based fuzzing and replace it with reboot-based
> >>> fuzzing for most cases. Misc notes about this change:
> >>>   * libfuzzer appears to be no longer in active development. As such, the
> >>>     current implementation of fork-based fuzzing (while having some nice
> >>>     advantages) is likely to hold us back in the future. If these changes
> >>>     are approved and appear to run successfully on OSS-Fuzz, we should be
> >>>     able to easily experiment with other fuzzing engines (AFL++).
> >>>   * Some device do not completely reset their state. This can lead to
> >>>     non-reproducible crashes. However, in my local tests, most crashes
> >>>     were reproducible. OSS-Fuzz shouldn't send us reports unless it can
> >>>     consistently reproduce a crash.
> >>>   * In theory, the corpus-format should not change, so the existing
> >>>     corpus-inputs on OSS-Fuzz will transfer to the new reset()-able
> >>>     fuzzers.
> >>>   * Each fuzzing process will now exit after a single crash is found. To
> >>>     continue the fuzzing process, use libfuzzer flags such as -jobs=-1
> >>>   * We no long control input-timeouts (those are handled by libfuzzer).
> >>>     Since timeouts on oss-fuzz can be many seconds long, I added a limit
> >>>     on the number of DMA bytes written.
> >>>
> >>> Alexander Bulekov (10):
> >>>    hw/sparse-mem: clear memory on reset
> >>>    fuzz: add fuzz_reboot API
> >>>    fuzz/generic-fuzz: use reboots instead of forks to reset state
> >>>    fuzz/generic-fuzz: add a limit on DMA bytes written
> >>>    fuzz/virtio-scsi: remove fork-based fuzzer
> >>>    fuzz/virtio-net: remove fork-based fuzzer
> >>>    fuzz/virtio-blk: remove fork-based fuzzer
> >>>    fuzz/i440fx: remove fork-based fuzzer
> >>>    fuzz: remove fork-fuzzing scaffolding
> >>>    docs/fuzz: remove mentions of fork-based fuzzing
> >>>
> >>>   docs/devel/fuzzing.rst              |  22 +-----
> >>>   hw/mem/sparse-mem.c                 |  13 +++-
> >>>   meson.build                         |   4 -
> >>>   tests/qtest/fuzz/fork_fuzz.c        |  41 ----------
> >>>   tests/qtest/fuzz/fork_fuzz.h        |  23 ------
> >>>   tests/qtest/fuzz/fork_fuzz.ld       |  56 --------------
> >>>   tests/qtest/fuzz/fuzz.c             |   6 ++
> >>>   tests/qtest/fuzz/fuzz.h             |   2 +-
> >>>   tests/qtest/fuzz/generic_fuzz.c     | 111 +++++++---------------------
> >>>   tests/qtest/fuzz/i440fx_fuzz.c      |  27 +------
> >>>   tests/qtest/fuzz/meson.build        |   6 +-
> >>>   tests/qtest/fuzz/virtio_blk_fuzz.c  |  51 ++-----------
> >>>   tests/qtest/fuzz/virtio_net_fuzz.c  |  54 ++------------
> >>>   tests/qtest/fuzz/virtio_scsi_fuzz.c |  51 ++-----------
> >>>   14 files changed, 72 insertions(+), 395 deletions(-)
> >>>   delete mode 100644 tests/qtest/fuzz/fork_fuzz.c
> >>>   delete mode 100644 tests/qtest/fuzz/fork_fuzz.h
> >>>   delete mode 100644 tests/qtest/fuzz/fork_fuzz.ld
> >>>
> >>> --
> >>> 2.39.0
> >>>
> >>
> >> Whose tree should this go through? Laurent's qtest tree?
> >
> > Do you mean Thomas?
> >
> > $ git shortlog -cs tests/qtest/fuzz | sort -rn
> >      32  Thomas Huth
> >      26  Paolo Bonzini
> >      19  Stefan Hajnoczi
> >       6  Markus Armbruster
> >       5  Alexander Bulekov
> >       4  Marc-André Lureau
> >       3  Peter Maydell
> >       2  Laurent Vivier
> >       1  Michael S. Tsirkin
> >       1  Gerd Hoffmann
> >
> > In doubt, cc'ing both :)
>
> Yes, Thomas is the real maintainer.

Want to update the ./MAINTAINERS file? That's where I found your name.
Thomas is only listed as a reviewer.

Stefan

Re: [PATCH 00/10] Retire Fork-Based Fuzzing

[Thomas Huth]

On 14/02/2023 17.08, Philippe Mathieu-Daudé wrote:
> On 14/2/23 16:38, Stefan Hajnoczi wrote:
>> On Sat, Feb 04, 2023 at 11:29:41PM -0500, Alexander Bulekov wrote:
>>> Hello,
>>> This series removes fork-based fuzzing.
>>> How does fork-based fuzzing work?
>>>   * A single parent process initializes QEMU
>>>   * We identify the devices we wish to fuzz (fuzzer-dependent)
>>>   * Use QTest to PCI enumerate the devices
>>>   * After that we start a fork-server which forks the process and executes
>>>     fuzzer inputs inside the disposable children.
>>>
>>> In a normal fuzzing process, everything happens in a single process.
>>>
>>> Pros of fork-based fuzzing:
>>>   * We only need to do common configuration once (e.g. PCI enumeration).
>>>   * Fork provides a strong guarantee that fuzzer inputs will not 
>>> interfere with
>>>     each-other
>>>   * The fuzzing process can continue even after a child-process crashes
>>>   * We can apply our-own timers to child-processes to exit slow inputs, 
>>> early
>>>
>>> Cons of fork-based fuzzing:
>>>   * Fork-based fuzzing is not supported by libfuzzer. We had to build our 
>>> own
>>>     fork-server and rely on tricks using linker-scripts and shared-memory to
>>>     support fuzzing. ( 
>>> https://physics.bu.edu/~alxndr/libfuzzer-forkserver/ )
>>>   * Fork-based fuzzing is currently the main blocker preventing us from 
>>> enabling
>>>     other fuzzers such as AFL++ on OSS-Fuzz
>>>   * Fork-based fuzzing may be a reason why coverage-builds are failing on
>>>     OSS-Fuzz. Coverage is an important fuzzing metric which would allow 
>>> us to
>>>     find parts of the code that are not well-covered.
>>>   * Fork-based fuzzing has high overhead. fork() is an expensive 
>>> system-call,
>>>     especially for processes running ASAN (with large/complex) VMA layouts.
>>>   * Fork prevents us from effectively fuzzing devices that rely on
>>>     threads (e.g. qxl).
>>>
>>> These patches remove fork-based fuzzing and replace it with reboot-based
>>> fuzzing for most cases. Misc notes about this change:
>>>   * libfuzzer appears to be no longer in active development. As such, the
>>>     current implementation of fork-based fuzzing (while having some nice
>>>     advantages) is likely to hold us back in the future. If these changes
>>>     are approved and appear to run successfully on OSS-Fuzz, we should be
>>>     able to easily experiment with other fuzzing engines (AFL++).
>>>   * Some device do not completely reset their state. This can lead to
>>>     non-reproducible crashes. However, in my local tests, most crashes
>>>     were reproducible. OSS-Fuzz shouldn't send us reports unless it can
>>>     consistently reproduce a crash.
>>>   * In theory, the corpus-format should not change, so the existing
>>>     corpus-inputs on OSS-Fuzz will transfer to the new reset()-able
>>>     fuzzers.
>>>   * Each fuzzing process will now exit after a single crash is found. To
>>>     continue the fuzzing process, use libfuzzer flags such as -jobs=-1
>>>   * We no long control input-timeouts (those are handled by libfuzzer).
>>>     Since timeouts on oss-fuzz can be many seconds long, I added a limit
>>>     on the number of DMA bytes written.
>>>
>>> Alexander Bulekov (10):
>>>    hw/sparse-mem: clear memory on reset
>>>    fuzz: add fuzz_reboot API
>>>    fuzz/generic-fuzz: use reboots instead of forks to reset state
>>>    fuzz/generic-fuzz: add a limit on DMA bytes written
>>>    fuzz/virtio-scsi: remove fork-based fuzzer
>>>    fuzz/virtio-net: remove fork-based fuzzer
>>>    fuzz/virtio-blk: remove fork-based fuzzer
>>>    fuzz/i440fx: remove fork-based fuzzer
>>>    fuzz: remove fork-fuzzing scaffolding
>>>    docs/fuzz: remove mentions of fork-based fuzzing
>>>
>>>   docs/devel/fuzzing.rst              |  22 +-----
>>>   hw/mem/sparse-mem.c                 |  13 +++-
>>>   meson.build                         |   4 -
>>>   tests/qtest/fuzz/fork_fuzz.c        |  41 ----------
>>>   tests/qtest/fuzz/fork_fuzz.h        |  23 ------
>>>   tests/qtest/fuzz/fork_fuzz.ld       |  56 --------------
>>>   tests/qtest/fuzz/fuzz.c             |   6 ++
>>>   tests/qtest/fuzz/fuzz.h             |   2 +-
>>>   tests/qtest/fuzz/generic_fuzz.c     | 111 +++++++---------------------
>>>   tests/qtest/fuzz/i440fx_fuzz.c      |  27 +------
>>>   tests/qtest/fuzz/meson.build        |   6 +-
>>>   tests/qtest/fuzz/virtio_blk_fuzz.c  |  51 ++-----------
>>>   tests/qtest/fuzz/virtio_net_fuzz.c  |  54 ++------------
>>>   tests/qtest/fuzz/virtio_scsi_fuzz.c |  51 ++-----------
>>>   14 files changed, 72 insertions(+), 395 deletions(-)
>>>   delete mode 100644 tests/qtest/fuzz/fork_fuzz.c
>>>   delete mode 100644 tests/qtest/fuzz/fork_fuzz.h
>>>   delete mode 100644 tests/qtest/fuzz/fork_fuzz.ld
>>>
>>> -- 
>>> 2.39.0
>>>
>>
>> Whose tree should this go through? Laurent's qtest tree?
> 
> Do you mean Thomas?

I thought Alexander would be doing pull requests for fuzzing-related patches 
nowadays (since he's the listed maintainer for these files)? Or did I get 
that wrong?

  Thomas

Re: [PATCH 00/10] Retire Fork-Based Fuzzing

[Alexander Bulekov]

On 230214 2009, Thomas Huth wrote:
> On 14/02/2023 17.08, Philippe Mathieu-Daudé wrote:
> > On 14/2/23 16:38, Stefan Hajnoczi wrote:
> > > On Sat, Feb 04, 2023 at 11:29:41PM -0500, Alexander Bulekov wrote:
> > > > Hello,
> > > > This series removes fork-based fuzzing.
> > > > How does fork-based fuzzing work?
> > > >   * A single parent process initializes QEMU
> > > >   * We identify the devices we wish to fuzz (fuzzer-dependent)
> > > >   * Use QTest to PCI enumerate the devices
> > > >   * After that we start a fork-server which forks the process and executes
> > > >     fuzzer inputs inside the disposable children.
> > > > 
> > > > In a normal fuzzing process, everything happens in a single process.
> > > > 
> > > > Pros of fork-based fuzzing:
> > > >   * We only need to do common configuration once (e.g. PCI enumeration).
> > > >   * Fork provides a strong guarantee that fuzzer inputs will not
> > > > interfere with
> > > >     each-other
> > > >   * The fuzzing process can continue even after a child-process crashes
> > > >   * We can apply our-own timers to child-processes to exit slow
> > > > inputs, early
> > > > 
> > > > Cons of fork-based fuzzing:
> > > >   * Fork-based fuzzing is not supported by libfuzzer. We had to
> > > > build our own
> > > >     fork-server and rely on tricks using linker-scripts and shared-memory to
> > > >     support fuzzing. (
> > > > https://physics.bu.edu/~alxndr/libfuzzer-forkserver/ )
> > > >   * Fork-based fuzzing is currently the main blocker preventing
> > > > us from enabling
> > > >     other fuzzers such as AFL++ on OSS-Fuzz
> > > >   * Fork-based fuzzing may be a reason why coverage-builds are failing on
> > > >     OSS-Fuzz. Coverage is an important fuzzing metric which
> > > > would allow us to
> > > >     find parts of the code that are not well-covered.
> > > >   * Fork-based fuzzing has high overhead. fork() is an expensive
> > > > system-call,
> > > >     especially for processes running ASAN (with large/complex) VMA layouts.
> > > >   * Fork prevents us from effectively fuzzing devices that rely on
> > > >     threads (e.g. qxl).
> > > > 
> > > > These patches remove fork-based fuzzing and replace it with reboot-based
> > > > fuzzing for most cases. Misc notes about this change:
> > > >   * libfuzzer appears to be no longer in active development. As such, the
> > > >     current implementation of fork-based fuzzing (while having some nice
> > > >     advantages) is likely to hold us back in the future. If these changes
> > > >     are approved and appear to run successfully on OSS-Fuzz, we should be
> > > >     able to easily experiment with other fuzzing engines (AFL++).
> > > >   * Some device do not completely reset their state. This can lead to
> > > >     non-reproducible crashes. However, in my local tests, most crashes
> > > >     were reproducible. OSS-Fuzz shouldn't send us reports unless it can
> > > >     consistently reproduce a crash.
> > > >   * In theory, the corpus-format should not change, so the existing
> > > >     corpus-inputs on OSS-Fuzz will transfer to the new reset()-able
> > > >     fuzzers.
> > > >   * Each fuzzing process will now exit after a single crash is found. To
> > > >     continue the fuzzing process, use libfuzzer flags such as -jobs=-1
> > > >   * We no long control input-timeouts (those are handled by libfuzzer).
> > > >     Since timeouts on oss-fuzz can be many seconds long, I added a limit
> > > >     on the number of DMA bytes written.
> > > > 
> > > > Alexander Bulekov (10):
> > > >    hw/sparse-mem: clear memory on reset
> > > >    fuzz: add fuzz_reboot API
> > > >    fuzz/generic-fuzz: use reboots instead of forks to reset state
> > > >    fuzz/generic-fuzz: add a limit on DMA bytes written
> > > >    fuzz/virtio-scsi: remove fork-based fuzzer
> > > >    fuzz/virtio-net: remove fork-based fuzzer
> > > >    fuzz/virtio-blk: remove fork-based fuzzer
> > > >    fuzz/i440fx: remove fork-based fuzzer
> > > >    fuzz: remove fork-fuzzing scaffolding
> > > >    docs/fuzz: remove mentions of fork-based fuzzing
> > > > 
> > > >   docs/devel/fuzzing.rst              |  22 +-----
> > > >   hw/mem/sparse-mem.c                 |  13 +++-
> > > >   meson.build                         |   4 -
> > > >   tests/qtest/fuzz/fork_fuzz.c        |  41 ----------
> > > >   tests/qtest/fuzz/fork_fuzz.h        |  23 ------
> > > >   tests/qtest/fuzz/fork_fuzz.ld       |  56 --------------
> > > >   tests/qtest/fuzz/fuzz.c             |   6 ++
> > > >   tests/qtest/fuzz/fuzz.h             |   2 +-
> > > >   tests/qtest/fuzz/generic_fuzz.c     | 111 +++++++---------------------
> > > >   tests/qtest/fuzz/i440fx_fuzz.c      |  27 +------
> > > >   tests/qtest/fuzz/meson.build        |   6 +-
> > > >   tests/qtest/fuzz/virtio_blk_fuzz.c  |  51 ++-----------
> > > >   tests/qtest/fuzz/virtio_net_fuzz.c  |  54 ++------------
> > > >   tests/qtest/fuzz/virtio_scsi_fuzz.c |  51 ++-----------
> > > >   14 files changed, 72 insertions(+), 395 deletions(-)
> > > >   delete mode 100644 tests/qtest/fuzz/fork_fuzz.c
> > > >   delete mode 100644 tests/qtest/fuzz/fork_fuzz.h
> > > >   delete mode 100644 tests/qtest/fuzz/fork_fuzz.ld
> > > > 
> > > > -- 
> > > > 2.39.0
> > > > 
> > > 
> > > Whose tree should this go through? Laurent's qtest tree?
> > 
> > Do you mean Thomas?
> 
> I thought Alexander would be doing pull requests for fuzzing-related patches
> nowadays (since he's the listed maintainer for these files)? Or did I get
> that wrong?

I have, though in the past I've been asked to send the PR to different
people. Who should I send this PR to?
-Alex

Re: [PATCH 00/10] Retire Fork-Based Fuzzing

[Thomas Huth]

On 14/02/2023 20.14, Alexander Bulekov wrote:
> On 230214 2009, Thomas Huth wrote:
>> On 14/02/2023 17.08, Philippe Mathieu-Daudé wrote:
>>> On 14/2/23 16:38, Stefan Hajnoczi wrote:
>>>> On Sat, Feb 04, 2023 at 11:29:41PM -0500, Alexander Bulekov wrote:
>>>>> Hello,
>>>>> This series removes fork-based fuzzing.
>>>>> How does fork-based fuzzing work?
>>>>>    * A single parent process initializes QEMU
>>>>>    * We identify the devices we wish to fuzz (fuzzer-dependent)
>>>>>    * Use QTest to PCI enumerate the devices
>>>>>    * After that we start a fork-server which forks the process and executes
>>>>>      fuzzer inputs inside the disposable children.
>>>>>
>>>>> In a normal fuzzing process, everything happens in a single process.
>>>>>
>>>>> Pros of fork-based fuzzing:
>>>>>    * We only need to do common configuration once (e.g. PCI enumeration).
>>>>>    * Fork provides a strong guarantee that fuzzer inputs will not
>>>>> interfere with
>>>>>      each-other
>>>>>    * The fuzzing process can continue even after a child-process crashes
>>>>>    * We can apply our-own timers to child-processes to exit slow
>>>>> inputs, early
>>>>>
>>>>> Cons of fork-based fuzzing:
>>>>>    * Fork-based fuzzing is not supported by libfuzzer. We had to
>>>>> build our own
>>>>>      fork-server and rely on tricks using linker-scripts and shared-memory to
>>>>>      support fuzzing. (
>>>>> https://physics.bu.edu/~alxndr/libfuzzer-forkserver/ )
>>>>>    * Fork-based fuzzing is currently the main blocker preventing
>>>>> us from enabling
>>>>>      other fuzzers such as AFL++ on OSS-Fuzz
>>>>>    * Fork-based fuzzing may be a reason why coverage-builds are failing on
>>>>>      OSS-Fuzz. Coverage is an important fuzzing metric which
>>>>> would allow us to
>>>>>      find parts of the code that are not well-covered.
>>>>>    * Fork-based fuzzing has high overhead. fork() is an expensive
>>>>> system-call,
>>>>>      especially for processes running ASAN (with large/complex) VMA layouts.
>>>>>    * Fork prevents us from effectively fuzzing devices that rely on
>>>>>      threads (e.g. qxl).
>>>>>
>>>>> These patches remove fork-based fuzzing and replace it with reboot-based
>>>>> fuzzing for most cases. Misc notes about this change:
>>>>>    * libfuzzer appears to be no longer in active development. As such, the
>>>>>      current implementation of fork-based fuzzing (while having some nice
>>>>>      advantages) is likely to hold us back in the future. If these changes
>>>>>      are approved and appear to run successfully on OSS-Fuzz, we should be
>>>>>      able to easily experiment with other fuzzing engines (AFL++).
>>>>>    * Some device do not completely reset their state. This can lead to
>>>>>      non-reproducible crashes. However, in my local tests, most crashes
>>>>>      were reproducible. OSS-Fuzz shouldn't send us reports unless it can
>>>>>      consistently reproduce a crash.
>>>>>    * In theory, the corpus-format should not change, so the existing
>>>>>      corpus-inputs on OSS-Fuzz will transfer to the new reset()-able
>>>>>      fuzzers.
>>>>>    * Each fuzzing process will now exit after a single crash is found. To
>>>>>      continue the fuzzing process, use libfuzzer flags such as -jobs=-1
>>>>>    * We no long control input-timeouts (those are handled by libfuzzer).
>>>>>      Since timeouts on oss-fuzz can be many seconds long, I added a limit
>>>>>      on the number of DMA bytes written.
>>>>>
>>>>> Alexander Bulekov (10):
>>>>>     hw/sparse-mem: clear memory on reset
>>>>>     fuzz: add fuzz_reboot API
>>>>>     fuzz/generic-fuzz: use reboots instead of forks to reset state
>>>>>     fuzz/generic-fuzz: add a limit on DMA bytes written
>>>>>     fuzz/virtio-scsi: remove fork-based fuzzer
>>>>>     fuzz/virtio-net: remove fork-based fuzzer
>>>>>     fuzz/virtio-blk: remove fork-based fuzzer
>>>>>     fuzz/i440fx: remove fork-based fuzzer
>>>>>     fuzz: remove fork-fuzzing scaffolding
>>>>>     docs/fuzz: remove mentions of fork-based fuzzing
>>>>>
>>>>>    docs/devel/fuzzing.rst              |  22 +-----
>>>>>    hw/mem/sparse-mem.c                 |  13 +++-
>>>>>    meson.build                         |   4 -
>>>>>    tests/qtest/fuzz/fork_fuzz.c        |  41 ----------
>>>>>    tests/qtest/fuzz/fork_fuzz.h        |  23 ------
>>>>>    tests/qtest/fuzz/fork_fuzz.ld       |  56 --------------
>>>>>    tests/qtest/fuzz/fuzz.c             |   6 ++
>>>>>    tests/qtest/fuzz/fuzz.h             |   2 +-
>>>>>    tests/qtest/fuzz/generic_fuzz.c     | 111 +++++++---------------------
>>>>>    tests/qtest/fuzz/i440fx_fuzz.c      |  27 +------
>>>>>    tests/qtest/fuzz/meson.build        |   6 +-
>>>>>    tests/qtest/fuzz/virtio_blk_fuzz.c  |  51 ++-----------
>>>>>    tests/qtest/fuzz/virtio_net_fuzz.c  |  54 ++------------
>>>>>    tests/qtest/fuzz/virtio_scsi_fuzz.c |  51 ++-----------
>>>>>    14 files changed, 72 insertions(+), 395 deletions(-)
>>>>>    delete mode 100644 tests/qtest/fuzz/fork_fuzz.c
>>>>>    delete mode 100644 tests/qtest/fuzz/fork_fuzz.h
>>>>>    delete mode 100644 tests/qtest/fuzz/fork_fuzz.ld
>>>>
>>>> Whose tree should this go through? Laurent's qtest tree?
>>>
>>> Do you mean Thomas?
>>
>> I thought Alexander would be doing pull requests for fuzzing-related patches
>> nowadays (since he's the listed maintainer for these files)? Or did I get
>> that wrong?
> 
> I have, though in the past I've been asked to send the PR to different
> people. Who should I send this PR to?

I assume you should have enough experience with sending PRs now, so if Peter 
does not mind, I'd suggest to directly send PRs to Peter now. If that does 
not work for some reason, feel free to send a “not for master” pull request 
to me instead, then I'll take it along with my next qtest-related PR.

  Thomas

Re: [PATCH 04/10] fuzz/generic-fuzz: add a limit on DMA bytes written

[Alexander Bulekov]

On 230213 1438, Darren Kenny wrote:
> Hi Alex,
> 
> On Saturday, 2023-02-04 at 23:29:45 -05, Alexander Bulekov wrote:
> > As we have repplaced fork-based fuzzing, with reboots - we can no longer
> > use a timeout+exit() to avoid slow inputs. Libfuzzer has its own timer
> > that it uses to catch slow inputs, however these timeouts are usually
> > seconds-minutes long: more than enough to bog-down the fuzzing process.
> > However, I found that slow inputs often attempt to fill overly large DMA
> > requests. Thus, we can mitigate most timeouts by setting a cap on the
> > total number of DMA bytes written by an input.
> >
> > Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> > ---
> >  tests/qtest/fuzz/generic_fuzz.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
> > index c2e5642150..eab92cbc23 100644
> > --- a/tests/qtest/fuzz/generic_fuzz.c
> > +++ b/tests/qtest/fuzz/generic_fuzz.c
> > @@ -52,6 +52,7 @@ enum cmds {
> >  #define USEC_IN_SEC 1000000000
> >  
> >  #define MAX_DMA_FILL_SIZE 0x10000
> > +#define MAX_TOTAL_DMA_SIZE 0x10000000
> >  
> >  #define PCI_HOST_BRIDGE_CFG 0xcf8
> >  #define PCI_HOST_BRIDGE_DATA 0xcfc
> > @@ -64,6 +65,7 @@ typedef struct {
> >  static useconds_t timeout = DEFAULT_TIMEOUT_US;
> >  
> >  static bool qtest_log_enabled;
> > +size_t dma_bytes_written;
> >  
> >  MemoryRegion *sparse_mem_mr;
> >  
> > @@ -197,6 +199,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr)
> >       */
> >      if (dma_patterns->len == 0
> >          || len == 0
> > +        || dma_bytes_written > MAX_TOTAL_DMA_SIZE
> 
> NIT: Just wondering if you should check dma_bytes_written + l as opposed
>      to dma_bytes_written? It's probably not important enough given it's
>      just an artificial limit, but thought I'd ask.
>

Done :)

> >          || (mr != current_machine->ram && mr != sparse_mem_mr)) {
> >          return;
> >      }
> > @@ -269,6 +272,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr)
> >                  fflush(stderr);
> >              }
> >              qtest_memwrite(qts_global, addr, buf, l);
> > +            dma_bytes_written += l;
> >          }
> >          len -= l;
> >          buf += l;
> > @@ -648,6 +652,7 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
> >  
> >      op_clear_dma_patterns(s, NULL, 0);
> >      pci_disabled = false;
> > +    dma_bytes_written = 0;
> >  
> >      QPCIBus *pcibus = qpci_new_pc(s, NULL);
> >      g_ptr_array_foreach(fuzzable_pci_devices, pci_enum, pcibus);
> > -- 
> > 2.39.0
> 
> While this will still consume the existing corpus, is it likely to
> cause these existing corpus to be trimmed?

Not sure - It would affect inputs that generate a lot of DMA
activity (though those should have been caught by our previous timeout
mechanism).

> 
> Otherwise, the changes look good:
> 
> Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
> 
> Thanks,
> 
> Darren.

Re: [PATCH 03/10] fuzz/generic-fuzz: use reboots instead of forks to reset state

[Alexander Bulekov]

On 230213 1426, Darren Kenny wrote:
> Hi Alex,
> 
> On Saturday, 2023-02-04 at 23:29:44 -05, Alexander Bulekov wrote:
> > Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> > ---
> >  tests/qtest/fuzz/generic_fuzz.c | 106 +++++++-------------------------
> >  1 file changed, 23 insertions(+), 83 deletions(-)
> >
> > diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
> > index 7326f6840b..c2e5642150 100644
> > --- a/tests/qtest/fuzz/generic_fuzz.c
> > +++ b/tests/qtest/fuzz/generic_fuzz.c
> > @@ -18,7 +18,6 @@
> >  #include "tests/qtest/libqtest.h"
> >  #include "tests/qtest/libqos/pci-pc.h"
> >  #include "fuzz.h"
> > -#include "fork_fuzz.h"
> >  #include "string.h"
> >  #include "exec/memory.h"
> >  #include "exec/ramblock.h"
> > @@ -29,6 +28,8 @@
> >  #include "generic_fuzz_configs.h"
> >  #include "hw/mem/sparse-mem.h"
> >  
> > +static void pci_enum(gpointer pcidev, gpointer bus);
> > +
> >  /*
> >   * SEPARATOR is used to separate "operations" in the fuzz input
> >   */
> > @@ -589,30 +590,6 @@ static void op_disable_pci(QTestState *s, const unsigned char *data, size_t len)
> >      pci_disabled = true;
> >  }
> >  
> > -static void handle_timeout(int sig)
> > -{
> > -    if (qtest_log_enabled) {
> > -        fprintf(stderr, "[Timeout]\n");
> > -        fflush(stderr);
> > -    }
> > -
> > -    /*
> > -     * If there is a crash, libfuzzer/ASAN forks a child to run an
> > -     * "llvm-symbolizer" process for printing out a pretty stacktrace. It
> > -     * communicates with this child using a pipe.  If we timeout+Exit, while
> > -     * libfuzzer is still communicating with the llvm-symbolizer child, we will
> > -     * be left with an orphan llvm-symbolizer process. Sometimes, this appears
> > -     * to lead to a deadlock in the forkserver. Use waitpid to check if there
> > -     * are any waitable children. If so, exit out of the signal-handler, and
> > -     * let libfuzzer finish communicating with the child, and exit, on its own.
> > -     */
> > -    if (waitpid(-1, NULL, WNOHANG) == 0) {
> > -        return;
> > -    }
> > -
> > -    _Exit(0);
> > -}
> > -
> >  /*
> >
> 
> I'm presuming that the timeout is being left to the fuzz orchestrator
> now, rather than us managing it directly in our own way?

Yes. The fuzzer should handle timeouts directly now. 

-Alex