All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/16] Add a General Virtual Device Fuzzer
@ 2020-09-21  2:24 Alexander Bulekov
  2020-09-21  2:24 ` [PATCH v3 01/16] memory: Add FlatView foreach function Alexander Bulekov
                   ` (26 more replies)
  0 siblings, 27 replies; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21  2:24 UTC (permalink / raw)
  To: qemu-devel; +Cc: darren.kenny, bsd, philmd, stefanha, Alexander Bulekov

v3:
	- Use flatviews to help select regions for fuzzing 
	- Meson-related changes
    - Add some documentation
	- Improve minimalization script to trim write{bwlq} commands
v2:
	- Remove QOS dependency.
	- Add a custom crossover function
	- Fix broken minimization scripts
	- Fixes to the IO region and DMA handling code

This is a general virtual-device fuzzer, designed to fuzz devices over Port IO,
MMIO, and DMA.

To get started with this:
 1. Build the fuzzers (see docs/devel/fuzzing.txt)
    Note: Build with --enable-sanitizers, or create a "dictionary file":
    echo kw1=\"FUZZ\" > dict
    and pass it as an argument to libFuzzer with -dict=./dict
    This magic value is a command separator that lets the fuzzer perform
    multiple IO actions with a single input.

 2. Pick the qemu arguments you wish to fuzz:
    export QEMU_FUZZ_ARGS="-M q35 -device virtio-balloon"

 3. Tell the fuzzer which QOM objects or MemoryRegion names to fuzz. I find the
 "info qom-tree", "info qtree" and "info mtree" commands useful for identifying
 these. Supports globbing. Here I will try to simultaneously fuzz(for no good
 reason) virtio-balloon and e1000e, which is included by default in the q35:
    export QEMU_FUZZ_OBJECTS='virtio* e1000*'
    You can also try to fuzz the whole machine:
    export QEMU_FUZZ_OBJECTS='*'

 4. Run the fuzzer for 0 inputs. The fuzzer should output a list of
 MemoryRegions/PCI Devices it will try to fuzz. Confirm that these match your
 expectations.
    ./i386-softmmu/qemu-fuzz-i386 --fuzz-target=general-fuzz -runs=0

 5. Run the fuzzer:
    ./i386-softmmu/qemu-fuzz-i386 --fuzz-target=general-fuzz 


Basically, at the core, this fuzzer is an interpreter that splits the input
into a series of commands, such as mmio_write, pio_write, etc. We structure
these commands to hit only MemoryRegions that are associated with the devices
specified in QEMU_FUZZ_OBJECTS. Additionally, these patches add "hooks" to
functions that are typically used by virtual-devices to read from RAM (DMA).
These hooks attempt to populate these DMA regions with fuzzed data, just in
time.

Some of the issues I have found or reproduced with this fuzzer:
https://bugs.launchpad.net/bugs/1525123
https://bugs.launchpad.net/bugs/1681439
https://bugs.launchpad.net/bugs/1777315
https://bugs.launchpad.net/bugs/1878034
https://bugs.launchpad.net/bugs/1878043
https://bugs.launchpad.net/bugs/1878054
https://bugs.launchpad.net/bugs/1878057
https://bugs.launchpad.net/bugs/1878067
https://bugs.launchpad.net/bugs/1878134
https://bugs.launchpad.net/bugs/1878136
https://bugs.launchpad.net/bugs/1878253
https://bugs.launchpad.net/bugs/1878255
https://bugs.launchpad.net/bugs/1878259
https://bugs.launchpad.net/bugs/1878263
https://bugs.launchpad.net/bugs/1878323
https://bugs.launchpad.net/bugs/1878641
https://bugs.launchpad.net/bugs/1878642
https://bugs.launchpad.net/bugs/1878645
https://bugs.launchpad.net/bugs/1878651
https://bugs.launchpad.net/bugs/1879223
https://bugs.launchpad.net/bugs/1879227
https://bugs.launchpad.net/bugs/1879531
https://bugs.launchpad.net/bugs/1880355
https://bugs.launchpad.net/bugs/1880539
https://bugs.launchpad.net/bugs/1884693
https://bugs.launchpad.net/bugs/1886362
https://bugs.launchpad.net/bugs/1887303
https://bugs.launchpad.net/bugs/1887309
https://bugs.launchpad.net/bugs/697510

Alexander Bulekov (16):
  memory: Add FlatView foreach function
  fuzz: Add general virtual-device fuzzer
  fuzz: Add PCI features to the general fuzzer
  fuzz: Add DMA support to the generic-fuzzer
  fuzz: Declare DMA Read callback function
  fuzz: Add fuzzer callbacks to DMA-read functions
  fuzz: Add support for custom crossover functions
  fuzz: add a DISABLE_PCI op to general-fuzzer
  fuzz: add a crossover function to generic-fuzzer
  scripts/oss-fuzz: Add wrapper program for generic fuzzer
  scripts/oss-fuzz: Add general-fuzzer build script
  scripts/oss-fuzz: Add general-fuzzer configs for oss-fuzz
  scripts/oss-fuzz: build the general-fuzzer configs
  scripts/oss-fuzz: Add script to reorder a general-fuzzer trace
  scripts/oss-fuzz: Add crash trace minimization script
  fuzz: Add instructions for using general-fuzz

 docs/devel/fuzzing.txt                        |  38 +
 exec.c                                        |   2 +
 include/exec/memory.h                         |  21 +
 include/exec/memory_ldst_cached.h.inc         |   3 +
 memory_ldst.c.inc                             |   4 +
 scripts/oss-fuzz/build.sh                     |   7 +
 scripts/oss-fuzz/build_general_fuzzers.py     |  69 ++
 scripts/oss-fuzz/general_fuzzer_configs.yml   | 103 +++
 scripts/oss-fuzz/minimize_qtest_trace.py      | 157 ++++
 .../oss-fuzz/reorder_fuzzer_qtest_trace.py    |  94 ++
 scripts/oss-fuzz/target_template.c            |  40 +
 softmmu/memory.c                              |  23 +
 tests/qtest/fuzz/fuzz.c                       |  13 +
 tests/qtest/fuzz/fuzz.h                       |  27 +
 tests/qtest/fuzz/general_fuzz.c               | 854 ++++++++++++++++++
 tests/qtest/fuzz/meson.build                  |   1 +
 16 files changed, 1456 insertions(+)
 create mode 100755 scripts/oss-fuzz/build_general_fuzzers.py
 create mode 100644 scripts/oss-fuzz/general_fuzzer_configs.yml
 create mode 100755 scripts/oss-fuzz/minimize_qtest_trace.py
 create mode 100755 scripts/oss-fuzz/reorder_fuzzer_qtest_trace.py
 create mode 100644 scripts/oss-fuzz/target_template.c
 create mode 100644 tests/qtest/fuzz/general_fuzz.c

-- 
2.28.0



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v3 01/16] memory: Add FlatView foreach function
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
@ 2020-09-21  2:24 ` Alexander Bulekov
  2020-10-08  6:57   ` Paolo Bonzini
  2020-09-21  2:24 ` [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer Alexander Bulekov
                   ` (25 subsequent siblings)
  26 siblings, 1 reply; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21  2:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alexander Bulekov, darren.kenny, bsd, stefanha, Paolo Bonzini, philmd

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 include/exec/memory.h | 5 +++++
 softmmu/memory.c      | 9 +++++++++
 2 files changed, 14 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index f1bb2a7df5..975a90c871 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -688,6 +688,11 @@ static inline FlatView *address_space_to_flatview(AddressSpace *as)
     return atomic_rcu_read(&as->current_map);
 }
 
