[PATCH v2 0/3] fuzz: Add a sparse-memory device to accelerate fuzzing

[PATCH v2 0/3] fuzz: Add a sparse-memory device to accelerate fuzzing

[Alexander Bulekov]

v2:
    - Make the device a TYPE_SYS_BUS_DEVICE device
    - Remove the qtest (the device cannot be enabled for testing outside
      of the fuzzing code).
    - Since this will only be used for short-lived fuzzing processes, do
      not keep track of empty regions.
    - Move some DMA callbacks to properly fill DMA buffers in sparse
      memory

The generic-fuzzer often provides virtual-devices with bogus DMA
addresses (e.g. 0x4141414141414141). The probability that these fuzzed
addresses actually land within RAM is quite small. The fuzzer eventually
finds valid addresses, however, this takes some time, and this problem is
compounded when the device accesses multiple DMA regions. This series
adds a "sparse" memory device, and configures it for the generic-fuzzer.
This allows us to simulate 16 EB ram (only a tiny portion actually
populated). Thus, almost any randomly generated 64-bit address will land
in memory that the fuzzer can populate with data.

Alexander Bulekov (3):
  memory: add a sparse memory device for fuzzing
  fuzz: configure a sparse-mem device, by default
  fuzz: move some DMA hooks

 MAINTAINERS                     |   1 +
 hw/mem/meson.build              |   1 +
 hw/mem/sparse-mem.c             | 152 ++++++++++++++++++++++++++++++++
 include/hw/mem/sparse-mem.h     |  19 ++++
 softmmu/memory.c                |   1 -
 softmmu/physmem.c               |   2 +-
 tests/qtest/fuzz/generic_fuzz.c |  14 ++-
 7 files changed, 185 insertions(+), 5 deletions(-)
 create mode 100644 hw/mem/sparse-mem.c
 create mode 100644 include/hw/mem/sparse-mem.h

-- 
2.28.0

[PATCH v2 1/3] memory: add a sparse memory device for fuzzing

[Alexander Bulekov]

For testing, it can be useful to simulate an enormous amount of memory
(e.g. 2^64 RAM). This adds an MMIO device that acts as sparse memory.
When something writes a nonzero value to a sparse-mem address, we
allocate a block of memory. This block is kept around, until all of the
bytes within the block are zero-ed. The device has a very low priority
(so it can be mapped beneath actual RAM, and virtual device MMIO
regions).

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 MAINTAINERS                 |   1 +
 hw/mem/meson.build          |   1 +
 hw/mem/sparse-mem.c         | 152 ++++++++++++++++++++++++++++++++++++
 include/hw/mem/sparse-mem.h |  19 +++++
 4 files changed, 173 insertions(+)
 create mode 100644 hw/mem/sparse-mem.c
 create mode 100644 include/hw/mem/sparse-mem.h

diff --git a/MAINTAINERS b/MAINTAINERS
index f22d83c178..9e3d8b1401 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2618,6 +2618,7 @@ R: Thomas Huth <thuth@redhat.com>
 S: Maintained
 F: tests/qtest/fuzz/
 F: scripts/oss-fuzz/
+F: hw/mem/sparse-mem.c
 F: docs/devel/fuzzing.rst
 
 Register API
diff --git a/hw/mem/meson.build b/hw/mem/meson.build
index 0d22f2b572..ef79e04678 100644
--- a/hw/mem/meson.build
+++ b/hw/mem/meson.build
@@ -1,5 +1,6 @@
 mem_ss = ss.source_set()
 mem_ss.add(files('memory-device.c'))
+mem_ss.add(when: 'CONFIG_FUZZ', if_true: files('sparse-mem.c'))
 mem_ss.add(when: 'CONFIG_DIMM', if_true: files('pc-dimm.c'))
 mem_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_mc.c'))
 mem_ss.add(when: 'CONFIG_NVDIMM', if_true: files('nvdimm.c'))
