[PATCH 0/2] tests/qtest/fuzz: Avoid QTest serialization

[PATCH 0/2] tests/qtest/fuzz: Avoid QTest serialization

[Philippe Mathieu-Daudé]

Hi Alexander,

I forgot to share these 2 patches wrote before
the direct MemoryRegion fuzzer sent yesterday.

Regards,

Phil.

Philippe Mathieu-Daudé (2):
  tests/qtest/fuzz: Avoid QTest ioport serialization
  tests/qtest/fuzz: Avoid QTest mmio serialization

 tests/qtest/fuzz/i440fx_fuzz.c      | 19 +++++++++++++------
 tests/qtest/fuzz/virtio_net_fuzz.c  |  6 ++++--
 tests/qtest/fuzz/virtio_scsi_fuzz.c |  6 +++++-
 3 files changed, 22 insertions(+), 9 deletions(-)

-- 
2.21.3

[PATCH 1/2] tests/qtest/fuzz: Avoid QTest ioport serialization

[Philippe Mathieu-Daudé]

We don't need to serialize over QTest chardev when we can
directly access the globally registered I/O address space.

i440fx-qtest-reboot-fuzz gets 2x performance improvement.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/qtest/fuzz/i440fx_fuzz.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c
index bcd6769b4c..d770caffa7 100644
--- a/tests/qtest/fuzz/i440fx_fuzz.c
+++ b/tests/qtest/fuzz/i440fx_fuzz.c
@@ -20,6 +20,7 @@
 #include "fuzz/qos_fuzz.h"
 #include "fuzz/fork_fuzz.h"
 
+#include "exec/address-spaces.h"
 
 #define I440FX_PCI_HOST_BRIDGE_CFG 0xcf8
 #define I440FX_PCI_HOST_BRIDGE_DATA 0xcfc
@@ -59,22 +60,28 @@ static void ioport_fuzz_qtest(QTestState *s,
                                       I440FX_PCI_HOST_BRIDGE_DATA;
         switch (a.opcode % ACTION_MAX) {
         case WRITEB:
-            qtest_outb(s, addr, (uint8_t)a.value);
+            address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
+                                &a.value, sizeof(uint8_t));
             break;
         case WRITEW:
-            qtest_outw(s, addr, (uint16_t)a.value);
+            address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
+                                &a.value, sizeof(uint16_t));
             break;
         case WRITEL:
-            qtest_outl(s, addr, (uint32_t)a.value);
+            address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
+                                &a.value, sizeof(uint32_t));
             break;
         case READB:
-            qtest_inb(s, addr);
+            address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
+                               &a.value, sizeof(uint8_t));
             break;
         case READW:
-            qtest_inw(s, addr);
+            address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
+                               &a.value, sizeof(uint16_t));
             break;
         case READL:
-            qtest_inl(s, addr);
+            address_space_read(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
+                               &a.value, sizeof(uint32_t));
             break;
         }
         /* Move to the next operation */
-- 
2.21.3

[PATCH 2/2] tests/qtest/fuzz: Avoid QTest mmio serialization

[Philippe Mathieu-Daudé]

We don't need to serialize over QTest chardev when we can
directly access the MMIO address space via the first
registered CPU view.

virtio-net-socket gets ~50% performance improvement.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/qtest/fuzz/virtio_net_fuzz.c  | 6 ++++--
 tests/qtest/fuzz/virtio_scsi_fuzz.c | 6 +++++-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/fuzz/virtio_net_fuzz.c b/tests/qtest/fuzz/virtio_net_fuzz.c
index d08a47e278..ec993c9d5f 100644
--- a/tests/qtest/fuzz/virtio_net_fuzz.c
+++ b/tests/qtest/fuzz/virtio_net_fuzz.c
@@ -19,6 +19,8 @@
 #include "fork_fuzz.h"
 #include "qos_fuzz.h"
 
+#include "exec/address-spaces.h"
+#include "hw/core/cpu.h"
 
 #define QVIRTIO_NET_TIMEOUT_US (30 * 1000 * 1000)
 #define QVIRTIO_RX_VQ 0