+typedef int (*flatview_cb)(ram_addr_t start,
+                           ram_addr_t len,
+                           const MemoryRegion*, void*);
+
+void flatview_for_each_range(FlatView *fv, flatview_cb cb , void *opaque);
 
 /**
  * MemoryRegionSection: describes a fragment of a #MemoryRegion
diff --git a/softmmu/memory.c b/softmmu/memory.c
index d030eb6f7c..9db5fbe43a 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -655,6 +655,15 @@ static void render_memory_region(FlatView *view,
     }
 }
 
+void flatview_for_each_range(FlatView *fv, flatview_cb cb , void *opaque)
+{
+    FlatRange *fr;
+    FOR_EACH_FLAT_RANGE(fr, fv) {
+        if (cb(fr->addr.start, fr->addr.size, fr->mr, opaque))
+            break;
+    }
+}
+
 static MemoryRegion *memory_region_get_flatview_root(MemoryRegion *mr)
 {
     while (mr->enabled) {
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
  2020-09-21  2:24 ` [PATCH v3 01/16] memory: Add FlatView foreach function Alexander Bulekov
@ 2020-09-21  2:24 ` Alexander Bulekov
  2020-09-21  5:43   ` Philippe Mathieu-Daudé
  2020-09-22 14:03   ` Alexander Bulekov
  2020-09-21  2:24 ` [PATCH v3 03/16] fuzz: Add PCI features to the general fuzzer Alexander Bulekov
                   ` (24 subsequent siblings)
  26 siblings, 2 replies; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21  2:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Alexander Bulekov, darren.kenny,
	bsd, stefanha, Paolo Bonzini, philmd

This is a generic fuzzer designed to fuzz a virtual device's
MemoryRegions, as long as they exist within the Memory or Port IO (if it
exists) AddressSpaces. The fuzzer's input is interpreted into a sequence
of qtest commands (outb, readw, etc). The interpreted commands are
separated by a magic seaparator, which should be easy for the fuzzer to
guess. Without ASan, the separator can be specified as a "dictionary
value" using the -dict argument (see libFuzzer documentation).

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 tests/qtest/fuzz/general_fuzz.c | 498 ++++++++++++++++++++++++++++++++
 tests/qtest/fuzz/meson.build    |   1 +
 2 files changed, 499 insertions(+)
 create mode 100644 tests/qtest/fuzz/general_fuzz.c

diff --git a/tests/qtest/fuzz/general_fuzz.c b/tests/qtest/fuzz/general_fuzz.c
new file mode 100644
index 0000000000..bf75b215ca
--- /dev/null
+++ b/tests/qtest/fuzz/general_fuzz.c
@@ -0,0 +1,498 @@
+/*
+ * General Virtual-Device Fuzzing Target
+ *
+ * Copyright Red Hat Inc., 2020
+ *
+ * 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 <wordexp.h>
+
+#include "hw/core/cpu.h"
+#include "tests/qtest/libqos/libqtest.h"
+#include "fuzz.h"
+#include "fork_fuzz.h"
+#include "exec/address-spaces.h"
+#include "string.h"
+#include "exec/memory.h"
+#include "exec/ramblock.h"
+#include "exec/address-spaces.h"
+#include "hw/qdev-core.h"
+
+/*
+ * SEPARATOR is used to separate "operations" in the fuzz input
+ */
+#define SEPARATOR "FUZZ"
+
+enum cmds {
+    OP_IN,
+    OP_OUT,
+    OP_READ,
+    OP_WRITE,
+    OP_CLOCK_STEP,
+};
+
+#define DEFAULT_TIMEOUT_US 100000
+#define USEC_IN_SEC 100000000
+
+typedef struct {
+    ram_addr_t addr;
+    ram_addr_t size; /* The number of bytes until the end of the I/O region */
+} address_range;
+
+static useconds_t timeout = 100000;
+
+static bool qtest_log_enabled;
+
+/*
+ * List of memory regions that are children of QOM objects specified by the
+ * user for fuzzing.
+ */
+static GHashTable *fuzzable_memoryregions;
+
+struct get_io_cb_info {
+    int index;
+    int found;
+    address_range result;
+};
+
+static int get_io_address_cb(ram_addr_t start, ram_addr_t size,
+                          const MemoryRegion *mr, void *opaque) {
+    struct get_io_cb_info *info = opaque;
+    if (g_hash_table_lookup(fuzzable_memoryregions, mr)) {
+        if (info->index == 0) {
+            info->result.addr = start;
+            info->result.size = size;
+            info->found = 1;
+            return 1;
+        }
+        info->index--;
+    }
+    return 0;
+}
+
+/*
+ * Here we want to convert a fuzzer-provided [io-region-index, offset] to
+ * a physical address. To do this, we iterate over all of the matched
+ * MemoryRegions. Check whether each region exists within the particular io
+ * space. Return the absolute address of the offset within the index'th region
+ * that is a subregion of the io_space and the distance until the end of the
+ * memory region.
+ */
+static bool get_io_address(address_range *result, AddressSpace *as,
+                            uint8_t index,
+                            uint32_t offset) {
+    FlatView *view;
+    view = as->current_map;
+    g_assert(view);
+    struct get_io_cb_info cb_info = {};
+
+    cb_info.index = index;
+
+    /*
+     * Loop around the FlatView until we match "index" number of
+     * fuzzable_memoryregions, or until we know that there are no matching
+     * memory_regions.
+     */
+    do {
+        flatview_for_each_range(view, get_io_address_cb , &cb_info);
+    } while (cb_info.index != index && !cb_info.found);
+
+    *result = cb_info.result;
+    return cb_info.found;
+}
+static bool get_pio_address(address_range *result,
+                            uint8_t index, uint16_t offset)
+{
+    /*
+     * PIO BARs can be set past the maximum port address (0xFFFF). Thus, result
+     * can contain an addr that extends past the PIO space. When we pass this
+     * address to qtest_in/qtest_out, it is cast to a uint16_t, so we might end
+     * up fuzzing a completely different MemoryRegion/Device. Therefore, check
+     * that the address here is within the PIO space limits.
+     */
+    bool found = get_io_address(result, &address_space_io, index, offset);
+    return result->addr <= 0xFFFF ? found : false;
+}
+static bool get_mmio_address(address_range *result,
+                             uint8_t index, uint32_t offset)
+{
+    return get_io_address(result, &address_space_memory, index, offset);
+}
+
+static void op_in(QTestState *s, const unsigned char * data, size_t len)
+{
+    enum Sizes {Byte, Word, Long, end_sizes};
+    struct {
+        uint8_t size;
+        uint8_t base;
+        uint16_t offset;
+    } a;
+    address_range abs;
+
+    if (len < sizeof(a)) {
+        return;
+    }
+    memcpy(&a, data, sizeof(a));
+    if (get_pio_address(&abs, a.base, a.offset) == 0) {
+        return;
+    }
+
+    switch (a.size %= end_sizes) {
+    case Byte:
+        qtest_inb(s, abs.addr);
+        break;
+    case Word:
+        if (abs.size >= 2) {
+            qtest_inw(s, abs.addr);
+        }
+        break;
+    case Long:
+        if (abs.size >= 4) {
+            qtest_inl(s, abs.addr);
+        }
+        break;
+    }
+}
+
+static void op_out(QTestState *s, const unsigned char * data, size_t len)
+{
+    enum Sizes {Byte, Word, Long, end_sizes};
+    struct {
+        uint8_t size;
+        uint8_t base;
+        uint16_t offset;
+        uint32_t value;
+    } a;
+    address_range abs;
+
+    if (len < sizeof(a)) {
+        return;
+    }
+    memcpy(&a, data, sizeof(a));
+
+    if (get_pio_address(&abs, a.base, a.offset) == 0) {
+        return;
+    }
+
+    switch (a.size %= end_sizes) {
+    case Byte:
+        qtest_outb(s, abs.addr, a.value & 0xFF);
+        break;
+    case Word:
+        if (abs.size >= 2) {
+            qtest_outw(s, abs.addr, a.value & 0xFFFF);
+        }
+        break;
+    case Long:
+        if (abs.size >= 4) {
+            qtest_outl(s, abs.addr, a.value);
+        }
+        break;
+    }
+}
+
+static void op_read(QTestState *s, const unsigned char * data, size_t len)
+{
+    enum Sizes {Byte, Word, Long, Quad, end_sizes};
+    struct {
+        uint8_t size;
+        uint8_t base;
+        uint32_t offset;
+    } a;
+    address_range abs;
+
+    if (len < sizeof(a)) {
+        return;
+    }
+    memcpy(&a, data, sizeof(a));
+
+    if (get_mmio_address(&abs, a.base, a.offset) == 0) {
+        return;
+    }
+
+    switch (a.size %= end_sizes) {
+    case Byte:
+        qtest_readb(s, abs.addr);
+        break;
+    case Word:
+        if (abs.size >= 2) {
+            qtest_readw(s, abs.addr);
+        }
+        break;
+    case Long:
+        if (abs.size >= 4) {
+            qtest_readl(s, abs.addr);
+        }
+        break;
+    case Quad:
+        if (abs.size >= 8) {
+            qtest_readq(s, abs.addr);
+        }
+        break;
+    }
+}
+
+static void op_write(QTestState *s, const unsigned char * data, size_t len)
+{
+    enum Sizes {Byte, Word, Long, Quad, end_sizes};
+    struct {
+        uint8_t size;
+        uint8_t base;
+        uint32_t offset;
+        uint64_t value;
+    } a;
+    address_range abs;
+
+    if (len < sizeof(a)) {
+        return;
+    }
+    memcpy(&a, data, sizeof(a));
+
+    if (get_mmio_address(&abs, a.base, a.offset) == 0) {
+        return;
+    }
+
+    switch (a.size %= end_sizes) {
+    case Byte:
+            qtest_writeb(s, abs.addr, a.value & 0xFF);
+        break;
+    case Word:
+        if (abs.size >= 2) {
+            qtest_writew(s, abs.addr, a.value & 0xFFFF);
+        }
+        break;
+    case Long:
+        if (abs.size >= 4) {
+            qtest_writel(s, abs.addr, a.value & 0xFFFFFFFF);
+        }
+        break;
+    case Quad:
+        if (abs.size >= 8) {
+            qtest_writeq(s, abs.addr, a.value);
+        }
+        break;
+    }
+}
+static void op_clock_step(QTestState *s, const unsigned char *data, size_t len)
+{
+    qtest_clock_step_next(s);
+}
+
+static void handle_timeout(int sig)
+{
+    if (qtest_log_enabled) {
+        fprintf(stderr, "[Timeout]\n");
+        fflush(stderr);
+    }
+    _Exit(0);
+}
+
+/*
+ * Here, we interpret random bytes from the fuzzer, as a sequence of commands.
+ * Our commands are variable-width, so we use a separator, SEPARATOR, to specify
+ * the boundaries between commands. This is just a random 32-bit value, which
+ * is easily identified by libfuzzer+AddressSanitizer, as long as we use
+ * memmem. It can also be included in the fuzzer's dictionary. More details
+ * here:
+ * https://github.com/google/fuzzing/blob/master/docs/split-inputs.md
+ *
+ * As a result, the stream of bytes is converted into a sequence of commands.
+ * In a simplified example where SEPARATOR is 0xFF:
+ * 00 01 02 FF 03 04 05 06 FF 01 FF ...
+ * becomes this sequence of commands:
+ * 00 01 02    -> op00 (0102)   -> in (0102, 2)
+ * 03 04 05 06 -> op03 (040506) -> write (040506, 3)
+ * 01          -> op01 (-,0)    -> out (-,0)
+ * ...
+ *
+ * Note here that it is the job of the individual opcode functions to check
+ * that enough data was provided. I.e. in the last command out (,0), out needs
+ * to check that there is not enough data provided to select an address/value
+ * for the operation.
+ */
+static void general_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
+{
+    void (*ops[]) (QTestState *s, const unsigned char* , size_t) = {
+        [OP_IN]                 = op_in,
+        [OP_OUT]                = op_out,
+        [OP_READ]               = op_read,
+        [OP_WRITE]              = op_write,
+        [OP_CLOCK_STEP]         = op_clock_step,
+    };
+    const unsigned char *cmd = Data;
+    const unsigned char *nextcmd;
+    size_t cmd_len;
+    uint8_t op;
+
+    if (fork() == 0) {
+        /*
+         * 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 to avoid hurting performance, too much
+         */
+        if (timeout) {
+            struct sigaction sact;
+            struct itimerval timer;
+
+            sigemptyset(&sact.sa_mask);
+            sact.sa_flags   = SA_NODEFER;
+            sact.sa_handler = handle_timeout;
+            sigaction(SIGALRM, &sact, NULL);
+
+            memset(&timer, 0, sizeof(timer));
+            timer.it_value.tv_sec = timeout / USEC_IN_SEC;
+            timer.it_value.tv_usec = timeout % USEC_IN_SEC;
+            setitimer(ITIMER_VIRTUAL, &timer, NULL);
+        }
+
+        while (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);
+
+                /* 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);
+        }
+        _Exit(0);
+    } else {
+        flush_events(s);
+        wait(0);
+    }
+}
+
+static void usage(void)
+{
+    printf("Please specify the following environment variables:\n");
+    printf("QEMU_FUZZ_ARGS= the command line arguments passed to qemu\n");
+    printf("QEMU_FUZZ_OBJECTS= "
+            "a space separated list of QOM type names for objects to fuzz\n");
+    printf("Optionally: QEMU_FUZZ_TIMEOUT= Specify a custom timeout (us). "
+            "0 to disable. %d by default\n", timeout);
+    exit(0);
+}
+
+static int locate_fuzz_memory_regions(Object *child, void *opaque)
+{
+    const char *name;
+    MemoryRegion *mr;
+    if (object_dynamic_cast(child, TYPE_MEMORY_REGION)) {
+        mr = MEMORY_REGION(child);
+        if ((memory_region_is_ram(mr) ||
+            memory_region_is_ram_device(mr) ||
+            memory_region_is_rom(mr) ||
+            memory_region_is_romd(mr)) == false) {
+            name = object_get_canonical_path_component(child);
+            /*
+             * We don't want duplicate pointers to the same MemoryRegion, so
+             * try to remove copies of the pointer, before adding it.
+             */
+            g_hash_table_insert(fuzzable_memoryregions, mr, (gpointer)true);
+        }
+    }
+    return 0;
+}
+static int locate_fuzz_objects(Object *child, void *opaque)
+{
+    char *pattern = opaque;
+    if (g_pattern_match_simple(pattern, object_get_typename(child))) {
+        /* Find and save ptrs to any child MemoryRegions */
+        object_child_foreach_recursive(child, locate_fuzz_memory_regions, NULL);
+
+    } else if (object_dynamic_cast(OBJECT(child), TYPE_MEMORY_REGION)) {
+        if (g_pattern_match_simple(pattern,
+            object_get_canonical_path_component(child))) {
+            MemoryRegion *mr;
+            mr = MEMORY_REGION(child);
+            if ((memory_region_is_ram(mr) ||
+                 memory_region_is_ram_device(mr) ||
+                 memory_region_is_rom(mr) ||
+                 memory_region_is_romd(mr)) == false) {
+                g_hash_table_insert(fuzzable_memoryregions, mr, (gpointer)true);
+            }
+        }
+    }
+    return 0;
+}
+
+static void general_pre_fuzz(QTestState *s)
+{
+    GHashTableIter iter;
+    MemoryRegion *mr;
+    char **result;
+
+    if (!getenv("QEMU_FUZZ_OBJECTS")) {
+        usage();
+    }
+    if (getenv("QTEST_LOG")) {
+        qtest_log_enabled = 1;
+    }
+    if (getenv("QEMU_FUZZ_TIMEOUT")) {
+        timeout = g_ascii_strtoll(getenv("QEMU_FUZZ_TIMEOUT"), NULL, 0);
+    }
+
+    fuzzable_memoryregions = g_hash_table_new(NULL, NULL);
+
+    result = g_strsplit(getenv("QEMU_FUZZ_OBJECTS"), " ", -1);
+    for (int i = 0; result[i] != NULL; i++) {
+        printf("Matching objects by name %s\n", result[i]);
+        object_child_foreach_recursive(qdev_get_machine(),
+                                    locate_fuzz_objects,
+                                    result[i]);
+    }
+    g_strfreev(result);
+    printf("This process will try to fuzz the following MemoryRegions:\n");
+
+    g_hash_table_iter_init(&iter, fuzzable_memoryregions);
+    while (g_hash_table_iter_next(&iter, (gpointer)&mr, NULL)) {
+        printf("  * %s (size %lx)\n",
+               object_get_canonical_path_component(&(mr->parent_obj)),
+               mr->addr);
+    }
+
+    if (!g_hash_table_size(fuzzable_memoryregions)) {
+        printf("No fuzzable memory regions found...\n");
+        exit(1);
+    }
+
+    counter_shm_init();
+}
+static GString *general_fuzz_cmdline(FuzzTarget *t)
+{
+    GString *cmd_line = g_string_new(TARGET_NAME);
+    if (!getenv("QEMU_FUZZ_ARGS")) {
+        usage();
+    }
+    g_string_append_printf(cmd_line, " -display none \
+                                      -machine accel=qtest, \
+                                      -m 64 %s ", getenv("QEMU_FUZZ_ARGS"));
+    return cmd_line;
+}
+
+static void register_general_fuzz_targets(void)
+{
+    fuzz_add_target(&(FuzzTarget){
+            .name = "general-fuzz",
+            .description = "Fuzz based on any qemu command-line args. ",
+            .get_init_cmdline = general_fuzz_cmdline,
+            .pre_fuzz = general_pre_fuzz,
+            .fuzz = general_fuzz});
+}
+
+fuzz_target_init(register_general_fuzz_targets);
diff --git a/tests/qtest/fuzz/meson.build b/tests/qtest/fuzz/meson.build
index b31ace7d5a..a59de6aa8c 100644
--- a/tests/qtest/fuzz/meson.build
+++ b/tests/qtest/fuzz/meson.build
@@ -5,6 +5,7 @@ specific_fuzz_ss.add(files('fuzz.c', 'fork_fuzz.c', 'qos_fuzz.c',
 specific_fuzz_ss.add(when: 'CONFIG_I440FX', if_true: files('i440fx_fuzz.c'))
 specific_fuzz_ss.add(when: 'CONFIG_VIRTIO_NET', if_true: files('virtio_net_fuzz.c'))
 specific_fuzz_ss.add(when: 'CONFIG_VIRTIO_SCSI', if_true: files('virtio_scsi_fuzz.c'))
+specific_fuzz_ss.add(files('general_fuzz.c'))
 
 fork_fuzz = declare_dependency(
   link_args: config_host['FUZZ_EXE_LDFLAGS'].split() +
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v3 03/16] fuzz: Add PCI features to the general fuzzer
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
  2020-09-21  2:24 ` [PATCH v3 01/16] memory: Add FlatView foreach function Alexander Bulekov
  2020-09-21  2:24 ` [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer Alexander Bulekov
@ 2020-09-21  2:24 ` Alexander Bulekov
  2020-09-21  5:44   ` Philippe Mathieu-Daudé
  2020-09-21  2:24 ` [PATCH v3 04/16] fuzz: Add DMA support to the generic-fuzzer Alexander Bulekov
                   ` (23 subsequent siblings)
  26 siblings, 1 reply; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21  2:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Alexander Bulekov, darren.kenny,
	bsd, stefanha, Paolo Bonzini, philmd

This patch compares TYPE_PCI_DEVICE objects against the user-provided
matching pattern. If there is a match, we use some hacks and leverage
QOS to map each possible BAR for that device. Now fuzzed inputs might be
converted to pci_read/write commands which target specific. This means
that we can fuzz a particular device's PCI configuration space,

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
---
 tests/qtest/fuzz/general_fuzz.c | 81 +++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/tests/qtest/fuzz/general_fuzz.c b/tests/qtest/fuzz/general_fuzz.c
index bf75b215ca..7c4c1398a7 100644
--- a/tests/qtest/fuzz/general_fuzz.c
+++ b/tests/qtest/fuzz/general_fuzz.c
@@ -24,6 +24,7 @@
 #include "exec/ramblock.h"
 #include "exec/address-spaces.h"
 #include "hw/qdev-core.h"
+#include "hw/pci/pci.h"
 
 /*
  * SEPARATOR is used to separate "operations" in the fuzz input
@@ -35,12 +36,17 @@ enum cmds {
     OP_OUT,
     OP_READ,
     OP_WRITE,
+    OP_PCI_READ,
+    OP_PCI_WRITE,
     OP_CLOCK_STEP,
 };
 
 #define DEFAULT_TIMEOUT_US 100000
 #define USEC_IN_SEC 100000000
 
+#define PCI_HOST_BRIDGE_CFG 0xcf8
+#define PCI_HOST_BRIDGE_DATA 0xcfc
+
 typedef struct {
     ram_addr_t addr;
     ram_addr_t size; /* The number of bytes until the end of the I/O region */
@@ -55,6 +61,7 @@ static bool qtest_log_enabled;
  * user for fuzzing.
  */
 static GHashTable *fuzzable_memoryregions;
+static GPtrArray *fuzzable_pci_devices;
 
 struct get_io_cb_info {
     int index;
@@ -280,6 +287,65 @@ static void op_write(QTestState *s, const unsigned char * data, size_t len)
         break;
     }
 }
+static void op_pci_read(QTestState *s, const unsigned char * data, size_t len)
+{
+    enum Sizes {Byte, Word, Long, end_sizes};
+    struct {
+        uint8_t size;
+        uint8_t base;
+        uint8_t offset;
+    } a;
+    if (len < sizeof(a) || fuzzable_pci_devices->len == 0) {
+        return;
+    }
+    memcpy(&a, data, sizeof(a));
+    PCIDevice *dev = g_ptr_array_index(fuzzable_pci_devices,
+                                  a.base % fuzzable_pci_devices->len);
+    int devfn = dev->devfn;
+    qtest_outl(s, PCI_HOST_BRIDGE_CFG, (1U << 31) | (devfn << 8) | a.offset);
+    switch (a.size %= end_sizes) {
+    case Byte:
+        qtest_inb(s, PCI_HOST_BRIDGE_DATA);
+        break;
+    case Word:
+        qtest_inw(s, PCI_HOST_BRIDGE_DATA);
+        break;
+    case Long:
+        qtest_inl(s, PCI_HOST_BRIDGE_DATA);
+        break;
+    }
+}
+
+static void op_pci_write(QTestState *s, const unsigned char * data, size_t len)
+{
+    enum Sizes {Byte, Word, Long, end_sizes};
+    struct {
+        uint8_t size;
+        uint8_t base;
+        uint8_t offset;
+        uint32_t value;
+    } a;
+    if (len < sizeof(a) || fuzzable_pci_devices->len == 0) {
+        return;
+    }
+    memcpy(&a, data, sizeof(a));
+    PCIDevice *dev = g_ptr_array_index(fuzzable_pci_devices,
+                                  a.base % fuzzable_pci_devices->len);
+    int devfn = dev->devfn;
+    qtest_outl(s, PCI_HOST_BRIDGE_CFG, (1U << 31) | (devfn << 8) | a.offset);
+    switch (a.size %= end_sizes) {
+    case Byte:
+        qtest_outb(s, PCI_HOST_BRIDGE_DATA, a.value & 0xFF);
+        break;
+    case Word:
+        qtest_outw(s, PCI_HOST_BRIDGE_DATA, a.value & 0xFFFF);
+        break;
+    case Long:
+        qtest_outl(s, PCI_HOST_BRIDGE_DATA, a.value & 0xFFFFFFFF);
+        break;
+    }
+}
+
 static void op_clock_step(QTestState *s, const unsigned char *data, size_t len)
 {
     qtest_clock_step_next(s);
@@ -324,6 +390,8 @@ static void general_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
         [OP_OUT]                = op_out,
         [OP_READ]               = op_read,
         [OP_WRITE]              = op_write,
+        [OP_PCI_READ]           = op_pci_read,
+        [OP_PCI_WRITE]          = op_pci_write,
         [OP_CLOCK_STEP]         = op_clock_step,
     };
     const unsigned char *cmd = Data;
@@ -415,6 +483,18 @@ static int locate_fuzz_objects(Object *child, void *opaque)
         /* Find and save ptrs to any child MemoryRegions */
         object_child_foreach_recursive(child, locate_fuzz_memory_regions, NULL);
 
+        /*
+         * We matched an object. If its a PCI device, store a pointer to it so
+         * we can map BARs and fuzz its config space.
+         */
+        if (object_dynamic_cast(OBJECT(child), TYPE_PCI_DEVICE)) {
+            /*
+             * Don't want duplicate pointers to the same PCIDevice, so remove
+             * copies of the pointer, before adding it.
+             */
+            g_ptr_array_remove_fast(fuzzable_pci_devices, PCI_DEVICE(child));
+            g_ptr_array_add(fuzzable_pci_devices, PCI_DEVICE(child));
+        }
     } else if (object_dynamic_cast(OBJECT(child), TYPE_MEMORY_REGION)) {
         if (g_pattern_match_simple(pattern,
             object_get_canonical_path_component(child))) {
@@ -448,6 +528,7 @@ static void general_pre_fuzz(QTestState *s)
     }
 
     fuzzable_memoryregions = g_hash_table_new(NULL, NULL);
+    fuzzable_pci_devices   = g_ptr_array_new();
 
     result = g_strsplit(getenv("QEMU_FUZZ_OBJECTS"), " ", -1);
     for (int i = 0; result[i] != NULL; i++) {
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v3 04/16] fuzz: Add DMA support to the generic-fuzzer
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (2 preceding siblings ...)
  2020-09-21  2:24 ` [PATCH v3 03/16] fuzz: Add PCI features to the general fuzzer Alexander Bulekov
@ 2020-09-21  2:24 ` Alexander Bulekov
  2020-10-08  7:43   ` Paolo Bonzini
  2020-09-21  2:24 ` [PATCH v3 05/16] fuzz: Declare DMA Read callback function Alexander Bulekov
                   ` (22 subsequent siblings)
  26 siblings, 1 reply; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21  2:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Alexander Bulekov, darren.kenny,
	bsd, stefanha, Paolo Bonzini, philmd

When a virtual-device tries to access some buffer in memory over DMA, we
add call-backs into the fuzzer(next commit). The fuzzer checks verifies
that the DMA request maps to a physical RAM address and fills the memory
with fuzzer-provided data. The patterns that we use to fill this memory
are specified using add_dma_pattern and clear_dma_patterns operations.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
---
 tests/qtest/fuzz/general_fuzz.c | 178 ++++++++++++++++++++++++++++++++
 1 file changed, 178 insertions(+)

diff --git a/tests/qtest/fuzz/general_fuzz.c b/tests/qtest/fuzz/general_fuzz.c
index 7c4c1398a7..5e42504821 100644
--- a/tests/qtest/fuzz/general_fuzz.c
+++ b/tests/qtest/fuzz/general_fuzz.c
@@ -25,6 +25,7 @@
 #include "exec/address-spaces.h"
 #include "hw/qdev-core.h"
 #include "hw/pci/pci.h"
+#include "hw/boards.h"
 
 /*
  * SEPARATOR is used to separate "operations" in the fuzz input
@@ -38,12 +39,16 @@ enum cmds {
     OP_WRITE,
     OP_PCI_READ,
     OP_PCI_WRITE,
+    OP_ADD_DMA_PATTERN,
+    OP_CLEAR_DMA_PATTERNS,
     OP_CLOCK_STEP,
 };
 
 #define DEFAULT_TIMEOUT_US 100000
 #define USEC_IN_SEC 100000000
 
+#define MAX_DMA_FILL_SIZE 0x10000
+
 #define PCI_HOST_BRIDGE_CFG 0xcf8
 #define PCI_HOST_BRIDGE_DATA 0xcfc
 
@@ -56,6 +61,24 @@ static useconds_t timeout = 100000;
 
 static bool qtest_log_enabled;
 
+/*
+ * A pattern used to populate a DMA region or perform a memwrite. This is
+ * useful for e.g. populating tables of unique addresses.
+ * Example {.index = 1; .stride = 2; .len = 3; .data = "\x00\x01\x02"}
+ * Renders as: 00 01 02   00 03 02   00 05 02   00 07 02 ...
+ */
+typedef struct {
+    uint8_t index;      /* Index of a byte to increment by stride */
+    uint8_t stride;     /* Increment each index'th byte by this amount */
+    size_t len;
+    const uint8_t *data;
+} pattern;
+
+/* Avoid filling the same DMA region between MMIO/PIO commands ? */
+static bool avoid_double_fetches;
+
+static QTestState *qts_global; /* Need a global for the DMA callback */
+
 /*
  * List of memory regions that are children of QOM objects specified by the
  * user for fuzzing.
@@ -84,6 +107,117 @@ static int get_io_address_cb(ram_addr_t start, ram_addr_t size,
     return 0;
 }
 
+/*
+ * List of dma regions populated since the last fuzzing command. Used to ensure
+ * that we only write to each DMA address once, to avoid race conditions when
+ * building reproducers.
+ */
+static GArray *dma_regions;
+
+static GArray *dma_patterns;
+static int dma_pattern_index;
+
+void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr, bool is_write);
+
+/*
+ * Allocate a block of memory and populate it with a pattern.
+ */
+static void *pattern_alloc(pattern p, size_t len)
+{
+    int i;
+    uint8_t *buf = g_malloc(len);
+    uint8_t sum = 0;
+
+    for (i = 0; i < len; ++i) {
+        buf[i] = p.data[i % p.len];
+        if ((i % p.len) == p.index) {
+            buf[i] += sum;
+            sum += p.stride;
+        }
+    }
+    return buf;
+}
+
+/*
+ * Call-back for functions that perform DMA reads from guest memory. Confirm
+ * that the region has not already been populated since the last loop in
+ * general_fuzz(), avoiding potential race-conditions, which we don't have
+ * a good way for reproducing right now.
+ */
+void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr, bool is_write)
+{
+    /* Are we in the general-fuzzer or are we using another fuzz-target? */
+    if (!qts_global) {
+        return;
+    }
+
+    /*
+     * Return immediately if:
+     * - We have no DMA patterns defined
+     * - The length of the DMA read request is zero
+     * - The DMA read is hitting an MR other than the machine's main RAM
+     * - The DMA request is not a read (what happens for a address_space_map
+     *   with is_write=True? Can the device use the same pointer to do reads?)
+     * - The DMA request hits past the bounds of our RAM
+     */
+    if (dma_patterns->len == 0
+        || len == 0
+        || mr != MACHINE(qdev_get_machine())->ram
+        || is_write
+        || addr > current_machine->ram_size) {
+        return;
+    }
+
+    /*
+     * If we overlap with any existing dma_regions, split the range and only
+     * populate the non-overlapping parts.
+     */
+    address_range region;
+    for (int i = 0; i < dma_regions->len && avoid_double_fetches; ++i) {
+        region = g_array_index(dma_regions, address_range, i);
+        if (addr < region.addr + region.size && addr + len > region.addr) {
+            if (addr < region.addr) {
+                fuzz_dma_read_cb(addr, region.addr - addr, mr, is_write);
+            }
+            if (addr + len > region.addr + region.size) {
+                fuzz_dma_read_cb(region.addr + region.size,
+                        addr + len - (region.addr + region.size), mr, is_write);
+            }
+            return;
+        }
+    }
+
+    /* Cap the length of the DMA access to something reasonable */
+    len = MIN(len, MAX_DMA_FILL_SIZE);
+
+    address_range ar = {addr, len};
+    g_array_append_val(dma_regions, ar);
+    pattern p = g_array_index(dma_patterns, pattern, dma_pattern_index);
+    void *buf = pattern_alloc(p, ar.size);
+    if (qtest_log_enabled) {
+        /*
+         * With QTEST_LOG, use a normal, slow QTest memwrite. Prefix the log
+         * that will be written by qtest.c with a DMA tag, so we can reorder
+         * the resulting QTest trace so the DMA fills precede the last PIO/MMIO
+         * command.
+         */
+        fprintf(stderr, "[DMA] ");
+        fflush(stderr);
+        qtest_memwrite(qts_global, ar.addr, buf, ar.size);
+    } else {
+       /*
+        * Populate the region using address_space_write_rom to avoid writing to
+        * any IO MemoryRegions
+        */
+        address_space_write_rom(first_cpu->as, ar.addr, MEMTXATTRS_UNSPECIFIED,
+                buf, ar.size);
+    }
+    g_free(buf);
+
+    /* Increment the index of the pattern for the next DMA access */
+    dma_pattern_index = (dma_pattern_index + 1) % dma_patterns->len;
+}
+
 /*
  * Here we want to convert a fuzzer-provided [io-region-index, offset] to
  * a physical address. To do this, we iterate over all of the matched
@@ -346,6 +480,35 @@ static void op_pci_write(QTestState *s, const unsigned char * data, size_t len)
     }
 }
 
+static void op_add_dma_pattern(QTestState *s,
+                               const unsigned char *data, size_t len)
+{
+    struct {
+        /*
+         * index and stride can be used to increment the index-th byte of the
+         * pattern by the value stride, for each loop of the pattern.
+         */
+        uint8_t index;
+        uint8_t stride;
+    } a;
+
+    if (len < sizeof(a) + 1) {
+        return;
+    }
+    memcpy(&a, data, sizeof(a));
+    pattern p = {a.index, a.stride, len - sizeof(a), data + sizeof(a)};
+    p.index = a.index % p.len;
+    g_array_append_val(dma_patterns, p);
+    return;
+}
+
+static void op_clear_dma_patterns(QTestState *s,
+                                  const unsigned char *data, size_t len)
+{
+    g_array_set_size(dma_patterns, 0);
+    dma_pattern_index = 0;
+}
+
 static void op_clock_step(QTestState *s, const unsigned char *data, size_t len)
 {
     qtest_clock_step_next(s);
@@ -392,6 +555,8 @@ static void general_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
         [OP_WRITE]              = op_write,
         [OP_PCI_READ]           = op_pci_read,
         [OP_PCI_WRITE]          = op_pci_write,
+        [OP_ADD_DMA_PATTERN]    = op_add_dma_pattern,
+        [OP_CLEAR_DMA_PATTERNS] = op_clear_dma_patterns,
         [OP_CLOCK_STEP]         = op_clock_step,
     };
     const unsigned char *cmd = Data;
@@ -421,6 +586,8 @@ static void general_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
             setitimer(ITIMER_VIRTUAL, &timer, NULL);
         }
 
+        op_clear_dma_patterns(s, NULL, 0);
+
         while (cmd && Size) {
             /* Get the length until the next command or end of input */
             nextcmd = memmem(cmd, Size, SEPARATOR, strlen(SEPARATOR));
@@ -437,6 +604,7 @@ static void general_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
             /* 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);
         }
         _Exit(0);
     } else {
@@ -451,6 +619,9 @@ static void usage(void)
     printf("QEMU_FUZZ_ARGS= the command line arguments passed to qemu\n");
     printf("QEMU_FUZZ_OBJECTS= "
             "a space separated list of QOM type names for objects to fuzz\n");
+    printf("Optionally: QEMU_AVOID_DOUBLE_FETCH= "
+            "Try to avoid racy DMA double fetch bugs? %d by default\n",
+            avoid_double_fetches);
     printf("Optionally: QEMU_FUZZ_TIMEOUT= Specify a custom timeout (us). "
             "0 to disable. %d by default\n", timeout);
     exit(0);
@@ -523,9 +694,16 @@ static void general_pre_fuzz(QTestState *s)
     if (getenv("QTEST_LOG")) {
         qtest_log_enabled = 1;
     }
+    if (getenv("QEMU_AVOID_DOUBLE_FETCH")) {
+        avoid_double_fetches = 1;
+    }
     if (getenv("QEMU_FUZZ_TIMEOUT")) {
         timeout = g_ascii_strtoll(getenv("QEMU_FUZZ_TIMEOUT"), NULL, 0);
     }
+    qts_global = s;
+
+    dma_regions = g_array_new(false, false, sizeof(address_range));
+    dma_patterns = g_array_new(false, false, sizeof(pattern));
 
     fuzzable_memoryregions = g_hash_table_new(NULL, NULL);
     fuzzable_pci_devices   = g_ptr_array_new();
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v3 05/16] fuzz: Declare DMA Read callback function
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (3 preceding siblings ...)
  2020-09-21  2:24 ` [PATCH v3 04/16] fuzz: Add DMA support to the generic-fuzzer Alexander Bulekov
@ 2020-09-21  2:24 ` Alexander Bulekov
  2020-10-08  7:39   ` Paolo Bonzini
  2020-09-21  2:24 ` [PATCH v3 06/16] fuzz: Add fuzzer callbacks to DMA-read functions Alexander Bulekov
                   ` (21 subsequent siblings)
  26 siblings, 1 reply; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21  2:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alexander Bulekov, darren.kenny, bsd, stefanha, Paolo Bonzini, philmd

This patch declares the fuzz_dma_read_cb function and uses the
preprocessor and linker(weak symbols) to handle these cases:

When we build softmmu/all with --enable-fuzzing, there should be no
strong symbol defined for fuzz_dma_read_cb, and we link against a weak
stub function.

When we build softmmu/fuzz with --enable-fuzzing, we link against the
strong symbol in general_fuzz.c

When we build softmmu/all without --enable-fuzzing, fuzz_dma_read_cb is
an empty, inlined function. As long as we don't call any other functions
when building the arguments, there should be no overhead.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
---
 include/exec/memory.h | 15 +++++++++++++++
 softmmu/memory.c      | 13 +++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 975a90c871..d5511c7222 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -42,6 +42,21 @@ typedef struct IOMMUMemoryRegionClass IOMMUMemoryRegionClass;
 DECLARE_OBJ_CHECKERS(IOMMUMemoryRegion, IOMMUMemoryRegionClass,
                      IOMMU_MEMORY_REGION, TYPE_IOMMU_MEMORY_REGION)
 
+#ifdef CONFIG_FUZZ
+void fuzz_dma_read_cb(size_t addr,
+                      size_t len,
+                      MemoryRegion *mr,
+                      bool is_write);
+#else
+static inline void fuzz_dma_read_cb(size_t addr,
+                                    size_t len,
+                                    MemoryRegion *mr,
+                                    bool is_write)
+{
+    /* Do Nothing */
+}
+#endif
+
 extern bool global_dirty_log;
 
 typedef struct MemoryRegionOps MemoryRegionOps;
diff --git a/softmmu/memory.c b/softmmu/memory.c
index 9db5fbe43a..24e59593ca 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -3232,6 +3232,19 @@ void memory_region_init_rom_device(MemoryRegion *mr,
     vmstate_register_ram(mr, owner_dev);
 }
 
+/*
+ * Support softmmu builds with CONFIG_FUZZ using a weak symbol and a stub for
+ * the fuzz_dma_read_cb callback
+ */
+#ifdef CONFIG_FUZZ
+void __attribute__((weak)) fuzz_dma_read_cb(size_t addr,
+                      size_t len,
+                      MemoryRegion *mr,
+                      bool is_write)
+{
+}
+#endif
+
 static const TypeInfo memory_region_info = {
     .parent             = TYPE_OBJECT,
     .name               = TYPE_MEMORY_REGION,
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v3 06/16] fuzz: Add fuzzer callbacks to DMA-read functions
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (4 preceding siblings ...)
  2020-09-21  2:24 ` [PATCH v3 05/16] fuzz: Declare DMA Read callback function Alexander Bulekov
@ 2020-09-21  2:24 ` Alexander Bulekov
  2020-09-21  2:24 ` [PATCH v3 07/16] fuzz: Add support for custom crossover functions Alexander Bulekov
                   ` (20 subsequent siblings)
  26 siblings, 0 replies; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21  2:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alexander Bulekov, darren.kenny, bsd, stefanha, Paolo Bonzini,
	philmd, Richard Henderson

We should be careful to not call any functions besides fuzz_dma_read_cb.
Without --enable-fuzzing, fuzz_dma_read_cb is an empty inlined function.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
---
 exec.c                                | 2 ++
 include/exec/memory.h                 | 1 +
 include/exec/memory_ldst_cached.h.inc | 3 +++
 memory_ldst.c.inc                     | 4 ++++
 softmmu/memory.c                      | 1 +
 5 files changed, 11 insertions(+)

diff --git a/exec.c b/exec.c
index e34b602bdf..5803e16be2 100644
--- a/exec.c
+++ b/exec.c
@@ -3242,6 +3242,7 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
             stn_he_p(buf, l, val);
         } else {
             /* RAM case */
+            fuzz_dma_read_cb(addr, len, mr, false);
             ram_ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l, false);
             memcpy(buf, ram_ptr, l);
         }