diff --git a/hw/mem/sparse-mem.c b/hw/mem/sparse-mem.c
new file mode 100644
index 0000000000..575a287f59
--- /dev/null
+++ b/hw/mem/sparse-mem.c
@@ -0,0 +1,152 @@
+/*
+ * A sparse memory device. Useful for fuzzing
+ *
+ * Copyright Red Hat Inc., 2021
+ *
+ * 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 "exec/address-spaces.h"
+#include "hw/qdev-properties.h"
+#include "hw/sysbus.h"
+#include "qapi/error.h"
+#include "qemu/units.h"
+#include "sysemu/qtest.h"
+#include "hw/mem/sparse-mem.h"
+
+#define SPARSE_MEM(obj) OBJECT_CHECK(SparseMemState, (obj), TYPE_SPARSE_MEM)
+#define SPARSE_BLOCK_SIZE 0x1000
+
+typedef struct SparseMemState {
+    SysBusDevice parent_obj;
+    MemoryRegion mmio;
+    uint64_t baseaddr;
+    uint64_t length;
+    uint64_t size_used;
+    uint64_t maxsize;
+    GHashTable *mapped;
+} SparseMemState;
+
+typedef struct sparse_mem_block {
+    uint8_t data[SPARSE_BLOCK_SIZE];
+} sparse_mem_block;
+
+static uint64_t sparse_mem_read(void *opaque, hwaddr addr, unsigned int size)
+{
+    printf("SPARSEREAD %lx\n", addr);
+    SparseMemState *s = opaque;
+    uint64_t ret = 0;
+    size_t pfn = addr / SPARSE_BLOCK_SIZE;
+    size_t offset = addr % SPARSE_BLOCK_SIZE;
+    sparse_mem_block *block;
+
+    block = g_hash_table_lookup(s->mapped, (void *)pfn);
+    if (block) {
+        assert(offset + size <= sizeof(block->data));
+        memcpy(&ret, block->data + offset, size);
+    }
+    return ret;
+}
+
+static void sparse_mem_write(void *opaque, hwaddr addr, uint64_t v,
+                             unsigned int size)
+{
+    SparseMemState *s = opaque;
+    size_t pfn = addr / SPARSE_BLOCK_SIZE;
+    size_t offset = addr % SPARSE_BLOCK_SIZE;
+    sparse_mem_block *block;
+
+    if (!g_hash_table_lookup(s->mapped, (void *)pfn) &&
+        s->size_used + SPARSE_BLOCK_SIZE < s->maxsize && v) {
+        g_hash_table_insert(s->mapped, (void *)pfn,
+                            g_new0(sparse_mem_block, 1));
+        s->size_used += sizeof(block->data);
+    }
+    block = g_hash_table_lookup(s->mapped, (void *)pfn);
+    if (!block) {
+        return;
+    }
+
+    assert(offset + size <= sizeof(block->data));
+
+    memcpy(block->data + offset, &v, size);
+
+}
+
+static const MemoryRegionOps sparse_mem_ops = {
+    .read = sparse_mem_read,
+    .write = sparse_mem_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+            .min_access_size = 1,
+            .max_access_size = 8,
+            .unaligned = false,
+        },
+};
+
+static Property sparse_mem_properties[] = {
+    /* The base address of the memory */
+    DEFINE_PROP_UINT64("baseaddr", SparseMemState, baseaddr, 0x0),
+    /* The length of the sparse memory region */
+    DEFINE_PROP_UINT64("length", SparseMemState, length, UINT64_MAX),
+    /* Max amount of actual memory that can be used to back the sparse memory */
+    DEFINE_PROP_UINT64("maxsize", SparseMemState, maxsize, 10 * MiB),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+MemoryRegion *sparse_mem_init(uint64_t addr, uint64_t length)
+{
+    DeviceState *dev;
+
+    dev = qdev_new(TYPE_SPARSE_MEM);
+    qdev_prop_set_uint64(dev, "baseaddr", addr);
+    qdev_prop_set_uint64(dev, "length", length);
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(dev), 0, addr, -10000);
+    return &SPARSE_MEM(dev)->mmio;
+}
+
+static void sparse_mem_realize(DeviceState *dev, Error **errp)
+{
+    SparseMemState *s = SPARSE_MEM(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+
+    if (!qtest_enabled()) {
+        error_setg(errp, "sparse_mem device should only be used "
+                         "for testing with QTest");
+        return;
+    }
+
+    assert(s->baseaddr + s->length > s->baseaddr);
+
+    s->mapped = g_hash_table_new(NULL, NULL);
+    memory_region_init_io(&s->mmio, OBJECT(s), &sparse_mem_ops, s,
+                          "sparse-mem", s->length);
+    sysbus_init_mmio(sbd, &s->mmio);
+}
+
+static void sparse_mem_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    device_class_set_props(dc, sparse_mem_properties);
+
+    dc->desc = "Sparse Memory Device";
+    dc->realize = sparse_mem_realize;
+}
+
+static const TypeInfo sparse_mem_types[] = {
+    {
+        .name = TYPE_SPARSE_MEM,
+        .parent = TYPE_SYS_BUS_DEVICE,
+        .instance_size = sizeof(SparseMemState),
+        .class_init = sparse_mem_class_init,
+    },
+};
+DEFINE_TYPES(sparse_mem_types);
diff --git a/include/hw/mem/sparse-mem.h b/include/hw/mem/sparse-mem.h
new file mode 100644
index 0000000000..f9863b154b
--- /dev/null
+++ b/include/hw/mem/sparse-mem.h
@@ -0,0 +1,19 @@
+/*
+ * A sparse memory device. Useful for fuzzing
+ *
+ * Copyright Red Hat Inc., 2021
+ *
+ * Authors:
+ *  Alexander Bulekov   <alxndr@bu.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef SPARSE_MEM_H
+#define SPARSE_MEM_H
+#define TYPE_SPARSE_MEM "sparse-mem"
+
+MemoryRegion *sparse_mem_init(uint64_t addr, uint64_t length);
+
+#endif
-- 
2.28.0

[PATCH v2 2/3] fuzz: configure a sparse-mem device, by default

[Alexander Bulekov]

The generic-fuzzer often provides randomized DMA addresses to
virtual-devices. For a 64-bit address-space, the chance of these
randomized addresses coinciding with RAM regions, is fairly small. Even
though the fuzzer's instrumentation eventually finds valid addresses,
this can take some-time, and slows-down fuzzing progress (especially,
when multiple DMA buffers are involved). To work around this, create
"fake" sparse-memory that spans all of the 64-bit address-space. Adjust
the DMA call-back to populate this sparse memory, correspondingly

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

diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
index 387ae2020a..b5fe27aae1 100644
--- a/tests/qtest/fuzz/generic_fuzz.c
+++ b/tests/qtest/fuzz/generic_fuzz.c
@@ -28,6 +28,7 @@
 #include "hw/pci/pci.h"
 #include "hw/boards.h"
 #include "generic_fuzz_configs.h"
+#include "hw/mem/sparse-mem.h"
 
 /*
  * SEPARATOR is used to separate "operations" in the fuzz input
@@ -64,6 +65,8 @@ static useconds_t timeout = DEFAULT_TIMEOUT_US;
 
 static bool qtest_log_enabled;
 
+MemoryRegion *sparse_mem_mr;
+
 /*
  * A pattern used to populate a DMA region or perform a memwrite. This is
  * useful for e.g. populating tables of unique addresses.
@@ -191,8 +194,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr)
      */
     if (dma_patterns->len == 0
         || len == 0
-        || mr != current_machine->ram
-        || addr > current_machine->ram_size) {
+        || (mr != current_machine->ram && mr != sparse_mem_mr)) {
         return;
     }
 