@@ -69,8 +71,8 @@ static void virtio_net_fuzz_multi(QTestState *s,
              * If checking used ring, ensure that the fuzzer doesn't trigger
              * trivial asserion failure on zero-zied buffer
              */
-            qtest_memwrite(s, req_addr, Data, vqa.length);
-
+            address_space_write(first_cpu->as, req_addr, MEMTXATTRS_UNSPECIFIED,
+                                &Data, vqa.length);
 
             free_head = qvirtqueue_add(s, q, req_addr, vqa.length,
                     vqa.write, vqa.next);
diff --git a/tests/qtest/fuzz/virtio_scsi_fuzz.c b/tests/qtest/fuzz/virtio_scsi_fuzz.c
index 3b95247f12..5096a5a730 100644
--- a/tests/qtest/fuzz/virtio_scsi_fuzz.c
+++ b/tests/qtest/fuzz/virtio_scsi_fuzz.c
@@ -23,6 +23,9 @@
 #include "fork_fuzz.h"
 #include "qos_fuzz.h"
 
+#include "exec/address-spaces.h"
+#include "hw/core/cpu.h"
+
 #define PCI_SLOT                0x02
 #define PCI_FN                  0x00
 #define QVIRTIO_SCSI_TIMEOUT_US (1 * 1000 * 1000)
@@ -108,7 +111,8 @@ static void virtio_scsi_fuzz(QTestState *s, QVirtioSCSIQueues* queues,
 
         /* Copy the data into ram, and place it on the virtqueue */
         uint64_t req_addr = guest_alloc(t_alloc, vqa.length);
-        qtest_memwrite(s, req_addr, Data, vqa.length);
+        address_space_write(first_cpu->as, req_addr, MEMTXATTRS_UNSPECIFIED,
+                            &Data, vqa.length);
         if (vq_touched[vqa.queue] == 0) {
             vq_touched[vqa.queue] = 1;
             free_head[vqa.queue] = qvirtqueue_add(s, q, req_addr, vqa.length,
-- 
2.21.3

Re: [PATCH 0/2] tests/qtest/fuzz: Avoid QTest serialization

[Stefan Hajnoczi]

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

On Tue, May 26, 2020 at 07:58:18AM +0200, Philippe Mathieu-Daudé wrote:
> Hi Alexander,
> 
> I forgot to share these 2 patches wrote before
> the direct MemoryRegion fuzzer sent yesterday.
> 
> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (2):
>   tests/qtest/fuzz: Avoid QTest ioport serialization
>   tests/qtest/fuzz: Avoid QTest mmio serialization
> 
>  tests/qtest/fuzz/i440fx_fuzz.c      | 19 +++++++++++++------
>  tests/qtest/fuzz/virtio_net_fuzz.c  |  6 ++++--
>  tests/qtest/fuzz/virtio_scsi_fuzz.c |  6 +++++-
>  3 files changed, 22 insertions(+), 9 deletions(-)

Will it still be possible to print qtest reproducer commands when a
crash is found?

Other than this concern, higher fuzzing rates would be great.

Stefan

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

Re: [PATCH 0/2] tests/qtest/fuzz: Avoid QTest serialization

[Philippe Mathieu-Daudé]

On 5/26/20 10:56 AM, Stefan Hajnoczi wrote:
> On Tue, May 26, 2020 at 07:58:18AM +0200, Philippe Mathieu-Daudé wrote:
>> Hi Alexander,
>>
>> I forgot to share these 2 patches wrote before
>> the direct MemoryRegion fuzzer sent yesterday.
>>
>> Regards,
>>
>> Phil.
>>
>> Philippe Mathieu-Daudé (2):
>>   tests/qtest/fuzz: Avoid QTest ioport serialization
>>   tests/qtest/fuzz: Avoid QTest mmio serialization
>>
>>  tests/qtest/fuzz/i440fx_fuzz.c      | 19 +++++++++++++------
>>  tests/qtest/fuzz/virtio_net_fuzz.c  |  6 ++++--
>>  tests/qtest/fuzz/virtio_scsi_fuzz.c |  6 +++++-
>>  3 files changed, 22 insertions(+), 9 deletions(-)
> 
> Will it still be possible to print qtest reproducer commands when a
> crash is found?

Yes, there is no change in the corpus format.

> 
> Other than this concern, higher fuzzing rates would be great.

Thanks,

Phil.

Re: [PATCH 0/2] tests/qtest/fuzz: Avoid QTest serialization

[Alexander Bulekov]

On 200526 1105, Philippe Mathieu-Daudé wrote:
> On 5/26/20 10:56 AM, Stefan Hajnoczi wrote:
> > On Tue, May 26, 2020 at 07:58:18AM +0200, Philippe Mathieu-Daudé wrote:
> >> Hi Alexander,
> >>
> >> I forgot to share these 2 patches wrote before
> >> the direct MemoryRegion fuzzer sent yesterday.
> >>
> >> Regards,
> >>
> >> Phil.
> >>
> >> Philippe Mathieu-Daudé (2):
> >>   tests/qtest/fuzz: Avoid QTest ioport serialization
> >>   tests/qtest/fuzz: Avoid QTest mmio serialization
> >>
> >>  tests/qtest/fuzz/i440fx_fuzz.c      | 19 +++++++++++++------
> >>  tests/qtest/fuzz/virtio_net_fuzz.c  |  6 ++++--
> >>  tests/qtest/fuzz/virtio_scsi_fuzz.c |  6 +++++-
> >>  3 files changed, 22 insertions(+), 9 deletions(-)
> > 
> > Will it still be possible to print qtest reproducer commands when a
> > crash is found?
> 
> Yes, there is no change in the corpus format.

Yes, though with these patches, the qtest-based code will be gone.
Should there be some option to switch between the two modes?

> > 
> > Other than this concern, higher fuzzing rates would be great.
> 
> Thanks,
> 
> Phil.

Re: [PATCH 0/2] tests/qtest/fuzz: Avoid QTest serialization

[Philippe Mathieu-Daudé]

On 5/26/20 4:56 PM, Alexander Bulekov wrote:
> On 200526 1105, Philippe Mathieu-Daudé wrote:
>> On 5/26/20 10:56 AM, Stefan Hajnoczi wrote:
>>> On Tue, May 26, 2020 at 07:58:18AM +0200, Philippe Mathieu-Daudé wrote:
>>>> Hi Alexander,
>>>>
>>>> I forgot to share these 2 patches wrote before
>>>> the direct MemoryRegion fuzzer sent yesterday.
>>>>
>>>> Regards,
>>>>
>>>> Phil.
>>>>
>>>> Philippe Mathieu-Daudé (2):
>>>>   tests/qtest/fuzz: Avoid QTest ioport serialization
>>>>   tests/qtest/fuzz: Avoid QTest mmio serialization
>>>>
>>>>  tests/qtest/fuzz/i440fx_fuzz.c      | 19 +++++++++++++------
>>>>  tests/qtest/fuzz/virtio_net_fuzz.c  |  6 ++++--
>>>>  tests/qtest/fuzz/virtio_scsi_fuzz.c |  6 +++++-
>>>>  3 files changed, 22 insertions(+), 9 deletions(-)
>>>
>>> Will it still be possible to print qtest reproducer commands when a
>>> crash is found?
>>
>> Yes, there is no change in the corpus format.
> 
> Yes, though with these patches, the qtest-based code will be gone.
> Should there be some option to switch between the two modes?

How so?

How do you generate your reproducers?

> 
>>>
>>> Other than this concern, higher fuzzing rates would be great.
>>
>> Thanks,
>>
>> Phil.
> 

Re: [PATCH 2/2] tests/qtest/fuzz: Avoid QTest mmio serialization

[Alexander Bulekov]

On 200526 0758, Philippe Mathieu-Daudé wrote:
> We don't need to serialize over QTest chardev when we can
> directly access the MMIO address space via the first
> registered CPU view.
> 
> virtio-net-socket gets ~50% performance improvement.

One option might be to write alternate (direct) implemtations for
qtest_out*, qtest_write*, qest_read*, qtest_bufread, qtest_bufwrite and
qtest_memset. Maybe these could even go into qtest.c, alleviating some
of the complexity of qtest_process_command(). Then there can be
a preprocessor option to link against libqtest or against the direct
access functions. In the case of qos-based virtio and scsi fuzzers
below, this would also mean that abstract functions such as
qvirtqueue_add would also go through the direct access layer, instead of
mixing direct access and qtest commands.

I don't think this is something we need right now, but it would be
useful for building qtest reproducers.

> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alexander Bulekov <alxndr@bu.edu>

> ---
>  tests/qtest/fuzz/virtio_net_fuzz.c  | 6 ++++--
>  tests/qtest/fuzz/virtio_scsi_fuzz.c | 6 +++++-
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/qtest/fuzz/virtio_net_fuzz.c b/tests/qtest/fuzz/virtio_net_fuzz.c
> index d08a47e278..ec993c9d5f 100644
> --- a/tests/qtest/fuzz/virtio_net_fuzz.c
> +++ b/tests/qtest/fuzz/virtio_net_fuzz.c
> @@ -19,6 +19,8 @@
>  #include "fork_fuzz.h"
>  #include "qos_fuzz.h"
>  
> +#include "exec/address-spaces.h"
> +#include "hw/core/cpu.h"
>  
>  #define QVIRTIO_NET_TIMEOUT_US (30 * 1000 * 1000)
>  #define QVIRTIO_RX_VQ 0
> @@ -69,8 +71,8 @@ static void virtio_net_fuzz_multi(QTestState *s,
>               * If checking used ring, ensure that the fuzzer doesn't trigger
>               * trivial asserion failure on zero-zied buffer
>               */
> -            qtest_memwrite(s, req_addr, Data, vqa.length);
> -
> +            address_space_write(first_cpu->as, req_addr, MEMTXATTRS_UNSPECIFIED,
> +                                &Data, vqa.length);
>  
>              free_head = qvirtqueue_add(s, q, req_addr, vqa.length,
>                      vqa.write, vqa.next);
> diff --git a/tests/qtest/fuzz/virtio_scsi_fuzz.c b/tests/qtest/fuzz/virtio_scsi_fuzz.c
> index 3b95247f12..5096a5a730 100644
> --- a/tests/qtest/fuzz/virtio_scsi_fuzz.c
> +++ b/tests/qtest/fuzz/virtio_scsi_fuzz.c
> @@ -23,6 +23,9 @@
>  #include "fork_fuzz.h"
>  #include "qos_fuzz.h"
>  
> +#include "exec/address-spaces.h"
> +#include "hw/core/cpu.h"
> +
>  #define PCI_SLOT                0x02
>  #define PCI_FN                  0x00
>  #define QVIRTIO_SCSI_TIMEOUT_US (1 * 1000 * 1000)
> @@ -108,7 +111,8 @@ static void virtio_scsi_fuzz(QTestState *s, QVirtioSCSIQueues* queues,
>  
>          /* Copy the data into ram, and place it on the virtqueue */
>          uint64_t req_addr = guest_alloc(t_alloc, vqa.length);
> -        qtest_memwrite(s, req_addr, Data, vqa.length);
> +        address_space_write(first_cpu->as, req_addr, MEMTXATTRS_UNSPECIFIED,
> +                            &Data, vqa.length);
>          if (vq_touched[vqa.queue] == 0) {
>              vq_touched[vqa.queue] = 1;
>              free_head[vqa.queue] = qvirtqueue_add(s, q, req_addr, vqa.length,
> -- 
> 2.21.3
> 

Re: [PATCH 0/2] tests/qtest/fuzz: Avoid QTest serialization

[Alexander Bulekov]

On 200526 1725, Philippe Mathieu-Daudé wrote:
> On 5/26/20 4:56 PM, Alexander Bulekov wrote:
> > On 200526 1105, Philippe Mathieu-Daudé wrote:
> >> On 5/26/20 10:56 AM, Stefan Hajnoczi wrote:
> >>> On Tue, May 26, 2020 at 07:58:18AM +0200, Philippe Mathieu-Daudé wrote:
> >>>> Hi Alexander,
> >>>>
> >>>> I forgot to share these 2 patches wrote before
> >>>> the direct MemoryRegion fuzzer sent yesterday.
> >>>>
> >>>> Regards,
> >>>>
> >>>> Phil.
> >>>>
> >>>> Philippe Mathieu-Daudé (2):
> >>>>   tests/qtest/fuzz: Avoid QTest ioport serialization
> >>>>   tests/qtest/fuzz: Avoid QTest mmio serialization
> >>>>
> >>>>  tests/qtest/fuzz/i440fx_fuzz.c      | 19 +++++++++++++------
> >>>>  tests/qtest/fuzz/virtio_net_fuzz.c  |  6 ++++--
> >>>>  tests/qtest/fuzz/virtio_scsi_fuzz.c |  6 +++++-
> >>>>  3 files changed, 22 insertions(+), 9 deletions(-)
> >>>
> >>> Will it still be possible to print qtest reproducer commands when a
> >>> crash is found?
> >>
> >> Yes, there is no change in the corpus format.
> > 
> > Yes, though with these patches, the qtest-based code will be gone.
> > Should there be some option to switch between the two modes?
> 
> How so?
> 
> How do you generate your reproducers?

Right now basically with this:

--- a/qtest.c
+++ b/qtest.c
@@ -808,6 +808,8 @@ bool qtest_driver(void)

 void qtest_server_inproc_recv(void *dummy, const char *buf)
 {
+    // It would be nice to add support for qtest's built in qtest_log_fp.
+    printf(">>> %s\n", buf);
     static GString *gstr;
     if (!gstr) {
         gstr = g_string_new(NULL);
--

It would be nice to add support for qtest's built in qtest_log_fp.
Unless I'm missing something, these address_space_writes completely
bypass qtest, so there has to be some additional step to build
reproducers(eg running against the QTest-based version, or adding some
way to spit out corresponding qtest commands for the
address_space_writes).

> > 
> >>>
> >>> Other than this concern, higher fuzzing rates would be great.
> >>
> >> Thanks,
> >>
> >> Phil.
> > 

Re: [PATCH 0/2] tests/qtest/fuzz: Avoid QTest serialization

[Philippe Mathieu-Daudé]

On 5/26/20 5:41 PM, Alexander Bulekov wrote:
> On 200526 1725, Philippe Mathieu-Daudé wrote:
>> On 5/26/20 4:56 PM, Alexander Bulekov wrote:
>>> On 200526 1105, Philippe Mathieu-Daudé wrote:
>>>> On 5/26/20 10:56 AM, Stefan Hajnoczi wrote:
>>>>> On Tue, May 26, 2020 at 07:58:18AM +0200, Philippe Mathieu-Daudé wrote:
>>>>>> Hi Alexander,
>>>>>>
>>>>>> I forgot to share these 2 patches wrote before
>>>>>> the direct MemoryRegion fuzzer sent yesterday.
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>> Phil.
>>>>>>
>>>>>> Philippe Mathieu-Daudé (2):
>>>>>>   tests/qtest/fuzz: Avoid QTest ioport serialization
>>>>>>   tests/qtest/fuzz: Avoid QTest mmio serialization
>>>>>>
>>>>>>  tests/qtest/fuzz/i440fx_fuzz.c      | 19 +++++++++++++------
>>>>>>  tests/qtest/fuzz/virtio_net_fuzz.c  |  6 ++++--
>>>>>>  tests/qtest/fuzz/virtio_scsi_fuzz.c |  6 +++++-
>>>>>>  3 files changed, 22 insertions(+), 9 deletions(-)
>>>>>
>>>>> Will it still be possible to print qtest reproducer commands when a
>>>>> crash is found?
>>>>
>>>> Yes, there is no change in the corpus format.
>>>
>>> Yes, though with these patches, the qtest-based code will be gone.
>>> Should there be some option to switch between the two modes?
>>
>> How so?
>>
>> How do you generate your reproducers?
> 
> Right now basically with this:
> 
> --- a/qtest.c
> +++ b/qtest.c
> @@ -808,6 +808,8 @@ bool qtest_driver(void)
> 
>  void qtest_server_inproc_recv(void *dummy, const char *buf)
>  {
> +    // It would be nice to add support for qtest's built in qtest_log_fp.
> +    printf(">>> %s\n", buf);

Uh =) I suppose you restart a single job with the offending corpus file?

>      static GString *gstr;
>      if (!gstr) {
>          gstr = g_string_new(NULL);
> --
> 
> It would be nice to add support for qtest's built in qtest_log_fp.
> Unless I'm missing something, these address_space_writes completely
> bypass qtest, so there has to be some additional step to build
> reproducers(eg running against the QTest-based version, or adding some
> way to spit out corresponding qtest commands for the
> address_space_writes).

I am using this hacky script, not committed yet because not ready but
still you can get the idea:

-- >8 --
import sys
import struct

# Tune to MemoryRegion properties
IOBASE = 0xa0002000
IOSIZE =     0x1000

action = { #           ASM        ADDR         VAL
    0x00: {'opcode': 'writeb', 'size': 8,  'fmt': 'B'},
    0x01: {'opcode': 'writew', 'size': 8,  'fmt': 'H'},
    0x02: {'opcode': 'writel', 'size': 8,  'fmt': 'I'},
    0x03: {'opcode': 'writeq', 'size': 8,  'fmt': 'L'},

    0x04: {'opcode': 'writeb', 'size': 16,  'fmt': 'B'},
    0x05: {'opcode': 'writew', 'size': 16,  'fmt': 'H'},
    0x06: {'opcode': 'writel', 'size': 16,  'fmt': 'I'},
    0x07: {'opcode': 'writeq', 'size': 16,  'fmt': 'L'},

    0x08: {'opcode': 'writeb', 'size': 32,  'fmt': 'B'},
    0x09: {'opcode': 'writew', 'size': 32,  'fmt': 'H'},
    0x0a: {'opcode': 'writel', 'size': 32,  'fmt': 'I'},
    0x0b: {'opcode': 'writeq', 'size': 32,  'fmt': 'L'},

    0x10: {'opcode': 'readb',  'size': 8,  'fmt': 'B'},
    0x11: {'opcode': 'readw',  'size': 8,  'fmt': 'H'},
    0x12: {'opcode': 'readl',  'size': 8,  'fmt': 'I'},
    0x13: {'opcode': 'readq',  'size': 8,  'fmt': 'L'},

    0x14: {'opcode': 'readb',  'size': 16,  'fmt': 'B'},
    0x15: {'opcode': 'readw',  'size': 16,  'fmt': 'H'},
    0x16: {'opcode': 'readl',  'size': 16,  'fmt': 'I'},
    0x17: {'opcode': 'readq',  'size': 16,  'fmt': 'L'},

    0x18: {'opcode': 'readb',  'size': 32,  'fmt': 'B'},
    0x19: {'opcode': 'readw',  'size': 32,  'fmt': 'H'},
    0x1a: {'opcode': 'readl',  'size': 32,  'fmt': 'I'},
    0x1b: {'opcode': 'readq',  'size': 32,  'fmt': 'L'},
}
AMASK = 0x1f
ADDR = {8: 'B', 16: 'H', 32: 'I', 64: 'L'}

def fuzz_parse_corpus_data(fn):
    fd = open(fn, 'rb')
    while True:
        buf = fd.read(1)
        if len(buf) < 1:
            break
        op, = struct.unpack("B", buf)
        op &= AMASK
        if op not in action:
            break
        a = action[op]
        fmt = "<" + ADDR[a['size']] + a['fmt']
        fmtsz = struct.calcsize(fmt)
        buf = fd.read(fmtsz)
        if len(buf) < fmtsz:
            break
        if a['fmt'] == 'x':
            addr, = struct.unpack(fmt, buf)
            val = 0
        else:
            addr, val = struct.unpack(fmt, buf)
        addr &= IOSIZE - 1
        print("%s 0x%02x 0x%x" % (a['opcode'], IOBASE + addr, val))

fuzz_parse_corpus_data(sys.argv[1])
---

$ python tests/qtest/fuzz/corpus2qtest.py \
  ./crash-12e481ba7c2a7a625152dc701821d5e184cddee8
writel 0xa0002000 0x20010000
writeb 0xa0002020 0x20
readl 0xa0002100 0x204204ff
writeb 0xa0002042 0x36
readl 0xa0002436 0xf4760024
writel 0xa0002020 0x4363636
writeb 0xa0002600 0xf4
writew 0xa0002001 0x2020
writeb 0xa0002020 0x4
writel 0xa0002020 0x4363636
writeb 0xa0002600 0xf4
writel 0xa0002020 0x36363a01
readl 0xa0002404 0x1f47600
writew 0xa0002020 0x2020
writeb 0xa0002004 0x42
writeb 0xa0002036 0x36
readl 0xa0002404 0x42f47600
writeb 0xa0002036 0x36
readl 0xa0002404 0x1f47600
writew 0xa0002020 0x420

> 
>>>
>>>>>
>>>>> Other than this concern, higher fuzzing rates would be great.
>>>>
>>>> Thanks,
>>>>
>>>> Phil.
>>>
> 

Re: [PATCH 0/2] tests/qtest/fuzz: Avoid QTest serialization

[Philippe Mathieu-Daudé]

On 5/26/20 5:52 PM, Philippe Mathieu-Daudé wrote:
> On 5/26/20 5:41 PM, Alexander Bulekov wrote:
>> On 200526 1725, Philippe Mathieu-Daudé wrote:
>>> On 5/26/20 4:56 PM, Alexander Bulekov wrote:
>>>> On 200526 1105, Philippe Mathieu-Daudé wrote:
>>>>> On 5/26/20 10:56 AM, Stefan Hajnoczi wrote:
>>>>>> On Tue, May 26, 2020 at 07:58:18AM +0200, Philippe Mathieu-Daudé wrote:
>>>>>>> Hi Alexander,
>>>>>>>
>>>>>>> I forgot to share these 2 patches wrote before
>>>>>>> the direct MemoryRegion fuzzer sent yesterday.
>>>>>>>
>>>>>>> Regards,
>>>>>>>
>>>>>>> Phil.
>>>>>>>
>>>>>>> Philippe Mathieu-Daudé (2):
>>>>>>>   tests/qtest/fuzz: Avoid QTest ioport serialization
>>>>>>>   tests/qtest/fuzz: Avoid QTest mmio serialization
>>>>>>>
>>>>>>>  tests/qtest/fuzz/i440fx_fuzz.c      | 19 +++++++++++++------
>>>>>>>  tests/qtest/fuzz/virtio_net_fuzz.c  |  6 ++++--
>>>>>>>  tests/qtest/fuzz/virtio_scsi_fuzz.c |  6 +++++-
>>>>>>>  3 files changed, 22 insertions(+), 9 deletions(-)
>>>>>>
>>>>>> Will it still be possible to print qtest reproducer commands when a
>>>>>> crash is found?
>>>>>
>>>>> Yes, there is no change in the corpus format.
>>>>
>>>> Yes, though with these patches, the qtest-based code will be gone.
>>>> Should there be some option to switch between the two modes?
>>>
>>> How so?
>>>
>>> How do you generate your reproducers?
>>
>> Right now basically with this:
>>
>> --- a/qtest.c
>> +++ b/qtest.c
>> @@ -808,6 +808,8 @@ bool qtest_driver(void)
>>
>>  void qtest_server_inproc_recv(void *dummy, const char *buf)
>>  {
>> +    // It would be nice to add support for qtest's built in qtest_log_fp.
>> +    printf(">>> %s\n", buf);
> 
> Uh =) I suppose you restart a single job with the offending corpus file?
> 
>>      static GString *gstr;
>>      if (!gstr) {
>>          gstr = g_string_new(NULL);
>> --
>>
>> It would be nice to add support for qtest's built in qtest_log_fp.
>> Unless I'm missing something, these address_space_writes completely
>> bypass qtest, so there has to be some additional step to build
>> reproducers(eg running against the QTest-based version, or adding some
>> way to spit out corresponding qtest commands for the
>> address_space_writes).
> 
> I am using this hacky script, not committed yet because not ready but
> still you can get the idea:
> 
> -- >8 --
> import sys
> import struct
> 
> # Tune to MemoryRegion properties
> IOBASE = 0xa0002000
> IOSIZE =     0x1000
> 
> action = { #           ASM        ADDR         VAL
>     0x00: {'opcode': 'writeb', 'size': 8,  'fmt': 'B'},
>     0x01: {'opcode': 'writew', 'size': 8,  'fmt': 'H'},
>     0x02: {'opcode': 'writel', 'size': 8,  'fmt': 'I'},
>     0x03: {'opcode': 'writeq', 'size': 8,  'fmt': 'L'},
> 
>     0x04: {'opcode': 'writeb', 'size': 16,  'fmt': 'B'},
>     0x05: {'opcode': 'writew', 'size': 16,  'fmt': 'H'},
>     0x06: {'opcode': 'writel', 'size': 16,  'fmt': 'I'},
>     0x07: {'opcode': 'writeq', 'size': 16,  'fmt': 'L'},
> 
>     0x08: {'opcode': 'writeb', 'size': 32,  'fmt': 'B'},
>     0x09: {'opcode': 'writew', 'size': 32,  'fmt': 'H'},
>     0x0a: {'opcode': 'writel', 'size': 32,  'fmt': 'I'},
>     0x0b: {'opcode': 'writeq', 'size': 32,  'fmt': 'L'},
> 
>     0x10: {'opcode': 'readb',  'size': 8,  'fmt': 'B'},
>     0x11: {'opcode': 'readw',  'size': 8,  'fmt': 'H'},
>     0x12: {'opcode': 'readl',  'size': 8,  'fmt': 'I'},
>     0x13: {'opcode': 'readq',  'size': 8,  'fmt': 'L'},
> 
>     0x14: {'opcode': 'readb',  'size': 16,  'fmt': 'B'},
>     0x15: {'opcode': 'readw',  'size': 16,  'fmt': 'H'},
>     0x16: {'opcode': 'readl',  'size': 16,  'fmt': 'I'},
>     0x17: {'opcode': 'readq',  'size': 16,  'fmt': 'L'},
> 
>     0x18: {'opcode': 'readb',  'size': 32,  'fmt': 'B'},
>     0x19: {'opcode': 'readw',  'size': 32,  'fmt': 'H'},
>     0x1a: {'opcode': 'readl',  'size': 32,  'fmt': 'I'},
>     0x1b: {'opcode': 'readq',  'size': 32,  'fmt': 'L'},
> }
> AMASK = 0x1f
> ADDR = {8: 'B', 16: 'H', 32: 'I', 64: 'L'}
> 
> def fuzz_parse_corpus_data(fn):
>     fd = open(fn, 'rb')
>     while True:
>         buf = fd.read(1)
>         if len(buf) < 1:
>             break
>         op, = struct.unpack("B", buf)
>         op &= AMASK
>         if op not in action:
>             break
>         a = action[op]
>         fmt = "<" + ADDR[a['size']] + a['fmt']
>         fmtsz = struct.calcsize(fmt)
>         buf = fd.read(fmtsz)
>         if len(buf) < fmtsz:
>             break
>         if a['fmt'] == 'x':
>             addr, = struct.unpack(fmt, buf)
>             val = 0
>         else:
>             addr, val = struct.unpack(fmt, buf)
>         addr &= IOSIZE - 1
>         print("%s 0x%02x 0x%x" % (a['opcode'], IOBASE + addr, val))
> 
> fuzz_parse_corpus_data(sys.argv[1])
> ---
> 
> $ python tests/qtest/fuzz/corpus2qtest.py \
>   ./crash-12e481ba7c2a7a625152dc701821d5e184cddee8
> writel 0xa0002000 0x20010000
> writeb 0xa0002020 0x20
> readl 0xa0002100 0x204204ff
> writeb 0xa0002042 0x36
> readl 0xa0002436 0xf4760024
> writel 0xa0002020 0x4363636
> writeb 0xa0002600 0xf4
> writew 0xa0002001 0x2020
> writeb 0xa0002020 0x4
> writel 0xa0002020 0x4363636
> writeb 0xa0002600 0xf4
> writel 0xa0002020 0x36363a01
> readl 0xa0002404 0x1f47600
> writew 0xa0002020 0x2020
> writeb 0xa0002004 0x42
> writeb 0xa0002036 0x36
> readl 0xa0002404 0x42f47600
> writeb 0xa0002036 0x36
> readl 0xa0002404 0x1f47600
> writew 0xa0002020 0x420

FYI talking with Stefan he kinda suggested it is cleaner to use a
'-replay corpus_data.bin' mode that enables the printf output from
qtest_*read/write (without reaching the device) rather than maintaining
a Python script.

> 
>>
>>>>
>>>>>>
>>>>>> Other than this concern, higher fuzzing rates would be great.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Phil.
>>>>
>>
> 

Re: [PATCH 0/2] tests/qtest/fuzz: Avoid QTest serialization

[Alexander Bulekov]

On 200528 1533, Philippe Mathieu-Daudé wrote:
> On 5/26/20 5:52 PM, Philippe Mathieu-Daudé wrote:
> > On 5/26/20 5:41 PM, Alexander Bulekov wrote:
> >> On 200526 1725, Philippe Mathieu-Daudé wrote:
> >>> On 5/26/20 4:56 PM, Alexander Bulekov wrote:
> >>>> On 200526 1105, Philippe Mathieu-Daudé wrote:
> >>>>> On 5/26/20 10:56 AM, Stefan Hajnoczi wrote:
> >>>>>> On Tue, May 26, 2020 at 07:58:18AM +0200, Philippe Mathieu-Daudé wrote:
> >>>>>>> Hi Alexander,
> >>>>>>>
> >>>>>>> I forgot to share these 2 patches wrote before
> >>>>>>> the direct MemoryRegion fuzzer sent yesterday.
> >>>>>>>
> >>>>>>> Regards,
> >>>>>>>
> >>>>>>> Phil.
> >>>>>>>
> >>>>>>> Philippe Mathieu-Daudé (2):
> >>>>>>>   tests/qtest/fuzz: Avoid QTest ioport serialization
> >>>>>>>   tests/qtest/fuzz: Avoid QTest mmio serialization
> >>>>>>>
> >>>>>>>  tests/qtest/fuzz/i440fx_fuzz.c      | 19 +++++++++++++------
> >>>>>>>  tests/qtest/fuzz/virtio_net_fuzz.c  |  6 ++++--
> >>>>>>>  tests/qtest/fuzz/virtio_scsi_fuzz.c |  6 +++++-
> >>>>>>>  3 files changed, 22 insertions(+), 9 deletions(-)
> >>>>>>
> >>>>>> Will it still be possible to print qtest reproducer commands when a
> >>>>>> crash is found?
> >>>>>
> >>>>> Yes, there is no change in the corpus format.
> >>>>
> >>>> Yes, though with these patches, the qtest-based code will be gone.
> >>>> Should there be some option to switch between the two modes?
> >>>
> >>> How so?
> >>>
> >>> How do you generate your reproducers?
> >>
> >> Right now basically with this:
> >>
> >> --- a/qtest.c
> >> +++ b/qtest.c
> >> @@ -808,6 +808,8 @@ bool qtest_driver(void)
> >>
> >>  void qtest_server_inproc_recv(void *dummy, const char *buf)
> >>  {
> >> +    // It would be nice to add support for qtest's built in qtest_log_fp.
> >> +    printf(">>> %s\n", buf);
> > 
> > Uh =) I suppose you restart a single job with the offending corpus file?
> > 
> >>      static GString *gstr;
> >>      if (!gstr) {
> >>          gstr = g_string_new(NULL);
> >> --
> >>
> >> It would be nice to add support for qtest's built in qtest_log_fp.
> >> Unless I'm missing something, these address_space_writes completely
> >> bypass qtest, so there has to be some additional step to build
> >> reproducers(eg running against the QTest-based version, or adding some
> >> way to spit out corresponding qtest commands for the
> >> address_space_writes).
> > 
> > I am using this hacky script, not committed yet because not ready but
> > still you can get the idea:
> > 
> > -- >8 --
> > import sys
> > import struct
> > 
> > # Tune to MemoryRegion properties
> > IOBASE = 0xa0002000
> > IOSIZE =     0x1000
> > 
> > action = { #           ASM        ADDR         VAL
> >     0x00: {'opcode': 'writeb', 'size': 8,  'fmt': 'B'},
> >     0x01: {'opcode': 'writew', 'size': 8,  'fmt': 'H'},
> >     0x02: {'opcode': 'writel', 'size': 8,  'fmt': 'I'},
> >     0x03: {'opcode': 'writeq', 'size': 8,  'fmt': 'L'},
> > 
> >     0x04: {'opcode': 'writeb', 'size': 16,  'fmt': 'B'},
> >     0x05: {'opcode': 'writew', 'size': 16,  'fmt': 'H'},
> >     0x06: {'opcode': 'writel', 'size': 16,  'fmt': 'I'},
> >     0x07: {'opcode': 'writeq', 'size': 16,  'fmt': 'L'},
> > 
> >     0x08: {'opcode': 'writeb', 'size': 32,  'fmt': 'B'},
> >     0x09: {'opcode': 'writew', 'size': 32,  'fmt': 'H'},
> >     0x0a: {'opcode': 'writel', 'size': 32,  'fmt': 'I'},
> >     0x0b: {'opcode': 'writeq', 'size': 32,  'fmt': 'L'},
> > 
> >     0x10: {'opcode': 'readb',  'size': 8,  'fmt': 'B'},
> >     0x11: {'opcode': 'readw',  'size': 8,  'fmt': 'H'},
> >     0x12: {'opcode': 'readl',  'size': 8,  'fmt': 'I'},
> >     0x13: {'opcode': 'readq',  'size': 8,  'fmt': 'L'},
> > 
> >     0x14: {'opcode': 'readb',  'size': 16,  'fmt': 'B'},
> >     0x15: {'opcode': 'readw',  'size': 16,  'fmt': 'H'},
> >     0x16: {'opcode': 'readl',  'size': 16,  'fmt': 'I'},
> >     0x17: {'opcode': 'readq',  'size': 16,  'fmt': 'L'},
> > 
> >     0x18: {'opcode': 'readb',  'size': 32,  'fmt': 'B'},
> >     0x19: {'opcode': 'readw',  'size': 32,  'fmt': 'H'},
> >     0x1a: {'opcode': 'readl',  'size': 32,  'fmt': 'I'},
> >     0x1b: {'opcode': 'readq',  'size': 32,  'fmt': 'L'},
> > }
> > AMASK = 0x1f
> > ADDR = {8: 'B', 16: 'H', 32: 'I', 64: 'L'}
> > 
> > def fuzz_parse_corpus_data(fn):
> >     fd = open(fn, 'rb')
> >     while True:
> >         buf = fd.read(1)
> >         if len(buf) < 1:
> >             break
> >         op, = struct.unpack("B", buf)
> >         op &= AMASK
> >         if op not in action:
> >             break
> >         a = action[op]
> >         fmt = "<" + ADDR[a['size']] + a['fmt']
> >         fmtsz = struct.calcsize(fmt)
> >         buf = fd.read(fmtsz)
> >         if len(buf) < fmtsz:
> >             break
> >         if a['fmt'] == 'x':
> >             addr, = struct.unpack(fmt, buf)
> >             val = 0
> >         else:
> >             addr, val = struct.unpack(fmt, buf)
> >         addr &= IOSIZE - 1
> >         print("%s 0x%02x 0x%x" % (a['opcode'], IOBASE + addr, val))
> > 
> > fuzz_parse_corpus_data(sys.argv[1])
> > ---
> > 
> > $ python tests/qtest/fuzz/corpus2qtest.py \
> >   ./crash-12e481ba7c2a7a625152dc701821d5e184cddee8
> > writel 0xa0002000 0x20010000
> > writeb 0xa0002020 0x20
> > readl 0xa0002100 0x204204ff
> > writeb 0xa0002042 0x36
> > readl 0xa0002436 0xf4760024
> > writel 0xa0002020 0x4363636
> > writeb 0xa0002600 0xf4
> > writew 0xa0002001 0x2020
> > writeb 0xa0002020 0x4
> > writel 0xa0002020 0x4363636
> > writeb 0xa0002600 0xf4
> > writel 0xa0002020 0x36363a01
> > readl 0xa0002404 0x1f47600
> > writew 0xa0002020 0x2020
> > writeb 0xa0002004 0x42
> > writeb 0xa0002036 0x36
> > readl 0xa0002404 0x42f47600
> > writeb 0xa0002036 0x36
> > readl 0xa0002404 0x1f47600
> > writew 0xa0002020 0x420
> 
> FYI talking with Stefan he kinda suggested it is cleaner to use a
> '-replay corpus_data.bin' mode that enables the printf output from
> qtest_*read/write (without reaching the device) rather than maintaining
> a Python script.

I think handling the QTEST_LOG=1 environment variable in
qtest_inproc_init would be a decent solution. Alternatively we could
have a -replay flag which just adds "-qtest-log /dev/fd/2" to the
qemu_main args. Writing scripts gets complicated for e.g. the
virtio-net-* fuzzers, where the input is converted into some high-level
libqos calls.
-Alex

> > 
> >>
> >>>>
> >>>>>>
> >>>>>> Other than this concern, higher fuzzing rates would be great.
> >>>>>
> >>>>> Thanks,
> >>>>>
> >>>>> Phil.
> >>>>
> >>
> >