@@ -3602,6 +3603,7 @@ void *address_space_map(AddressSpace *as,
     memory_region_ref(mr);
     *plen = flatview_extend_translation(fv, addr, len, mr, xlat,
                                         l, is_write, attrs);
+    fuzz_dma_read_cb(addr, *plen, mr, is_write);
     ptr = qemu_ram_ptr_length(mr->ram_block, xlat, plen, true);
 
     return ptr;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index d5511c7222..988c2866cd 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2444,6 +2444,7 @@ address_space_read_cached(MemoryRegionCache *cache, hwaddr addr,
                           void *buf, hwaddr len)
 {
     assert(addr < cache->len && len <= cache->len - addr);
+    fuzz_dma_read_cb(cache->xlat + addr, len, cache->mrs.mr, false);
     if (likely(cache->ptr)) {
         memcpy(buf, cache->ptr + addr, len);
         return MEMTX_OK;
diff --git a/include/exec/memory_ldst_cached.h.inc b/include/exec/memory_ldst_cached.h.inc
index fd4bbb40e7..aff574039f 100644
--- a/include/exec/memory_ldst_cached.h.inc
+++ b/include/exec/memory_ldst_cached.h.inc
@@ -28,6 +28,7 @@ static inline uint32_t ADDRESS_SPACE_LD_CACHED(l)(MemoryRegionCache *cache,
     hwaddr addr, MemTxAttrs attrs, MemTxResult *result)
 {
     assert(addr < cache->len && 4 <= cache->len - addr);
+    fuzz_dma_read_cb(cache->xlat + addr, 4, cache->mrs.mr, false);
     if (likely(cache->ptr)) {
         return LD_P(l)(cache->ptr + addr);
     } else {
@@ -39,6 +40,7 @@ static inline uint64_t ADDRESS_SPACE_LD_CACHED(q)(MemoryRegionCache *cache,
     hwaddr addr, MemTxAttrs attrs, MemTxResult *result)
 {
     assert(addr < cache->len && 8 <= cache->len - addr);
+    fuzz_dma_read_cb(cache->xlat + addr, 8, cache->mrs.mr, false);
     if (likely(cache->ptr)) {
         return LD_P(q)(cache->ptr + addr);
     } else {
@@ -50,6 +52,7 @@ static inline uint32_t ADDRESS_SPACE_LD_CACHED(uw)(MemoryRegionCache *cache,
     hwaddr addr, MemTxAttrs attrs, MemTxResult *result)
 {
     assert(addr < cache->len && 2 <= cache->len - addr);
+    fuzz_dma_read_cb(cache->xlat + addr, 2, cache->mrs.mr, false);
     if (likely(cache->ptr)) {
         return LD_P(uw)(cache->ptr + addr);
     } else {
diff --git a/memory_ldst.c.inc b/memory_ldst.c.inc
index c54aee4a95..8d45d2eeff 100644
--- a/memory_ldst.c.inc
+++ b/memory_ldst.c.inc
@@ -42,6 +42,7 @@ static inline uint32_t glue(address_space_ldl_internal, SUFFIX)(ARG1_DECL,
                                         MO_32 | devend_memop(endian), attrs);
     } else {
         /* RAM case */
+        fuzz_dma_read_cb(addr, 4, mr, false);
         ptr = qemu_map_ram_ptr(mr->ram_block, addr1);
         switch (endian) {
         case DEVICE_LITTLE_ENDIAN:
@@ -110,6 +111,7 @@ static inline uint64_t glue(address_space_ldq_internal, SUFFIX)(ARG1_DECL,
                                         MO_64 | devend_memop(endian), attrs);
     } else {
         /* RAM case */
+        fuzz_dma_read_cb(addr, 8, mr, false);
         ptr = qemu_map_ram_ptr(mr->ram_block, addr1);
         switch (endian) {
         case DEVICE_LITTLE_ENDIAN:
@@ -175,6 +177,7 @@ uint32_t glue(address_space_ldub, SUFFIX)(ARG1_DECL,
         r = memory_region_dispatch_read(mr, addr1, &val, MO_8, attrs);
     } else {
         /* RAM case */
+        fuzz_dma_read_cb(addr, 1, mr, false);
         ptr = qemu_map_ram_ptr(mr->ram_block, addr1);
         val = ldub_p(ptr);
         r = MEMTX_OK;
@@ -212,6 +215,7 @@ static inline uint32_t glue(address_space_lduw_internal, SUFFIX)(ARG1_DECL,
                                         MO_16 | devend_memop(endian), attrs);
     } else {
         /* RAM case */
+        fuzz_dma_read_cb(addr, 2, mr, false);
         ptr = qemu_map_ram_ptr(mr->ram_block, addr1);
         switch (endian) {
         case DEVICE_LITTLE_ENDIAN:
diff --git a/softmmu/memory.c b/softmmu/memory.c
index 24e59593ca..1adce47836 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -1414,6 +1414,7 @@ MemTxResult memory_region_dispatch_read(MemoryRegion *mr,
     unsigned size = memop_size(op);
     MemTxResult r;
 
+    fuzz_dma_read_cb(addr, size, mr, false);
     if (!memory_region_access_valid(mr, addr, size, false, attrs)) {
         *pval = unassigned_mem_read(mr, addr, size);
         return MEMTX_DECODE_ERROR;
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v3 07/16] fuzz: Add support for custom crossover functions
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (5 preceding siblings ...)
  2020-09-21  2:24 ` [PATCH v3 06/16] fuzz: Add fuzzer callbacks to DMA-read functions Alexander Bulekov
@ 2020-09-21  2:24 ` Alexander Bulekov
  2020-09-21  2:24 ` [PATCH v3 08/16] fuzz: add a DISABLE_PCI op to general-fuzzer Alexander Bulekov
                   ` (19 subsequent siblings)
  26 siblings, 0 replies; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21  2:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Alexander Bulekov, darren.kenny,
	bsd, stefanha, Paolo Bonzini, philmd

libfuzzer supports a "custom crossover function". Libfuzzer often tries
to blend two inputs to create a new interesting input. Sometimes, we
have a better idea about how to blend inputs together. This change
allows fuzzers to specify a custom function for blending two inputs
together.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
---
 tests/qtest/fuzz/fuzz.c | 13 +++++++++++++
 tests/qtest/fuzz/fuzz.h | 27 +++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
index 7f266ffc63..87a1b88f9b 100644
--- a/tests/qtest/fuzz/fuzz.c
+++ b/tests/qtest/fuzz/fuzz.c
@@ -118,6 +118,19 @@ static FuzzTarget *fuzz_get_target(char* name)
 }
 
 
+/* Sometimes called by libfuzzer to mutate two inputs into one */
+size_t LLVMFuzzerCustomCrossOver(const uint8_t *data1, size_t size1,
+                                 const uint8_t *data2, size_t size2,
+                                 uint8_t *out, size_t max_out_size,
+                                 unsigned int seed)
+{
+    if (fuzz_target->crossover) {
+        return fuzz_target->crossover(data1, size1, data2, size2, out,
+                                      max_out_size, seed);
+    }
+    return 0;
+}
+
 /* Executed for each fuzzing-input */
 int LLVMFuzzerTestOneInput(const unsigned char *Data, size_t Size)
 {
diff --git a/tests/qtest/fuzz/fuzz.h b/tests/qtest/fuzz/fuzz.h
index 8eb765edc8..ed9ce17154 100644
--- a/tests/qtest/fuzz/fuzz.h
+++ b/tests/qtest/fuzz/fuzz.h
@@ -77,6 +77,29 @@ typedef struct FuzzTarget {
      */
     void(*fuzz)(QTestState *, const unsigned char *, size_t);
 
+    /*
+     * The fuzzer can specify a "Custom Crossover" function for combining two
+     * inputs from the corpus. This function is sometimes called by libfuzzer
+     * when mutating inputs.
+     *
+     * data1: location of first input
+     * size1: length of first input
+     * data1: location of second input
+     * size1: length of second input
+     * out: where to place the resulting, mutated input
+     * max_out_size: the maximum length of the input that can be placed in out
+     * seed: the seed that should be used to make mutations deterministic, when
+     *       needed
+     *
+     * See libfuzzer's LLVMFuzzerCustomCrossOver API for more info.
+     *
+     * Can be NULL
+     */
+    size_t(*crossover)(const uint8_t *data1, size_t size1,
+                       const uint8_t *data2, size_t size2,
+                       uint8_t *out, size_t max_out_size,
+                       unsigned int seed);
+
 } FuzzTarget;
 
 void flush_events(QTestState *);
@@ -91,6 +114,10 @@ void fuzz_qtest_set_serialize(bool option);
  */
 void fuzz_add_target(const FuzzTarget *target);
 
+size_t LLVMFuzzerCustomCrossOver(const uint8_t *data1, size_t size1,
+                                 const uint8_t *data2, size_t size2,
+                                 uint8_t *out, size_t max_out_size,
+                                 unsigned int seed);
 int LLVMFuzzerTestOneInput(const unsigned char *Data, size_t Size);
 int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp);
 
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v3 08/16] fuzz: add a DISABLE_PCI op to general-fuzzer
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (6 preceding siblings ...)
  2020-09-21  2:24 ` [PATCH v3 07/16] fuzz: Add support for custom crossover functions Alexander Bulekov
@ 2020-09-21  2:24 ` Alexander Bulekov
  2020-09-21  2:24 ` [PATCH v3 09/16] fuzz: add a crossover function to generic-fuzzer Alexander Bulekov
                   ` (18 subsequent siblings)
  26 siblings, 0 replies; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21  2:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Alexander Bulekov, darren.kenny,
	bsd, stefanha, Paolo Bonzini, philmd

This new operation is used in the next commit, which concatenates two
fuzzer-generated inputs. With this operation, we can prevent the second
input from clobbering the PCI configuration performed by the first.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
---
 tests/qtest/fuzz/general_fuzz.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/fuzz/general_fuzz.c b/tests/qtest/fuzz/general_fuzz.c
index 5e42504821..656ec7fd55 100644
--- a/tests/qtest/fuzz/general_fuzz.c
+++ b/tests/qtest/fuzz/general_fuzz.c
@@ -39,6 +39,7 @@ enum cmds {
     OP_WRITE,
     OP_PCI_READ,
     OP_PCI_WRITE,
+    OP_DISABLE_PCI,
     OP_ADD_DMA_PATTERN,
     OP_CLEAR_DMA_PATTERNS,
     OP_CLOCK_STEP,
@@ -116,6 +117,7 @@ static GArray *dma_regions;
 
 static GArray *dma_patterns;
 static int dma_pattern_index;
+static bool pci_disabled;
 
 void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr, bool is_write);
 
@@ -429,7 +431,7 @@ static void op_pci_read(QTestState *s, const unsigned char * data, size_t len)
         uint8_t base;
         uint8_t offset;
     } a;
-    if (len < sizeof(a) || fuzzable_pci_devices->len == 0) {
+    if (len < sizeof(a) || fuzzable_pci_devices->len == 0 || pci_disabled) {
         return;
     }
     memcpy(&a, data, sizeof(a));
@@ -459,7 +461,7 @@ static void op_pci_write(QTestState *s, const unsigned char * data, size_t len)
         uint8_t offset;
         uint32_t value;
     } a;
-    if (len < sizeof(a) || fuzzable_pci_devices->len == 0) {
+    if (len < sizeof(a) || fuzzable_pci_devices->len == 0 || pci_disabled) {
         return;
     }
     memcpy(&a, data, sizeof(a));
@@ -514,6 +516,11 @@ static void op_clock_step(QTestState *s, const unsigned char *data, size_t len)
     qtest_clock_step_next(s);
 }
 
+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) {
@@ -555,6 +562,7 @@ static void general_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
         [OP_WRITE]              = op_write,
         [OP_PCI_READ]           = op_pci_read,
         [OP_PCI_WRITE]          = op_pci_write,
+        [OP_DISABLE_PCI]        = op_disable_pci,
         [OP_ADD_DMA_PATTERN]    = op_add_dma_pattern,
         [OP_CLEAR_DMA_PATTERNS] = op_clear_dma_patterns,
         [OP_CLOCK_STEP]         = op_clock_step,
@@ -587,6 +595,7 @@ static void general_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
         }
 
         op_clear_dma_patterns(s, NULL, 0);
+        pci_disabled = false;
 
         while (cmd && Size) {
             /* Get the length until the next command or end of input */
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v3 09/16] fuzz: add a crossover function to generic-fuzzer
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (7 preceding siblings ...)
  2020-09-21  2:24 ` [PATCH v3 08/16] fuzz: add a DISABLE_PCI op to general-fuzzer Alexander Bulekov
@ 2020-09-21  2:24 ` Alexander Bulekov
  2020-10-01 15:31   ` Darren Kenny
  2020-09-21  2:25 ` [PATCH v3 10/16] scripts/oss-fuzz: Add wrapper program for generic fuzzer Alexander Bulekov
                   ` (17 subsequent siblings)
  26 siblings, 1 reply; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21  2:24 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Alexander Bulekov, darren.kenny,
	bsd, stefanha, Paolo Bonzini, philmd

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

diff --git a/tests/qtest/fuzz/general_fuzz.c b/tests/qtest/fuzz/general_fuzz.c
index 656ec7fd55..3833b505c8 100644
--- a/tests/qtest/fuzz/general_fuzz.c
+++ b/tests/qtest/fuzz/general_fuzz.c
@@ -741,6 +741,92 @@ static void general_pre_fuzz(QTestState *s)
 
     counter_shm_init();
 }
+
+/*
+ * When libfuzzer gives us two inputs to combine, return a new input with the
+ * following structure:
+ *
+ * Input 1 (data1)
+ * SEPARATOR
+ * Clear out the DMA Patterns
+ * SEPARATOR
+ * Disable the pci_read/write instructions
+ * SEPARATOR
+ * Input 2 (data2)
+ *
+ * The idea is to collate the core behaviors of the two inputs.
+ * For example:
+ * Input 1: maps a device's BARs, sets up three DMA patterns, and triggers
+ *          device functionality A
+ * Input 2: maps a device's BARs, sets up one DMA pattern, and triggers device
+ *          functionality B
+ *
+ * This function attempts to produce an input that:
+ * Ouptut: maps a device's BARs, set up three DMA patterns, triggers
+ *          functionality A device, replaces the DMA patterns with a single
+ *          patten, and triggers device functionality B.
+ */
+static size_t general_fuzz_crossover(const uint8_t *data1, size_t size1, const
+                                     uint8_t *data2, size_t size2, uint8_t *out,
+                                     size_t max_out_size, unsigned int seed)
+{
+    size_t copy_len = 0, size = 0;
+
+    /* Check that we have enough space for data1 and at least part of data2 */
+    if (max_out_size <= size + strlen(SEPARATOR) * 3 + 2) {
+        return 0;
+    }
+
+    /* Copy_Len in the first input */
+    copy_len = size1;
+    memcpy(out + size, data1, copy_len);
+    size += copy_len;
+    max_out_size -= copy_len;
+
+    /* Append a separator */
+    copy_len = strlen(SEPARATOR);
+    memcpy(out + size, SEPARATOR, copy_len);
+    size += copy_len;
+    max_out_size -= copy_len;
+
+    /* Clear out the DMA Patterns */
+    copy_len = 1;
+    if (copy_len) {
+        out[size] = OP_CLEAR_DMA_PATTERNS;
+    }
+    size += copy_len;
+    max_out_size -= copy_len;
+
+    /* Append a separator */
+    copy_len = strlen(SEPARATOR);
+    memcpy(out + size, SEPARATOR, copy_len);
+    size += copy_len;
+    max_out_size -= copy_len;
+
+    /* Disable PCI ops. Assume data1 took care of setting up PCI */
+    copy_len = 1;
+    if (copy_len) {
+        out[size] = OP_DISABLE_PCI;
+    }
+    size += copy_len;
+    max_out_size -= copy_len;
+
+    /* Append a separator */
+    copy_len = strlen(SEPARATOR);
+    memcpy(out + size, SEPARATOR, copy_len);
+    size += copy_len;
+    max_out_size -= copy_len;
+
+    /* Copy_Len over the second input */
+    copy_len = MIN(size2, max_out_size);
+    memcpy(out + size, data2, copy_len);
+    size += copy_len;
+    max_out_size -= copy_len;
+
+    return  size;
+}
+
+
 static GString *general_fuzz_cmdline(FuzzTarget *t)
 {
     GString *cmd_line = g_string_new(TARGET_NAME);
@@ -760,7 +846,9 @@ static void register_general_fuzz_targets(void)
             .description = "Fuzz based on any qemu command-line args. ",
             .get_init_cmdline = general_fuzz_cmdline,
             .pre_fuzz = general_pre_fuzz,
-            .fuzz = general_fuzz});
+            .fuzz = general_fuzz,
+            .crossover = general_fuzz_crossover
+    });
 }
 
 fuzz_target_init(register_general_fuzz_targets);
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v3 10/16] scripts/oss-fuzz: Add wrapper program for generic fuzzer
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (8 preceding siblings ...)
  2020-09-21  2:24 ` [PATCH v3 09/16] fuzz: add a crossover function to generic-fuzzer Alexander Bulekov
@ 2020-09-21  2:25 ` Alexander Bulekov
  2020-09-21  2:25 ` [PATCH v3 11/16] scripts/oss-fuzz: Add general-fuzzer build script Alexander Bulekov
                   ` (16 subsequent siblings)
  26 siblings, 0 replies; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21  2:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Alexander Bulekov, darren.kenny, bsd, stefanha,
	Paolo Bonzini, philmd

On oss-fuzz we need some sort of wrapper to specify command-line
arguments or environment variables. When we had a similar problem with
other targets that I fixed with
05509c8e6d ("fuzz: select fuzz target using executable name")
by selecting the fuzz target based on the executable's name. In the
future should probably commit to one approach (wrapper binary or
argv0-based target selection).

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
---
 scripts/oss-fuzz/target_template.c | 40 ++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 scripts/oss-fuzz/target_template.c

diff --git a/scripts/oss-fuzz/target_template.c b/scripts/oss-fuzz/target_template.c
new file mode 100644
index 0000000000..4a7257412a
--- /dev/null
+++ b/scripts/oss-fuzz/target_template.c
@@ -0,0 +1,40 @@
+/*
+ * Copyright Red Hat Inc., 2020
+ *
+ * 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 <stdio.h>
+#include <stdlib.h>
+#include <limits.h>
+#include <libgen.h>
+#include <string.h>
+#include <unistd.h>
+
+
+/* Required for oss-fuzz to consider the binary a target. */
+static const char *magic __attribute__((used)) = "LLVMFuzzerTestOneInput";
+static const char args[] = {QEMU_FUZZ_ARGS, 0x00};
+static const char objects[] = {QEMU_FUZZ_OBJECTS, 0x00};
+
+int main(int argc, char *argv[])
+{
+    char path[PATH_MAX] = {0};
+    char *dir = dirname(argv[0]);
+    strncpy(path, dir, PATH_MAX);
+    strcat(path, "/deps/qemu-fuzz-i386-target-general-fuzz");
+
+    setenv("QEMU_FUZZ_ARGS", args, 0);
+    setenv("QEMU_FUZZ_OBJECTS", objects, 0);
+
+    argv[0] = path;
+    int ret = execvp(path, argv);
+    if (ret) {
+        perror("execv");
+    }
+    return ret;
+}
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v3 11/16] scripts/oss-fuzz: Add general-fuzzer build script
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (9 preceding siblings ...)
  2020-09-21  2:25 ` [PATCH v3 10/16] scripts/oss-fuzz: Add wrapper program for generic fuzzer Alexander Bulekov
@ 2020-09-21  2:25 ` Alexander Bulekov
  2020-10-01 15:40   ` Darren Kenny
  2020-10-08  7:35   ` Paolo Bonzini
  2020-09-21  2:25 ` [PATCH v3 12/16] scripts/oss-fuzz: Add general-fuzzer configs for oss-fuzz Alexander Bulekov
                   ` (15 subsequent siblings)
  26 siblings, 2 replies; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21  2:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Alexander Bulekov, darren.kenny, bsd, stefanha,
	Paolo Bonzini, philmd

This parses a yaml file containing general-fuzzer configs and builds a
separate oss-fuzz wrapper binary for each one, changing some
preprocessor macros for each configuration. To avoid dealing with
escaping and stringifying, convert each string into a byte-array
representation

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 scripts/oss-fuzz/build_general_fuzzers.py | 69 +++++++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100755 scripts/oss-fuzz/build_general_fuzzers.py

diff --git a/scripts/oss-fuzz/build_general_fuzzers.py b/scripts/oss-fuzz/build_general_fuzzers.py
new file mode 100755
index 0000000000..918f1143a5
--- /dev/null
+++ b/scripts/oss-fuzz/build_general_fuzzers.py
@@ -0,0 +1,69 @@
+#!/usr/bin/env python3
+# -*- coding: utf-8 -*-
+
+"""
+This script creates wrapper binaries that invoke the general-device-fuzzer with
+configurations specified in a yaml config file.
+"""
+import sys
+import os
+import yaml
+import tempfile
+
+CC = ""
+TEMPLATE_FILENAME = "target_template.c"
+TEMPLATE_PATH = ""
+
+
+def usage():
+    print("Usage: CC=COMPILER {} CONFIG_PATH \
+OUTPUT_PATH_PREFIX".format(sys.argv[0]))
+    sys.exit(0)
+
+
+def str_to_c_byte_array(s):
+    """
+    Convert strings to byte-arrays so we don't worry about formatting
+    strings to play nicely with cc -DQEMU_FUZZARGS etc
+    """
+    return ','.join('0x{:02x}'.format(ord(x)) for x in s)
+
+
+def compile_wrapper(cfg, path):
+    os.system('$CC -DQEMU_FUZZ_ARGS="{fuzz_args}" \
+               -DQEMU_FUZZ_OBJECTS="{fuzz_objs}" \
+               {wrapper_template} -o {output_bin}'.format(
+                   fuzz_args=str_to_c_byte_array(cfg["args"].replace("\n", " ")),
+                   fuzz_objs=str_to_c_byte_array(cfg["objects"].replace("\n", " ")),
+                   wrapper_template=TEMPLATE_PATH,
+                   output_bin=path))
+
+
+def main():
+    global CC
+    global TEMPLATE_PATH
+    global OUTPUT_BIN_NAME
+
+    if len(sys.argv) != 3:
+        usage()
+
+    cfg_path = sys.argv[1]
+    out_path = sys.argv[2]
+
+    CC = os.getenv("CC", default="cc")
+    TEMPLATE_PATH = os.path.join(os.path.dirname(__file__), TEMPLATE_FILENAME)
+    if not os.path.exists(TEMPLATE_PATH):
+        print("Error {} doesn't exist".format(TEMPLATE_PATH))
+        sys.exit(1)
+
+    with open(cfg_path, "r") as f:
+        configs = yaml.load(f)["configs"]
+    for cfg in configs:
+        assert "name" in cfg
+        assert "args" in cfg
+        assert "objects" in cfg
+        compile_wrapper(cfg, out_path + cfg["name"])
+
+
+if __name__ == '__main__':
+    main()
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v3 12/16] scripts/oss-fuzz: Add general-fuzzer configs for oss-fuzz
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (10 preceding siblings ...)
  2020-09-21  2:25 ` [PATCH v3 11/16] scripts/oss-fuzz: Add general-fuzzer build script Alexander Bulekov
@ 2020-09-21  2:25 ` Alexander Bulekov
  2020-09-21  2:25 ` [PATCH v3 13/16] scripts/oss-fuzz: build the general-fuzzer configs Alexander Bulekov
                   ` (14 subsequent siblings)
  26 siblings, 0 replies; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21  2:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Alexander Bulekov, darren.kenny, bsd, stefanha,
	Paolo Bonzini, philmd

Each of these entries is built into a wrapper binary that sets the
needed environment variables and executes the general virtual-device
fuzzer. In the future, we will need additional fields, such as arch=arm,
timeout_per_testcase=0, reset=reboot, etc...

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
---
 scripts/oss-fuzz/general_fuzzer_configs.yml | 103 ++++++++++++++++++++
 1 file changed, 103 insertions(+)
 create mode 100644 scripts/oss-fuzz/general_fuzzer_configs.yml

diff --git a/scripts/oss-fuzz/general_fuzzer_configs.yml b/scripts/oss-fuzz/general_fuzzer_configs.yml
new file mode 100644
index 0000000000..010e92a2a5
--- /dev/null
+++ b/scripts/oss-fuzz/general_fuzzer_configs.yml
@@ -0,0 +1,103 @@
+configs:
+    - name: virtio-net-pci-slirp
+      args: >
+        -M q35 -nodefaults
+        -device virtio-net,netdev=net0 -netdev user,id=net0
+      objects: virtio*
+
+    - name: virtio-blk
+      args: >
+        -machine q35 -device virtio-blk,drive=disk0
+        -drive file=null-co://,id=disk0,if=none,format=raw
+      objects: virtio*
+
+    - name: virtio-scsi
+      args: >
+        -machine q35 -device virtio-scsi,num_queues=8
+        -device scsi-hd,drive=disk0
+        -drive file=null-co://,id=disk0,if=none,format=raw
+      objects: scsi* virtio*
+
+    - name: virtio-gpu
+      args: -machine q35 -nodefaults -device virtio-gpu
+      objects: virtio*
+
+    - name: virtio-vga
+      args: -machine q35 -nodefaults -device virtio-vga
+      objects: virtio*
+
+    - name: virtio-rng
+      args: -machine q35 -nodefaults -device virtio-rng
+      objects: virtio*
+
+    - name: virtio-balloon
+      args: -machine q35 -nodefaults -device virtio-balloon
+      objects: virtio*
+
+    - name: virtio-serial
+      args: -machine q35 -nodefaults -device virtio-serial
+      objects: virtio*
+
+    - name: virtio-mouse
+      args: -machine q35 -nodefaults -device virtio-mouse
+      objects: virtio*
+
+    - name: e1000
+      args: >
+        -M q35 -nodefaults
+        -device e1000,netdev=net0 -netdev user,id=net0
+      objects: e1000
+
+    - name: e1000e
+      args: >
+        -M q35 -nodefaults
+        -device e1000e,netdev=net0 -netdev user,id=net0
+      objects: e1000e
+
+    - name: cirrus-vga
+      args: -machine q35 -nodefaults -device cirrus-vga
+      objects: cirrus*
+
+    - name: bochs-display
+      args: -machine q35 -nodefaults -device bochs-display
+      objects: bochs*
+
+    - name: intel-hda
+      args: >
+        -machine q35 -nodefaults -device intel-hda,id=hda0
+        -device hda-output,bus=hda0.0 -device hda-micro,bus=hda0.0
+        -device hda-duplex,bus=hda0.0
+      objects: intel-hda
+
+    - name: ide-hd
+      args: >
+        -machine q35 -nodefaults
+        -drive file=null-co://,if=none,format=raw,id=disk0
+        -device ide-hd,drive=disk0
+      objects: ahci*
+
+    - name: floppy
+      args: >
+        -machine pc -nodefaults -device floppy,id=floppy0
+        -drive id=disk0,file=null-co://,file.read-zeroes=on,if=none
+        -device floppy,drive=disk0,drive-type=288
+      objects: fd* floppy*
+
+    - name: xhci
+      args: >
+        -machine q35 -nodefaults
+        -drive file=null-co://,if=none,format=raw,id=disk0
+        -device qemu-xhci,id=xhci -device usb-tablet,bus=xhci.0 -device usb-bot
+        -device usb-storage,drive=disk0 -chardev null,id=cd0 -chardev null,id=cd1
+        -device usb-braille,chardev=cd0 -device usb-ccid -device usb-ccid
+        -device usb-kbd -device usb-mouse -device usb-serial,chardev=cd1
+        -device usb-tablet -device usb-wacom-tablet -device usb-audio
+      objects: "*usb* *uhci* *xhci*"
+
+    - name: pc-i440fx
+      args: -machine pc
+      objects: "*"
+
+    - name: pc-q35
+      args: -machine q35
+      objects: "*"
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v3 13/16] scripts/oss-fuzz: build the general-fuzzer configs
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (11 preceding siblings ...)
  2020-09-21  2:25 ` [PATCH v3 12/16] scripts/oss-fuzz: Add general-fuzzer configs for oss-fuzz Alexander Bulekov
@ 2020-09-21  2:25 ` Alexander Bulekov
  2020-09-21  2:25 ` [PATCH v3 14/16] scripts/oss-fuzz: Add script to reorder a general-fuzzer trace Alexander Bulekov
                   ` (13 subsequent siblings)
  26 siblings, 0 replies; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21  2:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Alexander Bulekov, darren.kenny, bsd, stefanha,
	Paolo Bonzini, philmd

Build general-fuzzer wrappers for each configuration defined in
general_fuzzer_configs.yml and move the actual general-fuzzer to a
subdirectory, so oss-fuzz doesn't treat it as a standalone fuzzer.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
---
 scripts/oss-fuzz/build.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
index d16207eb67..16b9f39d07 100755
--- a/scripts/oss-fuzz/build.sh
+++ b/scripts/oss-fuzz/build.sh
@@ -87,6 +87,7 @@ make "-j$(nproc)" qemu-fuzz-i386 V=1
 
 # Copy over the datadir
 cp  -r ../pc-bios/ "$DEST_DIR/pc-bios"
+cp  -r ../pc-bios/ "$DEST_DIR/deps/pc-bios"
 
 # Run the fuzzer with no arguments, to print the help-string and get the list
 # of available fuzz-targets. Copy over the qemu-fuzz-i386, naming it according
@@ -97,5 +98,11 @@ do
     cp qemu-fuzz-i386 "$DEST_DIR/qemu-fuzz-i386-target-$target"
 done
 
+mkdir -p "$DEST_DIR/deps"
+mv "$DEST_DIR/qemu-fuzz-i386-target-general-fuzz" "$DEST_DIR/deps/"
+
+./scripts/oss-fuzz/build_general_fuzzers.py \
+    "./scripts/oss-fuzz/general_fuzzer_configs.yml" "$DEST_DIR/general-fuzz-"
+
 echo "Done. The fuzzers are located in $DEST_DIR"
 exit 0
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v3 14/16] scripts/oss-fuzz: Add script to reorder a general-fuzzer trace
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (12 preceding siblings ...)
  2020-09-21  2:25 ` [PATCH v3 13/16] scripts/oss-fuzz: build the general-fuzzer configs Alexander Bulekov
@ 2020-09-21  2:25 ` Alexander Bulekov
  2020-10-08  7:42   ` Paolo Bonzini
  2020-09-21  2:25 ` [PATCH v3 15/16] scripts/oss-fuzz: Add crash trace minimization script Alexander Bulekov
                   ` (12 subsequent siblings)
  26 siblings, 1 reply; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21  2:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Alexander Bulekov, darren.kenny, bsd, stefanha,
	Paolo Bonzini, philmd

The general-fuzzer uses hooks to fulfill DMA requests just-in-time.
This means that if we try to use QTEST_LOG=1 to build a reproducer, the
DMA writes will be logged _after_ the in/out/read/write that triggered
the DMA read. To work work around this, the general-fuzzer annotates
these just-in time DMA fulfilments with a tag that we can use to
discern them. This script simply iterates over a raw qtest
trace (including log messages, errors, timestamps etc), filters it and
re-orders it so that DMA fulfillments are placed directly _before_ the
qtest command that will cause the DMA access.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
---
 .../oss-fuzz/reorder_fuzzer_qtest_trace.py    | 94 +++++++++++++++++++
 1 file changed, 94 insertions(+)
 create mode 100755 scripts/oss-fuzz/reorder_fuzzer_qtest_trace.py

diff --git a/scripts/oss-fuzz/reorder_fuzzer_qtest_trace.py b/scripts/oss-fuzz/reorder_fuzzer_qtest_trace.py
new file mode 100755
index 0000000000..9fb7edb6ee
--- /dev/null
+++ b/scripts/oss-fuzz/reorder_fuzzer_qtest_trace.py
@@ -0,0 +1,94 @@
+#!/usr/bin/env python3
+# -*- coding: utf-8 -*-
+
+"""
+Use this to convert qtest log info from a generic fuzzer input into a qtest
+trace that you can feed into a standard qemu-system process. Example usage:
+
+QEMU_FUZZ_ARGS="-machine q35,accel=qtest" QEMU_FUZZ_OBJECTS="*" \
+        ./i386-softmmu/qemu-fuzz-i386 --fuzz-target=general-pci-fuzz
+# .. Finds some crash
+QTEST_LOG=1 FUZZ_SERIALIZE_QTEST=1 \
+QEMU_FUZZ_ARGS="-machine q35,accel=qtest" QEMU_FUZZ_OBJECTS="*" \
+        ./i386-softmmu/qemu-fuzz-i386 --fuzz-target=general-pci-fuzz
+        /path/to/crash 2> qtest_log_output
+scripts/oss-fuzz/reorder_fuzzer_qtest_trace.py qtest_log_output > qtest_trace
+./i386-softmmu/qemu-fuzz-i386 -machine q35,accel=qtest \
+        -qtest stdin < qtest_trace
+
+### Details ###
+
+Some fuzzer make use of hooks that allow us to populate some memory range, just
+before a DMA read from that range. This means that the fuzzer can produce
+activity that looks like:
+    [start] read from mmio addr
+    [end]   read from mmio addr
+    [start] write to pio addr
+        [start] fill a DMA buffer just in time
+        [end]   fill a DMA buffer just in time
+        [start] fill a DMA buffer just in time
+        [end]   fill a DMA buffer just in time
+    [end]   write to pio addr
+    [start] read from mmio addr
+    [end]   read from mmio addr
+
+We annotate these "nested" DMA writes, so with QTEST_LOG=1 the QTest trace
+might look something like:
+[R +0.028431] readw 0x10000
+[R +0.028434] outl 0xc000 0xbeef  # Triggers a DMA read from 0xbeef and 0xbf00
+[DMA][R +0.034639] write 0xbeef 0x2 0xAAAA
+[DMA][R +0.034639] write 0xbf00 0x2 0xBBBB
+[R +0.028431] readw 0xfc000
+
+This script would reorder the above trace so it becomes:
+readw 0x10000
+write 0xbeef 0x2 0xAAAA
+write 0xbf00 0x2 0xBBBB
+outl 0xc000 0xbeef
+readw 0xfc000
+
+I.e. by the time, 0xc000 tries to read from DMA, those DMA buffers have already
+been set up, removing the need for the DMA hooks. We can simply provide this
+reordered trace via -qtest stdio to reproduce the input
+
+Note: this won't work for traces where the device tries to read from the same
+DMA region twice in between MMIO/PIO commands. E.g:
+    [R +0.028434] outl 0xc000 0xbeef
+    [DMA][R +0.034639] write 0xbeef 0x2 0xAAAA
+    [DMA][R +0.034639] write 0xbeef 0x2 0xBBBB
+"""
+
+import sys
+
+__author__     = "Alexander Bulekov <alxndr@bu.edu>"
+__copyright__  = "Copyright (C) 2020, Red Hat, Inc."
+__license__    = "GPL version 2 or (at your option) any later version"
+
+__maintainer__ = "Alexander Bulekov"
+__email__      = "alxndr@bu.edu"
+
+
+def usage():
+    sys.exit("Usage: {} /path/to/qtest_log_output".format((sys.argv[0])))
+
+
+def main(filename):
+    with open(filename, "r") as f:
+        trace = f.readlines()
+
+    # Leave only lines that look like logged qtest commands
+    trace[:] = [x.strip() for x in trace if "[R +" in x
+                or "[S +" in x and "CLOSED" not in x]
+
+    for i in range(len(trace)):
+        if i+1 < len(trace):
+            if "[DMA]" in trace[i+1]:
+                trace[i], trace[i+1] = trace[i+1], trace[i]
+    for line in trace:
+        print(line.split("]")[-1].strip())
+
+
+if __name__ == '__main__':
+    if len(sys.argv) == 1:
+        usage()
+    main(sys.argv[1])
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v3 15/16] scripts/oss-fuzz: Add crash trace minimization script
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (13 preceding siblings ...)
  2020-09-21  2:25 ` [PATCH v3 14/16] scripts/oss-fuzz: Add script to reorder a general-fuzzer trace Alexander Bulekov
@ 2020-09-21  2:25 ` Alexander Bulekov
  2020-09-21  2:25 ` [PATCH v3 16/16] fuzz: Add instructions for using general-fuzz Alexander Bulekov
                   ` (11 subsequent siblings)
  26 siblings, 0 replies; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21  2:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Alexander Bulekov, darren.kenny, bsd, stefanha,
	Paolo Bonzini, philmd

Once we find a crash, we can convert it into a QTest trace. Usually this
trace will contain many operations that are unneeded to reproduce the
crash. This script tries to minimize the crashing trace, by removing
operations and trimming QTest bufwrite(write addr len data...) commands.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
---
 scripts/oss-fuzz/minimize_qtest_trace.py | 157 +++++++++++++++++++++++
 1 file changed, 157 insertions(+)
 create mode 100755 scripts/oss-fuzz/minimize_qtest_trace.py

diff --git a/scripts/oss-fuzz/minimize_qtest_trace.py b/scripts/oss-fuzz/minimize_qtest_trace.py
new file mode 100755
index 0000000000..05596d6f9c
--- /dev/null
+++ b/scripts/oss-fuzz/minimize_qtest_trace.py
@@ -0,0 +1,157 @@
+#!/usr/bin/env python3
+# -*- coding: utf-8 -*-
+
+"""
+This takes a crashing qtest trace and tries to remove superflous operations
+"""
+
+import sys
+import os
+import subprocess
+import time
+import struct
+
+QEMU_ARGS = None
+QEMU_PATH = None
+TIMEOUT = 5
+CRASH_TOKEN = None
+
+write_suffix_lookup = {"b": (1, "B"),
+                       "w": (2, "H"),
+                       "l": (4, "L"),
+                       "q": (8, "Q")}
+
+def usage():
+    sys.exit("""\
+Usage: QEMU_PATH="/path/to/qemu" QEMU_ARGS="args" {} input_trace output_trace
+By default, will try to use the second-to-last line in the output to identify
+whether the crash occred. Optionally, manually set a string that idenitifes the
+crash by setting CRASH_TOKEN=
+""".format((sys.argv[0])))
+
+def check_if_trace_crashes(trace, path):
+    global CRASH_TOKEN
+    with open(path, "w") as tracefile:
+        tracefile.write("".join(trace))
+
+    rc = subprocess.Popen("timeout -s 9 {timeout}s {qemu_path} {qemu_args} 2>&1\
+    < {trace_path}".format(timeout=TIMEOUT,
+                           qemu_path=QEMU_PATH,
+                           qemu_args=QEMU_ARGS,
+                           trace_path=path),
+                          shell=True,
+                          stdin=subprocess.PIPE,
+                          stdout=subprocess.PIPE)
+    stdo = rc.communicate()[0]
+    output = stdo.decode('unicode_escape')
+    if rc.returncode == 137:    # Timed Out
+        return False
+    if len(output.splitlines()) < 2:
+        return False
+
+    if CRASH_TOKEN is None:
+        CRASH_TOKEN = output.splitlines()[-2]
+
+    return CRASH_TOKEN in output
+
+
+def minimize_trace(inpath, outpath):
+    global TIMEOUT
+    with open(inpath) as f:
+        trace = f.readlines()
+    start = time.time()
+    if not check_if_trace_crashes(trace, outpath):
+        sys.exit("The input qtest trace didn't cause a crash...")
+    end = time.time()
+    print("Crashed in {} seconds".format(end-start))
+    TIMEOUT = (end-start)*5
+    print("Setting the timeout for {} seconds".format(TIMEOUT))
+    print("Identifying Crashes by this string: {}".format(CRASH_TOKEN))
+
+    i = 0
+    newtrace = trace[:]
+    # For each line
+    while i < len(newtrace):
+        # 1.) Try to remove it completely and reproduce the crash. If it works,
+        # we're done.
+        prior = newtrace[i]
+        print("Trying to remove {}".format(newtrace[i]))
+        # Try to remove the line completely
+        newtrace[i] = ""
+        if check_if_trace_crashes(newtrace, outpath):
+            i += 1
+            continue
+        newtrace[i] = prior
+
+        # 2.) Try to replace write{bwlq} commands with a write addr, len
+        # command. Since this can require swapping endianness, try both LE and
+        # BE options. We do this, so we can "trim" the writes in (3)
+        if (newtrace[i].startswith("write") and not
+            newtrace[i].startswith("write ")):
+            suffix = newtrace[i].split()[0][-1]
+            assert(suffix in write_suffix_lookup)
+            addr = int(newtrace[i].split()[1], 16)
+            value = int(newtrace[i].split()[2], 16)
+            for endianness in ['<', '>']:
+                data = struct.pack("{end}{size}".format(end=endianness,
+                                   size=write_suffix_lookup[suffix][1]),
+                                   value)
+                newtrace[i] = "write {addr} {size} 0x{data}\n".format(
+                    addr=hex(addr),
+                    size=hex(write_suffix_lookup[suffix][0]),
+                    data=data.hex())
+                if(check_if_trace_crashes(newtrace, outpath)):
+                    break
+            else:
+                newtrace[i] = prior
+
+        # 3.) If it is a qtest write command: write addr len data, try to split
+        # it into two separate write commands. If splitting the write down the
+        # middle does not work, try to move the pivot "left" and retry, until
+        # there is no space left. The idea is to prune unneccessary bytes from
+        # long writes, while accommodating arbitrary MemoryRegion access sizes
+        # and alignments.
+        if newtrace[i].startswith("write "):
+            addr = int(newtrace[i].split()[1], 16)
+            length = int(newtrace[i].split()[2], 16)
+            data = newtrace[i].split()[3][2:]
+            if length > 1:
+                leftlength = int(length/2)
+                rightlength = length - leftlength
+                newtrace.insert(i+1, "")
+                while leftlength > 0:
+                    newtrace[i] = "write {} {} 0x{}\n".format(
+                            hex(addr),
+                            hex(leftlength),
+                            data[:leftlength*2])
+                    newtrace[i+1] = "write {} {} 0x{}\n".format(
+                            hex(addr+leftlength),
+                            hex(rightlength),
+                            data[leftlength*2:])
+                    if check_if_trace_crashes(newtrace, outpath):
+                        break
+                    else:
+                        leftlength -= 1
+                        rightlength += 1
+                if check_if_trace_crashes(newtrace, outpath):
+                    i -= 1
+                else:
+                    newtrace[i] = prior
+                    del newtrace[i+1]
+        i += 1
+    check_if_trace_crashes(newtrace, outpath)
+
+
+if __name__ == '__main__':
+    if len(sys.argv) < 3:
+        usage()
+
+    QEMU_PATH = os.getenv("QEMU_PATH")
+    QEMU_ARGS = os.getenv("QEMU_ARGS")
+    if QEMU_PATH is None or QEMU_ARGS is None:
+        usage()
+    # if "accel" not in QEMU_ARGS:
+    #     QEMU_ARGS += " -accel qtest"
+    CRASH_TOKEN = os.getenv("CRASH_TOKEN")
+    QEMU_ARGS += " -qtest stdio -monitor none -serial none "
+    minimize_trace(sys.argv[1], sys.argv[2])
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [PATCH v3 16/16] fuzz: Add instructions for using general-fuzz
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (14 preceding siblings ...)
  2020-09-21  2:25 ` [PATCH v3 15/16] scripts/oss-fuzz: Add crash trace minimization script Alexander Bulekov
@ 2020-09-21  2:25 ` Alexander Bulekov
  2020-10-01 15:44   ` Darren Kenny
  2020-09-21  2:45 ` [PATCH v3 00/16] Add a General Virtual Device Fuzzer no-reply
                   ` (10 subsequent siblings)
  26 siblings, 1 reply; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21  2:25 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Alexander Bulekov, darren.kenny, bsd, stefanha,
	Paolo Bonzini, philmd

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 docs/devel/fuzzing.txt | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/docs/devel/fuzzing.txt b/docs/devel/fuzzing.txt
index 96d71c94d7..208b0c8360 100644
--- a/docs/devel/fuzzing.txt
+++ b/docs/devel/fuzzing.txt
@@ -125,6 +125,44 @@ provided by libfuzzer. Libfuzzer passes a byte array and length. Commonly the
 fuzzer loops over the byte-array interpreting it as a list of qtest commands,
 addresses, or values.
 
+== The General Fuzzer ==
+Writing a fuzz target can be a lot of effort (especially if a device driver has
+not be built-out within libqos). Many devices can be fuzzed to some degree,
+without any device-specific code, using the general-fuzz target.
+
+The general-fuzz target is capable of fuzzing devices over their PIO, MMIO,
+and DMA input-spaces. To apply the general-fuzz to a device, we need to define
+two env-variables, at minimum:
+
+QEMU_FUZZ_ARGS= is the set of QEMU arguments used to configure a machine, with
+the device attached. For example, if we want to fuzz the virtio-net device
+attached to a pc-i440fx machine, we can specify:
+QEMU_FUZZ_ARGS="-M pc -nodefaults -netdev user,id=user0 \
+                -device virtio-net,netdev=user0"
+
+QEMU_FUZZ_OBJECTS= is a set of space-delimited strings used to identify the
+MemoryRegions that will be fuzzed. These strings are compared against
+MemoryRegion names and MemoryRegion owner names, to decide whether each
+MemoryRegion should be fuzzed. These strings support globbing. For the
+virtio-net example, we could use
+QEMU_FUZZ_OBJECTS='virtio-net'
+QEMU_FUZZ_OBJECTS='virtio*'
+QEMU_FUZZ_OBJECTS='virtio* pcspk' # Fuzz the virtio-net device and the PC speaker...
+QEMU_FUZZ_OBJECTS='*' # Fuzz the whole machine
+
+The "info mtree" and "info qom-tree" monitor commands can be especially useful
+for identifying the MemoryRegion and Object names used for matching.
+
+As a general rule-of-thumb, the more MemoryRegions/Devices we match, the greater
+the input-space, and the smaller the probability of finding crashing inputs for
+individual devices. As such, it is usually a good idea to limit the fuzzer to
+only a few MemoryRegions.
+
+To ensure that these env variables have been configured correctly, we can use:
+./qemu-fuzz-i386 --fuzz-target=general-fuzz -runs=0
+
+The output should contain a complete list of matched MemoryRegions.
+
 = Implementation Details =
 
 == The Fuzzer's Lifecycle ==
-- 
2.28.0



^ permalink raw reply related	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 00/16] Add a General Virtual Device Fuzzer
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (15 preceding siblings ...)
  2020-09-21  2:25 ` [PATCH v3 16/16] fuzz: Add instructions for using general-fuzz Alexander Bulekov
@ 2020-09-21  2:45 ` no-reply
  2020-09-21  2:58 ` no-reply
                   ` (9 subsequent siblings)
  26 siblings, 0 replies; 53+ messages in thread
From: no-reply @ 2020-09-21  2:45 UTC (permalink / raw)
  To: alxndr; +Cc: alxndr, qemu-devel, darren.kenny, bsd, stefanha, philmd

Patchew URL: https://patchew.org/QEMU/20200921022506.873303-1-alxndr@bu.edu/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20200921022506.873303-1-alxndr@bu.edu
Subject: [PATCH v3 00/16] Add a General Virtual Device Fuzzer

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20200920114416.353277-1-f4bug@amsat.org -> patchew/20200920114416.353277-1-f4bug@amsat.org
 - [tag update]      patchew/20200920155042.400737-1-f4bug@amsat.org -> patchew/20200920155042.400737-1-f4bug@amsat.org
 * [new tag]         patchew/20200921022506.873303-1-alxndr@bu.edu -> patchew/20200921022506.873303-1-alxndr@bu.edu
Switched to a new branch 'test'
885f529 fuzz: Add instructions for using general-fuzz
c60146b scripts/oss-fuzz: Add crash trace minimization script
a8bc368 scripts/oss-fuzz: Add script to reorder a general-fuzzer trace
316bf46 scripts/oss-fuzz: build the general-fuzzer configs
9a97bb7 scripts/oss-fuzz: Add general-fuzzer configs for oss-fuzz
23556b7 scripts/oss-fuzz: Add general-fuzzer build script
245e070 scripts/oss-fuzz: Add wrapper program for generic fuzzer
891a00a fuzz: add a crossover function to generic-fuzzer
f814247 fuzz: add a DISABLE_PCI op to general-fuzzer
5c574ed fuzz: Add support for custom crossover functions
11ed4b4 fuzz: Add fuzzer callbacks to DMA-read functions
6613942 fuzz: Declare DMA Read callback function
ec2c675 fuzz: Add DMA support to the generic-fuzzer
6eb92b4 fuzz: Add PCI features to the general fuzzer
d0d5046 fuzz: Add general virtual-device fuzzer
c925032 memory: Add FlatView foreach function

=== OUTPUT BEGIN ===
1/16 Checking commit c925032667fa (memory: Add FlatView foreach function)
2/16 Checking commit d0d5046ad116 (fuzz: Add general virtual-device fuzzer)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#19: 
new file mode 100644

total: 0 errors, 1 warnings, 505 lines checked

Patch 2/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/16 Checking commit 6eb92b44834e (fuzz: Add PCI features to the general fuzzer)
4/16 Checking commit ec2c675d3c66 (fuzz: Add DMA support to the generic-fuzzer)
ERROR: externs should be avoided in .c files
#84: FILE: tests/qtest/fuzz/general_fuzz.c:120:
+void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr, bool is_write);

total: 1 errors, 0 warnings, 247 lines checked

Patch 4/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/16 Checking commit 6613942d235c (fuzz: Declare DMA Read callback function)
6/16 Checking commit 11ed4b463abe (fuzz: Add fuzzer callbacks to DMA-read functions)
7/16 Checking commit 5c574ed8159a (fuzz: Add support for custom crossover functions)
8/16 Checking commit f8142473c0f9 (fuzz: add a DISABLE_PCI op to general-fuzzer)
9/16 Checking commit 891a00a3410a (fuzz: add a crossover function to generic-fuzzer)
10/16 Checking commit 245e07058f48 (scripts/oss-fuzz: Add wrapper program for generic fuzzer)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#20: 
new file mode 100644

total: 0 errors, 1 warnings, 40 lines checked

Patch 10/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/16 Checking commit 23556b7712cd (scripts/oss-fuzz: Add general-fuzzer build script)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100755

WARNING: line over 80 characters
#57: FILE: scripts/oss-fuzz/build_general_fuzzers.py:36:
+                   fuzz_args=str_to_c_byte_array(cfg["args"].replace("\n", " ")),

WARNING: line over 80 characters
#58: FILE: scripts/oss-fuzz/build_general_fuzzers.py:37:
+                   fuzz_objs=str_to_c_byte_array(cfg["objects"].replace("\n", " ")),

total: 0 errors, 3 warnings, 69 lines checked

Patch 11/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/16 Checking commit 9a97bb7982d2 (scripts/oss-fuzz: Add general-fuzzer configs for oss-fuzz)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100644

total: 0 errors, 1 warnings, 103 lines checked

Patch 12/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
13/16 Checking commit 316bf46fb3a7 (scripts/oss-fuzz: build the general-fuzzer configs)
14/16 Checking commit a8bc368fc111 (scripts/oss-fuzz: Add script to reorder a general-fuzzer trace)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#22: 
new file mode 100755

total: 0 errors, 1 warnings, 94 lines checked

Patch 14/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
15/16 Checking commit c60146b50a2b (scripts/oss-fuzz: Add crash trace minimization script)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100755

total: 0 errors, 1 warnings, 157 lines checked

Patch 15/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
16/16 Checking commit 885f5295ce5f (fuzz: Add instructions for using general-fuzz)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200921022506.873303-1-alxndr@bu.edu/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 00/16] Add a General Virtual Device Fuzzer
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (16 preceding siblings ...)
  2020-09-21  2:45 ` [PATCH v3 00/16] Add a General Virtual Device Fuzzer no-reply
@ 2020-09-21  2:58 ` no-reply
  2020-09-21  3:30 ` no-reply
                   ` (8 subsequent siblings)
  26 siblings, 0 replies; 53+ messages in thread
From: no-reply @ 2020-09-21  2:58 UTC (permalink / raw)
  To: alxndr; +Cc: alxndr, qemu-devel, darren.kenny, bsd, stefanha, philmd

Patchew URL: https://patchew.org/QEMU/20200921022506.873303-1-alxndr@bu.edu/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

Host machine cpu: x86_64
Target machine cpu family: x86
Target machine cpu: x86_64
../src/meson.build:10: WARNING: Module unstable-keyval has no backwards or forwards compatibility and might not exist in future releases.
Program sh found: YES
Program python3 found: YES (/usr/bin/python3)
Configuring ninjatool using configuration
---
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory '/tmp/qemu-test/src/slirp'
Generating nsis with a custom command
make[1]: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.
make[1]: Entering directory '/tmp/qemu-test/build'
make[2]: Entering directory '/tmp/qemu-test/src/slirp'
make[2]: Nothing to be done for 'all'.
---
Host machine cpu: i386
Target machine cpu family: x86
Target machine cpu: i386
../src/meson.build:10: WARNING: Module unstable-keyval has no backwards or forwards compatibility and might not exist in future releases.
Program sh found: YES
Program python3 found: YES (/usr/bin/python3)
Configuring ninjatool using configuration
---
Compiling C object libqemu-x86_64-softmmu.fa.p/meson-generated_.._qapi_qapi-introspect.c.obj
Compiling C object libqemu-x86_64-softmmu.fa.p/meson-generated_.._qapi_qapi-types-misc-target.c.obj
../src/softmmu/memory.c: In function 'flatview_for_each_range':
../src/softmmu/memory.c:662:24: error: incompatible type for argument 1 of 'cb'
  662 |         if (cb(fr->addr.start, fr->addr.size, fr->mr, opaque))
      |                ~~~~~~~~^~~~~~
      |                        |
      |                        Int128 {aka struct Int128}
../src/softmmu/memory.c:662:24: note: expected 'ram_addr_t' {aka 'unsigned int'} but argument is of type 'Int128' {aka 'struct Int128'}
../src/softmmu/memory.c:662:40: error: incompatible type for argument 2 of 'cb'
  662 |         if (cb(fr->addr.start, fr->addr.size, fr->mr, opaque))
      |                                ~~~~~~~~^~~~~
      |                                        |
      |                                        Int128 {aka struct Int128}
../src/softmmu/memory.c:662:40: note: expected 'ram_addr_t' {aka 'unsigned int'} but argument is of type 'Int128' {aka 'struct Int128'}
Compiling C object libqemu-x86_64-softmmu.fa.p/meson-generated_.._qapi_qapi-init-commands.c.obj
make: *** [Makefile.ninja:1642: libqemu-x86_64-softmmu.fa.p/softmmu_memory.c.obj] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 709, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--rm', '--label', 'com.qemu.instance.uuid=a0ab34f4ceae48228c5f799018992ddf', '-u', '1001', '--security-opt', 'seccomp=unconfined', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-5y5gpn49/src/docker-src.2020-09-20-22.46.26.4071:/var/tmp/qemu:z,ro', 'qemu/fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=a0ab34f4ceae48228c5f799018992ddf
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-5y5gpn49/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    12m30.289s
user    0m20.383s


The full log is available at
http://patchew.org/logs/20200921022506.873303-1-alxndr@bu.edu/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 00/16] Add a General Virtual Device Fuzzer
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (17 preceding siblings ...)
  2020-09-21  2:58 ` no-reply
@ 2020-09-21  3:30 ` no-reply
  2020-09-21  3:43 ` no-reply
                   ` (7 subsequent siblings)
  26 siblings, 0 replies; 53+ messages in thread
From: no-reply @ 2020-09-21  3:30 UTC (permalink / raw)
  To: alxndr; +Cc: alxndr, qemu-devel, darren.kenny, bsd, stefanha, philmd

Patchew URL: https://patchew.org/QEMU/20200921022506.873303-1-alxndr@bu.edu/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20200921022506.873303-1-alxndr@bu.edu
Subject: [PATCH v3 00/16] Add a General Virtual Device Fuzzer

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20200921022506.873303-1-alxndr@bu.edu -> patchew/20200921022506.873303-1-alxndr@bu.edu
Switched to a new branch 'test'
d7cd74b fuzz: Add instructions for using general-fuzz
3134005 scripts/oss-fuzz: Add crash trace minimization script
21ca1ac scripts/oss-fuzz: Add script to reorder a general-fuzzer trace
78e1a44 scripts/oss-fuzz: build the general-fuzzer configs
c4d54eb scripts/oss-fuzz: Add general-fuzzer configs for oss-fuzz
d1d8bf7 scripts/oss-fuzz: Add general-fuzzer build script
775f979 scripts/oss-fuzz: Add wrapper program for generic fuzzer
9a0bb4d fuzz: add a crossover function to generic-fuzzer
023fcf2 fuzz: add a DISABLE_PCI op to general-fuzzer
d47550a fuzz: Add support for custom crossover functions
4280a71 fuzz: Add fuzzer callbacks to DMA-read functions
620fc01 fuzz: Declare DMA Read callback function
051fb28 fuzz: Add DMA support to the generic-fuzzer
b154f43 fuzz: Add PCI features to the general fuzzer
dc4ebfd fuzz: Add general virtual-device fuzzer
8136cd1 memory: Add FlatView foreach function

=== OUTPUT BEGIN ===
1/16 Checking commit 8136cd116e64 (memory: Add FlatView foreach function)
2/16 Checking commit dc4ebfd71ea9 (fuzz: Add general virtual-device fuzzer)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#19: 
new file mode 100644

total: 0 errors, 1 warnings, 505 lines checked

Patch 2/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/16 Checking commit b154f4362734 (fuzz: Add PCI features to the general fuzzer)
4/16 Checking commit 051fb28424cd (fuzz: Add DMA support to the generic-fuzzer)
ERROR: externs should be avoided in .c files
#84: FILE: tests/qtest/fuzz/general_fuzz.c:120:
+void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr, bool is_write);

total: 1 errors, 0 warnings, 247 lines checked

Patch 4/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/16 Checking commit 620fc01dff92 (fuzz: Declare DMA Read callback function)
6/16 Checking commit 4280a71ba60b (fuzz: Add fuzzer callbacks to DMA-read functions)
7/16 Checking commit d47550afc30c (fuzz: Add support for custom crossover functions)
8/16 Checking commit 023fcf2cdeae (fuzz: add a DISABLE_PCI op to general-fuzzer)
9/16 Checking commit 9a0bb4de7516 (fuzz: add a crossover function to generic-fuzzer)
10/16 Checking commit 775f9797c7f2 (scripts/oss-fuzz: Add wrapper program for generic fuzzer)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#20: 
new file mode 100644

total: 0 errors, 1 warnings, 40 lines checked

Patch 10/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/16 Checking commit d1d8bf728884 (scripts/oss-fuzz: Add general-fuzzer build script)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100755

WARNING: line over 80 characters
#57: FILE: scripts/oss-fuzz/build_general_fuzzers.py:36:
+                   fuzz_args=str_to_c_byte_array(cfg["args"].replace("\n", " ")),

WARNING: line over 80 characters
#58: FILE: scripts/oss-fuzz/build_general_fuzzers.py:37:
+                   fuzz_objs=str_to_c_byte_array(cfg["objects"].replace("\n", " ")),

total: 0 errors, 3 warnings, 69 lines checked

Patch 11/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/16 Checking commit c4d54ebe8ed2 (scripts/oss-fuzz: Add general-fuzzer configs for oss-fuzz)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100644

total: 0 errors, 1 warnings, 103 lines checked

Patch 12/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
13/16 Checking commit 78e1a443f0df (scripts/oss-fuzz: build the general-fuzzer configs)
14/16 Checking commit 21ca1ac3dc9e (scripts/oss-fuzz: Add script to reorder a general-fuzzer trace)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#22: 
new file mode 100755

total: 0 errors, 1 warnings, 94 lines checked

Patch 14/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
15/16 Checking commit 313400585806 (scripts/oss-fuzz: Add crash trace minimization script)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100755

total: 0 errors, 1 warnings, 157 lines checked

Patch 15/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
16/16 Checking commit d7cd74b8caec (fuzz: Add instructions for using general-fuzz)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200921022506.873303-1-alxndr@bu.edu/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 00/16] Add a General Virtual Device Fuzzer
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (18 preceding siblings ...)
  2020-09-21  3:30 ` no-reply
@ 2020-09-21  3:43 ` no-reply
  2020-09-21  3:46 ` no-reply
                   ` (6 subsequent siblings)
  26 siblings, 0 replies; 53+ messages in thread
From: no-reply @ 2020-09-21  3:43 UTC (permalink / raw)
  To: alxndr; +Cc: alxndr, qemu-devel, darren.kenny, bsd, stefanha, philmd

Patchew URL: https://patchew.org/QEMU/20200921022506.873303-1-alxndr@bu.edu/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

Host machine cpu: x86_64
Target machine cpu family: x86
Target machine cpu: x86_64
../src/meson.build:10: WARNING: Module unstable-keyval has no backwards or forwards compatibility and might not exist in future releases.
Program sh found: YES
Program python3 found: YES (/usr/bin/python3)
Configuring ninjatool using configuration
---
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory '/tmp/qemu-test/src/slirp'
Generating nsis with a custom command
make[1]: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.
make[1]: Entering directory '/tmp/qemu-test/build'
make[2]: Entering directory '/tmp/qemu-test/src/slirp'
make[2]: Nothing to be done for 'all'.
---
Host machine cpu: i386
Target machine cpu family: x86
Target machine cpu: i386
../src/meson.build:10: WARNING: Module unstable-keyval has no backwards or forwards compatibility and might not exist in future releases.
Program sh found: YES
Program python3 found: YES (/usr/bin/python3)
Configuring ninjatool using configuration
---
Compiling C object migration/libmigration.fa.p/qemu-file-channel.c.obj
Compiling C object migration/libmigration.fa.p/qemu-file.c.obj
../src/softmmu/memory.c: In function 'flatview_for_each_range':
../src/softmmu/memory.c:662:24: error: incompatible type for argument 1 of 'cb'
  662 |         if (cb(fr->addr.start, fr->addr.size, fr->mr, opaque))
      |                ~~~~~~~~^~~~~~
      |                        |
      |                        Int128 {aka struct Int128}
../src/softmmu/memory.c:662:24: note: expected 'ram_addr_t' {aka 'unsigned int'} but argument is of type 'Int128' {aka 'struct Int128'}
../src/softmmu/memory.c:662:40: error: incompatible type for argument 2 of 'cb'
  662 |         if (cb(fr->addr.start, fr->addr.size, fr->mr, opaque))
      |                                ~~~~~~~~^~~~~
      |                                        |
      |                                        Int128 {aka struct Int128}
../src/softmmu/memory.c:662:40: note: expected 'ram_addr_t' {aka 'unsigned int'} but argument is of type 'Int128' {aka 'struct Int128'}
make: *** [Makefile.ninja:1640: libqemu-x86_64-softmmu.fa.p/softmmu_memory.c.obj] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 709, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--rm', '--label', 'com.qemu.instance.uuid=72afe850b45249cb90682d80e1fe24de', '-u', '1003', '--security-opt', 'seccomp=unconfined', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-ly488yk7/src/docker-src.2020-09-20-23.31.12.24444:/var/tmp/qemu:z,ro', 'qemu/fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=72afe850b45249cb90682d80e1fe24de
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-ly488yk7/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    12m33.413s
user    0m22.763s


The full log is available at
http://patchew.org/logs/20200921022506.873303-1-alxndr@bu.edu/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 00/16] Add a General Virtual Device Fuzzer
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (19 preceding siblings ...)
  2020-09-21  3:43 ` no-reply
@ 2020-09-21  3:46 ` no-reply
  2020-09-21  4:30 ` no-reply
                   ` (5 subsequent siblings)
  26 siblings, 0 replies; 53+ messages in thread
From: no-reply @ 2020-09-21  3:46 UTC (permalink / raw)
  To: alxndr; +Cc: alxndr, qemu-devel, darren.kenny, bsd, stefanha, philmd

Patchew URL: https://patchew.org/QEMU/20200921022506.873303-1-alxndr@bu.edu/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20200921022506.873303-1-alxndr@bu.edu
Subject: [PATCH v3 00/16] Add a General Virtual Device Fuzzer

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20200921022506.873303-1-alxndr@bu.edu -> patchew/20200921022506.873303-1-alxndr@bu.edu
Switched to a new branch 'test'
08f9672 fuzz: Add instructions for using general-fuzz
410c8be scripts/oss-fuzz: Add crash trace minimization script
8e0abed scripts/oss-fuzz: Add script to reorder a general-fuzzer trace
7513795 scripts/oss-fuzz: build the general-fuzzer configs
05353cf scripts/oss-fuzz: Add general-fuzzer configs for oss-fuzz
7abda11 scripts/oss-fuzz: Add general-fuzzer build script
6261e77 scripts/oss-fuzz: Add wrapper program for generic fuzzer
29fd853 fuzz: add a crossover function to generic-fuzzer
f69413a fuzz: add a DISABLE_PCI op to general-fuzzer
7013ef0 fuzz: Add support for custom crossover functions
230a7bd fuzz: Add fuzzer callbacks to DMA-read functions
e713ca2 fuzz: Declare DMA Read callback function
0fb3331 fuzz: Add DMA support to the generic-fuzzer
585e6f0 fuzz: Add PCI features to the general fuzzer
44bac90 fuzz: Add general virtual-device fuzzer
d68075b memory: Add FlatView foreach function

=== OUTPUT BEGIN ===
1/16 Checking commit d68075b23fc3 (memory: Add FlatView foreach function)
2/16 Checking commit 44bac90b4413 (fuzz: Add general virtual-device fuzzer)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#19: 
new file mode 100644

total: 0 errors, 1 warnings, 505 lines checked

Patch 2/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/16 Checking commit 585e6f0153b5 (fuzz: Add PCI features to the general fuzzer)
4/16 Checking commit 0fb3331d35db (fuzz: Add DMA support to the generic-fuzzer)
ERROR: externs should be avoided in .c files
#84: FILE: tests/qtest/fuzz/general_fuzz.c:120:
+void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr, bool is_write);

total: 1 errors, 0 warnings, 247 lines checked

Patch 4/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/16 Checking commit e713ca2fcd7d (fuzz: Declare DMA Read callback function)
6/16 Checking commit 230a7bdf81ad (fuzz: Add fuzzer callbacks to DMA-read functions)
7/16 Checking commit 7013ef029b3d (fuzz: Add support for custom crossover functions)
8/16 Checking commit f69413a4bf8f (fuzz: add a DISABLE_PCI op to general-fuzzer)
9/16 Checking commit 29fd85365df7 (fuzz: add a crossover function to generic-fuzzer)
10/16 Checking commit 6261e7752953 (scripts/oss-fuzz: Add wrapper program for generic fuzzer)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#20: 
new file mode 100644

total: 0 errors, 1 warnings, 40 lines checked

Patch 10/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/16 Checking commit 7abda1159bd0 (scripts/oss-fuzz: Add general-fuzzer build script)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100755

WARNING: line over 80 characters
#57: FILE: scripts/oss-fuzz/build_general_fuzzers.py:36:
+                   fuzz_args=str_to_c_byte_array(cfg["args"].replace("\n", " ")),

WARNING: line over 80 characters
#58: FILE: scripts/oss-fuzz/build_general_fuzzers.py:37:
+                   fuzz_objs=str_to_c_byte_array(cfg["objects"].replace("\n", " ")),

total: 0 errors, 3 warnings, 69 lines checked

Patch 11/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/16 Checking commit 05353cfded4a (scripts/oss-fuzz: Add general-fuzzer configs for oss-fuzz)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100644

total: 0 errors, 1 warnings, 103 lines checked

Patch 12/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
13/16 Checking commit 7513795c708f (scripts/oss-fuzz: build the general-fuzzer configs)
14/16 Checking commit 8e0abed9fb55 (scripts/oss-fuzz: Add script to reorder a general-fuzzer trace)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#22: 
new file mode 100755

total: 0 errors, 1 warnings, 94 lines checked

Patch 14/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
15/16 Checking commit 410c8be1d76e (scripts/oss-fuzz: Add crash trace minimization script)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100755

total: 0 errors, 1 warnings, 157 lines checked

Patch 15/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
16/16 Checking commit 08f967285a5d (fuzz: Add instructions for using general-fuzz)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200921022506.873303-1-alxndr@bu.edu/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 00/16] Add a General Virtual Device Fuzzer
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (20 preceding siblings ...)
  2020-09-21  3:46 ` no-reply
@ 2020-09-21  4:30 ` no-reply
  2020-09-21  4:39 ` no-reply
                   ` (4 subsequent siblings)
  26 siblings, 0 replies; 53+ messages in thread
From: no-reply @ 2020-09-21  4:30 UTC (permalink / raw)
  To: alxndr; +Cc: alxndr, qemu-devel, darren.kenny, bsd, stefanha, philmd

Patchew URL: https://patchew.org/QEMU/20200921022506.873303-1-alxndr@bu.edu/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20200921022506.873303-1-alxndr@bu.edu
Subject: [PATCH v3 00/16] Add a General Virtual Device Fuzzer

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20200921022506.873303-1-alxndr@bu.edu -> patchew/20200921022506.873303-1-alxndr@bu.edu
Switched to a new branch 'test'
c5a12c5 fuzz: Add instructions for using general-fuzz
c891dcf scripts/oss-fuzz: Add crash trace minimization script
b407f74 scripts/oss-fuzz: Add script to reorder a general-fuzzer trace
6b4a6ed scripts/oss-fuzz: build the general-fuzzer configs
f3d8717 scripts/oss-fuzz: Add general-fuzzer configs for oss-fuzz
4f27ac9 scripts/oss-fuzz: Add general-fuzzer build script
12d3cc3 scripts/oss-fuzz: Add wrapper program for generic fuzzer
4757c0c fuzz: add a crossover function to generic-fuzzer
f6dea40 fuzz: add a DISABLE_PCI op to general-fuzzer
78c0bba fuzz: Add support for custom crossover functions
7622426 fuzz: Add fuzzer callbacks to DMA-read functions
172f58e fuzz: Declare DMA Read callback function
e69d9d2 fuzz: Add DMA support to the generic-fuzzer
f318099 fuzz: Add PCI features to the general fuzzer
b158f12 fuzz: Add general virtual-device fuzzer
bb981ec memory: Add FlatView foreach function

=== OUTPUT BEGIN ===
1/16 Checking commit bb981ecfaec7 (memory: Add FlatView foreach function)
2/16 Checking commit b158f12ff17a (fuzz: Add general virtual-device fuzzer)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#19: 
new file mode 100644

total: 0 errors, 1 warnings, 505 lines checked

Patch 2/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/16 Checking commit f31809900b15 (fuzz: Add PCI features to the general fuzzer)
4/16 Checking commit e69d9d2d1eff (fuzz: Add DMA support to the generic-fuzzer)
ERROR: externs should be avoided in .c files
#84: FILE: tests/qtest/fuzz/general_fuzz.c:120:
+void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr, bool is_write);

total: 1 errors, 0 warnings, 247 lines checked

Patch 4/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/16 Checking commit 172f58e72b9f (fuzz: Declare DMA Read callback function)
6/16 Checking commit 7622426e63a6 (fuzz: Add fuzzer callbacks to DMA-read functions)
7/16 Checking commit 78c0bba4f019 (fuzz: Add support for custom crossover functions)
8/16 Checking commit f6dea40297ff (fuzz: add a DISABLE_PCI op to general-fuzzer)
9/16 Checking commit 4757c0ce2ff6 (fuzz: add a crossover function to generic-fuzzer)
10/16 Checking commit 12d3cc318c7b (scripts/oss-fuzz: Add wrapper program for generic fuzzer)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#20: 
new file mode 100644

total: 0 errors, 1 warnings, 40 lines checked

Patch 10/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/16 Checking commit 4f27ac983f0c (scripts/oss-fuzz: Add general-fuzzer build script)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100755

WARNING: line over 80 characters
#57: FILE: scripts/oss-fuzz/build_general_fuzzers.py:36:
+                   fuzz_args=str_to_c_byte_array(cfg["args"].replace("\n", " ")),

WARNING: line over 80 characters
#58: FILE: scripts/oss-fuzz/build_general_fuzzers.py:37:
+                   fuzz_objs=str_to_c_byte_array(cfg["objects"].replace("\n", " ")),

total: 0 errors, 3 warnings, 69 lines checked

Patch 11/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/16 Checking commit f3d87177b8a8 (scripts/oss-fuzz: Add general-fuzzer configs for oss-fuzz)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100644

total: 0 errors, 1 warnings, 103 lines checked

Patch 12/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
13/16 Checking commit 6b4a6ede3e7c (scripts/oss-fuzz: build the general-fuzzer configs)
14/16 Checking commit b407f741be78 (scripts/oss-fuzz: Add script to reorder a general-fuzzer trace)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#22: 
new file mode 100755

total: 0 errors, 1 warnings, 94 lines checked

Patch 14/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
15/16 Checking commit c891dcf25dc8 (scripts/oss-fuzz: Add crash trace minimization script)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100755

total: 0 errors, 1 warnings, 157 lines checked

Patch 15/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
16/16 Checking commit c5a12c542fe1 (fuzz: Add instructions for using general-fuzz)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200921022506.873303-1-alxndr@bu.edu/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 00/16] Add a General Virtual Device Fuzzer
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (21 preceding siblings ...)
  2020-09-21  4:30 ` no-reply
@ 2020-09-21  4:39 ` no-reply
  2020-09-21  5:22 ` no-reply
                   ` (3 subsequent siblings)
  26 siblings, 0 replies; 53+ messages in thread
From: no-reply @ 2020-09-21  4:39 UTC (permalink / raw)
  To: alxndr; +Cc: alxndr, qemu-devel, darren.kenny, bsd, stefanha, philmd

Patchew URL: https://patchew.org/QEMU/20200921022506.873303-1-alxndr@bu.edu/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

Host machine cpu: x86_64
Target machine cpu family: x86
Target machine cpu: x86_64
../src/meson.build:10: WARNING: Module unstable-keyval has no backwards or forwards compatibility and might not exist in future releases.
Program sh found: YES
Program python3 found: YES (/usr/bin/python3)
Configuring ninjatool using configuration
---
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory '/tmp/qemu-test/src/slirp'
Generating nsis with a custom command
make[1]: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.
make[1]: Entering directory '/tmp/qemu-test/build'
make[2]: Entering directory '/tmp/qemu-test/src/slirp'
make[2]: Nothing to be done for 'all'.
---
Host machine cpu: i386
Target machine cpu family: x86
Target machine cpu: i386
../src/meson.build:10: WARNING: Module unstable-keyval has no backwards or forwards compatibility and might not exist in future releases.
Program sh found: YES
Program python3 found: YES (/usr/bin/python3)
Configuring ninjatool using configuration
---
Compiling C object libqemu-x86_64-softmmu.fa.p/meson-generated_.._qapi_qapi-commands-misc-target.c.obj
Compiling C object libqemu-x86_64-softmmu.fa.p/meson-generated_.._qapi_qapi-introspect.c.obj
../src/softmmu/memory.c: In function 'flatview_for_each_range':
../src/softmmu/memory.c:662:24: error: incompatible type for argument 1 of 'cb'
  662 |         if (cb(fr->addr.start, fr->addr.size, fr->mr, opaque))
      |                ~~~~~~~~^~~~~~
      |                        |
      |                        Int128 {aka struct Int128}
../src/softmmu/memory.c:662:24: note: expected 'ram_addr_t' {aka 'unsigned int'} but argument is of type 'Int128' {aka 'struct Int128'}
../src/softmmu/memory.c:662:40: error: incompatible type for argument 2 of 'cb'
  662 |         if (cb(fr->addr.start, fr->addr.size, fr->mr, opaque))
      |                                ~~~~~~~~^~~~~
      |                                        |
---
Compiling C object libqemu-x86_64-softmmu.fa.p/meson-generated_.._qapi_qapi-commands-machine-target.c.obj
Compiling C object libqemu-x86_64-softmmu.fa.p/meson-generated_.._qapi_qapi-events-machine-target.c.obj
Compiling C object libqemu-x86_64-softmmu.fa.p/meson-generated_.._qapi_qapi-visit.c.obj
make: *** [Makefile.ninja:1614: libqemu-x86_64-softmmu.fa.p/softmmu_memory.c.obj] Error 1
make: *** Waiting for unfinished jobs....
writing output... [ 63%] quickstart
writing output... [ 65%] s390x/3270
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--rm', '--label', 'com.qemu.instance.uuid=4fd88488a3e949c5bfcb744e06e6e10b', '-u', '1003', '--security-opt', 'seccomp=unconfined', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-g98bmkoq/src/docker-src.2020-09-21-00.30.50.13082:/var/tmp/qemu:z,ro', 'qemu/fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=4fd88488a3e949c5bfcb744e06e6e10b
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-g98bmkoq/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    8m48.046s
user    0m21.161s


The full log is available at
http://patchew.org/logs/20200921022506.873303-1-alxndr@bu.edu/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 00/16] Add a General Virtual Device Fuzzer
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (22 preceding siblings ...)
  2020-09-21  4:39 ` no-reply
@ 2020-09-21  5:22 ` no-reply
  2020-09-21  5:31 ` no-reply
                   ` (2 subsequent siblings)
  26 siblings, 0 replies; 53+ messages in thread
From: no-reply @ 2020-09-21  5:22 UTC (permalink / raw)
  To: alxndr; +Cc: alxndr, qemu-devel, darren.kenny, bsd, stefanha, philmd

Patchew URL: https://patchew.org/QEMU/20200921022506.873303-1-alxndr@bu.edu/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20200921022506.873303-1-alxndr@bu.edu
Subject: [PATCH v3 00/16] Add a General Virtual Device Fuzzer

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20200921022506.873303-1-alxndr@bu.edu -> patchew/20200921022506.873303-1-alxndr@bu.edu
 - [tag update]      patchew/20200921040231.437653-1-f4bug@amsat.org -> patchew/20200921040231.437653-1-f4bug@amsat.org
Switched to a new branch 'test'
ac49b50 fuzz: Add instructions for using general-fuzz
03cfb56 scripts/oss-fuzz: Add crash trace minimization script
4a95274 scripts/oss-fuzz: Add script to reorder a general-fuzzer trace
7b5df76 scripts/oss-fuzz: build the general-fuzzer configs
f0600bf scripts/oss-fuzz: Add general-fuzzer configs for oss-fuzz
c749add scripts/oss-fuzz: Add general-fuzzer build script
b477a48 scripts/oss-fuzz: Add wrapper program for generic fuzzer
1afef40 fuzz: add a crossover function to generic-fuzzer
0336e30 fuzz: add a DISABLE_PCI op to general-fuzzer
466bdd9 fuzz: Add support for custom crossover functions
7aeb4d9 fuzz: Add fuzzer callbacks to DMA-read functions
0d8a3cf fuzz: Declare DMA Read callback function
5181305 fuzz: Add DMA support to the generic-fuzzer
2d31fe3 fuzz: Add PCI features to the general fuzzer
163ca9e fuzz: Add general virtual-device fuzzer
ebe51bc memory: Add FlatView foreach function

=== OUTPUT BEGIN ===
1/16 Checking commit ebe51bc5fd2f (memory: Add FlatView foreach function)
2/16 Checking commit 163ca9ee783f (fuzz: Add general virtual-device fuzzer)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#19: 
new file mode 100644

total: 0 errors, 1 warnings, 505 lines checked

Patch 2/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/16 Checking commit 2d31fe390ee1 (fuzz: Add PCI features to the general fuzzer)
4/16 Checking commit 5181305b5987 (fuzz: Add DMA support to the generic-fuzzer)
ERROR: externs should be avoided in .c files
#84: FILE: tests/qtest/fuzz/general_fuzz.c:120:
+void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr, bool is_write);

total: 1 errors, 0 warnings, 247 lines checked

Patch 4/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/16 Checking commit 0d8a3cf19a0a (fuzz: Declare DMA Read callback function)
6/16 Checking commit 7aeb4d97aa66 (fuzz: Add fuzzer callbacks to DMA-read functions)
7/16 Checking commit 466bdd9666a5 (fuzz: Add support for custom crossover functions)
8/16 Checking commit 0336e304bd66 (fuzz: add a DISABLE_PCI op to general-fuzzer)
9/16 Checking commit 1afef402ddef (fuzz: add a crossover function to generic-fuzzer)
10/16 Checking commit b477a487ad70 (scripts/oss-fuzz: Add wrapper program for generic fuzzer)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#20: 
new file mode 100644

total: 0 errors, 1 warnings, 40 lines checked

Patch 10/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/16 Checking commit c749addcaa6b (scripts/oss-fuzz: Add general-fuzzer build script)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100755

WARNING: line over 80 characters
#57: FILE: scripts/oss-fuzz/build_general_fuzzers.py:36:
+                   fuzz_args=str_to_c_byte_array(cfg["args"].replace("\n", " ")),

WARNING: line over 80 characters
#58: FILE: scripts/oss-fuzz/build_general_fuzzers.py:37:
+                   fuzz_objs=str_to_c_byte_array(cfg["objects"].replace("\n", " ")),

total: 0 errors, 3 warnings, 69 lines checked

Patch 11/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/16 Checking commit f0600bfb96db (scripts/oss-fuzz: Add general-fuzzer configs for oss-fuzz)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100644

total: 0 errors, 1 warnings, 103 lines checked

Patch 12/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
13/16 Checking commit 7b5df76f494b (scripts/oss-fuzz: build the general-fuzzer configs)
14/16 Checking commit 4a9527426d38 (scripts/oss-fuzz: Add script to reorder a general-fuzzer trace)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#22: 
new file mode 100755

total: 0 errors, 1 warnings, 94 lines checked

Patch 14/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
15/16 Checking commit 03cfb56d5c0c (scripts/oss-fuzz: Add crash trace minimization script)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100755

total: 0 errors, 1 warnings, 157 lines checked

Patch 15/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
16/16 Checking commit ac49b50e7b45 (fuzz: Add instructions for using general-fuzz)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200921022506.873303-1-alxndr@bu.edu/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 00/16] Add a General Virtual Device Fuzzer
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (23 preceding siblings ...)
  2020-09-21  5:22 ` no-reply
@ 2020-09-21  5:31 ` no-reply
  2020-09-21  6:17 ` no-reply
  2020-09-21  6:26 ` no-reply
  26 siblings, 0 replies; 53+ messages in thread
From: no-reply @ 2020-09-21  5:31 UTC (permalink / raw)
  To: alxndr; +Cc: alxndr, qemu-devel, darren.kenny, bsd, stefanha, philmd

Patchew URL: https://patchew.org/QEMU/20200921022506.873303-1-alxndr@bu.edu/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

Host machine cpu: x86_64
Target machine cpu family: x86
Target machine cpu: x86_64
../src/meson.build:10: WARNING: Module unstable-keyval has no backwards or forwards compatibility and might not exist in future releases.
Program sh found: YES
Program python3 found: YES (/usr/bin/python3)
Configuring ninjatool using configuration
---
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory '/tmp/qemu-test/src/slirp'
Generating nsis with a custom command
make[1]: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.
make[1]: Entering directory '/tmp/qemu-test/build'
make[2]: Entering directory '/tmp/qemu-test/src/slirp'
make[2]: Nothing to be done for 'all'.
---
Host machine cpu: i386
Target machine cpu family: x86
Target machine cpu: i386
../src/meson.build:10: WARNING: Module unstable-keyval has no backwards or forwards compatibility and might not exist in future releases.
Program sh found: YES
Program python3 found: YES (/usr/bin/python3)
Configuring ninjatool using configuration
---
Compiling C object libblock.fa.p/nbd_client.c.obj
Compiling C object libblock.fa.p/block_backup-top.c.obj
../src/softmmu/memory.c: In function 'flatview_for_each_range':
../src/softmmu/memory.c:662:24: error: incompatible type for argument 1 of 'cb'
  662 |         if (cb(fr->addr.start, fr->addr.size, fr->mr, opaque))
      |                ~~~~~~~~^~~~~~
      |                        |
      |                        Int128 {aka struct Int128}
../src/softmmu/memory.c:662:24: note: expected 'ram_addr_t' {aka 'unsigned int'} but argument is of type 'Int128' {aka 'struct Int128'}
../src/softmmu/memory.c:662:40: error: incompatible type for argument 2 of 'cb'
  662 |         if (cb(fr->addr.start, fr->addr.size, fr->mr, opaque))
      |                                ~~~~~~~~^~~~~
      |                                        |
      |                                        Int128 {aka struct Int128}
../src/softmmu/memory.c:662:40: note: expected 'ram_addr_t' {aka 'unsigned int'} but argument is of type 'Int128' {aka 'struct Int128'}
make: *** [Makefile.ninja:1619: libqemu-x86_64-softmmu.fa.p/softmmu_memory.c.obj] Error 1
make: *** Waiting for unfinished jobs....
writing output... [ 72%] s390x/vfio-ap
writing output... [ 73%] s390x/vfio-ccw
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--rm', '--label', 'com.qemu.instance.uuid=65ea885b2e8140d5b330f237b0927fc7', '-u', '1001', '--security-opt', 'seccomp=unconfined', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-3iv7vq71/src/docker-src.2020-09-21-01.23.02.30031:/var/tmp/qemu:z,ro', 'qemu/fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=65ea885b2e8140d5b330f237b0927fc7
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-3iv7vq71/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    8m48.240s
user    0m20.729s


The full log is available at
http://patchew.org/logs/20200921022506.873303-1-alxndr@bu.edu/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer
  2020-09-21  2:24 ` [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer Alexander Bulekov
@ 2020-09-21  5:43   ` Philippe Mathieu-Daudé
  2020-09-21 14:34     ` Alexander Bulekov
  2020-09-22 14:03   ` Alexander Bulekov
  1 sibling, 1 reply; 53+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-21  5:43 UTC (permalink / raw)
  To: Alexander Bulekov, qemu-devel
  Cc: Laurent Vivier, Thomas Huth, darren.kenny, bsd, stefanha, Paolo Bonzini

Hi Alexander,

On 9/21/20 4:24 AM, Alexander Bulekov wrote:
> This is a generic fuzzer designed to fuzz a virtual device's
> MemoryRegions, as long as they exist within the Memory or Port IO (if it
> exists) AddressSpaces. The fuzzer's input is interpreted into a sequence
> of qtest commands (outb, readw, etc). The interpreted commands are
> separated by a magic seaparator, which should be easy for the fuzzer to
> guess. Without ASan, the separator can be specified as a "dictionary
> value" using the -dict argument (see libFuzzer documentation).
> 
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  tests/qtest/fuzz/general_fuzz.c | 498 ++++++++++++++++++++++++++++++++
>  tests/qtest/fuzz/meson.build    |   1 +
>  2 files changed, 499 insertions(+)
>  create mode 100644 tests/qtest/fuzz/general_fuzz.c
> 
> diff --git a/tests/qtest/fuzz/general_fuzz.c b/tests/qtest/fuzz/general_fuzz.c
> new file mode 100644
> index 0000000000..bf75b215ca
> --- /dev/null
> +++ b/tests/qtest/fuzz/general_fuzz.c
> @@ -0,0 +1,498 @@
> +/*
> + * General Virtual-Device Fuzzing Target
> + *
> + * Copyright Red Hat Inc., 2020
> + *
> + * 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 <wordexp.h>
> +
> +#include "hw/core/cpu.h"
> +#include "tests/qtest/libqos/libqtest.h"
> +#include "fuzz.h"
> +#include "fork_fuzz.h"
> +#include "exec/address-spaces.h"
> +#include "string.h"
> +#include "exec/memory.h"
> +#include "exec/ramblock.h"
> +#include "exec/address-spaces.h"
> +#include "hw/qdev-core.h"
> +
> +/*
> + * SEPARATOR is used to separate "operations" in the fuzz input
> + */
> +#define SEPARATOR "FUZZ"

Why use a separator when all pkt sizes are known?

Can you fuzz writing "FUZZ" in memory? Like:
OP_WRITE(0x100000, "UsingLibFUZZerString")?

> +
> +enum cmds {
> +    OP_IN,
> +    OP_OUT,
> +    OP_READ,
> +    OP_WRITE,
> +    OP_CLOCK_STEP,
> +};
> +
> +#define DEFAULT_TIMEOUT_US 100000
> +#define USEC_IN_SEC 100000000

Are you sure this definition is correct?

> +
> +typedef struct {
> +    ram_addr_t addr;
> +    ram_addr_t size; /* The number of bytes until the end of the I/O region */
> +} address_range;
> +
> +static useconds_t timeout = 100000;
[...]



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 03/16] fuzz: Add PCI features to the general fuzzer
  2020-09-21  2:24 ` [PATCH v3 03/16] fuzz: Add PCI features to the general fuzzer Alexander Bulekov
@ 2020-09-21  5:44   ` Philippe Mathieu-Daudé
  2020-09-21 14:41     ` Alexander Bulekov
  0 siblings, 1 reply; 53+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-21  5:44 UTC (permalink / raw)
  To: Alexander Bulekov, qemu-devel
  Cc: Laurent Vivier, Thomas Huth, darren.kenny, bsd, stefanha, Paolo Bonzini

On 9/21/20 4:24 AM, Alexander Bulekov wrote:
> This patch compares TYPE_PCI_DEVICE objects against the user-provided
> matching pattern. If there is a match, we use some hacks and leverage
> QOS to map each possible BAR for that device. Now fuzzed inputs might be
> converted to pci_read/write commands which target specific. This means
> that we can fuzz a particular device's PCI configuration space,
> 
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
> ---
>  tests/qtest/fuzz/general_fuzz.c | 81 +++++++++++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
> 
> diff --git a/tests/qtest/fuzz/general_fuzz.c b/tests/qtest/fuzz/general_fuzz.c
> index bf75b215ca..7c4c1398a7 100644
> --- a/tests/qtest/fuzz/general_fuzz.c
> +++ b/tests/qtest/fuzz/general_fuzz.c
> @@ -24,6 +24,7 @@
>  #include "exec/ramblock.h"
>  #include "exec/address-spaces.h"
>  #include "hw/qdev-core.h"
> +#include "hw/pci/pci.h"
>  
>  /*
>   * SEPARATOR is used to separate "operations" in the fuzz input
> @@ -35,12 +36,17 @@ enum cmds {
>      OP_OUT,
>      OP_READ,
>      OP_WRITE,
> +    OP_PCI_READ,
> +    OP_PCI_WRITE,
>      OP_CLOCK_STEP,
>  };

As there is no versioning, does adding new commands
invalidates the corpus?

[...]



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 00/16] Add a General Virtual Device Fuzzer
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (24 preceding siblings ...)
  2020-09-21  5:31 ` no-reply
@ 2020-09-21  6:17 ` no-reply
  2020-09-21  6:26 ` no-reply
  26 siblings, 0 replies; 53+ messages in thread
From: no-reply @ 2020-09-21  6:17 UTC (permalink / raw)
  To: alxndr; +Cc: alxndr, qemu-devel, darren.kenny, bsd, stefanha, philmd

Patchew URL: https://patchew.org/QEMU/20200921022506.873303-1-alxndr@bu.edu/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20200921022506.873303-1-alxndr@bu.edu
Subject: [PATCH v3 00/16] Add a General Virtual Device Fuzzer

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20200921022506.873303-1-alxndr@bu.edu -> patchew/20200921022506.873303-1-alxndr@bu.edu
Switched to a new branch 'test'
d9d0265 fuzz: Add instructions for using general-fuzz
d950d49 scripts/oss-fuzz: Add crash trace minimization script
168befa scripts/oss-fuzz: Add script to reorder a general-fuzzer trace
ef2230a scripts/oss-fuzz: build the general-fuzzer configs
1888cf5 scripts/oss-fuzz: Add general-fuzzer configs for oss-fuzz
c715a72 scripts/oss-fuzz: Add general-fuzzer build script
091fb32 scripts/oss-fuzz: Add wrapper program for generic fuzzer
64f57dd fuzz: add a crossover function to generic-fuzzer
c403f95 fuzz: add a DISABLE_PCI op to general-fuzzer
c8f8177 fuzz: Add support for custom crossover functions
3ea40a4 fuzz: Add fuzzer callbacks to DMA-read functions
8a42707 fuzz: Declare DMA Read callback function
2cc6034 fuzz: Add DMA support to the generic-fuzzer
39e32b1 fuzz: Add PCI features to the general fuzzer
d451370 fuzz: Add general virtual-device fuzzer
5c8d0e4 memory: Add FlatView foreach function

=== OUTPUT BEGIN ===
1/16 Checking commit 5c8d0e4bae0b (memory: Add FlatView foreach function)
2/16 Checking commit d451370ff085 (fuzz: Add general virtual-device fuzzer)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#19: 
new file mode 100644

total: 0 errors, 1 warnings, 505 lines checked

Patch 2/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/16 Checking commit 39e32b1703bc (fuzz: Add PCI features to the general fuzzer)
4/16 Checking commit 2cc603435c94 (fuzz: Add DMA support to the generic-fuzzer)
ERROR: externs should be avoided in .c files
#84: FILE: tests/qtest/fuzz/general_fuzz.c:120:
+void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr, bool is_write);

total: 1 errors, 0 warnings, 247 lines checked

Patch 4/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/16 Checking commit 8a42707161e0 (fuzz: Declare DMA Read callback function)
6/16 Checking commit 3ea40a4d49b0 (fuzz: Add fuzzer callbacks to DMA-read functions)
7/16 Checking commit c8f8177fdacf (fuzz: Add support for custom crossover functions)
8/16 Checking commit c403f9555371 (fuzz: add a DISABLE_PCI op to general-fuzzer)
9/16 Checking commit 64f57ddcf7e6 (fuzz: add a crossover function to generic-fuzzer)
10/16 Checking commit 091fb32210ca (scripts/oss-fuzz: Add wrapper program for generic fuzzer)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#20: 
new file mode 100644

total: 0 errors, 1 warnings, 40 lines checked

Patch 10/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/16 Checking commit c715a727b0a4 (scripts/oss-fuzz: Add general-fuzzer build script)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100755

WARNING: line over 80 characters
#57: FILE: scripts/oss-fuzz/build_general_fuzzers.py:36:
+                   fuzz_args=str_to_c_byte_array(cfg["args"].replace("\n", " ")),

WARNING: line over 80 characters
#58: FILE: scripts/oss-fuzz/build_general_fuzzers.py:37:
+                   fuzz_objs=str_to_c_byte_array(cfg["objects"].replace("\n", " ")),

total: 0 errors, 3 warnings, 69 lines checked

Patch 11/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/16 Checking commit 1888cf57cb94 (scripts/oss-fuzz: Add general-fuzzer configs for oss-fuzz)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100644

total: 0 errors, 1 warnings, 103 lines checked

Patch 12/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
13/16 Checking commit ef2230afae0a (scripts/oss-fuzz: build the general-fuzzer configs)
14/16 Checking commit 168befa7c4fc (scripts/oss-fuzz: Add script to reorder a general-fuzzer trace)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#22: 
new file mode 100755

total: 0 errors, 1 warnings, 94 lines checked

Patch 14/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
15/16 Checking commit d950d4983bf8 (scripts/oss-fuzz: Add crash trace minimization script)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#17: 
new file mode 100755

total: 0 errors, 1 warnings, 157 lines checked

Patch 15/16 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
16/16 Checking commit d9d02650fb57 (fuzz: Add instructions for using general-fuzz)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200921022506.873303-1-alxndr@bu.edu/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 00/16] Add a General Virtual Device Fuzzer
  2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
                   ` (25 preceding siblings ...)
  2020-09-21  6:17 ` no-reply
@ 2020-09-21  6:26 ` no-reply
  26 siblings, 0 replies; 53+ messages in thread
From: no-reply @ 2020-09-21  6:26 UTC (permalink / raw)
  To: alxndr; +Cc: alxndr, qemu-devel, darren.kenny, bsd, stefanha, philmd

Patchew URL: https://patchew.org/QEMU/20200921022506.873303-1-alxndr@bu.edu/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

Host machine cpu: x86_64
Target machine cpu family: x86
Target machine cpu: x86_64
../src/meson.build:10: WARNING: Module unstable-keyval has no backwards or forwards compatibility and might not exist in future releases.
Program sh found: YES
Program python3 found: YES (/usr/bin/python3)
Configuring ninjatool using configuration
---
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory '/tmp/qemu-test/src/slirp'
Generating nsis with a custom command
make[1]: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.
make[1]: Entering directory '/tmp/qemu-test/build'
make[2]: Entering directory '/tmp/qemu-test/src/slirp'
make[2]: Nothing to be done for 'all'.
---
Host machine cpu: i386
Target machine cpu family: x86
Target machine cpu: i386
../src/meson.build:10: WARNING: Module unstable-keyval has no backwards or forwards compatibility and might not exist in future releases.
Program sh found: YES
Program python3 found: YES (/usr/bin/python3)
Configuring ninjatool using configuration
---
Compiling C object libblock.fa.p/block_crypto.c.obj
Compiling C object libblock.fa.p/nbd_client.c.obj
../src/softmmu/memory.c: In function 'flatview_for_each_range':
../src/softmmu/memory.c:662:24: error: incompatible type for argument 1 of 'cb'
  662 |         if (cb(fr->addr.start, fr->addr.size, fr->mr, opaque))
      |                ~~~~~~~~^~~~~~
      |                        |
      |                        Int128 {aka struct Int128}
../src/softmmu/memory.c:662:24: note: expected 'ram_addr_t' {aka 'unsigned int'} but argument is of type 'Int128' {aka 'struct Int128'}
../src/softmmu/memory.c:662:40: error: incompatible type for argument 2 of 'cb'
  662 |         if (cb(fr->addr.start, fr->addr.size, fr->mr, opaque))
      |                                ~~~~~~~~^~~~~
      |                                        |
      |                                        Int128 {aka struct Int128}
../src/softmmu/memory.c:662:40: note: expected 'ram_addr_t' {aka 'unsigned int'} but argument is of type 'Int128' {aka 'struct Int128'}
Compiling C object libblock.fa.p/block_blkreplay.c.obj
make: *** [Makefile.ninja:1631: libqemu-x86_64-softmmu.fa.p/softmmu_memory.c.obj] Error 1
make: *** Waiting for unfinished jobs....
writing output... [ 91%] target-sparc64
writing output... [ 93%] target-xtensa
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--rm', '--label', 'com.qemu.instance.uuid=cdc507cddaae436bb961ff51ce93414a', '-u', '1001', '--security-opt', 'seccomp=unconfined', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-7s3xp4k6/src/docker-src.2020-09-21-02.18.35.17865:/var/tmp/qemu:z,ro', 'qemu/fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=cdc507cddaae436bb961ff51ce93414a
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-7s3xp4k6/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    7m26.992s
user    0m22.230s


The full log is available at
http://patchew.org/logs/20200921022506.873303-1-alxndr@bu.edu/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer
  2020-09-21  5:43   ` Philippe Mathieu-Daudé
@ 2020-09-21 14:34     ` Alexander Bulekov
  2020-10-01 15:29       ` Darren Kenny
  2020-10-08  7:03       ` Paolo Bonzini
  0 siblings, 2 replies; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21 14:34 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Laurent Vivier, Thomas Huth, qemu-devel, darren.kenny, bsd,
	stefanha, Paolo Bonzini

On 200921 0743, Philippe Mathieu-Daudé wrote:
> Hi Alexander,
> 
> On 9/21/20 4:24 AM, Alexander Bulekov wrote:
> > This is a generic fuzzer designed to fuzz a virtual device's
> > MemoryRegions, as long as they exist within the Memory or Port IO (if it
> > exists) AddressSpaces. The fuzzer's input is interpreted into a sequence
> > of qtest commands (outb, readw, etc). The interpreted commands are
> > separated by a magic seaparator, which should be easy for the fuzzer to
> > guess. Without ASan, the separator can be specified as a "dictionary
> > value" using the -dict argument (see libFuzzer documentation).
> > 
> > Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> > ---
> >  tests/qtest/fuzz/general_fuzz.c | 498 ++++++++++++++++++++++++++++++++
> >  tests/qtest/fuzz/meson.build    |   1 +
> >  2 files changed, 499 insertions(+)
> >  create mode 100644 tests/qtest/fuzz/general_fuzz.c
> > 
> > diff --git a/tests/qtest/fuzz/general_fuzz.c b/tests/qtest/fuzz/general_fuzz.c
> > new file mode 100644
> > index 0000000000..bf75b215ca
> > --- /dev/null
> > +++ b/tests/qtest/fuzz/general_fuzz.c
> > @@ -0,0 +1,498 @@
> > +/*
> > + * General Virtual-Device Fuzzing Target
> > + *
> > + * Copyright Red Hat Inc., 2020
> > + *
> > + * 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 <wordexp.h>
> > +
> > +#include "hw/core/cpu.h"
> > +#include "tests/qtest/libqos/libqtest.h"
> > +#include "fuzz.h"
> > +#include "fork_fuzz.h"
> > +#include "exec/address-spaces.h"
> > +#include "string.h"
> > +#include "exec/memory.h"
> > +#include "exec/ramblock.h"
> > +#include "exec/address-spaces.h"
> > +#include "hw/qdev-core.h"
> > +
> > +/*
> > + * SEPARATOR is used to separate "operations" in the fuzz input
> > + */
> > +#define SEPARATOR "FUZZ"
> 
> Why use a separator when all pkt sizes are known?
Good point. 
1. When we add the DMA Pattern OP in patch 04/16, we now have
variable-width OPs.
2. Even when everything has a known size, take for example the input:
Acb Bd Caaaa Effff
Where Operation A has size 3, B: size 2, C size 5 ...:
Simply by removing the first byte, we now have a completely different
sequence of operations:
Cbbdc Aaa Aef Ff...
Thus the separators "add some stability" to random mutations:
Cb FUZZ Bd FUZZ Caaaa FUZZ Effff ...
(Cb is now invalid/ignored, but the rest of the commands are still
intact)
There is some libfuzzer documentation about this technique:
https://github.com/google/fuzzing/blob/master/docs/split-inputs.md#magic-separator

There is also a promising "FuzzDataProvider" header library that lets
you directly call functions, such as ConsumeBytes, or
ConsumeIntegralInRange, but unfortunately it is a C++ header.
> 
> Can you fuzz writing "FUZZ" in memory? Like:
> OP_WRITE(0x100000, "UsingLibFUZZerString")?

No.. Hopefully that's not a huge problem.

> > +
> > +enum cmds {
> > +    OP_IN,
> > +    OP_OUT,
> > +    OP_READ,
> > +    OP_WRITE,
> > +    OP_CLOCK_STEP,
> > +};
> > +
> > +#define DEFAULT_TIMEOUT_US 100000
> > +#define USEC_IN_SEC 100000000
> 
> Are you sure this definition is correct?
> 
Thanks for the catch...

> > +
> > +typedef struct {
> > +    ram_addr_t addr;
> > +    ram_addr_t size; /* The number of bytes until the end of the I/O region */
> > +} address_range;
> > +
> > +static useconds_t timeout = 100000;
> [...]
> 


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 03/16] fuzz: Add PCI features to the general fuzzer
  2020-09-21  5:44   ` Philippe Mathieu-Daudé
@ 2020-09-21 14:41     ` Alexander Bulekov
  0 siblings, 0 replies; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-21 14:41 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Laurent Vivier, Thomas Huth, qemu-devel, darren.kenny, bsd,
	stefanha, Paolo Bonzini

On 200921 0744, Philippe Mathieu-Daudé wrote:
> On 9/21/20 4:24 AM, Alexander Bulekov wrote:
> > This patch compares TYPE_PCI_DEVICE objects against the user-provided
> > matching pattern. If there is a match, we use some hacks and leverage
> > QOS to map each possible BAR for that device. Now fuzzed inputs might be
> > converted to pci_read/write commands which target specific. This means
> > that we can fuzz a particular device's PCI configuration space,
> > 
> > Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> > Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
> > ---
> >  tests/qtest/fuzz/general_fuzz.c | 81 +++++++++++++++++++++++++++++++++
> >  1 file changed, 81 insertions(+)
> > 
> > diff --git a/tests/qtest/fuzz/general_fuzz.c b/tests/qtest/fuzz/general_fuzz.c
> > index bf75b215ca..7c4c1398a7 100644
> > --- a/tests/qtest/fuzz/general_fuzz.c
> > +++ b/tests/qtest/fuzz/general_fuzz.c
> > @@ -24,6 +24,7 @@
> >  #include "exec/ramblock.h"
> >  #include "exec/address-spaces.h"
> >  #include "hw/qdev-core.h"
> > +#include "hw/pci/pci.h"
> >  
> >  /*
> >   * SEPARATOR is used to separate "operations" in the fuzz input
> > @@ -35,12 +36,17 @@ enum cmds {
> >      OP_OUT,
> >      OP_READ,
> >      OP_WRITE,
> > +    OP_PCI_READ,
> > +    OP_PCI_WRITE,
> >      OP_CLOCK_STEP,
> >  };
> 
> As there is no versioning, does adding new commands
> invalidates the corpus?
> 
> [...]
>

Yes. I think there are a few approaches:
1.) Write a separate OP parser/converter in python that will convert the
corpus to a newer version. Each time we change the code:
    a. write a converter
    b. download corpus from oss-fuzz
    c. convert the corpus
    d. commit new corpus to the seed corpus repo (we don't have one
    right now)
2.) Same as (1) but instead of corpus_v1 -> corpus_v2, 
try to convert corpus_v1 -> qtest commands -> corpus_v2
3.) Use 1st byte to pick corpus format version. I think this could lead
to a lot of code-bloat and could hurt fuzzing performance (fuzzer
mutations could make new inputs that still use old corpus versions)...
4.) Do nothing and wait..

Do you have any suggestion?
-Alex


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer
  2020-09-21  2:24 ` [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer Alexander Bulekov
  2020-09-21  5:43   ` Philippe Mathieu-Daudé
@ 2020-09-22 14:03   ` Alexander Bulekov
  2020-10-08  7:04     ` Paolo Bonzini
  1 sibling, 1 reply; 53+ messages in thread
From: Alexander Bulekov @ 2020-09-22 14:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Laurent Vivier, Thomas Huth, darren.kenny, bsd, stefanha,
	Paolo Bonzini, philmd

On 200920 2224, Alexander Bulekov wrote:
[snip]
> +static int locate_fuzz_memory_regions(Object *child, void *opaque)
> +{
> +    const char *name;
> +    MemoryRegion *mr;
> +    if (object_dynamic_cast(child, TYPE_MEMORY_REGION)) {
> +        mr = MEMORY_REGION(child);
> +        if ((memory_region_is_ram(mr) ||
> +            memory_region_is_ram_device(mr) ||
> +            memory_region_is_rom(mr) ||
> +            memory_region_is_romd(mr)) == false) {
> +            name = object_get_canonical_path_component(child);

This isn't a great way to check whether MRs have ops with code that is
interesting to fuzz (for example the pflash MemoryRegions do not pass
these checks, so you can't fuzz the pflash device). Need to think of
some better checks to identify MRs that we are interested in fuzzing.

-Alex

> +            /*
> +             * We don't want duplicate pointers to the same MemoryRegion, so
> +             * try to remove copies of the pointer, before adding it.
> +             */
> +            g_hash_table_insert(fuzzable_memoryregions, mr, (gpointer)true);
> +        }
> +    }
> +    return 0;
> +}
> +static int locate_fuzz_objects(Object *child, void *opaque)
> +{
> +    char *pattern = opaque;
> +    if (g_pattern_match_simple(pattern, object_get_typename(child))) {
> +        /* Find and save ptrs to any child MemoryRegions */
> +        object_child_foreach_recursive(child, locate_fuzz_memory_regions, NULL);
> +
> +    } else if (object_dynamic_cast(OBJECT(child), TYPE_MEMORY_REGION)) {
> +        if (g_pattern_match_simple(pattern,
> +            object_get_canonical_path_component(child))) {
> +            MemoryRegion *mr;
> +            mr = MEMORY_REGION(child);
> +            if ((memory_region_is_ram(mr) ||
> +                 memory_region_is_ram_device(mr) ||
> +                 memory_region_is_rom(mr) ||
> +                 memory_region_is_romd(mr)) == false) {
> +                g_hash_table_insert(fuzzable_memoryregions, mr, (gpointer)true);
> +            }
> +        }
> +    }
> +    return 0;
> +}


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer
  2020-09-21 14:34     ` Alexander Bulekov
@ 2020-10-01 15:29       ` Darren Kenny
  2020-10-07 13:39         ` Alexander Bulekov
  2020-10-08  7:03       ` Paolo Bonzini
  1 sibling, 1 reply; 53+ messages in thread
From: Darren Kenny @ 2020-10-01 15:29 UTC (permalink / raw)
  To: Alexander Bulekov, Philippe Mathieu-Daudé
  Cc: Laurent Vivier, Thomas Huth, qemu-devel, bsd, stefanha, Paolo Bonzini

Hi Alex,

On Monday, 2020-09-21 at 10:34:05 -04, Alexander Bulekov wrote:
> On 200921 0743, Philippe Mathieu-Daudé wrote:
>> Hi Alexander,
>> 
>> On 9/21/20 4:24 AM, Alexander Bulekov wrote:
>> > This is a generic fuzzer designed to fuzz a virtual device's
>> > MemoryRegions, as long as they exist within the Memory or Port IO (if it
>> > exists) AddressSpaces. The fuzzer's input is interpreted into a sequence
>> > of qtest commands (outb, readw, etc). The interpreted commands are
>> > separated by a magic seaparator, which should be easy for the fuzzer to
>> > guess. Without ASan, the separator can be specified as a "dictionary
>> > value" using the -dict argument (see libFuzzer documentation).
>> > 
>> > Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
>> > ---
>> >  tests/qtest/fuzz/general_fuzz.c | 498 ++++++++++++++++++++++++++++++++
>> >  tests/qtest/fuzz/meson.build    |   1 +
>> >  2 files changed, 499 insertions(+)
>> >  create mode 100644 tests/qtest/fuzz/general_fuzz.c
>> > 
>> > diff --git a/tests/qtest/fuzz/general_fuzz.c b/tests/qtest/fuzz/general_fuzz.c
>> > new file mode 100644
>> > index 0000000000..bf75b215ca
>> > --- /dev/null
>> > +++ b/tests/qtest/fuzz/general_fuzz.c
>> > @@ -0,0 +1,498 @@
>> > +/*
>> > + * General Virtual-Device Fuzzing Target
>> > + *
>> > + * Copyright Red Hat Inc., 2020
>> > + *
>> > + * 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 <wordexp.h>
>> > +
>> > +#include "hw/core/cpu.h"
>> > +#include "tests/qtest/libqos/libqtest.h"
>> > +#include "fuzz.h"
>> > +#include "fork_fuzz.h"
>> > +#include "exec/address-spaces.h"
>> > +#include "string.h"
>> > +#include "exec/memory.h"
>> > +#include "exec/ramblock.h"
>> > +#include "exec/address-spaces.h"
>> > +#include "hw/qdev-core.h"
>> > +
>> > +/*
>> > + * SEPARATOR is used to separate "operations" in the fuzz input
>> > + */
>> > +#define SEPARATOR "FUZZ"
>> 
>> Why use a separator when all pkt sizes are known?
> Good point. 
> 1. When we add the DMA Pattern OP in patch 04/16, we now have
> variable-width OPs.
> 2. Even when everything has a known size, take for example the input:
> Acb Bd Caaaa Effff
> Where Operation A has size 3, B: size 2, C size 5 ...:
> Simply by removing the first byte, we now have a completely different
> sequence of operations:
> Cbbdc Aaa Aef Ff...
> Thus the separators "add some stability" to random mutations:
> Cb FUZZ Bd FUZZ Caaaa FUZZ Effff ...
> (Cb is now invalid/ignored, but the rest of the commands are still
> intact)
> There is some libfuzzer documentation about this technique:
> https://github.com/google/fuzzing/blob/master/docs/split-inputs.md#magic-separator
>
> There is also a promising "FuzzDataProvider" header library that lets
> you directly call functions, such as ConsumeBytes, or
> ConsumeIntegralInRange, but unfortunately it is a C++ header.

It might make sense to put the definition of SEPARATOR and some variant
of the above the comments in patch 9 where you're adding this related
functionality?

It seems a little out of place here.

Thanks,

Darren.

>> 
>> Can you fuzz writing "FUZZ" in memory? Like:
>> OP_WRITE(0x100000, "UsingLibFUZZerString")?
>
> No.. Hopefully that's not a huge problem.
>
>> > +
>> > +enum cmds {
>> > +    OP_IN,
>> > +    OP_OUT,
>> > +    OP_READ,
>> > +    OP_WRITE,
>> > +    OP_CLOCK_STEP,
>> > +};
>> > +
>> > +#define DEFAULT_TIMEOUT_US 100000
>> > +#define USEC_IN_SEC 100000000
>> 
>> Are you sure this definition is correct?
>> 
> Thanks for the catch...
>
>> > +
>> > +typedef struct {
>> > +    ram_addr_t addr;
>> > +    ram_addr_t size; /* The number of bytes until the end of the I/O region */
>> > +} address_range;
>> > +
>> > +static useconds_t timeout = 100000;
>> [...]
>> 


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 09/16] fuzz: add a crossover function to generic-fuzzer
  2020-09-21  2:24 ` [PATCH v3 09/16] fuzz: add a crossover function to generic-fuzzer Alexander Bulekov
@ 2020-10-01 15:31   ` Darren Kenny
  2020-10-15 13:43     ` Alexander Bulekov
  0 siblings, 1 reply; 53+ messages in thread
From: Darren Kenny @ 2020-10-01 15:31 UTC (permalink / raw)
  To: Alexander Bulekov, qemu-devel
  Cc: Laurent Vivier, Thomas Huth, Alexander Bulekov, bsd, stefanha,
	Paolo Bonzini, philmd

As mentioned in an earlier patch, maybe the definition of SEPARATOR
should be here as well as some of the comments you provided in the
replies to it.

Otherwise, this looks good,

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

Thanks,

Darren.

On Sunday, 2020-09-20 at 22:24:59 -04, Alexander Bulekov wrote:
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  tests/qtest/fuzz/general_fuzz.c | 90 ++++++++++++++++++++++++++++++++-
>  1 file changed, 89 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qtest/fuzz/general_fuzz.c b/tests/qtest/fuzz/general_fuzz.c
> index 656ec7fd55..3833b505c8 100644
> --- a/tests/qtest/fuzz/general_fuzz.c
> +++ b/tests/qtest/fuzz/general_fuzz.c
> @@ -741,6 +741,92 @@ static void general_pre_fuzz(QTestState *s)
>  
>      counter_shm_init();
>  }
> +
> +/*
> + * When libfuzzer gives us two inputs to combine, return a new input with the
> + * following structure:
> + *
> + * Input 1 (data1)
> + * SEPARATOR
> + * Clear out the DMA Patterns
> + * SEPARATOR
> + * Disable the pci_read/write instructions
> + * SEPARATOR
> + * Input 2 (data2)
> + *
> + * The idea is to collate the core behaviors of the two inputs.
> + * For example:
> + * Input 1: maps a device's BARs, sets up three DMA patterns, and triggers
> + *          device functionality A
> + * Input 2: maps a device's BARs, sets up one DMA pattern, and triggers device
> + *          functionality B
> + *
> + * This function attempts to produce an input that:
> + * Ouptut: maps a device's BARs, set up three DMA patterns, triggers
> + *          functionality A device, replaces the DMA patterns with a single
> + *          patten, and triggers device functionality B.
> + */
> +static size_t general_fuzz_crossover(const uint8_t *data1, size_t size1, const
> +                                     uint8_t *data2, size_t size2, uint8_t *out,
> +                                     size_t max_out_size, unsigned int seed)
> +{
> +    size_t copy_len = 0, size = 0;
> +
> +    /* Check that we have enough space for data1 and at least part of data2 */
> +    if (max_out_size <= size + strlen(SEPARATOR) * 3 + 2) {
> +        return 0;
> +    }
> +
> +    /* Copy_Len in the first input */
> +    copy_len = size1;
> +    memcpy(out + size, data1, copy_len);
> +    size += copy_len;
> +    max_out_size -= copy_len;
> +
> +    /* Append a separator */
> +    copy_len = strlen(SEPARATOR);
> +    memcpy(out + size, SEPARATOR, copy_len);
> +    size += copy_len;
> +    max_out_size -= copy_len;
> +
> +    /* Clear out the DMA Patterns */
> +    copy_len = 1;
> +    if (copy_len) {
> +        out[size] = OP_CLEAR_DMA_PATTERNS;
> +    }
> +    size += copy_len;
> +    max_out_size -= copy_len;
> +
> +    /* Append a separator */
> +    copy_len = strlen(SEPARATOR);
> +    memcpy(out + size, SEPARATOR, copy_len);
> +    size += copy_len;
> +    max_out_size -= copy_len;
> +
> +    /* Disable PCI ops. Assume data1 took care of setting up PCI */
> +    copy_len = 1;
> +    if (copy_len) {
> +        out[size] = OP_DISABLE_PCI;
> +    }
> +    size += copy_len;
> +    max_out_size -= copy_len;
> +
> +    /* Append a separator */
> +    copy_len = strlen(SEPARATOR);
> +    memcpy(out + size, SEPARATOR, copy_len);
> +    size += copy_len;
> +    max_out_size -= copy_len;
> +
> +    /* Copy_Len over the second input */
> +    copy_len = MIN(size2, max_out_size);
> +    memcpy(out + size, data2, copy_len);
> +    size += copy_len;
> +    max_out_size -= copy_len;
> +
> +    return  size;
> +}
> +
> +
>  static GString *general_fuzz_cmdline(FuzzTarget *t)
>  {
>      GString *cmd_line = g_string_new(TARGET_NAME);
> @@ -760,7 +846,9 @@ static void register_general_fuzz_targets(void)
>              .description = "Fuzz based on any qemu command-line args. ",
>              .get_init_cmdline = general_fuzz_cmdline,
>              .pre_fuzz = general_pre_fuzz,
> -            .fuzz = general_fuzz});
> +            .fuzz = general_fuzz,
> +            .crossover = general_fuzz_crossover
> +    });
>  }
>  
>  fuzz_target_init(register_general_fuzz_targets);
> -- 
> 2.28.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 11/16] scripts/oss-fuzz: Add general-fuzzer build script
  2020-09-21  2:25 ` [PATCH v3 11/16] scripts/oss-fuzz: Add general-fuzzer build script Alexander Bulekov
@ 2020-10-01 15:40   ` Darren Kenny
  2020-10-08  7:35   ` Paolo Bonzini
  1 sibling, 0 replies; 53+ messages in thread
From: Darren Kenny @ 2020-10-01 15:40 UTC (permalink / raw)
  To: Alexander Bulekov, qemu-devel
  Cc: Thomas Huth, Alexander Bulekov, bsd, stefanha, Paolo Bonzini, philmd

Hi Alex,

On Sunday, 2020-09-20 at 22:25:01 -04, Alexander Bulekov wrote:
> This parses a yaml file containing general-fuzzer configs and builds a
> separate oss-fuzz wrapper binary for each one, changing some
> preprocessor macros for each configuration. To avoid dealing with
> escaping and stringifying, convert each string into a byte-array
> representation
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  scripts/oss-fuzz/build_general_fuzzers.py | 69 +++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100755 scripts/oss-fuzz/build_general_fuzzers.py
>
> diff --git a/scripts/oss-fuzz/build_general_fuzzers.py b/scripts/oss-fuzz/build_general_fuzzers.py
> new file mode 100755
> index 0000000000..918f1143a5
> --- /dev/null
> +++ b/scripts/oss-fuzz/build_general_fuzzers.py
> @@ -0,0 +1,69 @@
> +#!/usr/bin/env python3
> +# -*- coding: utf-8 -*-
> +
> +"""
> +This script creates wrapper binaries that invoke the general-device-fuzzer with
> +configurations specified in a yaml config file.
> +"""
> +import sys
> +import os
> +import yaml
> +import tempfile
> +
> +CC = ""
> +TEMPLATE_FILENAME = "target_template.c"
> +TEMPLATE_PATH = ""
> +
> +
> +def usage():
> +    print("Usage: CC=COMPILER {} CONFIG_PATH \
> +OUTPUT_PATH_PREFIX".format(sys.argv[0]))

The indentation of this seems off.

Python will concatenate 2 or more strings that appear one after the
other, so it might be cleaner to write them like:

    print("Usage: CC=COMPILER {} CONFIG_PATH "
          "OUTPUT_PATH_PREFIX".format(sys.argv[0]))

There is no need for the backslash at the end due to the use of the
braces '()' here.


> +    sys.exit(0)
> +
> +
> +def str_to_c_byte_array(s):
> +    """
> +    Convert strings to byte-arrays so we don't worry about formatting
> +    strings to play nicely with cc -DQEMU_FUZZARGS etc
> +    """
> +    return ','.join('0x{:02x}'.format(ord(x)) for x in s)
> +
> +
> +def compile_wrapper(cfg, path):
> +    os.system('$CC -DQEMU_FUZZ_ARGS="{fuzz_args}" \
> +               -DQEMU_FUZZ_OBJECTS="{fuzz_objs}" \
> +               {wrapper_template} -o {output_bin}'.format(
> +                   fuzz_args=str_to_c_byte_array(cfg["args"].replace("\n", " ")),
> +                   fuzz_objs=str_to_c_byte_array(cfg["objects"].replace("\n", " ")),
> +                   wrapper_template=TEMPLATE_PATH,
> +                   output_bin=path))
> +

Similarly here, it might look better as:

    os.system('$CC -DQEMU_FUZZ_ARGS="{fuzz_args}" '
              '-DQEMU_FUZZ_OBJECTS="{fuzz_objs}" '
              '{wrapper_template} -o {output_bin}'.format(
                   fuzz_args=str_to_c_byte_array(cfg["args"].replace("\n", " ")),
                   fuzz_objs=str_to_c_byte_array(cfg["objects"].replace("\n", " ")),
                   wrapper_template=TEMPLATE_PATH,
                   output_bin=path))

> +
> +
> +def main():
> +    global CC
> +    global TEMPLATE_PATH
> +    global OUTPUT_BIN_NAME
> +
> +    if len(sys.argv) != 3:
> +        usage()
> +
> +    cfg_path = sys.argv[1]
> +    out_path = sys.argv[2]
> +
> +    CC = os.getenv("CC", default="cc")
> +    TEMPLATE_PATH = os.path.join(os.path.dirname(__file__), TEMPLATE_FILENAME)
> +    if not os.path.exists(TEMPLATE_PATH):
> +        print("Error {} doesn't exist".format(TEMPLATE_PATH))
> +        sys.exit(1)
> +
> +    with open(cfg_path, "r") as f:
> +        configs = yaml.load(f)["configs"]

Just in case, the use of .get("config". []) might work better here.

But also check if yaml.load() could possibly throw an exception if the
file, despite existing isn't able to be parsed.

Thanks,

Darren.


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 16/16] fuzz: Add instructions for using general-fuzz
  2020-09-21  2:25 ` [PATCH v3 16/16] fuzz: Add instructions for using general-fuzz Alexander Bulekov
@ 2020-10-01 15:44   ` Darren Kenny
  0 siblings, 0 replies; 53+ messages in thread
From: Darren Kenny @ 2020-10-01 15:44 UTC (permalink / raw)
  To: Alexander Bulekov, qemu-devel
  Cc: Thomas Huth, Alexander Bulekov, bsd, stefanha, Paolo Bonzini, philmd

On Sunday, 2020-09-20 at 22:25:06 -04, Alexander Bulekov wrote:
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  docs/devel/fuzzing.txt | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/docs/devel/fuzzing.txt b/docs/devel/fuzzing.txt
> index 96d71c94d7..208b0c8360 100644
> --- a/docs/devel/fuzzing.txt
> +++ b/docs/devel/fuzzing.txt
> @@ -125,6 +125,44 @@ provided by libfuzzer. Libfuzzer passes a byte array and length. Commonly the
>  fuzzer loops over the byte-array interpreting it as a list of qtest commands,
>  addresses, or values.
>  
> +== The General Fuzzer ==
> +Writing a fuzz target can be a lot of effort (especially if a device driver has
> +not be built-out within libqos). Many devices can be fuzzed to some degree,
> +without any device-specific code, using the general-fuzz target.
> +
> +The general-fuzz target is capable of fuzzing devices over their PIO, MMIO,
> +and DMA input-spaces. To apply the general-fuzz to a device, we need to define
> +two env-variables, at minimum:
> +
> +QEMU_FUZZ_ARGS= is the set of QEMU arguments used to configure a machine, with
> +the device attached. For example, if we want to fuzz the virtio-net device
> +attached to a pc-i440fx machine, we can specify:
> +QEMU_FUZZ_ARGS="-M pc -nodefaults -netdev user,id=user0 \
> +                -device virtio-net,netdev=user0"
> +
> +QEMU_FUZZ_OBJECTS= is a set of space-delimited strings used to identify the
> +MemoryRegions that will be fuzzed. These strings are compared against
> +MemoryRegion names and MemoryRegion owner names, to decide whether each
> +MemoryRegion should be fuzzed. These strings support globbing. For the
> +virtio-net example, we could use
> +QEMU_FUZZ_OBJECTS='virtio-net'
> +QEMU_FUZZ_OBJECTS='virtio*'
> +QEMU_FUZZ_OBJECTS='virtio* pcspk' # Fuzz the virtio-net device and the PC speaker...
> +QEMU_FUZZ_OBJECTS='*' # Fuzz the whole machine
> +

Maybe format these as lists, where each list item is the variable?

> +The "info mtree" and "info qom-tree" monitor commands can be especially useful
> +for identifying the MemoryRegion and Object names used for matching.
> +
> +As a general rule-of-thumb, the more MemoryRegions/Devices we match, the greater
> +the input-space, and the smaller the probability of finding crashing inputs for
> +individual devices. As such, it is usually a good idea to limit the fuzzer to
> +only a few MemoryRegions.
> +
> +To ensure that these env variables have been configured correctly, we can use:

Add a blank line here?

> +./qemu-fuzz-i386 --fuzz-target=general-fuzz -runs=0
> +
> +The output should contain a complete list of matched MemoryRegions.
> +
>  = Implementation Details =
>  
>  == The Fuzzer's Lifecycle ==


Not this patch, but this text file is partially written in Restructured
Text format, but not completely. Should it be converted to RsT format
properly - doesn't have to be now, but something we could consider.

Otherwise, it looks find to me, so:

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

Thanks,

Darren.


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer
  2020-10-01 15:29       ` Darren Kenny
@ 2020-10-07 13:39         ` Alexander Bulekov
  2020-10-07 13:53           ` Darren Kenny
  0 siblings, 1 reply; 53+ messages in thread
From: Alexander Bulekov @ 2020-10-07 13:39 UTC (permalink / raw)
  To: Darren Kenny
  Cc: Laurent Vivier, Thomas Huth, qemu-devel, bsd, stefanha,
	Paolo Bonzini, Philippe Mathieu-Daudé

On 201001 1629, Darren Kenny wrote:
> Hi Alex,
> 
> On Monday, 2020-09-21 at 10:34:05 -04, Alexander Bulekov wrote:
> > On 200921 0743, Philippe Mathieu-Daudé wrote:
> >> Hi Alexander,
> >> 
> >> On 9/21/20 4:24 AM, Alexander Bulekov wrote:
> >> > This is a generic fuzzer designed to fuzz a virtual device's
> >> > MemoryRegions, as long as they exist within the Memory or Port IO (if it
> >> > exists) AddressSpaces. The fuzzer's input is interpreted into a sequence
> >> > of qtest commands (outb, readw, etc). The interpreted commands are
> >> > separated by a magic seaparator, which should be easy for the fuzzer to
> >> > guess. Without ASan, the separator can be specified as a "dictionary
> >> > value" using the -dict argument (see libFuzzer documentation).
> >> > 
> >> > Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> >> > ---
> >> >  tests/qtest/fuzz/general_fuzz.c | 498 ++++++++++++++++++++++++++++++++
> >> >  tests/qtest/fuzz/meson.build    |   1 +
> >> >  2 files changed, 499 insertions(+)
> >> >  create mode 100644 tests/qtest/fuzz/general_fuzz.c
> >> > 
> >> > diff --git a/tests/qtest/fuzz/general_fuzz.c b/tests/qtest/fuzz/general_fuzz.c
> >> > new file mode 100644
> >> > index 0000000000..bf75b215ca
> >> > --- /dev/null
> >> > +++ b/tests/qtest/fuzz/general_fuzz.c
> >> > @@ -0,0 +1,498 @@
> >> > +/*
> >> > + * General Virtual-Device Fuzzing Target
> >> > + *
> >> > + * Copyright Red Hat Inc., 2020
> >> > + *
> >> > + * 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 <wordexp.h>
> >> > +
> >> > +#include "hw/core/cpu.h"
> >> > +#include "tests/qtest/libqos/libqtest.h"
> >> > +#include "fuzz.h"
> >> > +#include "fork_fuzz.h"
> >> > +#include "exec/address-spaces.h"
> >> > +#include "string.h"
> >> > +#include "exec/memory.h"
> >> > +#include "exec/ramblock.h"
> >> > +#include "exec/address-spaces.h"
> >> > +#include "hw/qdev-core.h"
> >> > +
> >> > +/*
> >> > + * SEPARATOR is used to separate "operations" in the fuzz input
> >> > + */
> >> > +#define SEPARATOR "FUZZ"
> >> 
> >> Why use a separator when all pkt sizes are known?
> > Good point. 
> > 1. When we add the DMA Pattern OP in patch 04/16, we now have
> > variable-width OPs.
> > 2. Even when everything has a known size, take for example the input:
> > Acb Bd Caaaa Effff
> > Where Operation A has size 3, B: size 2, C size 5 ...:
> > Simply by removing the first byte, we now have a completely different
> > sequence of operations:
> > Cbbdc Aaa Aef Ff...
> > Thus the separators "add some stability" to random mutations:
> > Cb FUZZ Bd FUZZ Caaaa FUZZ Effff ...
> > (Cb is now invalid/ignored, but the rest of the commands are still
> > intact)
> > There is some libfuzzer documentation about this technique:
> > https://github.com/google/fuzzing/blob/master/docs/split-inputs.md#magic-separator
> >
> > There is also a promising "FuzzDataProvider" header library that lets
> > you directly call functions, such as ConsumeBytes, or
> > ConsumeIntegralInRange, but unfortunately it is a C++ header.
> 
> It might make sense to put the definition of SEPARATOR and some variant
> of the above the comments in patch 9 where you're adding this related
> functionality?
> 
> It seems a little out of place here.
> 
> Thanks,
> 
> Darren.
> 

Hi Darren,
If I move the definition of SEPARATOR to Patch 9, I would need some
different way to parse commands here, to keep everything bisectable. I
don't think the separator is only important in the context of the
Crossover functionality (Patch 9) - it is useful in general as a
"stable" way to parse an input into multiple commands.
Is it OK if I keep SEPARATOR in this patch and add the comments you
mention to both this patch and patch 9?
Thanks
-Alex

> >> 
> >> Can you fuzz writing "FUZZ" in memory? Like:
> >> OP_WRITE(0x100000, "UsingLibFUZZerString")?
> >
> > No.. Hopefully that's not a huge problem.
> >
> >> > +
> >> > +enum cmds {
> >> > +    OP_IN,
> >> > +    OP_OUT,
> >> > +    OP_READ,
> >> > +    OP_WRITE,
> >> > +    OP_CLOCK_STEP,
> >> > +};
> >> > +
> >> > +#define DEFAULT_TIMEOUT_US 100000
> >> > +#define USEC_IN_SEC 100000000
> >> 
> >> Are you sure this definition is correct?
> >> 
> > Thanks for the catch...
> >
> >> > +
> >> > +typedef struct {
> >> > +    ram_addr_t addr;
> >> > +    ram_addr_t size; /* The number of bytes until the end of the I/O region */
> >> > +} address_range;
> >> > +
> >> > +static useconds_t timeout = 100000;
> >> [...]
> >> 


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer
  2020-10-07 13:39         ` Alexander Bulekov
@ 2020-10-07 13:53           ` Darren Kenny
  0 siblings, 0 replies; 53+ messages in thread
From: Darren Kenny @ 2020-10-07 13:53 UTC (permalink / raw)
  To: Alexander Bulekov
  Cc: Laurent Vivier, Thomas Huth, qemu-devel, bsd, stefanha,
	Paolo Bonzini, Philippe Mathieu-Daudé

On Wednesday, 2020-10-07 at 09:39:32 -04, Alexander Bulekov wrote:
> On 201001 1629, Darren Kenny wrote:

...

>>
>> It might make sense to put the definition of SEPARATOR and some variant
>> of the above the comments in patch 9 where you're adding this related
>> functionality?
>> 
>> It seems a little out of place here.
>> 
>> Thanks,
>> 
>> Darren.
>> 
>
> Hi Darren,
> If I move the definition of SEPARATOR to Patch 9, I would need some
> different way to parse commands here, to keep everything bisectable. I
> don't think the separator is only important in the context of the
> Crossover functionality (Patch 9) - it is useful in general as a
> "stable" way to parse an input into multiple commands.
> Is it OK if I keep SEPARATOR in this patch and add the comments you
> mention to both this patch and patch 9?

Sounds fine, it was just a suggestion since I hadn't seen it being used
in this file, but maybe I missed something.

Thanks,

Darren.

> Thanks
> -Alex
>
>> >> 
>> >> Can you fuzz writing "FUZZ" in memory? Like:
>> >> OP_WRITE(0x100000, "UsingLibFUZZerString")?
>> >
>> > No.. Hopefully that's not a huge problem.
>> >
>> >> > +
>> >> > +enum cmds {
>> >> > +    OP_IN,
>> >> > +    OP_OUT,
>> >> > +    OP_READ,
>> >> > +    OP_WRITE,
>> >> > +    OP_CLOCK_STEP,
>> >> > +};
>> >> > +
>> >> > +#define DEFAULT_TIMEOUT_US 100000
>> >> > +#define USEC_IN_SEC 100000000
>> >> 
>> >> Are you sure this definition is correct?
>> >> 
>> > Thanks for the catch...
>> >
>> >> > +
>> >> > +typedef struct {
>> >> > +    ram_addr_t addr;
>> >> > +    ram_addr_t size; /* The number of bytes until the end of the I/O region */
>> >> > +} address_range;
>> >> > +
>> >> > +static useconds_t timeout = 100000;
>> >> [...]
>> >> 


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 01/16] memory: Add FlatView foreach function
  2020-09-21  2:24 ` [PATCH v3 01/16] memory: Add FlatView foreach function Alexander Bulekov
@ 2020-10-08  6:57   ` Paolo Bonzini
  0 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2020-10-08  6:57 UTC (permalink / raw)
  To: Alexander Bulekov, qemu-devel; +Cc: darren.kenny, bsd, philmd, stefanha

On 21/09/20 04:24, Alexander Bulekov wrote:
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  include/exec/memory.h | 5 +++++
>  softmmu/memory.c      | 9 +++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index f1bb2a7df5..975a90c871 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -688,6 +688,11 @@ static inline FlatView *address_space_to_flatview(AddressSpace *as)
>      return atomic_rcu_read(&as->current_map);
>  }
>  
> +typedef int (*flatview_cb)(ram_addr_t start,
> +                           ram_addr_t len,
> +                           const MemoryRegion*, void*);
> +
> +void flatview_for_each_range(FlatView *fv, flatview_cb cb , void *opaque);
>  
>  /**
>   * MemoryRegionSection: describes a fragment of a #MemoryRegion
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index d030eb6f7c..9db5fbe43a 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -655,6 +655,15 @@ static void render_memory_region(FlatView *view,
>      }
>  }
>  
> +void flatview_for_each_range(FlatView *fv, flatview_cb cb , void *opaque)
> +{
> +    FlatRange *fr;
> +    FOR_EACH_FLAT_RANGE(fr, fv) {
> +        if (cb(fr->addr.start, fr->addr.size, fr->mr, opaque))
> +            break;
> +    }
> +}

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

>  static MemoryRegion *memory_region_get_flatview_root(MemoryRegion *mr)
>  {
>      while (mr->enabled) {
> 



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer
  2020-09-21 14:34     ` Alexander Bulekov
  2020-10-01 15:29       ` Darren Kenny
@ 2020-10-08  7:03       ` Paolo Bonzini
  2020-10-11 15:35         ` Alexander Bulekov
  1 sibling, 1 reply; 53+ messages in thread
From: Paolo Bonzini @ 2020-10-08  7:03 UTC (permalink / raw)
  To: Alexander Bulekov, Philippe Mathieu-Daudé
  Cc: Laurent Vivier, Thomas Huth, qemu-devel, darren.kenny, bsd, stefanha

On 21/09/20 16:34, Alexander Bulekov wrote:
>> Can you fuzz writing "FUZZ" in memory? Like:
>> OP_WRITE(0x100000, "UsingLibFUZZerString")?
> No.. Hopefully that's not a huge problem.
> 

Instead of always looking for a separator, can you:

1) skip over it if you find it naturally at the end of a command (that
is, "FUZZ" is like a comment command)

2) actively search for it only if you stumble upon an unrecognized command?

In that case, if you have

  AbcFUZZD0x100000UsingLibFUZZerFUZZ

The first and third instances would be ignored, while the second would
be part of the input.  On the other hand if you have

  bcFUZZD0x100000UsingLibFUZZerFUZZ

"b" is an invalid command and therefore you'd skip directly to "D".

Paolo



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer
  2020-09-22 14:03   ` Alexander Bulekov
@ 2020-10-08  7:04     ` Paolo Bonzini
  0 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2020-10-08  7:04 UTC (permalink / raw)
  To: Alexander Bulekov, qemu-devel
  Cc: Laurent Vivier, Thomas Huth, darren.kenny, bsd, stefanha, philmd

On 22/09/20 16:03, Alexander Bulekov wrote:
>> +        if ((memory_region_is_ram(mr) ||
>> +            memory_region_is_ram_device(mr) ||
>> +            memory_region_is_rom(mr) ||
>> +            memory_region_is_romd(mr)) == false) {
>> +            name = object_get_canonical_path_component(child);
> This isn't a great way to check whether MRs have ops with code that is
> interesting to fuzz (for example the pflash MemoryRegions do not pass
> these checks, so you can't fuzz the pflash device). Need to think of
> some better checks to identify MRs that we are interested in fuzzing.

I think you can simply remove the ROMD check.

Paolo



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 11/16] scripts/oss-fuzz: Add general-fuzzer build script
  2020-09-21  2:25 ` [PATCH v3 11/16] scripts/oss-fuzz: Add general-fuzzer build script Alexander Bulekov
  2020-10-01 15:40   ` Darren Kenny
@ 2020-10-08  7:35   ` Paolo Bonzini
  2020-10-15 13:46     ` Alexander Bulekov
  1 sibling, 1 reply; 53+ messages in thread
From: Paolo Bonzini @ 2020-10-08  7:35 UTC (permalink / raw)
  To: Alexander Bulekov, qemu-devel
  Cc: darren.kenny, bsd, philmd, Thomas Huth, stefanha

On 21/09/20 04:25, Alexander Bulekov wrote:
> This parses a yaml file containing general-fuzzer configs and builds a
> separate oss-fuzz wrapper binary for each one, changing some
> preprocessor macros for each configuration. To avoid dealing with
> escaping and stringifying, convert each string into a byte-array
> representation
> 
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  scripts/oss-fuzz/build_general_fuzzers.py | 69 +++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100755 scripts/oss-fuzz/build_general_fuzzers.py
> 
> diff --git a/scripts/oss-fuzz/build_general_fuzzers.py b/scripts/oss-fuzz/build_general_fuzzers.py
> new file mode 100755
> index 0000000000..918f1143a5
> --- /dev/null
> +++ b/scripts/oss-fuzz/build_general_fuzzers.py
> @@ -0,0 +1,69 @@
> +#!/usr/bin/env python3
> +# -*- coding: utf-8 -*-
> +
> +"""
> +This script creates wrapper binaries that invoke the general-device-fuzzer with
> +configurations specified in a yaml config file.
> +"""
> +import sys
> +import os
> +import yaml
> +import tempfile
> +
> +CC = ""
> +TEMPLATE_FILENAME = "target_template.c"
> +TEMPLATE_PATH = ""
> +
> +
> +def usage():
> +    print("Usage: CC=COMPILER {} CONFIG_PATH \
> +OUTPUT_PATH_PREFIX".format(sys.argv[0]))
> +    sys.exit(0)
> +
> +
> +def str_to_c_byte_array(s):
> +    """
> +    Convert strings to byte-arrays so we don't worry about formatting
> +    strings to play nicely with cc -DQEMU_FUZZARGS etc
> +    """
> +    return ','.join('0x{:02x}'.format(ord(x)) for x in s)
> +
> +
> +def compile_wrapper(cfg, path):
> +    os.system('$CC -DQEMU_FUZZ_ARGS="{fuzz_args}" \
> +               -DQEMU_FUZZ_OBJECTS="{fuzz_objs}" \
> +               {wrapper_template} -o {output_bin}'.format(
> +                   fuzz_args=str_to_c_byte_array(cfg["args"].replace("\n", " ")),
> +                   fuzz_objs=str_to_c_byte_array(cfg["objects"].replace("\n", " ")),
> +                   wrapper_template=TEMPLATE_PATH,
> +                   output_bin=path))
> +
> +
> +def main():
> +    global CC
> +    global TEMPLATE_PATH
> +    global OUTPUT_BIN_NAME
> +
> +    if len(sys.argv) != 3:
> +        usage()
> +
> +    cfg_path = sys.argv[1]
> +    out_path = sys.argv[2]
> +
> +    CC = os.getenv("CC", default="cc")
> +    TEMPLATE_PATH = os.path.join(os.path.dirname(__file__), TEMPLATE_FILENAME)
> +    if not os.path.exists(TEMPLATE_PATH):
> +        print("Error {} doesn't exist".format(TEMPLATE_PATH))
> +        sys.exit(1)
> +
> +    with open(cfg_path, "r") as f:
> +        configs = yaml.load(f)["configs"]
> +    for cfg in configs:
> +        assert "name" in cfg
> +        assert "args" in cfg
> +        assert "objects" in cfg
> +        compile_wrapper(cfg, out_path + cfg["name"])
> +
> +
> +if __name__ == '__main__':
> +    main()
> 

Can you instead write an array of

struct {
    const char *name, *args, *objects;
}

and use it in the normal argv0-based selection?  The advantage would be
that you can do the whole build within tests/qtest/fuzz/meson.build
instead of having yet another undocumented shell script (cue all the
mess I made when trying to modify scripts/oss-fuzz/build.sh).

Paolo



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 05/16] fuzz: Declare DMA Read callback function
  2020-09-21  2:24 ` [PATCH v3 05/16] fuzz: Declare DMA Read callback function Alexander Bulekov
@ 2020-10-08  7:39   ` Paolo Bonzini
  2020-10-11 15:45     ` Alexander Bulekov
  0 siblings, 1 reply; 53+ messages in thread
From: Paolo Bonzini @ 2020-10-08  7:39 UTC (permalink / raw)
  To: Alexander Bulekov, qemu-devel; +Cc: darren.kenny, bsd, philmd, stefanha

On 21/09/20 04:24, Alexander Bulekov wrote:
> This patch declares the fuzz_dma_read_cb function and uses the
> preprocessor and linker(weak symbols) to handle these cases:
> 
> When we build softmmu/all with --enable-fuzzing, there should be no
> strong symbol defined for fuzz_dma_read_cb, and we link against a weak
> stub function.
> 
> When we build softmmu/fuzz with --enable-fuzzing, we link against the
> strong symbol in general_fuzz.c
> 
> When we build softmmu/all without --enable-fuzzing, fuzz_dma_read_cb is
> an empty, inlined function. As long as we don't call any other functions
> when building the arguments, there should be no overhead.

Can you move the weak function somewhere in tests/qtest/fuzz instead?
Then you don't need an #ifdef because you can add it to specific_fuzz_ss.

Paolo



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 14/16] scripts/oss-fuzz: Add script to reorder a general-fuzzer trace
  2020-09-21  2:25 ` [PATCH v3 14/16] scripts/oss-fuzz: Add script to reorder a general-fuzzer trace Alexander Bulekov
@ 2020-10-08  7:42   ` Paolo Bonzini
  0 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2020-10-08  7:42 UTC (permalink / raw)
  To: Alexander Bulekov, qemu-devel
  Cc: darren.kenny, bsd, philmd, Thomas Huth, stefanha

On 21/09/20 04:25, Alexander Bulekov wrote:
> +
> +Note: this won't work for traces where the device tries to read from the same
> +DMA region twice in between MMIO/PIO commands. E.g:
> +    [R +0.028434] outl 0xc000 0xbeef
> +    [DMA][R +0.034639] write 0xbeef 0x2 0xAAAA
> +    [DMA][R +0.034639] write 0xbeef 0x2 0xBBBB

Can you detect this and print an error?

Paolo



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 04/16] fuzz: Add DMA support to the generic-fuzzer
  2020-09-21  2:24 ` [PATCH v3 04/16] fuzz: Add DMA support to the generic-fuzzer Alexander Bulekov
@ 2020-10-08  7:43   ` Paolo Bonzini
  2020-10-08 13:26     ` Alexander Bulekov
  0 siblings, 1 reply; 53+ messages in thread
From: Paolo Bonzini @ 2020-10-08  7:43 UTC (permalink / raw)
  To: Alexander Bulekov, qemu-devel
  Cc: Laurent Vivier, Thomas Huth, darren.kenny, bsd, stefanha, philmd

On 21/09/20 04:24, Alexander Bulekov wrote:
> +    if (qtest_log_enabled) {
> +        /*
> +         * With QTEST_LOG, use a normal, slow QTest memwrite. Prefix the log
> +         * that will be written by qtest.c with a DMA tag, so we can reorder
> +         * the resulting QTest trace so the DMA fills precede the last PIO/MMIO
> +         * command.
> +         */
> +        fprintf(stderr, "[DMA] ");
> +        fflush(stderr);
> +        qtest_memwrite(qts_global, ar.addr, buf, ar.size);
> +    } else {
> +       /*
> +        * Populate the region using address_space_write_rom to avoid writing to
> +        * any IO MemoryRegions
> +        */
> +        address_space_write_rom(first_cpu->as, ar.addr, MEMTXATTRS_UNSPECIFIED,
> +                buf, ar.size);
> +    }

I wonder if you should just copy address_space_write_rom to your own
code.  This way you can log the write just like qtest_memwrite would,
while skipping memwrites that would access IO regions.

Paolo



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 04/16] fuzz: Add DMA support to the generic-fuzzer
  2020-10-08  7:43   ` Paolo Bonzini
@ 2020-10-08 13:26     ` Alexander Bulekov
  0 siblings, 0 replies; 53+ messages in thread
From: Alexander Bulekov @ 2020-10-08 13:26 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Laurent Vivier, Thomas Huth, qemu-devel, darren.kenny, bsd,
	stefanha, philmd

On 201008 0943, Paolo Bonzini wrote:
> On 21/09/20 04:24, Alexander Bulekov wrote:
> > +    if (qtest_log_enabled) {
> > +        /*
> > +         * With QTEST_LOG, use a normal, slow QTest memwrite. Prefix the log
> > +         * that will be written by qtest.c with a DMA tag, so we can reorder
> > +         * the resulting QTest trace so the DMA fills precede the last PIO/MMIO
> > +         * command.
> > +         */
> > +        fprintf(stderr, "[DMA] ");
> > +        fflush(stderr);
> > +        qtest_memwrite(qts_global, ar.addr, buf, ar.size);
> > +    } else {
> > +       /*
> > +        * Populate the region using address_space_write_rom to avoid writing to
> > +        * any IO MemoryRegions
> > +        */
> > +        address_space_write_rom(first_cpu->as, ar.addr, MEMTXATTRS_UNSPECIFIED,
> > +                buf, ar.size);
> > +    }
> 
> I wonder if you should just copy address_space_write_rom to your own
> code.  This way you can log the write just like qtest_memwrite would,
> while skipping memwrites that would access IO regions.
> 
> Paolo

I took a quick look, and I think this should be possible, though I might
also need to carry over memory_access_size and invalidate_and_set_dirty.
That would certainly make things better for building reproducers, since
I have seen cases where bugs were not reproducible with QTEST_LOG=1,
because we were not using address_space_write_rom. I'll give it a shot!
Thanks


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer
  2020-10-08  7:03       ` Paolo Bonzini
@ 2020-10-11 15:35         ` Alexander Bulekov
  2020-10-12  7:02           ` Paolo Bonzini
  0 siblings, 1 reply; 53+ messages in thread
From: Alexander Bulekov @ 2020-10-11 15:35 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Laurent Vivier, Thomas Huth, qemu-devel, darren.kenny, bsd,
	stefanha, Philippe Mathieu-Daudé

On 201008 0903, Paolo Bonzini wrote:
> On 21/09/20 16:34, Alexander Bulekov wrote:
> >> Can you fuzz writing "FUZZ" in memory? Like:
> >> OP_WRITE(0x100000, "UsingLibFUZZerString")?
> > No.. Hopefully that's not a huge problem.
> > 
> 
> Instead of always looking for a separator, can you:
> 
> 1) skip over it if you find it naturally at the end of a command (that
> is, "FUZZ" is like a comment command)
> 
> 2) actively search for it only if you stumble upon an unrecognized command?
> 