@@ -238,7 +240,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr)
                                       MEMTXATTRS_UNSPECIFIED);
 
         if (!(memory_region_is_ram(mr1) ||
-              memory_region_is_romd(mr1))) {
+              memory_region_is_romd(mr1)) && mr1 != sparse_mem_mr) {
             l = memory_access_size(mr1, l, addr1);
         } else {
             /* ROM/RAM case */
@@ -814,6 +816,12 @@ static void generic_pre_fuzz(QTestState *s)
     }
     qts_global = s;
 
+    /*
+     * Create a special device that we can use to back DMA buffers at very
+     * high memory addresses
+     */
+    sparse_mem_mr = sparse_mem_init(0, UINT64_MAX);
+
     dma_regions = g_array_new(false, false, sizeof(address_range));
     dma_patterns = g_array_new(false, false, sizeof(pattern));
 
-- 
2.28.0

[PATCH v2 3/3] fuzz: move some DMA hooks

[Alexander Bulekov]

For the sparse-mem device, we want the fuzzer to populate entire DMA
reads from sparse-mem, rather than hooking into the individual MMIO
memory_region_dispatch_read operations. Otherwise, the fuzzer will treat
each sequential read separately (and populate it with a separate
pattern). Work around this by rearranging some DMA hooks. Since the
fuzzer has it's own logic to skip accidentally writing to MMIO regions,
we can call the DMA cb, outside the flatview_translate loop.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 softmmu/memory.c  | 1 -
 softmmu/physmem.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/softmmu/memory.c b/softmmu/memory.c