What is the end goal? Is it to be able to use the "FUZZ" bytes to fuzz
devices?
My concern is that we want to keep the "stability" added by the FUZZ
separators (ie removing a single byte shouldn't completely change the
sequence of operations).

> In that case, if you have
> 
>   AbcFUZZD0x100000UsingLibFUZZerFUZZ
> 
> The first and third instances would be ignored, while the second would
> be part of the input.  On the other hand if you have
> 
>   bcFUZZD0x100000UsingLibFUZZerFUZZ
> 
> "b" is an invalid command and therefore you'd skip directly to "D".

There aren't any invalid OPCodes, since we interpret the opcode modulo
the size of the OPcode table. We only have invalid/skipped commands when
there isn't enough data after the opcode to figure out what we should do.

> 
> Paolo
> 


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 05/16] fuzz: Declare DMA Read callback function
  2020-10-08  7:39   ` Paolo Bonzini
@ 2020-10-11 15:45     ` Alexander Bulekov
  2020-10-12  6:59       ` Paolo Bonzini
  0 siblings, 1 reply; 53+ messages in thread
From: Alexander Bulekov @ 2020-10-11 15:45 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: darren.kenny, bsd, philmd, qemu-devel, stefanha

On 201008 0939, Paolo Bonzini wrote:
> On 21/09/20 04:24, Alexander Bulekov wrote:
> > This patch declares the fuzz_dma_read_cb function and uses the
> > preprocessor and linker(weak symbols) to handle these cases:
> > 
> > When we build softmmu/all with --enable-fuzzing, there should be no
> > strong symbol defined for fuzz_dma_read_cb, and we link against a weak
> > stub function.
> > 
> > When we build softmmu/fuzz with --enable-fuzzing, we link against the
> > strong symbol in general_fuzz.c
> > 
> > When we build softmmu/all without --enable-fuzzing, fuzz_dma_read_cb is
> > an empty, inlined function. As long as we don't call any other functions
> > when building the arguments, there should be no overhead.
> 
> Can you move the weak function somewhere in tests/qtest/fuzz instead?
> Then you don't need an #ifdef because you can add it to specific_fuzz_ss.
> 
> Paolo
> 

If I understand correctly, specific_fuzz_ss is only used to build
qemu-fuzz targets. The goal here was to support building qemu-system
with --enable-fuzzing (ie CONFIG_FUZZ=y), where specific_fuzz isn't
used. If its too ugly, we could make a stub file under tests/qtest/fuzz
and add it to specific_ss when: 'CONFIG_FUZZ'.
-Alex


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 05/16] fuzz: Declare DMA Read callback function
  2020-10-11 15:45     ` Alexander Bulekov
@ 2020-10-12  6:59       ` Paolo Bonzini
  0 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2020-10-12  6:59 UTC (permalink / raw)
  To: Alexander Bulekov; +Cc: darren.kenny, bsd, philmd, qemu-devel, stefanha

On 11/10/20 17:45, Alexander Bulekov wrote:
> On 201008 0939, Paolo Bonzini wrote:
>> On 21/09/20 04:24, Alexander Bulekov wrote:
>>> This patch declares the fuzz_dma_read_cb function and uses the
>>> preprocessor and linker(weak symbols) to handle these cases:
>>>
>>> When we build softmmu/all with --enable-fuzzing, there should be no
>>> strong symbol defined for fuzz_dma_read_cb, and we link against a weak
>>> stub function.
>>>
>>> When we build softmmu/fuzz with --enable-fuzzing, we link against the
>>> strong symbol in general_fuzz.c
>>>
>>> When we build softmmu/all without --enable-fuzzing, fuzz_dma_read_cb is
>>> an empty, inlined function. As long as we don't call any other functions
>>> when building the arguments, there should be no overhead.
>>
>> Can you move the weak function somewhere in tests/qtest/fuzz instead?
>> Then you don't need an #ifdef because you can add it to specific_fuzz_ss.
> 
> If I understand correctly, specific_fuzz_ss is only used to build
> qemu-fuzz targets. The goal here was to support building qemu-system
> with --enable-fuzzing (ie CONFIG_FUZZ=y), where specific_fuzz isn't
> used. If its too ugly, we could make a stub file under tests/qtest/fuzz
> and add it to specific_ss when: 'CONFIG_FUZZ'.

You're right.

Paolo



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer
  2020-10-11 15:35         ` Alexander Bulekov