index 874a8fccde..3b8e428064 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -1440,7 +1440,6 @@ MemTxResult memory_region_dispatch_read(MemoryRegion *mr,
     unsigned size = memop_size(op);
     MemTxResult r;
 
-    fuzz_dma_read_cb(addr, size, mr);
     if (!memory_region_access_valid(mr, addr, size, false, attrs)) {
         *pval = unassigned_mem_read(mr, addr, size);
         return MEMTX_DECODE_ERROR;
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 7e8b0fab89..6a58c86750 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2831,6 +2831,7 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
     bool release_lock = false;
     uint8_t *buf = ptr;
 
+    fuzz_dma_read_cb(addr, len, mr);
     for (;;) {
         if (!memory_access_is_direct(mr, false)) {
             /* I/O case */
@@ -2841,7 +2842,6 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
             stn_he_p(buf, l, val);
         } else {
             /* RAM case */
-            fuzz_dma_read_cb(addr, len, mr);
             ram_ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l, false);
             memcpy(buf, ram_ptr, l);
         }
-- 
2.28.0

Re: [PATCH v2 1/3] memory: add a sparse memory device for fuzzing

[Alexander Bulekov]

On 210313 1818, Alexander Bulekov wrote:
> For testing, it can be useful to simulate an enormous amount of memory
> (e.g. 2^64 RAM). This adds an MMIO device that acts as sparse memory.
> When something writes a nonzero value to a sparse-mem address, we
> allocate a block of memory. This block is kept around, until all of the
> bytes within the block are zero-ed. The device has a very low priority
> (so it can be mapped beneath actual RAM, and virtual device MMIO
> regions).
> 
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  MAINTAINERS                 |   1 +
>  hw/mem/meson.build          |   1 +
>  hw/mem/sparse-mem.c         | 152 ++++++++++++++++++++++++++++++++++++
>  include/hw/mem/sparse-mem.h |  19 +++++
>  4 files changed, 173 insertions(+)
>  create mode 100644 hw/mem/sparse-mem.c
>  create mode 100644 include/hw/mem/sparse-mem.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f22d83c178..9e3d8b1401 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2618,6 +2618,7 @@ R: Thomas Huth <thuth@redhat.com>
>  S: Maintained
>  F: tests/qtest/fuzz/
>  F: scripts/oss-fuzz/
> +F: hw/mem/sparse-mem.c
>  F: docs/devel/fuzzing.rst
>  
>  Register API
> diff --git a/hw/mem/meson.build b/hw/mem/meson.build
> index 0d22f2b572..ef79e04678 100644
> --- a/hw/mem/meson.build
> +++ b/hw/mem/meson.build
> @@ -1,5 +1,6 @@
>  mem_ss = ss.source_set()
>  mem_ss.add(files('memory-device.c'))
> +mem_ss.add(when: 'CONFIG_FUZZ', if_true: files('sparse-mem.c'))
>  mem_ss.add(when: 'CONFIG_DIMM', if_true: files('pc-dimm.c'))
>  mem_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_mc.c'))
>  mem_ss.add(when: 'CONFIG_NVDIMM', if_true: files('nvdimm.c'))
> diff --git a/hw/mem/sparse-mem.c b/hw/mem/sparse-mem.c
> new file mode 100644
> index 0000000000..575a287f59
> --- /dev/null
> +++ b/hw/mem/sparse-mem.c
> @@ -0,0 +1,152 @@
> +/*
> + * A sparse memory device. Useful for fuzzing
> + *
> + * Copyright Red Hat Inc., 2021
> + *
> + * 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 "exec/address-spaces.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/sysbus.h"
> +#include "qapi/error.h"
> +#include "qemu/units.h"
> +#include "sysemu/qtest.h"
> +#include "hw/mem/sparse-mem.h"
> +
> +#define SPARSE_MEM(obj) OBJECT_CHECK(SparseMemState, (obj), TYPE_SPARSE_MEM)
> +#define SPARSE_BLOCK_SIZE 0x1000
> +
> +typedef struct SparseMemState {
> +    SysBusDevice parent_obj;
> +    MemoryRegion mmio;
> +    uint64_t baseaddr;
> +    uint64_t length;
> +    uint64_t size_used;
> +    uint64_t maxsize;
> +    GHashTable *mapped;
> +} SparseMemState;
> +
> +typedef struct sparse_mem_block {
> +    uint8_t data[SPARSE_BLOCK_SIZE];
> +} sparse_mem_block;
> +
> +static uint64_t sparse_mem_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> +    printf("SPARSEREAD %lx\n", addr);
This debug-printf shouldn't be here..
-Alex

Re: [PATCH v2 1/3] memory: add a sparse memory device for fuzzing

[Darren Kenny]

Hi Alex,

On Saturday, 2021-03-13 at 18:18:57 -05, Alexander Bulekov wrote:
> For testing, it can be useful to simulate an enormous amount of memory
> (e.g. 2^64 RAM). This adds an MMIO device that acts as sparse memory.
> When something writes a nonzero value to a sparse-mem address, we
> allocate a block of memory. This block is kept around, until all of the
> bytes within the block are zero-ed. The device has a very low priority

I don't see code below that actually checks if a block is zero-ed and
removes it from the hash table, so is this comment correct?

> (so it can be mapped beneath actual RAM, and virtual device MMIO
> regions).
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  MAINTAINERS                 |   1 +
>  hw/mem/meson.build          |   1 +
>  hw/mem/sparse-mem.c         | 152 ++++++++++++++++++++++++++++++++++++
>  include/hw/mem/sparse-mem.h |  19 +++++
>  4 files changed, 173 insertions(+)
>  create mode 100644 hw/mem/sparse-mem.c
>  create mode 100644 include/hw/mem/sparse-mem.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f22d83c178..9e3d8b1401 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2618,6 +2618,7 @@ R: Thomas Huth <thuth@redhat.com>
>  S: Maintained
>  F: tests/qtest/fuzz/
>  F: scripts/oss-fuzz/
> +F: hw/mem/sparse-mem.c
>  F: docs/devel/fuzzing.rst
>  
>  Register API
> diff --git a/hw/mem/meson.build b/hw/mem/meson.build
> index 0d22f2b572..ef79e04678 100644
> --- a/hw/mem/meson.build
> +++ b/hw/mem/meson.build
> @@ -1,5 +1,6 @@
>  mem_ss = ss.source_set()
>  mem_ss.add(files('memory-device.c'))
> +mem_ss.add(when: 'CONFIG_FUZZ', if_true: files('sparse-mem.c'))
>  mem_ss.add(when: 'CONFIG_DIMM', if_true: files('pc-dimm.c'))
>  mem_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_mc.c'))
>  mem_ss.add(when: 'CONFIG_NVDIMM', if_true: files('nvdimm.c'))
> diff --git a/hw/mem/sparse-mem.c b/hw/mem/sparse-mem.c
> new file mode 100644
> index 0000000000..575a287f59
> --- /dev/null
> +++ b/hw/mem/sparse-mem.c
> @@ -0,0 +1,152 @@
> +/*
> + * A sparse memory device. Useful for fuzzing
> + *
> + * Copyright Red Hat Inc., 2021
> + *
> + * 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 "exec/address-spaces.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/sysbus.h"
> +#include "qapi/error.h"
> +#include "qemu/units.h"
> +#include "sysemu/qtest.h"
> +#include "hw/mem/sparse-mem.h"
> +
> +#define SPARSE_MEM(obj) OBJECT_CHECK(SparseMemState, (obj), TYPE_SPARSE_MEM)
> +#define SPARSE_BLOCK_SIZE 0x1000

This is assuming a 4K block size, should that be the same as the system
pagesize is? Or will it not matter w.r.t. how this is being consumed?

> +
> +typedef struct SparseMemState {
> +    SysBusDevice parent_obj;
> +    MemoryRegion mmio;
> +    uint64_t baseaddr;
> +    uint64_t length;
> +    uint64_t size_used;
> +    uint64_t maxsize;
> +    GHashTable *mapped;
> +} SparseMemState;
> +
> +typedef struct sparse_mem_block {
> +    uint8_t data[SPARSE_BLOCK_SIZE];
> +} sparse_mem_block;
> +
> +static uint64_t sparse_mem_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> +    printf("SPARSEREAD %lx\n", addr);

Should this printf() be a logging/trace call? Or do you really want it to be
printed all the time? Also seems out of place before the declaration of
the variables.

Thanks,

Darren.

Re: [PATCH v2 2/3] fuzz: configure a sparse-mem device, by default

[Darren Kenny]

On Saturday, 2021-03-13 at 18:18:58 -05, Alexander Bulekov wrote:
> The generic-fuzzer often provides randomized DMA addresses to
> virtual-devices. For a 64-bit address-space, the chance of these
> randomized addresses coinciding with RAM regions, is fairly small. Even
> though the fuzzer's instrumentation eventually finds valid addresses,
> this can take some-time, and slows-down fuzzing progress (especially,
> when multiple DMA buffers are involved). To work around this, create
> "fake" sparse-memory that spans all of the 64-bit address-space. Adjust
> the DMA call-back to populate this sparse memory, correspondingly
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

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

> ---
>  tests/qtest/fuzz/generic_fuzz.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
> index 387ae2020a..b5fe27aae1 100644
> --- a/tests/qtest/fuzz/generic_fuzz.c
> +++ b/tests/qtest/fuzz/generic_fuzz.c
> @@ -28,6 +28,7 @@
>  #include "hw/pci/pci.h"
>  #include "hw/boards.h"
>  #include "generic_fuzz_configs.h"
> +#include "hw/mem/sparse-mem.h"
>  
>  /*
>   * SEPARATOR is used to separate "operations" in the fuzz input
> @@ -64,6 +65,8 @@ static useconds_t timeout = DEFAULT_TIMEOUT_US;
>  
>  static bool qtest_log_enabled;
>  
> +MemoryRegion *sparse_mem_mr;
> +
>  /*
>   * A pattern used to populate a DMA region or perform a memwrite. This is
>   * useful for e.g. populating tables of unique addresses.
> @@ -191,8 +194,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr)
>       */
>      if (dma_patterns->len == 0
>          || len == 0
> -        || mr != current_machine->ram
> -        || addr > current_machine->ram_size) {
> +        || (mr != current_machine->ram && mr != sparse_mem_mr)) {
>          return;
>      }
>  
> @@ -238,7 +240,7 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr)
>                                        MEMTXATTRS_UNSPECIFIED);
>  
>          if (!(memory_region_is_ram(mr1) ||
> -              memory_region_is_romd(mr1))) {
> +              memory_region_is_romd(mr1)) && mr1 != sparse_mem_mr) {
>              l = memory_access_size(mr1, l, addr1);
>          } else {
>              /* ROM/RAM case */
> @@ -814,6 +816,12 @@ static void generic_pre_fuzz(QTestState *s)
>      }
>      qts_global = s;
>  
> +    /*
> +     * Create a special device that we can use to back DMA buffers at very
> +     * high memory addresses
> +     */
> +    sparse_mem_mr = sparse_mem_init(0, UINT64_MAX);
> +
>      dma_regions = g_array_new(false, false, sizeof(address_range));
>      dma_patterns = g_array_new(false, false, sizeof(pattern));
>  
> -- 
> 2.28.0