@ 2020-10-12  7:02           ` Paolo Bonzini
  0 siblings, 0 replies; 53+ messages in thread
From: Paolo Bonzini @ 2020-10-12  7:02 UTC (permalink / raw)
  To: Alexander Bulekov
  Cc: Laurent Vivier, Thomas Huth, qemu-devel, darren.kenny, bsd,
	stefanha, Philippe Mathieu-Daudé

On 11/10/20 17:35, Alexander Bulekov wrote:
>> Instead of always looking for a separator, can you:
>>
>> 1) skip over it if you find it naturally at the end of a command (that
>> is, "FUZZ" is like a comment command)
>>
>> 2) actively search for it only if you stumble upon an unrecognized command?
>>
> What is the end goal? Is it to be able to use the "FUZZ" bytes to fuzz
> devices?

Yes, possibly, and perhaps also using a shorter separator that is easier
to learn.  But if the dictionary is enough to work around the learning
time and it's unlikely that crossover produces inputs like that, I guess
it's okay to have the separator.

Paolo



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 09/16] fuzz: add a crossover function to generic-fuzzer
  2020-10-01 15:31   ` Darren Kenny
@ 2020-10-15 13:43     ` Alexander Bulekov
  0 siblings, 0 replies; 53+ messages in thread
From: Alexander Bulekov @ 2020-10-15 13:43 UTC (permalink / raw)
  To: Darren Kenny
  Cc: Laurent Vivier, Thomas Huth, qemu-devel, bsd, stefanha,
	Paolo Bonzini, philmd

Thanks, I added some clarifications, but I added them to 02/16 (where I
first define and use SEPARATOR).

On 201001 1631, Darren Kenny wrote:
> As mentioned in an earlier patch, maybe the definition of SEPARATOR
> should be here as well as some of the comments you provided in the
> replies to it.
> 
> Otherwise, this looks good,
> 
> Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
> 
> Thanks,
> 
> Darren.
> 
> On Sunday, 2020-09-20 at 22:24:59 -04, Alexander Bulekov wrote:
> > Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> > ---
> >  tests/qtest/fuzz/general_fuzz.c | 90 ++++++++++++++++++++++++++++++++-
> >  1 file changed, 89 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/qtest/fuzz/general_fuzz.c b/tests/qtest/fuzz/general_fuzz.c
> > index 656ec7fd55..3833b505c8 100644
> > --- a/tests/qtest/fuzz/general_fuzz.c
> > +++ b/tests/qtest/fuzz/general_fuzz.c
> > @@ -741,6 +741,92 @@ static void general_pre_fuzz(QTestState *s)
> >  
> >      counter_shm_init();
> >  }
> > +
> > +/*
> > + * When libfuzzer gives us two inputs to combine, return a new input with the
> > + * following structure:
> > + *
> > + * Input 1 (data1)
> > + * SEPARATOR
> > + * Clear out the DMA Patterns
> > + * SEPARATOR
> > + * Disable the pci_read/write instructions
> > + * SEPARATOR
> > + * Input 2 (data2)
> > + *
> > + * The idea is to collate the core behaviors of the two inputs.
> > + * For example:
> > + * Input 1: maps a device's BARs, sets up three DMA patterns, and triggers
> > + *          device functionality A
> > + * Input 2: maps a device's BARs, sets up one DMA pattern, and triggers device
> > + *          functionality B
> > + *
> > + * This function attempts to produce an input that:
> > + * Ouptut: maps a device's BARs, set up three DMA patterns, triggers
> > + *          functionality A device, replaces the DMA patterns with a single
> > + *          patten, and triggers device functionality B.
> > + */
> > +static size_t general_fuzz_crossover(const uint8_t *data1, size_t size1, const
> > +                                     uint8_t *data2, size_t size2, uint8_t *out,
> > +                                     size_t max_out_size, unsigned int seed)
> > +{
> > +    size_t copy_len = 0, size = 0;
> > +
> > +    /* Check that we have enough space for data1 and at least part of data2 */
> > +    if (max_out_size <= size + strlen(SEPARATOR) * 3 + 2) {
> > +        return 0;
> > +    }
> > +
> > +    /* Copy_Len in the first input */
> > +    copy_len = size1;
> > +    memcpy(out + size, data1, copy_len);
> > +    size += copy_len;
> > +    max_out_size -= copy_len;
> > +
> > +    /* Append a separator */
> > +    copy_len = strlen(SEPARATOR);
> > +    memcpy(out + size, SEPARATOR, copy_len);
> > +    size += copy_len;
> > +    max_out_size -= copy_len;
> > +
> > +    /* Clear out the DMA Patterns */
> > +    copy_len = 1;
> > +    if (copy_len) {
> > +        out[size] = OP_CLEAR_DMA_PATTERNS;
> > +    }
> > +    size += copy_len;
> > +    max_out_size -= copy_len;
> > +
> > +    /* Append a separator */
> > +    copy_len = strlen(SEPARATOR);
> > +    memcpy(out + size, SEPARATOR, copy_len);
> > +    size += copy_len;
> > +    max_out_size -= copy_len;
> > +
> > +    /* Disable PCI ops. Assume data1 took care of setting up PCI */
> > +    copy_len = 1;
> > +    if (copy_len) {
> > +        out[size] = OP_DISABLE_PCI;
> > +    }
> > +    size += copy_len;
> > +    max_out_size -= copy_len;
> > +
> > +    /* Append a separator */
> > +    copy_len = strlen(SEPARATOR);
> > +    memcpy(out + size, SEPARATOR, copy_len);
> > +    size += copy_len;
> > +    max_out_size -= copy_len;
> > +
> > +    /* Copy_Len over the second input */
> > +    copy_len = MIN(size2, max_out_size);
> > +    memcpy(out + size, data2, copy_len);
> > +    size += copy_len;
> > +    max_out_size -= copy_len;
> > +
> > +    return  size;
> > +}
> > +
> > +
> >  static GString *general_fuzz_cmdline(FuzzTarget *t)
> >  {
> >      GString *cmd_line = g_string_new(TARGET_NAME);
> > @@ -760,7 +846,9 @@ static void register_general_fuzz_targets(void)
> >              .description = "Fuzz based on any qemu command-line args. ",
> >              .get_init_cmdline = general_fuzz_cmdline,
> >              .pre_fuzz = general_pre_fuzz,
> > -            .fuzz = general_fuzz});
> > +            .fuzz = general_fuzz,
> > +            .crossover = general_fuzz_crossover
> > +    });
> >  }
> >  
> >  fuzz_target_init(register_general_fuzz_targets);
> > -- 
> > 2.28.0


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v3 11/16] scripts/oss-fuzz: Add general-fuzzer build script
  2020-10-08  7:35   ` Paolo Bonzini
@ 2020-10-15 13:46     ` Alexander Bulekov
  0 siblings, 0 replies; 53+ messages in thread
From: Alexander Bulekov @ 2020-10-15 13:46 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Thomas Huth, qemu-devel, darren.kenny, bsd, stefanha, philmd

On 201008 0935, Paolo Bonzini wrote:
> On 21/09/20 04:25, Alexander Bulekov wrote:
> > This parses a yaml file containing general-fuzzer configs and builds a
> > separate oss-fuzz wrapper binary for each one, changing some
> > preprocessor macros for each configuration. To avoid dealing with
> > escaping and stringifying, convert each string into a byte-array
> > representation
> > 
> > Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> > ---
> >  scripts/oss-fuzz/build_general_fuzzers.py | 69 +++++++++++++++++++++++
> >  1 file changed, 69 insertions(+)
> >  create mode 100755 scripts/oss-fuzz/build_general_fuzzers.py
> > 
> > diff --git a/scripts/oss-fuzz/build_general_fuzzers.py b/scripts/oss-fuzz/build_general_fuzzers.py
> > new file mode 100755
> > index 0000000000..918f1143a5
> > --- /dev/null
> > +++ b/scripts/oss-fuzz/build_general_fuzzers.py
> > @@ -0,0 +1,69 @@
> > +#!/usr/bin/env python3
> > +# -*- coding: utf-8 -*-
> > +
> > +"""
> > +This script creates wrapper binaries that invoke the general-device-fuzzer with
> > +configurations specified in a yaml config file.
> > +"""
> > +import sys
> > +import os
> > +import yaml
> > +import tempfile
> > +
> > +CC = ""
> > +TEMPLATE_FILENAME = "target_template.c"
> > +TEMPLATE_PATH = ""
> > +
> > +
> > +def usage():
> > +    print("Usage: CC=COMPILER {} CONFIG_PATH \
> > +OUTPUT_PATH_PREFIX".format(sys.argv[0]))
> > +    sys.exit(0)
> > +
> > +
> > +def str_to_c_byte_array(s):
> > +    """
> > +    Convert strings to byte-arrays so we don't worry about formatting
> > +    strings to play nicely with cc -DQEMU_FUZZARGS etc
> > +    """
> > +    return ','.join('0x{:02x}'.format(ord(x)) for x in s)
> > +
> > +
> > +def compile_wrapper(cfg, path):
> > +    os.system('$CC -DQEMU_FUZZ_ARGS="{fuzz_args}" \
> > +               -DQEMU_FUZZ_OBJECTS="{fuzz_objs}" \
> > +               {wrapper_template} -o {output_bin}'.format(
> > +                   fuzz_args=str_to_c_byte_array(cfg["args"].replace("\n", " ")),
> > +                   fuzz_objs=str_to_c_byte_array(cfg["objects"].replace("\n", " ")),
> > +                   wrapper_template=TEMPLATE_PATH,
> > +                   output_bin=path))
> > +
> > +
> > +def main():
> > +    global CC
> > +    global TEMPLATE_PATH
> > +    global OUTPUT_BIN_NAME
> > +
> > +    if len(sys.argv) != 3:
> > +        usage()
> > +
> > +    cfg_path = sys.argv[1]
> > +    out_path = sys.argv[2]
> > +
> > +    CC = os.getenv("CC", default="cc")
> > +    TEMPLATE_PATH = os.path.join(os.path.dirname(__file__), TEMPLATE_FILENAME)
> > +    if not os.path.exists(TEMPLATE_PATH):
> > +        print("Error {} doesn't exist".format(TEMPLATE_PATH))
> > +        sys.exit(1)
> > +
> > +    with open(cfg_path, "r") as f:
> > +        configs = yaml.load(f)["configs"]
> > +    for cfg in configs:
> > +        assert "name" in cfg
> > +        assert "args" in cfg
> > +        assert "objects" in cfg
> > +        compile_wrapper(cfg, out_path + cfg["name"])
> > +
> > +
> > +if __name__ == '__main__':
> > +    main()
> > 
> 
> Can you instead write an array of
> 
> struct {
>     const char *name, *args, *objects;
> }
> 
> and use it in the normal argv0-based selection?  The advantage would be
> that you can do the whole build within tests/qtest/fuzz/meson.build
> instead of having yet another undocumented shell script (cue all the
> mess I made when trying to modify scripts/oss-fuzz/build.sh).
> 
> Paolo

Thanks for the suggestion. I did this in v4, and I think it is much
nicer. No more python script, c template, and preprocessor hacking. I
don't think the way I defined the configs is ideal, however I think it
is already a better solution.
-Alex

> 


^ permalink raw reply	[flat|nested] 53+ messages in thread

end of thread, other threads:[~2020-10-15 14:04 UTC | newest]

Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-21  2:24 [PATCH v3 00/16] Add a General Virtual Device Fuzzer Alexander Bulekov
2020-09-21  2:24 ` [PATCH v3 01/16] memory: Add FlatView foreach function Alexander Bulekov
2020-10-08  6:57   ` Paolo Bonzini
2020-09-21  2:24 ` [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer Alexander Bulekov
2020-09-21  5:43   ` Philippe Mathieu-Daudé
2020-09-21 14:34     ` Alexander Bulekov
2020-10-01 15:29       ` Darren Kenny
2020-10-07 13:39         ` Alexander Bulekov
2020-10-07 13:53           ` Darren Kenny
2020-10-08  7:03       ` Paolo Bonzini
2020-10-11 15:35         ` Alexander Bulekov
2020-10-12  7:02           ` Paolo Bonzini
2020-09-22 14:03   ` Alexander Bulekov
2020-10-08  7:04     ` Paolo Bonzini
2020-09-21  2:24 ` [PATCH v3 03/16] fuzz: Add PCI features to the general fuzzer Alexander Bulekov
2020-09-21  5:44   ` Philippe Mathieu-Daudé
2020-09-21 14:41     ` Alexander Bulekov
2020-09-21  2:24 ` [PATCH v3 04/16] fuzz: Add DMA support to the generic-fuzzer Alexander Bulekov
2020-10-08  7:43   ` Paolo Bonzini
2020-10-08 13:26     ` Alexander Bulekov
2020-09-21  2:24 ` [PATCH v3 05/16] fuzz: Declare DMA Read callback function Alexander Bulekov
2020-10-08  7:39   ` Paolo Bonzini
2020-10-11 15:45     ` Alexander Bulekov
2020-10-12  6:59       ` Paolo Bonzini
2020-09-21  2:24 ` [PATCH v3 06/16] fuzz: Add fuzzer callbacks to DMA-read functions Alexander Bulekov
2020-09-21  2:24 ` [PATCH v3 07/16] fuzz: Add support for custom crossover functions Alexander Bulekov
2020-09-21  2:24 ` [PATCH v3 08/16] fuzz: add a DISABLE_PCI op to general-fuzzer Alexander Bulekov
2020-09-21  2:24 ` [PATCH v3 09/16] fuzz: add a crossover function to generic-fuzzer Alexander Bulekov
2020-10-01 15:31   ` Darren Kenny
2020-10-15 13:43     ` Alexander Bulekov
2020-09-21  2:25 ` [PATCH v3 10/16] scripts/oss-fuzz: Add wrapper program for generic fuzzer Alexander Bulekov
2020-09-21  2:25 ` [PATCH v3 11/16] scripts/oss-fuzz: Add general-fuzzer build script Alexander Bulekov
2020-10-01 15:40   ` Darren Kenny
2020-10-08  7:35   ` Paolo Bonzini
2020-10-15 13:46     ` Alexander Bulekov
2020-09-21  2:25 ` [PATCH v3 12/16] scripts/oss-fuzz: Add general-fuzzer configs for oss-fuzz Alexander Bulekov
2020-09-21  2:25 ` [PATCH v3 13/16] scripts/oss-fuzz: build the general-fuzzer configs Alexander Bulekov
2020-09-21  2:25 ` [PATCH v3 14/16] scripts/oss-fuzz: Add script to reorder a general-fuzzer trace Alexander Bulekov
2020-10-08  7:42   ` Paolo Bonzini
2020-09-21  2:25 ` [PATCH v3 15/16] scripts/oss-fuzz: Add crash trace minimization script Alexander Bulekov
2020-09-21  2:25 ` [PATCH v3 16/16] fuzz: Add instructions for using general-fuzz Alexander Bulekov
2020-10-01 15:44   ` Darren Kenny
2020-09-21  2:45 ` [PATCH v3 00/16] Add a General Virtual Device Fuzzer no-reply
2020-09-21  2:58 ` no-reply
2020-09-21  3:30 ` no-reply
2020-09-21  3:43 ` no-reply
2020-09-21  3:46 ` no-reply
2020-09-21  4:30 ` no-reply
2020-09-21  4:39 ` no-reply
2020-09-21  5:22 ` no-reply
2020-09-21  5:31 ` no-reply
2020-09-21  6:17 ` no-reply
2020-09-21  6:26 ` no-reply

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.