Re: [PATCH v2 3/3] fuzz: move some DMA hooks

[Darren Kenny]

On Saturday, 2021-03-13 at 18:18:59 -05, Alexander Bulekov wrote:
> For the sparse-mem device, we want the fuzzer to populate entire DMA
> reads from sparse-mem, rather than hooking into the individual MMIO
> memory_region_dispatch_read operations. Otherwise, the fuzzer will treat
> each sequential read separately (and populate it with a separate
> pattern). Work around this by rearranging some DMA hooks. Since the
> fuzzer has it's own logic to skip accidentally writing to MMIO regions,
> we can call the DMA cb, outside the flatview_translate loop.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

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

> ---
>  softmmu/memory.c  | 1 -
>  softmmu/physmem.c | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index 874a8fccde..3b8e428064 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -1440,7 +1440,6 @@ MemTxResult memory_region_dispatch_read(MemoryRegion *mr,
>      unsigned size = memop_size(op);
>      MemTxResult r;
>  
> -    fuzz_dma_read_cb(addr, size, mr);
>      if (!memory_region_access_valid(mr, addr, size, false, attrs)) {
>          *pval = unassigned_mem_read(mr, addr, size);
>          return MEMTX_DECODE_ERROR;
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index 7e8b0fab89..6a58c86750 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -2831,6 +2831,7 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
>      bool release_lock = false;
>      uint8_t *buf = ptr;
>  
> +    fuzz_dma_read_cb(addr, len, mr);
>      for (;;) {
>          if (!memory_access_is_direct(mr, false)) {
>              /* I/O case */
> @@ -2841,7 +2842,6 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
>              stn_he_p(buf, l, val);
>          } else {
>              /* RAM case */
> -            fuzz_dma_read_cb(addr, len, mr);
>              ram_ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l, false);
>              memcpy(buf, ram_ptr, l);
>          }
> -- 
> 2.28.0

Re: [PATCH v2 1/3] memory: add a sparse memory device for fuzzing

[Alexander Bulekov]

On 210315 1209, Darren Kenny wrote:
> Hi Alex,
> 
> On Saturday, 2021-03-13 at 18:18:57 -05, Alexander Bulekov wrote:
> > For testing, it can be useful to simulate an enormous amount of memory
> > (e.g. 2^64 RAM). This adds an MMIO device that acts as sparse memory.
> > When something writes a nonzero value to a sparse-mem address, we
> > allocate a block of memory. This block is kept around, until all of the
> > bytes within the block are zero-ed. The device has a very low priority
> 
> I don't see code below that actually checks if a block is zero-ed and
> removes it from the hash table, so is this comment correct?

No.. I will update it

> 
> > (so it can be mapped beneath actual RAM, and virtual device MMIO
> > regions).
> >
> > Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> > ---
> >  MAINTAINERS                 |   1 +
> >  hw/mem/meson.build          |   1 +
> >  hw/mem/sparse-mem.c         | 152 ++++++++++++++++++++++++++++++++++++
> >  include/hw/mem/sparse-mem.h |  19 +++++
> >  4 files changed, 173 insertions(+)
> >  create mode 100644 hw/mem/sparse-mem.c
> >  create mode 100644 include/hw/mem/sparse-mem.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index f22d83c178..9e3d8b1401 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2618,6 +2618,7 @@ R: Thomas Huth <thuth@redhat.com>
> >  S: Maintained
> >  F: tests/qtest/fuzz/
> >  F: scripts/oss-fuzz/
> > +F: hw/mem/sparse-mem.c
> >  F: docs/devel/fuzzing.rst
> >  
> >  Register API
> > diff --git a/hw/mem/meson.build b/hw/mem/meson.build
> > index 0d22f2b572..ef79e04678 100644
> > --- a/hw/mem/meson.build
> > +++ b/hw/mem/meson.build
> > @@ -1,5 +1,6 @@
> >  mem_ss = ss.source_set()
> >  mem_ss.add(files('memory-device.c'))
> > +mem_ss.add(when: 'CONFIG_FUZZ', if_true: files('sparse-mem.c'))
> >  mem_ss.add(when: 'CONFIG_DIMM', if_true: files('pc-dimm.c'))
> >  mem_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_mc.c'))
> >  mem_ss.add(when: 'CONFIG_NVDIMM', if_true: files('nvdimm.c'))
> > diff --git a/hw/mem/sparse-mem.c b/hw/mem/sparse-mem.c
> > new file mode 100644
> > index 0000000000..575a287f59
> > --- /dev/null
> > +++ b/hw/mem/sparse-mem.c
> > @@ -0,0 +1,152 @@
> > +/*
> > + * A sparse memory device. Useful for fuzzing
> > + *
> > + * Copyright Red Hat Inc., 2021
> > + *
> > + * 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 "exec/address-spaces.h"
> > +#include "hw/qdev-properties.h"
> > +#include "hw/sysbus.h"
> > +#include "qapi/error.h"
> > +#include "qemu/units.h"
> > +#include "sysemu/qtest.h"
> > +#include "hw/mem/sparse-mem.h"
> > +
> > +#define SPARSE_MEM(obj) OBJECT_CHECK(SparseMemState, (obj), TYPE_SPARSE_MEM)
> > +#define SPARSE_BLOCK_SIZE 0x1000
> 
> This is assuming a 4K block size, should that be the same as the system
> pagesize is? Or will it not matter w.r.t. how this is being consumed?
> 

It shouldn't make a difference, as long as it is a multiple of the
MMIO region's alignment size.

> > +
> > +typedef struct SparseMemState {
> > +    SysBusDevice parent_obj;
> > +    MemoryRegion mmio;
> > +    uint64_t baseaddr;
> > +    uint64_t length;
> > +    uint64_t size_used;
> > +    uint64_t maxsize;
> > +    GHashTable *mapped;
> > +} SparseMemState;
> > +
> > +typedef struct sparse_mem_block {
> > +    uint8_t data[SPARSE_BLOCK_SIZE];
> > +} sparse_mem_block;
> > +
> > +static uint64_t sparse_mem_read(void *opaque, hwaddr addr, unsigned int size)
> > +{
> > +    printf("SPARSEREAD %lx\n", addr);
> 
> Should this printf() be a logging/trace call? Or do you really want it to be
> printed all the time? Also seems out of place before the declaration of
> the variables.

No. I will remove it

> 
> Thanks,
> 
> Darren.