[Qemu-devel] [PATCH v3 00/22] Add virtual device fuzzing support

[Qemu-devel] [PATCH v3 00/22] Add virtual device fuzzing support

[Oleinik, Alexander]

This series adds a framework for coverage-guided fuzzing of
virtual-devices. Fuzzing targets are based on qtest and can make use of
the libqos abstractions.

Build instructions in docs/devel/fuzzing.txt

V3:
 * Rebased onto v4.1.0+
 * Add the fuzzer as a new build-target type in the build-system
 * Add indirection to qtest client/server communication functions
 * Remove ramfile and snapshot-based fuzzing support
 * Add i440fx fuzz-target as a reference for developers.
 * Add linker-script to assist with fork-based fuzzer

V2:
 * Split off changes to qos virtio-net and qtest server to other patches
 * Move vl:main initialization into new func: qemu_init
 * Moved useful functions from qos-test.c to a separate object
 * Use struct of function pointers for add_fuzz_target(), instead of
   arguments
 * Move ramfile to migration/qemu-file
 * Rewrite fork-based fuzzer pending patch to libfuzzer
 * Pass check-patch

Alexander Oleinik (22):
  softmmu: split off vl.c:main() into main.c
  libqos: Rename i2c_send and i2c_recv
  fuzz: Add FUZZ_TARGET module type
  qtest: add qtest_server_send abstraction
  libqtest: Add a layer of abstraciton to send/recv
  fuzz: add configure flag --enable-fuzzing
  fuzz: Add target/fuzz makefile rules
  module: check module wasn't already initialized
  qtest: add in-process incoming command handler
  tests: provide test variables to other targets
  libqos: split qos-test and libqos makefile vars
  libqos: move useful qos-test funcs to qos_external
  libqtest: make qtest_bufwrite send "atomic"
  libqtest: add in-process qtest.c tx/rx handlers
  fuzz: Add target/fuzz makefile rules
  fuzz: add fuzzer skeleton
  fuzz: add support for fork-based fuzzing.
  fuzz: expose fuzz target name
  fuzz: add support for qos-assisted fuzz targets
  fuzz: add i440fx fuzz targets
  fuzz: add virtio-net fuzz target
  fuzz: add documentation to docs/devel/

 Makefile                     |  26 ++++-
 Makefile.objs                |   6 +
 Makefile.target              |  17 +++
 configure                    |  13 +++
 docs/devel/fuzzing.txt       | 107 ++++++++++++++++++
 exec.c                       |   2 +
 include/qemu/module.h        |   4 +-
 include/sysemu/qtest.h       |   4 +
 include/sysemu/sysemu.h      |   4 +
 main.c                       |  29 +++++
 qtest.c                      |  24 +++-
 tests/Makefile.include       |  74 ++++++------
 tests/fuzz/Makefile.include  |  11 ++
 tests/fuzz/fork_fuzz.c       |  27 +++++
 tests/fuzz/fork_fuzz.h       |  12 ++
 tests/fuzz/fork_fuzz.ld      |  46 ++++++++
 tests/fuzz/fuzz.c            | 182 ++++++++++++++++++++++++++++++
 tests/fuzz/fuzz.h            |  31 +++++
 tests/fuzz/i440fx_fuzz.c     | 158 ++++++++++++++++++++++++++
 tests/fuzz/qos_fuzz.c        | 212 +++++++++++++++++++++++++++++++++++
 tests/fuzz/qos_fuzz.h        |  19 ++++
 tests/fuzz/virtio_net_fuzz.c | 120 ++++++++++++++++++++
 tests/libqos/i2c-imx.c       |   8 +-
 tests/libqos/i2c-omap.c      |   8 +-
 tests/libqos/i2c.c           |  10 +-
 tests/libqos/i2c.h           |   4 +-
 tests/libqos/qos_external.c  | 151 +++++++++++++++++++++++++
 tests/libqos/qos_external.h  |  10 ++
 tests/libqtest.c             | 114 ++++++++++++++++---
 tests/libqtest.h             |   4 +
 tests/pca9552-test.c         |  10 +-
 tests/qos-test.c             | 140 +----------------------
 util/module.c                |   7 ++
 vl.c                         |  25 ++---
 34 files changed, 1393 insertions(+), 226 deletions(-)
 create mode 100644 docs/devel/fuzzing.txt
 create mode 100644 main.c
 create mode 100644 tests/fuzz/Makefile.include
 create mode 100644 tests/fuzz/fork_fuzz.c
 create mode 100644 tests/fuzz/fork_fuzz.h
 create mode 100644 tests/fuzz/fork_fuzz.ld
 create mode 100644 tests/fuzz/fuzz.c
 create mode 100644 tests/fuzz/fuzz.h
 create mode 100644 tests/fuzz/i440fx_fuzz.c
 create mode 100644 tests/fuzz/qos_fuzz.c
 create mode 100644 tests/fuzz/qos_fuzz.h
 create mode 100644 tests/fuzz/virtio_net_fuzz.c
 create mode 100644 tests/libqos/qos_external.c
 create mode 100644 tests/libqos/qos_external.h

-- 
2.23.0

[Qemu-devel] [PATCH v3 01/22] softmmu: split off vl.c:main() into main.c

[Oleinik, Alexander]

A program might rely on functions implemented in vl.c, but implement its
own main(). By placing main into a separate source file, there are no
complaints about duplicate main()s when linking against vl.o. For
example, the virtual-device fuzzer uses a main() provided by libfuzzer,
and needs to perform some initialization before running the softmmu
initialization. Now, main simply calls three vl.c functions which
handle the guest initialization, main loop and cleanup.

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 Makefile                |  1 +
 Makefile.objs           |  2 ++
 include/sysemu/sysemu.h |  4 ++++
 main.c                  | 29 +++++++++++++++++++++++++++++
 vl.c                    | 25 +++++++------------------
 5 files changed, 43 insertions(+), 18 deletions(-)
 create mode 100644 main.c

diff --git a/Makefile b/Makefile
index b3528617e4..f628783571 100644
--- a/Makefile
+++ b/Makefile
@@ -463,6 +463,7 @@ $(SOFTMMU_ALL_RULES): $(crypto-obj-y)
 $(SOFTMMU_ALL_RULES): $(io-obj-y)
 $(SOFTMMU_ALL_RULES): config-all-devices.mak
 $(SOFTMMU_ALL_RULES): $(edk2-decompressed)
+$(SOFTMMU_ALL_RULES): $(softmmu-main-y)
 
 .PHONY: $(TARGET_DIRS_RULES)
 # The $(TARGET_DIRS_RULES) are of the form SUBDIR/GOAL, so that
diff --git a/Makefile.objs b/Makefile.objs
index 6a143dcd57..bb1cfa05ef 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -86,6 +86,8 @@ common-obj-$(CONFIG_FDT) += device_tree.o
 # qapi
 
 common-obj-y += qapi/
+
+softmmu-main-y = main.o
 endif
 
 #######################################################################
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 44f18eb739..aa204ebbb0 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -114,6 +114,10 @@ QemuOpts *qemu_get_machine_opts(void);
 
 bool defaults_enabled(void);
 
+void main_loop(void);
+int qemu_init(int argc, char **argv, char **envp);
+void qemu_cleanup(void);
+
 extern QemuOptsList qemu_legacy_drive_opts;
 extern QemuOptsList qemu_common_drive_opts;
 extern QemuOptsList qemu_drive_opts;
diff --git a/main.c b/main.c
new file mode 100644
index 0000000000..279275069d
--- /dev/null
+++ b/main.c
@@ -0,0 +1,29 @@
+#include "qemu/osdep.h"
+#include "sysemu/sysemu.h"
+
+#ifdef CONFIG_SDL
+#if defined(__APPLE__) || defined(main)
+#include <SDL.h>
+int qemu_main(int argc, char **argv, char **envp);
+int main(int argc, char **argv)
+{
+    return qemu_main(argc, argv, NULL);
+}
+#undef main
+#define main qemu_main
+#endif
+#endif /* CONFIG_SDL */
+
+int main(int argc, char **argv, char **envp)
+{
+    int ret = qemu_init(argc, argv, envp);
+    if (ret != 0) {
+        return ret;
+    }
+
+    main_loop();
+
+    qemu_cleanup();
+
+    return 0;
+}
diff --git a/vl.c b/vl.c
index 630f5c5e9c..327510c81f 100644
--- a/vl.c
+++ b/vl.c
@@ -36,18 +36,6 @@
 #include "sysemu/seccomp.h"
 #include "sysemu/tcg.h"
 
-#ifdef CONFIG_SDL
-#if defined(__APPLE__) || defined(main)
-#include <SDL.h>
-int qemu_main(int argc, char **argv, char **envp);
-int main(int argc, char **argv)
-{
-    return qemu_main(argc, argv, NULL);
-}
-#undef main
-#define main qemu_main
-#endif
-#endif /* CONFIG_SDL */
 
 #ifdef CONFIG_COCOA
 #undef main
@@ -1794,7 +1782,7 @@ static bool main_loop_should_exit(void)
     return false;
 }
 
-static void main_loop(void)
+void main_loop(void)
 {
 #ifdef CONFIG_PROFILER
     int64_t ti;
@@ -2869,7 +2857,7 @@ static void user_register_global_props(void)
                       global_init_func, NULL, NULL);
 }
 
-int main(int argc, char **argv, char **envp)
+int qemu_init(int argc, char **argv, char **envp)
 {
     int i;
     int snapshot, linux_boot;
@@ -4468,7 +4456,7 @@ int main(int argc, char **argv, char **envp)
     if (vmstate_dump_file) {
         /* dump and exit */
         dump_vmstate_json_to_file(vmstate_dump_file);
-        return 0;
+        exit(0);
     }
 
     if (incoming) {
@@ -4485,8 +4473,11 @@ int main(int argc, char **argv, char **envp)
     accel_setup_post(current_machine);
     os_setup_post();
 
-    main_loop();
+    return 0;
+}
 
+void qemu_cleanup()
+{
     gdbserver_cleanup();
 
     /*
@@ -4522,6 +4513,4 @@ int main(int argc, char **argv, char **envp)
     qemu_chr_cleanup();
     user_creatable_cleanup();
     /* TODO: unref root container, check all devices are ok */
-
-    return 0;
 }
-- 
2.23.0

Re: [Qemu-devel] [PATCH v3 01/22] softmmu: split off vl.c:main() into main.c

[Stefan Hajnoczi]

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

On Wed, Sep 18, 2019 at 11:19:28PM +0000, Oleinik, Alexander wrote:
> diff --git a/vl.c b/vl.c
> index 630f5c5e9c..327510c81f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -36,18 +36,6 @@
>  #include "sysemu/seccomp.h"
>  #include "sysemu/tcg.h"
>  
> -#ifdef CONFIG_SDL
> -#if defined(__APPLE__) || defined(main)
> -#include <SDL.h>
> -int qemu_main(int argc, char **argv, char **envp);
> -int main(int argc, char **argv)
> -{
> -    return qemu_main(argc, argv, NULL);
> -}
> -#undef main
> -#define main qemu_main
> -#endif
> -#endif /* CONFIG_SDL */
>  
>  #ifdef CONFIG_COCOA
>  #undef main

This looks suspicious.  Should the #ifdef CONFIG_COCOA be moved into
main.c?

> @@ -1794,7 +1782,7 @@ static bool main_loop_should_exit(void)
>      return false;
>  }
>  
> -static void main_loop(void)
> +void main_loop(void)
>  {
>  #ifdef CONFIG_PROFILER
>      int64_t ti;
> @@ -2869,7 +2857,7 @@ static void user_register_global_props(void)
>                        global_init_func, NULL, NULL);
>  }
>  
> -int main(int argc, char **argv, char **envp)
> +int qemu_init(int argc, char **argv, char **envp)

The return value is confusing.  0 = success, >0 error exit code, but the
function may also invoke exit(0) to terminate successfully.

It's simpler to make this function void and invoke exit(3) directly.

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

Re: [Qemu-devel] [PATCH v3 01/22] softmmu: split off vl.c:main() into main.c

[Oleinik, Alexander]

On Thu, 2019-09-19 at 11:03 +0100, Stefan Hajnoczi wrote:
> On Wed, Sep 18, 2019 at 11:19:28PM +0000, Oleinik, Alexander wrote:
> >  #ifdef CONFIG_COCOA
> >  #undef main
> 
> This looks suspicious.  Should the #ifdef CONFIG_COCOA be moved into
> main.c?

> The return value is confusing.  0 = success, >0 error exit code, but
> the
> function may also invoke exit(0) to terminate successfully.
> 
> It's simpler to make this function void and invoke exit(3) directly.

I'll make these changes. Thanks

[Qemu-devel] [PATCH v3 02/22] libqos: Rename i2c_send and i2c_recv

[Oleinik, Alexander]

The names i2c_send and i2c_recv collide with functions defined in
hw/i2c/core.c. This causes an error when linking against libqos and
softmmu simultaneously (for example when using qtest inproc). Rename the
libqos functions to avoid this.

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 tests/libqos/i2c-imx.c  |  8 ++++----
 tests/libqos/i2c-omap.c |  8 ++++----
 tests/libqos/i2c.c      | 10 +++++-----
 tests/libqos/i2c.h      |  4 ++--
 tests/pca9552-test.c    | 10 +++++-----
 5 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/tests/libqos/i2c-imx.c b/tests/libqos/i2c-imx.c
index f33ece55a3..c1dcc9fb1d 100644
--- a/tests/libqos/i2c-imx.c
+++ b/tests/libqos/i2c-imx.c
@@ -37,7 +37,7 @@ static void imx_i2c_set_slave_addr(IMXI2C *s, uint8_t addr,
                  (addr << 1) | (direction == IMX_I2C_READ ? 1 : 0));
 }
 
-static void imx_i2c_send(I2CAdapter *i2c, uint8_t addr,
+static void imx_i2c_test_send(I2CAdapter *i2c, uint8_t addr,
                          const uint8_t *buf, uint16_t len)
 {
     IMXI2C *s = container_of(i2c, IMXI2C, parent);
@@ -97,7 +97,7 @@ static void imx_i2c_send(I2CAdapter *i2c, uint8_t addr,
     g_assert((status & I2SR_IBB) == 0);
 }
 
-static void imx_i2c_recv(I2CAdapter *i2c, uint8_t addr,
+static void imx_i2c_test_recv(I2CAdapter *i2c, uint8_t addr,
                          uint8_t *buf, uint16_t len)
 {
     IMXI2C *s = container_of(i2c, IMXI2C, parent);
@@ -202,8 +202,8 @@ void imx_i2c_init(IMXI2C *s, QTestState *qts, uint64_t addr)
 
     s->obj.get_driver = imx_i2c_get_driver;
 
-    s->parent.send = imx_i2c_send;
-    s->parent.recv = imx_i2c_recv;
+    s->parent.send = imx_i2c_test_send;
+    s->parent.recv = imx_i2c_test_recv;
     s->parent.qts = qts;
 }
 
diff --git a/tests/libqos/i2c-omap.c b/tests/libqos/i2c-omap.c
index 9ae8214fa8..284f765a3b 100644
--- a/tests/libqos/i2c-omap.c
+++ b/tests/libqos/i2c-omap.c
@@ -50,7 +50,7 @@ static void omap_i2c_set_slave_addr(OMAPI2C *s, uint8_t addr)
     g_assert_cmphex(data, ==, addr);
 }
 
-static void omap_i2c_send(I2CAdapter *i2c, uint8_t addr,
+static void omap_i2c_test_send(I2CAdapter *i2c, uint8_t addr,
                           const uint8_t *buf, uint16_t len)
 {
     OMAPI2C *s = container_of(i2c, OMAPI2C, parent);
@@ -94,7 +94,7 @@ static void omap_i2c_send(I2CAdapter *i2c, uint8_t addr,
     g_assert((data & OMAP_I2C_CON_STP) == 0);
 }
 
-static void omap_i2c_recv(I2CAdapter *i2c, uint8_t addr,
+static void omap_i2c_test_recv(I2CAdapter *i2c, uint8_t addr,
                           uint8_t *buf, uint16_t len)
 {
     OMAPI2C *s = container_of(i2c, OMAPI2C, parent);
@@ -182,8 +182,8 @@ void omap_i2c_init(OMAPI2C *s, QTestState *qts, uint64_t addr)
     s->obj.get_driver = omap_i2c_get_driver;
     s->obj.start_hw = omap_i2c_start_hw;
 
-    s->parent.send = omap_i2c_send;
-    s->parent.recv = omap_i2c_recv;
+    s->parent.send = omap_i2c_test_send;
+    s->parent.recv = omap_i2c_test_recv;
     s->parent.qts = qts;
 }
 
diff --git a/tests/libqos/i2c.c b/tests/libqos/i2c.c
index 156114e745..b96a37b69b 100644
--- a/tests/libqos/i2c.c
+++ b/tests/libqos/i2c.c
@@ -10,12 +10,12 @@
 #include "libqos/i2c.h"
 #include "libqtest.h"
 
-void i2c_send(QI2CDevice *i2cdev, const uint8_t *buf, uint16_t len)
+void i2c_test_send(QI2CDevice *i2cdev, const uint8_t *buf, uint16_t len)
 {
     i2cdev->bus->send(i2cdev->bus, i2cdev->addr, buf, len);
 }
 
-void i2c_recv(QI2CDevice *i2cdev, uint8_t *buf, uint16_t len)
+void i2c_test_recv(QI2CDevice *i2cdev, uint8_t *buf, uint16_t len)
 {
     i2cdev->bus->recv(i2cdev->bus, i2cdev->addr, buf, len);
 }
@@ -23,8 +23,8 @@ void i2c_recv(QI2CDevice *i2cdev, uint8_t *buf, uint16_t len)
 void i2c_read_block(QI2CDevice *i2cdev, uint8_t reg,
                     uint8_t *buf, uint16_t len)
 {
-    i2c_send(i2cdev, &reg, 1);
-    i2c_recv(i2cdev, buf, len);
+    i2c_test_send(i2cdev, &reg, 1);
+    i2c_test_recv(i2cdev, buf, len);
 }
 
 void i2c_write_block(QI2CDevice *i2cdev, uint8_t reg,
@@ -33,7 +33,7 @@ void i2c_write_block(QI2CDevice *i2cdev, uint8_t reg,
     uint8_t *cmd = g_malloc(len + 1);
     cmd[0] = reg;
     memcpy(&cmd[1], buf, len);
-    i2c_send(i2cdev, cmd, len + 1);
+    i2c_test_send(i2cdev, cmd, len + 1);
     g_free(cmd);
 }
 
diff --git a/tests/libqos/i2c.h b/tests/libqos/i2c.h
index 945b65b34c..9a4d6579a2 100644
--- a/tests/libqos/i2c.h
+++ b/tests/libqos/i2c.h
@@ -47,8 +47,8 @@ struct QI2CDevice {
 void *i2c_device_create(void *i2c_bus, QGuestAllocator *alloc, void *addr);
 void add_qi2c_address(QOSGraphEdgeOptions *opts, QI2CAddress *addr);
 
-void i2c_send(QI2CDevice *dev, const uint8_t *buf, uint16_t len);
-void i2c_recv(QI2CDevice *dev, uint8_t *buf, uint16_t len);
+void i2c_test_send(QI2CDevice *dev, const uint8_t *buf, uint16_t len);
+void i2c_test_recv(QI2CDevice *dev, uint8_t *buf, uint16_t len);
 
 void i2c_read_block(QI2CDevice *dev, uint8_t reg,
                     uint8_t *buf, uint16_t len);
diff --git a/tests/pca9552-test.c b/tests/pca9552-test.c
index 4b800d3c3e..9844177d79 100644
--- a/tests/pca9552-test.c
+++ b/tests/pca9552-test.c
@@ -32,22 +32,22 @@ static void receive_autoinc(void *obj, void *data, QGuestAllocator *alloc)
 
     pca9552_init(i2cdev);
 
-    i2c_send(i2cdev, &reg, 1);
+    i2c_test_send(i2cdev, &reg, 1);
 
     /* PCA9552_LS0 */
-    i2c_recv(i2cdev, &resp, 1);
+    i2c_test_recv(i2cdev, &resp, 1);
     g_assert_cmphex(resp, ==, 0x54);
 
     /* PCA9552_LS1 */
-    i2c_recv(i2cdev, &resp, 1);
+    i2c_test_recv(i2cdev, &resp, 1);
     g_assert_cmphex(resp, ==, 0x55);
 
     /* PCA9552_LS2 */
-    i2c_recv(i2cdev, &resp, 1);
+    i2c_test_recv(i2cdev, &resp, 1);
     g_assert_cmphex(resp, ==, 0x55);
 
     /* PCA9552_LS3 */
-    i2c_recv(i2cdev, &resp, 1);
+    i2c_test_recv(i2cdev, &resp, 1);
     g_assert_cmphex(resp, ==, 0x54);
 }
 
-- 
2.23.0

Re: [Qemu-devel] [PATCH v3 02/22] libqos: Rename i2c_send and i2c_recv

[Thomas Huth]

On 19/09/2019 01.19, Oleinik, Alexander wrote:
> The names i2c_send and i2c_recv collide with functions defined in
> hw/i2c/core.c. This causes an error when linking against libqos and
> softmmu simultaneously (for example when using qtest inproc). Rename the
> libqos functions to avoid this.
> 
> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> ---
>  tests/libqos/i2c-imx.c  |  8 ++++----
>  tests/libqos/i2c-omap.c |  8 ++++----
>  tests/libqos/i2c.c      | 10 +++++-----
>  tests/libqos/i2c.h      |  4 ++--
>  tests/pca9552-test.c    | 10 +++++-----
>  5 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/tests/libqos/i2c-imx.c b/tests/libqos/i2c-imx.c
> index f33ece55a3..c1dcc9fb1d 100644
> --- a/tests/libqos/i2c-imx.c
> +++ b/tests/libqos/i2c-imx.c
> @@ -37,7 +37,7 @@ static void imx_i2c_set_slave_addr(IMXI2C *s, uint8_t addr,
>                   (addr << 1) | (direction == IMX_I2C_READ ? 1 : 0));
>  }
>  
> -static void imx_i2c_send(I2CAdapter *i2c, uint8_t addr,
> +static void imx_i2c_test_send(I2CAdapter *i2c, uint8_t addr,
>                           const uint8_t *buf, uint16_t len)
>  {
>      IMXI2C *s = container_of(i2c, IMXI2C, parent);
> @@ -97,7 +97,7 @@ static void imx_i2c_send(I2CAdapter *i2c, uint8_t addr,
>      g_assert((status & I2SR_IBB) == 0);
>  }
>  
> -static void imx_i2c_recv(I2CAdapter *i2c, uint8_t addr,
> +static void imx_i2c_test_recv(I2CAdapter *i2c, uint8_t addr,
>                           uint8_t *buf, uint16_t len)
>  {
>      IMXI2C *s = container_of(i2c, IMXI2C, parent);
> @@ -202,8 +202,8 @@ void imx_i2c_init(IMXI2C *s, QTestState *qts, uint64_t addr)
>  
>      s->obj.get_driver = imx_i2c_get_driver;
>  
> -    s->parent.send = imx_i2c_send;
> -    s->parent.recv = imx_i2c_recv;
> +    s->parent.send = imx_i2c_test_send;
> +    s->parent.recv = imx_i2c_test_recv;
>      s->parent.qts = qts;
>  }
>  
> diff --git a/tests/libqos/i2c-omap.c b/tests/libqos/i2c-omap.c
> index 9ae8214fa8..284f765a3b 100644
> --- a/tests/libqos/i2c-omap.c
> +++ b/tests/libqos/i2c-omap.c
> @@ -50,7 +50,7 @@ static void omap_i2c_set_slave_addr(OMAPI2C *s, uint8_t addr)
>      g_assert_cmphex(data, ==, addr);
>  }
>  
> -static void omap_i2c_send(I2CAdapter *i2c, uint8_t addr,
> +static void omap_i2c_test_send(I2CAdapter *i2c, uint8_t addr,
>                            const uint8_t *buf, uint16_t len)
>  {
>      OMAPI2C *s = container_of(i2c, OMAPI2C, parent);
> @@ -94,7 +94,7 @@ static void omap_i2c_send(I2CAdapter *i2c, uint8_t addr,
>      g_assert((data & OMAP_I2C_CON_STP) == 0);
>  }
>  
> -static void omap_i2c_recv(I2CAdapter *i2c, uint8_t addr,
> +static void omap_i2c_test_recv(I2CAdapter *i2c, uint8_t addr,
>                            uint8_t *buf, uint16_t len)
>  {
>      OMAPI2C *s = container_of(i2c, OMAPI2C, parent);
> @@ -182,8 +182,8 @@ void omap_i2c_init(OMAPI2C *s, QTestState *qts, uint64_t addr)
>      s->obj.get_driver = omap_i2c_get_driver;
>      s->obj.start_hw = omap_i2c_start_hw;
>  
> -    s->parent.send = omap_i2c_send;
> -    s->parent.recv = omap_i2c_recv;
> +    s->parent.send = omap_i2c_test_send;
> +    s->parent.recv = omap_i2c_test_recv;
>      s->parent.qts = qts;
>  }
>  
> diff --git a/tests/libqos/i2c.c b/tests/libqos/i2c.c
> index 156114e745..b96a37b69b 100644
> --- a/tests/libqos/i2c.c
> +++ b/tests/libqos/i2c.c
> @@ -10,12 +10,12 @@
>  #include "libqos/i2c.h"
>  #include "libqtest.h"
>  
> -void i2c_send(QI2CDevice *i2cdev, const uint8_t *buf, uint16_t len)
> +void i2c_test_send(QI2CDevice *i2cdev, const uint8_t *buf, uint16_t len)
>  {
>      i2cdev->bus->send(i2cdev->bus, i2cdev->addr, buf, len);
>  }
>  
> -void i2c_recv(QI2CDevice *i2cdev, uint8_t *buf, uint16_t len)
> +void i2c_test_recv(QI2CDevice *i2cdev, uint8_t *buf, uint16_t len)
>  {
>      i2cdev->bus->recv(i2cdev->bus, i2cdev->addr, buf, len);
>  }
> @@ -23,8 +23,8 @@ void i2c_recv(QI2CDevice *i2cdev, uint8_t *buf, uint16_t len)
>  void i2c_read_block(QI2CDevice *i2cdev, uint8_t reg,
>                      uint8_t *buf, uint16_t len)
>  {
> -    i2c_send(i2cdev, &reg, 1);
> -    i2c_recv(i2cdev, buf, len);
> +    i2c_test_send(i2cdev, &reg, 1);
> +    i2c_test_recv(i2cdev, buf, len);
>  }
>  
>  void i2c_write_block(QI2CDevice *i2cdev, uint8_t reg,
> @@ -33,7 +33,7 @@ void i2c_write_block(QI2CDevice *i2cdev, uint8_t reg,
>      uint8_t *cmd = g_malloc(len + 1);
>      cmd[0] = reg;
>      memcpy(&cmd[1], buf, len);
> -    i2c_send(i2cdev, cmd, len + 1);
> +    i2c_test_send(i2cdev, cmd, len + 1);
>      g_free(cmd);
>  }
>  
> diff --git a/tests/libqos/i2c.h b/tests/libqos/i2c.h
> index 945b65b34c..9a4d6579a2 100644
> --- a/tests/libqos/i2c.h
> +++ b/tests/libqos/i2c.h
> @@ -47,8 +47,8 @@ struct QI2CDevice {
>  void *i2c_device_create(void *i2c_bus, QGuestAllocator *alloc, void *addr);
>  void add_qi2c_address(QOSGraphEdgeOptions *opts, QI2CAddress *addr);
>  
> -void i2c_send(QI2CDevice *dev, const uint8_t *buf, uint16_t len);
> -void i2c_recv(QI2CDevice *dev, uint8_t *buf, uint16_t len);
> +void i2c_test_send(QI2CDevice *dev, const uint8_t *buf, uint16_t len);
> +void i2c_test_recv(QI2CDevice *dev, uint8_t *buf, uint16_t len);
>  
>  void i2c_read_block(QI2CDevice *dev, uint8_t reg,
>                      uint8_t *buf, uint16_t len);
> diff --git a/tests/pca9552-test.c b/tests/pca9552-test.c
> index 4b800d3c3e..9844177d79 100644
> --- a/tests/pca9552-test.c
> +++ b/tests/pca9552-test.c
> @@ -32,22 +32,22 @@ static void receive_autoinc(void *obj, void *data, QGuestAllocator *alloc)
>  
>      pca9552_init(i2cdev);
>  
> -    i2c_send(i2cdev, &reg, 1);
> +    i2c_test_send(i2cdev, &reg, 1);
>  
>      /* PCA9552_LS0 */
> -    i2c_recv(i2cdev, &resp, 1);
> +    i2c_test_recv(i2cdev, &resp, 1);
>      g_assert_cmphex(resp, ==, 0x54);
>  
>      /* PCA9552_LS1 */
> -    i2c_recv(i2cdev, &resp, 1);
> +    i2c_test_recv(i2cdev, &resp, 1);
>      g_assert_cmphex(resp, ==, 0x55);
>  
>      /* PCA9552_LS2 */
> -    i2c_recv(i2cdev, &resp, 1);
> +    i2c_test_recv(i2cdev, &resp, 1);
>      g_assert_cmphex(resp, ==, 0x55);
>  
>      /* PCA9552_LS3 */
> -    i2c_recv(i2cdev, &resp, 1);
> +    i2c_test_recv(i2cdev, &resp, 1);
>      g_assert_cmphex(resp, ==, 0x54);
>  }

Acked-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH v3 02/22] libqos: Rename i2c_send and i2c_recv

[Stefan Hajnoczi]

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

On Wed, Sep 18, 2019 at 11:19:29PM +0000, Oleinik, Alexander wrote:
> The names i2c_send and i2c_recv collide with functions defined in
> hw/i2c/core.c. This causes an error when linking against libqos and
> softmmu simultaneously (for example when using qtest inproc). Rename the
> libqos functions to avoid this.
> 
> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> ---
>  tests/libqos/i2c-imx.c  |  8 ++++----
>  tests/libqos/i2c-omap.c |  8 ++++----
>  tests/libqos/i2c.c      | 10 +++++-----
>  tests/libqos/i2c.h      |  4 ++--
>  tests/pca9552-test.c    | 10 +++++-----
>  5 files changed, 20 insertions(+), 20 deletions(-)

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

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

Re: [Qemu-devel] [PATCH v3 02/22] libqos: Rename i2c_send and i2c_recv

[Paolo Bonzini]

On 19/09/19 01:19, Oleinik, Alexander wrote:
> The names i2c_send and i2c_recv collide with functions defined in
> hw/i2c/core.c. This causes an error when linking against libqos and
> softmmu simultaneously (for example when using qtest inproc). Rename the
> libqos functions to avoid this.

I'd prefer "qi2c_{send,recv}" since that is consistent with "qvirtio_"
functions and with the name of the "QI2CDevice" struct.

Paolo

Re: [Qemu-devel] [PATCH v3 02/22] libqos: Rename i2c_send and i2c_recv

[Oleinik, Alexander]

On Thu, 2019-09-19 at 13:15 +0200, Paolo Bonzini wrote:
> On 19/09/19 01:19, Oleinik, Alexander wrote:
> > The names i2c_send and i2c_recv collide with functions defined in
> > hw/i2c/core.c. This causes an error when linking against libqos and
> > softmmu simultaneously (for example when using qtest inproc).
> > Rename the
> > libqos functions to avoid this.
> 
> I'd prefer "qi2c_{send,recv}" since that is consistent with
> "qvirtio_"
> functions and with the name of the "QI2CDevice" struct.

Ok - i'll change this.

[Qemu-devel] [PATCH v3 03/22] fuzz: Add FUZZ_TARGET module type

[Oleinik, Alexander]

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 include/qemu/module.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/qemu/module.h b/include/qemu/module.h
index 65ba596e46..684753d808 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -46,6 +46,7 @@ typedef enum {
     MODULE_INIT_TRACE,
     MODULE_INIT_XEN_BACKEND,
     MODULE_INIT_LIBQOS,
+    MODULE_INIT_FUZZ_TARGET,
     MODULE_INIT_MAX
 } module_init_type;
 
@@ -56,7 +57,8 @@ typedef enum {
 #define xen_backend_init(function) module_init(function, \
                                                MODULE_INIT_XEN_BACKEND)
 #define libqos_init(function) module_init(function, MODULE_INIT_LIBQOS)
-
+#define fuzz_target_init(function) module_init(function, \
+                                               MODULE_INIT_FUZZ_TARGET)
 #define block_module_load_one(lib) module_load_one("block-", lib)
 #define ui_module_load_one(lib) module_load_one("ui-", lib)
 #define audio_module_load_one(lib) module_load_one("audio-", lib)
-- 
2.23.0

Re: [Qemu-devel] [PATCH v3 03/22] fuzz: Add FUZZ_TARGET module type

[Stefan Hajnoczi]

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

On Wed, Sep 18, 2019 at 11:19:30PM +0000, Oleinik, Alexander wrote:
> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> ---
>  include/qemu/module.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

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

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

[Qemu-devel] [PATCH v3 04/22] qtest: add qtest_server_send abstraction

[Oleinik, Alexander]

qtest_server_send is a function pointer specifying the handler used to
transmit data to the qtest client. In the standard configuration, this
calls the CharBackend handler, but now it is possible for other types of
handlers, e.g direct-function calls if the qtest client and server
exist within the same process (inproc)

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 include/sysemu/qtest.h |  3 +++
 qtest.c                | 17 +++++++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h
index 5ed09c80b1..fda7000d2c 100644
--- a/include/sysemu/qtest.h
+++ b/include/sysemu/qtest.h
@@ -26,4 +26,7 @@ bool qtest_driver(void);
 
 void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **errp);
 
+void qtest_server_set_tx_handler(void (*send)(void *, const char *, size_t),
+                                 void *opaque);
+
 #endif
diff --git a/qtest.c b/qtest.c
index 8b50e2783e..ae7e6d779d 100644
--- a/qtest.c
+++ b/qtest.c
@@ -42,6 +42,8 @@ static GString *inbuf;
 static int irq_levels[MAX_IRQ];
 static qemu_timeval start_time;
 static bool qtest_opened;
+static void (*qtest_server_send)(void*, const char*, size_t);
+static void *qtest_server_send_opaque;
 
 #define FMT_timeval "%ld.%06ld"
 
@@ -228,8 +230,9 @@ static void GCC_FMT_ATTR(1, 2) qtest_log_send(const char *fmt, ...)
     va_end(ap);
 }
 
-static void do_qtest_send(CharBackend *chr, const char *str, size_t len)
+static void qtest_server_char_be_send(void *opaque, const char *str, size_t len)
 {
+    CharBackend* chr = (CharBackend *)opaque;
     qemu_chr_fe_write_all(chr, (uint8_t *)str, len);
     if (qtest_log_fp && qtest_opened) {
         fprintf(qtest_log_fp, "%s", str);
@@ -238,7 +241,7 @@ static void do_qtest_send(CharBackend *chr, const char *str, size_t len)
 
 static void qtest_send(CharBackend *chr, const char *str)
 {
-    do_qtest_send(chr, str, strlen(str));
+    qtest_server_send(qtest_server_send_opaque, str, strlen(str));
 }
 
 static void GCC_FMT_ATTR(2, 3) qtest_sendf(CharBackend *chr,
@@ -783,6 +786,16 @@ void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **
     qemu_chr_fe_set_echo(&qtest_chr, true);
 
     inbuf = g_string_new("");
+
+    if (!qtest_server_send) {
+        qtest_server_set_tx_handler(qtest_server_char_be_send, &qtest_chr);
+    }
+}
+
+void qtest_server_set_tx_handler(void (*send)(void*, const char*, size_t), void *opaque)
+{
+    qtest_server_send = send;
+    qtest_server_send_opaque = opaque;
 }
 
 bool qtest_driver(void)
-- 
2.23.0

Re: [Qemu-devel] [PATCH v3 04/22] qtest: add qtest_server_send abstraction

[Stefan Hajnoczi]

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

On Wed, Sep 18, 2019 at 11:19:31PM +0000, Oleinik, Alexander wrote:
> qtest_server_send is a function pointer specifying the handler used to
> transmit data to the qtest client. In the standard configuration, this
> calls the CharBackend handler, but now it is possible for other types of
> handlers, e.g direct-function calls if the qtest client and server
> exist within the same process (inproc)
> 
> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> ---
>  include/sysemu/qtest.h |  3 +++
>  qtest.c                | 17 +++++++++++++++--
>  2 files changed, 18 insertions(+), 2 deletions(-)

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

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

[Qemu-devel] [PATCH v3 05/22] libqtest: Add a layer of abstraciton to send/recv

[Oleinik, Alexander]

This makes it simple to swap the transport functions for qtest commands
to and from the qtest client. For example, now it is possible to
directly pass qtest commands to a server handler that exists within the
same process, without the standard way of writing to a file descriptor.

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 tests/libqtest.c | 64 +++++++++++++++++++++++++++++++++++++++---------
 tests/libqtest.h |  1 -
 2 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 4a7556462d..19feea9e17 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -35,6 +35,15 @@
 #define SOCKET_TIMEOUT 50
 #define SOCKET_MAX_FDS 16
 
+
+typedef struct QTestClientTransportOps {
+    void     (*send)(void* , const char*, size_t);
+    void     *send_opaque;
+
+    GString* (*recv_line)(void *);
+    void     *recv_line_opaque;
+} QTestTransportOps;
+
 struct QTestState
 {
     int fd;
@@ -45,13 +54,28 @@ struct QTestState
     bool big_endian;
     bool irq_level[MAX_IRQ];
     GString *rx;
+    QTestTransportOps ops;
 };
 
 static GHookList abrt_hooks;
 static struct sigaction sigact_old;
+static GString *recv_str;
 
 static int qtest_query_target_endianness(QTestState *s);
 
+static void qtest_client_socket_send(void *opaque,
+        const char *buf, size_t size);
+static void socket_send(int fd, const char *buf, size_t size);
+
+static GString *qtest_client_socket_recv_line(void *);
+static GString *qtest_client_inproc_recv_line(void *);
+
+static void qtest_client_set_tx_handler(QTestState *s,
+        void (*send)(void *, const char *, size_t), void *opaque);
+static void qtest_client_set_rx_handler(QTestState *s,
+        GString * (*recv)(void *), void *opaque);
+
+
 static int init_socket(const char *socket_path)
 {
     struct sockaddr_un addr;
@@ -234,6 +258,9 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
     sock = init_socket(socket_path);
     qmpsock = init_socket(qmp_socket_path);
 
+    qtest_client_set_rx_handler(s, qtest_client_socket_recv_line, s);
+    qtest_client_set_tx_handler(s, qtest_client_socket_send, &(s->fd));
+
     qtest_add_abrt_handler(kill_qemu_hook_func, s);
 
     command = g_strdup_printf("exec %s "
@@ -360,8 +387,9 @@ void qtest_quit(QTestState *s)
     g_free(s);
 }
 
-static void socket_send(int fd, const char *buf, size_t size)
+static void qtest_client_socket_send(void *opaque, const char *buf, size_t size)
 {
+    int fd = *((int *)opaque);
     size_t offset;
 
     offset = 0;
@@ -379,22 +407,22 @@ static void socket_send(int fd, const char *buf, size_t size)
     }
 }
 
-static void socket_sendf(int fd, const char *fmt, va_list ap)
+static void socket_send(int fd, const char *buf, size_t size)
 {
-    gchar *str = g_strdup_vprintf(fmt, ap);
-    size_t size = strlen(str);
-
-    socket_send(fd, str, size);
-    g_free(str);
+    qtest_client_socket_send(&fd, buf, size);
 }
 
 static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...)
 {
     va_list ap;
-
     va_start(ap, fmt);
-    socket_sendf(s->fd, fmt, ap);
+    gchar *str = g_strdup_vprintf(fmt, ap);
     va_end(ap);
+
+    size_t size = strlen(str);
+
+    s->ops.send(s, str, size);
+    g_free(str);
 }
 
 /* Sends a message and file descriptors to the socket.
@@ -431,8 +459,9 @@ static void socket_send_fds(int socket_fd, int *fds, size_t fds_num,
     g_assert_cmpint(ret, >, 0);
 }
 
-static GString *qtest_recv_line(QTestState *s)
+static GString *qtest_client_socket_recv_line(void* opaque)
 {
+    QTestState *s = opaque;
     GString *line;
     size_t offset;
     char *eol;
@@ -468,7 +497,7 @@ static gchar **qtest_rsp(QTestState *s, int expected_args)
     int i;
 
 redo:
-    line = qtest_recv_line(s);
+    line = s->ops.recv_line(s->ops.recv_line_opaque);
     words = g_strsplit(line->str, " ", 0);
     g_string_free(line, TRUE);
 
@@ -1327,3 +1356,16 @@ void qmp_assert_error_class(QDict *rsp, const char *class)
 
     qobject_unref(rsp);
 }
+
+static void qtest_client_set_tx_handler(QTestState *s,
+                    void (*send)(void*, const char*, size_t), void *opaque)
+{
+    s->ops.send = send;
+    s->ops.send_opaque = opaque;
+}
+static void qtest_client_set_rx_handler(QTestState *s,
+                    GString* (*recv)(void *), void *opaque)
+{
+    s->ops.recv_line = recv;
+    s->ops.recv_line_opaque = opaque;
+}
diff --git a/tests/libqtest.h b/tests/libqtest.h
index a177e502d9..40fa235a52 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -716,5 +716,4 @@ bool qtest_probe_child(QTestState *s);
  * Set expected exit status of the child.
  */
 void qtest_set_expected_status(QTestState *s, int status);
-
 #endif
-- 
2.23.0

Re: [Qemu-devel] [PATCH v3 05/22] libqtest: Add a layer of abstraciton to send/recv

[Stefan Hajnoczi]

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

On Wed, Sep 18, 2019 at 11:19:32PM +0000, Oleinik, Alexander wrote:
> This makes it simple to swap the transport functions for qtest commands
> to and from the qtest client. For example, now it is possible to
> directly pass qtest commands to a server handler that exists within the
> same process, without the standard way of writing to a file descriptor.
> 
> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> ---
>  tests/libqtest.c | 64 +++++++++++++++++++++++++++++++++++++++---------
>  tests/libqtest.h |  1 -
>  2 files changed, 53 insertions(+), 12 deletions(-)

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

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

Re: [Qemu-devel] [PATCH v3 05/22] libqtest: Add a layer of abstraciton to send/recv

[Paolo Bonzini]

On 19/09/19 01:19, Oleinik, Alexander wrote:
> +static GString *qtest_client_inproc_recv_line(void *);

This is not defined as of this patch, please move it to patch 14.

>  
> +    qtest_client_set_rx_handler(s, qtest_client_socket_recv_line, s);
> +    qtest_client_set_tx_handler(s, qtest_client_socket_send, &(s->fd));
> +

I think you can pass "s" to the tx handler as well, and remove the
send_opaque and recv_opaque fields?

Paolo

Re: [Qemu-devel] [PATCH v3 05/22] libqtest: Add a layer of abstraciton to send/recv

[Oleinik, Alexander]

On Thu, 2019-09-19 at 13:18 +0200, Paolo Bonzini wrote:
> I think you can pass "s" to the tx handler as well, and remove the
> send_opaque and recv_opaque fields?

Qtest also uses this function to communicate over qmp  (different fd).
I can probably make the tx handler a wrapper which accepts "s", and
passes s->fd to the real socket_send.

Re: [PATCH v3 05/22] libqtest: Add a layer of abstraciton to send/recv

[Paolo Bonzini]

On 19/09/19 15:27, Oleinik, Alexander wrote:
> On Thu, 2019-09-19 at 13:18 +0200, Paolo Bonzini wrote:
>> I think you can pass "s" to the tx handler as well, and remove the
>> send_opaque and recv_opaque fields?
> 
> Qtest also uses this function to communicate over qmp  (different fd).
> I can probably make the tx handler a wrapper which accepts "s", and
> passes s->fd to the real socket_send.

Yes, exactly.

Thanks,

Paolo

[Qemu-devel] [PATCH v3 06/22] fuzz: add configure flag --enable-fuzzing

[Oleinik, Alexander]

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 configure | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/configure b/configure
index 30aad233d1..775f46f55a 100755
--- a/configure
+++ b/configure
@@ -498,6 +498,7 @@ libxml2=""
 debug_mutex="no"
 libpmem=""
 default_devices="yes"
+fuzzing="no"
 
 supported_cpu="no"
 supported_os="no"
@@ -1521,6 +1522,8 @@ for opt do
   ;;
   --disable-libpmem) libpmem=no
   ;;
+  --enable-fuzzing) fuzzing=yes
+  ;;
   *)
       echo "ERROR: unknown option $opt"
       echo "Try '$0 --help' for more information"
@@ -6429,6 +6432,7 @@ echo "capstone          $capstone"
 echo "libpmem support   $libpmem"
 echo "libudev           $libudev"
 echo "default devices   $default_devices"
+echo "fuzzing support   $fuzzing"
 
 if test "$supported_cpu" = "no"; then
     echo
@@ -7258,6 +7262,10 @@ fi
 if test "$sheepdog" = "yes" ; then
   echo "CONFIG_SHEEPDOG=y" >> $config_host_mak
 fi
+if test "$fuzzing" = "yes" ; then
+  FUZZ_LDFLAGS=" -fsanitize=address,fuzzer"
+  FUZZ_CFLAGS=" -fsanitize=address,fuzzer"
+fi
 
 if test "$tcg_interpreter" = "yes"; then
   QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
@@ -7339,6 +7347,11 @@ if test "$libudev" != "no"; then
     echo "CONFIG_LIBUDEV=y" >> $config_host_mak
     echo "LIBUDEV_LIBS=$libudev_libs" >> $config_host_mak
 fi
+if test "$fuzzing" != "no"; then
+    echo "CONFIG_FUZZ=y" >> $config_host_mak
+    echo "FUZZ_CFLAGS=$FUZZ_CFLAGS" >> $config_host_mak
+    echo "FUZZ_LDFLAGS=$FUZZ_LDFLAGS" >> $config_host_mak
+fi
 
 # use included Linux headers
 if test "$linux" = "yes" ; then
-- 
2.23.0

Re: [Qemu-devel] [PATCH v3 06/22] fuzz: add configure flag --enable-fuzzing

[Stefan Hajnoczi]

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

On Wed, Sep 18, 2019 at 11:19:33PM +0000, Oleinik, Alexander wrote:
> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> ---
>  configure | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/configure b/configure
> index 30aad233d1..775f46f55a 100755
> --- a/configure
> +++ b/configure
> @@ -498,6 +498,7 @@ libxml2=""
>  debug_mutex="no"
>  libpmem=""
>  default_devices="yes"
> +fuzzing="no"
>  
>  supported_cpu="no"
>  supported_os="no"
> @@ -1521,6 +1522,8 @@ for opt do
>    ;;
>    --disable-libpmem) libpmem=no
>    ;;
> +  --enable-fuzzing) fuzzing=yes
> +  ;;

Please add --disable-fuzzing, the ./configure script says:

  # Always add --enable-foo and --disable-foo command line args.

>    *)
>        echo "ERROR: unknown option $opt"
>        echo "Try '$0 --help' for more information"
> @@ -6429,6 +6432,7 @@ echo "capstone          $capstone"
>  echo "libpmem support   $libpmem"
>  echo "libudev           $libudev"
>  echo "default devices   $default_devices"
> +echo "fuzzing support   $fuzzing"
>  
>  if test "$supported_cpu" = "no"; then
>      echo
> @@ -7258,6 +7262,10 @@ fi
>  if test "$sheepdog" = "yes" ; then
>    echo "CONFIG_SHEEPDOG=y" >> $config_host_mak
>  fi
> +if test "$fuzzing" = "yes" ; then
> +  FUZZ_LDFLAGS=" -fsanitize=address,fuzzer"
> +  FUZZ_CFLAGS=" -fsanitize=address,fuzzer"
> +fi

Is -fsanitize=fuzzer a clang-only option?  If yes, then please ensure
that there is a friendly error message when ./configure is run with gcc.
You could probe if -fsanitize=fuzzer works and then check that
fuzzing=no when this feature is unavailable.

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

Re: [Qemu-devel] [PATCH v3 06/22] fuzz: add configure flag --enable-fuzzing

[Oleinik, Alexander]

On Thu, 2019-09-19 at 11:28 +0100, Stefan Hajnoczi wrote:
> Is -fsanitize=fuzzer a clang-only option?  If yes, then please ensure
> that there is a friendly error message when ./configure is run with
> gcc.
> You could probe if -fsanitize=fuzzer works and then check that
> fuzzing=no when this feature is unavailable.
Yes, fsantizier=fuzzer is a clang-only feature. I'll add a probe for
it.

[Qemu-devel] [PATCH v3 07/22] fuzz: Add target/fuzz makefile rules

[Oleinik, Alexander]

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
Note that with this you cannot configure with --enable-fuzzing to build
/all targets. Even if you could, you would need to clean all of the *.o
between builds, since fuzzing adds instrumentation CFLAGS.


 Makefile                    | 15 ++++++++++++++-
 Makefile.target             | 16 ++++++++++++++++
 tests/fuzz/Makefile.include |  4 ++++
 3 files changed, 34 insertions(+), 1 deletion(-)
 create mode 100644 tests/fuzz/Makefile.include

diff --git a/Makefile b/Makefile
index f628783571..3a43492340 100644
--- a/Makefile
+++ b/Makefile
@@ -453,7 +453,7 @@ config-host.h-timestamp: config-host.mak
 qemu-options.def: $(SRC_PATH)/qemu-options.hx $(SRC_PATH)/scripts/hxtool
 	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"GEN","$@")
 
-TARGET_DIRS_RULES := $(foreach t, all clean install, $(addsuffix /$(t), $(TARGET_DIRS)))
+TARGET_DIRS_RULES := $(foreach t, all fuzz clean install, $(addsuffix /$(t), $(TARGET_DIRS)))
 
 SOFTMMU_ALL_RULES=$(filter %-softmmu/all, $(TARGET_DIRS_RULES))
 $(SOFTMMU_ALL_RULES): $(authz-obj-y)
@@ -465,6 +465,15 @@ $(SOFTMMU_ALL_RULES): config-all-devices.mak
 $(SOFTMMU_ALL_RULES): $(edk2-decompressed)
 $(SOFTMMU_ALL_RULES): $(softmmu-main-y)
 
+SOFTMMU_FUZZ_RULES=$(filter %-softmmu/fuzz, $(TARGET_DIRS_RULES))
+$(SOFTMMU_FUZZ_RULES): $(authz-obj-y)
+$(SOFTMMU_FUZZ_RULES): $(block-obj-y)
+$(SOFTMMU_FUZZ_RULES): $(chardev-obj-y)
+$(SOFTMMU_FUZZ_RULES): $(crypto-obj-y)
+$(SOFTMMU_FUZZ_RULES): $(io-obj-y)
+$(SOFTMMU_FUZZ_RULES): config-all-devices.mak
+$(SOFTMMU_FUZZ_RULES): $(edk2-decompressed)
+
 .PHONY: $(TARGET_DIRS_RULES)
 # The $(TARGET_DIRS_RULES) are of the form SUBDIR/GOAL, so that
 # $(dir $@) yields the sub-directory, and $(notdir $@) yields the sub-goal
@@ -511,6 +520,9 @@ subdir-slirp: slirp/all
 $(filter %/all, $(TARGET_DIRS_RULES)): libqemuutil.a $(common-obj-y) \
 	$(qom-obj-y) $(crypto-user-obj-$(CONFIG_USER_ONLY))
 
+$(filter %/fuzz, $(TARGET_DIRS_RULES)): libqemuutil.a $(common-obj-y) \
+	$(qom-obj-y) $(crypto-user-obj-$(CONFIG_USER_ONLY))
+
 ROM_DIRS = $(addprefix pc-bios/, $(ROMS))
 ROM_DIRS_RULES=$(foreach t, all clean, $(addsuffix /$(t), $(ROM_DIRS)))
 # Only keep -O and -g cflags
@@ -520,6 +532,7 @@ $(ROM_DIRS_RULES):
 
 .PHONY: recurse-all recurse-clean recurse-install
 recurse-all: $(addsuffix /all, $(TARGET_DIRS) $(ROM_DIRS))
+recurse-fuzz: $(addsuffix /fuzz, $(TARGET_DIRS) $(ROM_DIRS))
 recurse-clean: $(addsuffix /clean, $(TARGET_DIRS) $(ROM_DIRS))
 recurse-install: $(addsuffix /install, $(TARGET_DIRS))
 $(addsuffix /install, $(TARGET_DIRS)): all
diff --git a/Makefile.target b/Makefile.target
index 5e916230c4..6065989964 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -227,6 +227,22 @@ ifdef CONFIG_TRACE_SYSTEMTAP
 	rm -f *.stp
 endif
 
+ifdef CONFIG_FUZZ
+include $(SRC_PATH)/tests/fuzz/Makefile.include
+include $(SRC_PATH)/tests/Makefile.include
+
+fuzz: fuzz-vars
+fuzz-vars: QEMU_CFLAGS := $(FUZZ_CFLAGS) $(QEMU_CFLAGS)
+fuzz-vars: QEMU_LDFLAGS := $(FUZZ_LDFLAGS) $(QEMU_LDFLAGS)
+fuzz-vars: $(QEMU_PROG_FUZZ)
+dummy := $(call unnest-vars,, fuzz-obj-y)
+
+
+$(QEMU_PROG_FUZZ): config-devices.mak $(all-obj-y) $(COMMON_LDADDS) $(fuzz-obj-y)
+	$(call LINK, $(filter-out %.mak, $^))
+
+endif
+
 install: all
 ifneq ($(PROGS),)
 	$(call install-prog,$(PROGS),$(DESTDIR)$(bindir))
diff --git a/tests/fuzz/Makefile.include b/tests/fuzz/Makefile.include
new file mode 100644
index 0000000000..324e6c1433
--- /dev/null
+++ b/tests/fuzz/Makefile.include
@@ -0,0 +1,4 @@
+# QEMU_PROG_FUZZ=qemu-fuzz-$(TARGET_NAME)$(EXESUF)
+fuzz-obj-y = $(libqos-obj-y)
+fuzz-obj-y += tests/libqtest.o
+
-- 
2.23.0

[Qemu-devel] [PATCH v3 08/22] module: check module wasn't already initialized

[Oleinik, Alexander]

The virtual-device fuzzer must initialize QOM, prior to running
vl:qemu_init, so that it can use the qos_graph to identify the arguments
required to initialize a guest for libqos-assisted fuzzing. This change
prevents errors when vl:qemu_init tries to (re)initialize the previously
initialized QOM module.

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 util/module.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/util/module.c b/util/module.c
index e9fe3e5422..841e490e06 100644
--- a/util/module.c
+++ b/util/module.c
@@ -30,6 +30,7 @@ typedef struct ModuleEntry
 typedef QTAILQ_HEAD(, ModuleEntry) ModuleTypeList;
 
 static ModuleTypeList init_type_list[MODULE_INIT_MAX];
+static bool modules_init_done[MODULE_INIT_MAX];
 
 static ModuleTypeList dso_init_list;
 
@@ -91,11 +92,17 @@ void module_call_init(module_init_type type)
     ModuleTypeList *l;
     ModuleEntry *e;
 
+    if (modules_init_done[type]) {
+        return;
+    }
+
     l = find_type(type);
 
     QTAILQ_FOREACH(e, l, node) {
         e->init();
     }
+
+    modules_init_done[type] = true;
 }
 
 #ifdef CONFIG_MODULES
-- 
2.23.0

Re: [Qemu-devel] [PATCH v3 08/22] module: check module wasn't already initialized

[Stefan Hajnoczi]

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

On Wed, Sep 18, 2019 at 11:19:35PM +0000, Oleinik, Alexander wrote:
> The virtual-device fuzzer must initialize QOM, prior to running
> vl:qemu_init, so that it can use the qos_graph to identify the arguments
> required to initialize a guest for libqos-assisted fuzzing. This change
> prevents errors when vl:qemu_init tries to (re)initialize the previously
> initialized QOM module.
> 
> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> ---
>  util/module.c | 7 +++++++
>  1 file changed, 7 insertions(+)

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

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

[Qemu-devel] [PATCH v3 09/22] qtest: add in-process incoming command handler

[Oleinik, Alexander]

The handler allows a qtest client to send commands to the server by
directly calling a function, rather than using a file/CharBackend

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 include/sysemu/qtest.h | 1 +
 qtest.c                | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h
index fda7000d2c..3f365522d5 100644
--- a/include/sysemu/qtest.h
+++ b/include/sysemu/qtest.h
@@ -28,5 +28,6 @@ void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **
 
 void qtest_server_set_tx_handler(void (*send)(void *, const char *, size_t),
                                  void *opaque);
+void qtest_server_inproc_recv(void *opaque, const char *buf, size_t size);
 
 #endif
diff --git a/qtest.c b/qtest.c
index ae7e6d779d..44a916485f 100644
--- a/qtest.c
+++ b/qtest.c
@@ -802,3 +802,10 @@ bool qtest_driver(void)
 {
     return qtest_chr.chr != NULL;
 }
+
+void qtest_server_inproc_recv(void *opaque, const char *buf, size_t size)
+{
+    GString *gstr = g_string_new_len(buf, size);
+    qtest_process_inbuf(NULL, gstr);
+    g_string_free(gstr, true);
+}
-- 
2.23.0

Re: [Qemu-devel] [PATCH v3 09/22] qtest: add in-process incoming command handler

[Stefan Hajnoczi]

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

On Wed, Sep 18, 2019 at 11:19:36PM +0000, Oleinik, Alexander wrote:
> The handler allows a qtest client to send commands to the server by
> directly calling a function, rather than using a file/CharBackend
> 
> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> ---
>  include/sysemu/qtest.h | 1 +
>  qtest.c                | 7 +++++++
>  2 files changed, 8 insertions(+)

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

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

[Qemu-devel] [PATCH v3 10/22] tests: provide test variables to other targets

[Oleinik, Alexander]

Before, when tests/Makefile.include was included, the contents would be
ignored if config-host.mak was defined. Moving the ifneq responsible for
this allows a target to depend on both testing-related and host-related
objects. For example the virtual-device fuzzer relies on both
libqtest/libqos objects and softmmu objects.

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 tests/Makefile.include | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index b39860a8d0..a9b3c36259 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -27,7 +27,6 @@ check-help:
 	@echo "Default options are -k and (for $(MAKE) V=1) --verbose; they can be"
 	@echo "changed with variable GTESTER_OPTIONS."
 
-ifneq ($(wildcard config-host.mak),)
 export SRC_PATH
 
 # TODO don't duplicate $(SRC_PATH)/Makefile's qapi-py here
@@ -860,6 +859,8 @@ tests/test-qga$(EXESUF): tests/test-qga.o $(qtest-obj-y)
 
 SPEED = quick
 
+ifneq ($(wildcard config-host.mak),)
+
 # gtester tests, possibly with verbose output
 # do_test_tap runs all tests, even if some of them fail, while do_test_human
 # stops at the first failure unless -k is given on the command line
-- 
2.23.0

[Qemu-devel] [PATCH v3 11/22] libqos: split qos-test and libqos makefile vars

[Oleinik, Alexander]

Most qos-related objects were specified in the qos-test-obj-y variable.
qos-test-obj-y also included qos-test.o which defines a main().
This made it difficult to repurpose qos-test-obj-y to link anything
beside tests/qos-test against libqos. This change separates objects that
are libqos-specific and ones that are qos-test specific into different
variables.

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 tests/Makefile.include | 70 ++++++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 34 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index a9b3c36259..da665ca7eb 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -687,51 +687,53 @@ tests/test-crypto-block$(EXESUF): tests/test-crypto-block.o $(test-crypto-obj-y)
 
 libqgraph-obj-y = tests/libqos/qgraph.o
 
-libqos-obj-y = $(libqgraph-obj-y) tests/libqos/pci.o tests/libqos/fw_cfg.o
-libqos-obj-y += tests/libqos/malloc.o
-libqos-obj-y += tests/libqos/libqos.o
-libqos-spapr-obj-y = $(libqos-obj-y) tests/libqos/malloc-spapr.o
+libqos-core-obj-y = $(libqgraph-obj-y) tests/libqos/pci.o tests/libqos/fw_cfg.o
+libqos-core-obj-y += tests/libqos/malloc.o
+libqos-core-obj-y += tests/libqos/libqos.o
+libqos-spapr-obj-y = $(libqos-core-obj-y) tests/libqos/malloc-spapr.o
 libqos-spapr-obj-y += tests/libqos/libqos-spapr.o
 libqos-spapr-obj-y += tests/libqos/rtas.o
 libqos-spapr-obj-y += tests/libqos/pci-spapr.o
-libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
+libqos-pc-obj-y = $(libqos-core-obj-y) tests/libqos/pci-pc.o
 libqos-pc-obj-y += tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o
 libqos-pc-obj-y += tests/libqos/ahci.o
 libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/usb.o
 
 # Devices
-qos-test-obj-y = tests/qos-test.o $(libqgraph-obj-y)
-qos-test-obj-y += $(libqos-pc-obj-y) $(libqos-spapr-obj-y)
-qos-test-obj-y += tests/libqos/e1000e.o
-qos-test-obj-y += tests/libqos/i2c.o
-qos-test-obj-y += tests/libqos/i2c-imx.o
-qos-test-obj-y += tests/libqos/i2c-omap.o
-qos-test-obj-y += tests/libqos/sdhci.o
-qos-test-obj-y += tests/libqos/tpci200.o
-qos-test-obj-y += tests/libqos/virtio.o
-qos-test-obj-$(CONFIG_VIRTFS) += tests/libqos/virtio-9p.o
-qos-test-obj-y += tests/libqos/virtio-balloon.o
-qos-test-obj-y += tests/libqos/virtio-blk.o
-qos-test-obj-y += tests/libqos/virtio-mmio.o
-qos-test-obj-y += tests/libqos/virtio-net.o
-qos-test-obj-y += tests/libqos/virtio-pci.o
-qos-test-obj-y += tests/libqos/virtio-rng.o
-qos-test-obj-y += tests/libqos/virtio-scsi.o
-qos-test-obj-y += tests/libqos/virtio-serial.o
+libqos-obj-y = $(libqos-core-obj-y)
+libqos-obj-y += $(libqgraph-obj-y)
+libqos-obj-y += $(libqos-pc-obj-y) $(libqos-spapr-obj-y)
+libqos-obj-y += tests/libqos/e1000e.o
+libqos-obj-y += tests/libqos/i2c.o
+libqos-obj-y += tests/libqos/i2c-imx.o
+libqos-obj-y += tests/libqos/i2c-omap.o
+libqos-obj-y += tests/libqos/sdhci.o
+libqos-obj-y += tests/libqos/tpci200.o
+libqos-obj-y += tests/libqos/virtio.o
+libqos-obj-$(CONFIG_VIRTFS) += tests/libqos/virtio-9p.o
+libqos-obj-y += tests/libqos/virtio-balloon.o
+libqos-obj-y += tests/libqos/virtio-blk.o
+libqos-obj-y += tests/libqos/virtio-mmio.o
+libqos-obj-y += tests/libqos/virtio-net.o
+libqos-obj-y += tests/libqos/virtio-pci.o
+libqos-obj-y += tests/libqos/virtio-rng.o
+libqos-obj-y += tests/libqos/virtio-scsi.o
+libqos-obj-y += tests/libqos/virtio-serial.o
 
 # Machines
-qos-test-obj-y += tests/libqos/aarch64-xlnx-zcu102-machine.o
-qos-test-obj-y += tests/libqos/arm-imx25-pdk-machine.o
-qos-test-obj-y += tests/libqos/arm-n800-machine.o
-qos-test-obj-y += tests/libqos/arm-raspi2-machine.o
-qos-test-obj-y += tests/libqos/arm-sabrelite-machine.o
-qos-test-obj-y += tests/libqos/arm-smdkc210-machine.o
-qos-test-obj-y += tests/libqos/arm-virt-machine.o
-qos-test-obj-y += tests/libqos/arm-xilinx-zynq-a9-machine.o
-qos-test-obj-y += tests/libqos/ppc64_pseries-machine.o
-qos-test-obj-y += tests/libqos/x86_64_pc-machine.o
+libqos-obj-y += tests/libqos/aarch64-xlnx-zcu102-machine.o
+libqos-obj-y += tests/libqos/arm-imx25-pdk-machine.o
+libqos-obj-y += tests/libqos/arm-n800-machine.o
+libqos-obj-y += tests/libqos/arm-raspi2-machine.o
+libqos-obj-y += tests/libqos/arm-sabrelite-machine.o
+libqos-obj-y += tests/libqos/arm-smdkc210-machine.o
+libqos-obj-y += tests/libqos/arm-virt-machine.o
+libqos-obj-y += tests/libqos/arm-xilinx-zynq-a9-machine.o
+libqos-obj-y += tests/libqos/ppc64_pseries-machine.o
+libqos-obj-y += tests/libqos/x86_64_pc-machine.o
 
 # Tests
+qos-test-obj-y = tests/qos-test.o
 qos-test-obj-y += tests/ac97-test.o
 qos-test-obj-y += tests/ds1338-test.o
 qos-test-obj-y += tests/e1000-test.o
@@ -763,7 +765,7 @@ check-unit-y += tests/test-qgraph$(EXESUF)
 tests/test-qgraph$(EXESUF): tests/test-qgraph.o $(libqgraph-obj-y)
 
 check-qtest-generic-y += tests/qos-test$(EXESUF)
-tests/qos-test$(EXESUF): $(qos-test-obj-y)
+tests/qos-test$(EXESUF): $(qos-test-obj-y) $(libqos-obj-y)
 
 tests/qmp-test$(EXESUF): tests/qmp-test.o
 tests/qmp-cmd-test$(EXESUF): tests/qmp-cmd-test.o
-- 
2.23.0

Re: [Qemu-devel] [PATCH v3 11/22] libqos: split qos-test and libqos makefile vars

[Thomas Huth]

On 19/09/2019 01.19, Oleinik, Alexander wrote:
> Most qos-related objects were specified in the qos-test-obj-y variable.
> qos-test-obj-y also included qos-test.o which defines a main().
> This made it difficult to repurpose qos-test-obj-y to link anything
> beside tests/qos-test against libqos. This change separates objects that
> are libqos-specific and ones that are qos-test specific into different
> variables.
> 
> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> ---
>  tests/Makefile.include | 70 ++++++++++++++++++++++--------------------
>  1 file changed, 36 insertions(+), 34 deletions(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index a9b3c36259..da665ca7eb 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -687,51 +687,53 @@ tests/test-crypto-block$(EXESUF): tests/test-crypto-block.o $(test-crypto-obj-y)
>  
>  libqgraph-obj-y = tests/libqos/qgraph.o
>  
> -libqos-obj-y = $(libqgraph-obj-y) tests/libqos/pci.o tests/libqos/fw_cfg.o
> -libqos-obj-y += tests/libqos/malloc.o
> -libqos-obj-y += tests/libqos/libqos.o
> -libqos-spapr-obj-y = $(libqos-obj-y) tests/libqos/malloc-spapr.o
> +libqos-core-obj-y = $(libqgraph-obj-y) tests/libqos/pci.o tests/libqos/fw_cfg.o
> +libqos-core-obj-y += tests/libqos/malloc.o
> +libqos-core-obj-y += tests/libqos/libqos.o
> +libqos-spapr-obj-y = $(libqos-core-obj-y) tests/libqos/malloc-spapr.o
>  libqos-spapr-obj-y += tests/libqos/libqos-spapr.o
>  libqos-spapr-obj-y += tests/libqos/rtas.o
>  libqos-spapr-obj-y += tests/libqos/pci-spapr.o
> -libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
> +libqos-pc-obj-y = $(libqos-core-obj-y) tests/libqos/pci-pc.o
>  libqos-pc-obj-y += tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o
>  libqos-pc-obj-y += tests/libqos/ahci.o
>  libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/usb.o
>  
>  # Devices
> -qos-test-obj-y = tests/qos-test.o $(libqgraph-obj-y)
> -qos-test-obj-y += $(libqos-pc-obj-y) $(libqos-spapr-obj-y)
> -qos-test-obj-y += tests/libqos/e1000e.o
> -qos-test-obj-y += tests/libqos/i2c.o
> -qos-test-obj-y += tests/libqos/i2c-imx.o
> -qos-test-obj-y += tests/libqos/i2c-omap.o
> -qos-test-obj-y += tests/libqos/sdhci.o
> -qos-test-obj-y += tests/libqos/tpci200.o
> -qos-test-obj-y += tests/libqos/virtio.o
> -qos-test-obj-$(CONFIG_VIRTFS) += tests/libqos/virtio-9p.o
> -qos-test-obj-y += tests/libqos/virtio-balloon.o
> -qos-test-obj-y += tests/libqos/virtio-blk.o
> -qos-test-obj-y += tests/libqos/virtio-mmio.o
> -qos-test-obj-y += tests/libqos/virtio-net.o
> -qos-test-obj-y += tests/libqos/virtio-pci.o
> -qos-test-obj-y += tests/libqos/virtio-rng.o
> -qos-test-obj-y += tests/libqos/virtio-scsi.o
> -qos-test-obj-y += tests/libqos/virtio-serial.o
> +libqos-obj-y = $(libqos-core-obj-y)
> +libqos-obj-y += $(libqgraph-obj-y)
> +libqos-obj-y += $(libqos-pc-obj-y) $(libqos-spapr-obj-y)
> +libqos-obj-y += tests/libqos/e1000e.o
> +libqos-obj-y += tests/libqos/i2c.o
> +libqos-obj-y += tests/libqos/i2c-imx.o
> +libqos-obj-y += tests/libqos/i2c-omap.o
> +libqos-obj-y += tests/libqos/sdhci.o
> +libqos-obj-y += tests/libqos/tpci200.o
> +libqos-obj-y += tests/libqos/virtio.o
> +libqos-obj-$(CONFIG_VIRTFS) += tests/libqos/virtio-9p.o
> +libqos-obj-y += tests/libqos/virtio-balloon.o
> +libqos-obj-y += tests/libqos/virtio-blk.o
> +libqos-obj-y += tests/libqos/virtio-mmio.o
> +libqos-obj-y += tests/libqos/virtio-net.o
> +libqos-obj-y += tests/libqos/virtio-pci.o
> +libqos-obj-y += tests/libqos/virtio-rng.o
> +libqos-obj-y += tests/libqos/virtio-scsi.o
> +libqos-obj-y += tests/libqos/virtio-serial.o
>  
>  # Machines
> -qos-test-obj-y += tests/libqos/aarch64-xlnx-zcu102-machine.o
> -qos-test-obj-y += tests/libqos/arm-imx25-pdk-machine.o
> -qos-test-obj-y += tests/libqos/arm-n800-machine.o
> -qos-test-obj-y += tests/libqos/arm-raspi2-machine.o
> -qos-test-obj-y += tests/libqos/arm-sabrelite-machine.o
> -qos-test-obj-y += tests/libqos/arm-smdkc210-machine.o
> -qos-test-obj-y += tests/libqos/arm-virt-machine.o
> -qos-test-obj-y += tests/libqos/arm-xilinx-zynq-a9-machine.o
> -qos-test-obj-y += tests/libqos/ppc64_pseries-machine.o
> -qos-test-obj-y += tests/libqos/x86_64_pc-machine.o
> +libqos-obj-y += tests/libqos/aarch64-xlnx-zcu102-machine.o
> +libqos-obj-y += tests/libqos/arm-imx25-pdk-machine.o
> +libqos-obj-y += tests/libqos/arm-n800-machine.o
> +libqos-obj-y += tests/libqos/arm-raspi2-machine.o
> +libqos-obj-y += tests/libqos/arm-sabrelite-machine.o
> +libqos-obj-y += tests/libqos/arm-smdkc210-machine.o
> +libqos-obj-y += tests/libqos/arm-virt-machine.o
> +libqos-obj-y += tests/libqos/arm-xilinx-zynq-a9-machine.o
> +libqos-obj-y += tests/libqos/ppc64_pseries-machine.o
> +libqos-obj-y += tests/libqos/x86_64_pc-machine.o
>  
>  # Tests
> +qos-test-obj-y = tests/qos-test.o
>  qos-test-obj-y += tests/ac97-test.o
>  qos-test-obj-y += tests/ds1338-test.o
>  qos-test-obj-y += tests/e1000-test.o
> @@ -763,7 +765,7 @@ check-unit-y += tests/test-qgraph$(EXESUF)
>  tests/test-qgraph$(EXESUF): tests/test-qgraph.o $(libqgraph-obj-y)
>  
>  check-qtest-generic-y += tests/qos-test$(EXESUF)
> -tests/qos-test$(EXESUF): $(qos-test-obj-y)
> +tests/qos-test$(EXESUF): $(qos-test-obj-y) $(libqos-obj-y)
>  
>  tests/qmp-test$(EXESUF): tests/qmp-test.o
>  tests/qmp-cmd-test$(EXESUF): tests/qmp-cmd-test.o
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

[Qemu-devel] [PATCH v3 12/22] libqos: move useful qos-test funcs to qos_external

[Oleinik, Alexander]

The moved functions are not specific to qos-test and might be useful
elsewhere. For example the virtual-device fuzzer makes use of them for
qos-assisted fuzz-targets.

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 tests/Makefile.include      |   1 +
 tests/libqos/qos_external.c | 151 ++++++++++++++++++++++++++++++++++++
 tests/libqos/qos_external.h |  10 +++
 tests/qos-test.c            | 140 ++-------------------------------
 4 files changed, 167 insertions(+), 135 deletions(-)
 create mode 100644 tests/libqos/qos_external.c
 create mode 100644 tests/libqos/qos_external.h

diff --git a/tests/Makefile.include b/tests/Makefile.include
index da665ca7eb..5d4d12503a 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -703,6 +703,7 @@ libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/usb.o
 libqos-obj-y = $(libqos-core-obj-y)
 libqos-obj-y += $(libqgraph-obj-y)
 libqos-obj-y += $(libqos-pc-obj-y) $(libqos-spapr-obj-y)
+libqos-obj-y += tests/libqos/qos_external.o
 libqos-obj-y += tests/libqos/e1000e.o
 libqos-obj-y += tests/libqos/i2c.o
 libqos-obj-y += tests/libqos/i2c-imx.o
diff --git a/tests/libqos/qos_external.c b/tests/libqos/qos_external.c
new file mode 100644
index 0000000000..81d1a50c8c
--- /dev/null
+++ b/tests/libqos/qos_external.c
@@ -0,0 +1,151 @@
+
+#include "qemu/osdep.h"
+#include <getopt.h>
+#include "libqtest.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qbool.h"
+#include "qapi/qmp/qstring.h"
+#include "qemu/module.h"
+#include "qapi/qmp/qlist.h"
+#include "libqos/malloc.h"
+#include "libqos/qgraph.h"
+#include "libqos/qgraph_internal.h"
+#include "libqos/qos_external.h"
+
+
+
+void apply_to_node(const char *name, bool is_machine, bool is_abstract)
+{
+    char *machine_name = NULL;
+    if (is_machine) {
+        const char *arch = qtest_get_arch();
+        machine_name = g_strconcat(arch, "/", name, NULL);
+        name = machine_name;
+    }
+    qos_graph_node_set_availability(name, true);
+    if (is_abstract) {
+        qos_delete_cmd_line(name);
+    }
+    g_free(machine_name);
+}
+
+/**
+ * apply_to_qlist(): using QMP queries QEMU for a list of
+ * machines and devices available, and sets the respective node
+ * as true. If a node is found, also all its produced and contained
+ * child are marked available.
+ *
+ * See qos_graph_node_set_availability() for more info
+ */
+void apply_to_qlist(QList *list, bool is_machine)
+{
+    const QListEntry *p;
+    const char *name;
+    bool abstract;
+    QDict *minfo;
+    QObject *qobj;
+    QString *qstr;
+    QBool *qbool;
+
+    for (p = qlist_first(list); p; p = qlist_next(p)) {
+        minfo = qobject_to(QDict, qlist_entry_obj(p));
+        qobj = qdict_get(minfo, "name");
+        qstr = qobject_to(QString, qobj);
+        name = qstring_get_str(qstr);
+
+        qobj = qdict_get(minfo, "abstract");
+        if (qobj) {
+            qbool = qobject_to(QBool, qobj);
+            abstract = qbool_get_bool(qbool);
+        } else {
+            abstract = false;
+        }
+
+        apply_to_node(name, is_machine, abstract);
+        qobj = qdict_get(minfo, "alias");
+        if (qobj) {
+            qstr = qobject_to(QString, qobj);
+            name = qstring_get_str(qstr);
+            apply_to_node(name, is_machine, abstract);
+        }
+    }
+}
+
+QGuestAllocator *get_machine_allocator(QOSGraphObject *obj)
+{
+    return obj->get_driver(obj, "memory");
+}
+
+/**
+ * allocate_objects(): given an array of nodes @arg,
+ * walks the path invoking all constructors and
+ * passing the corresponding parameter in order to
+ * continue the objects allocation.
+ * Once the test is reached, return the object it consumes.
+ *
+ * Since the machine and QEDGE_CONSUMED_BY nodes allocate
+ * memory in the constructor, g_test_queue_destroy is used so
+ * that after execution they can be safely free'd.  (The test's
+ * ->before callback is also welcome to use g_test_queue_destroy).
+ *
+ * Note: as specified in walk_path() too, @arg is an array of
+ * char *, where arg[0] is a pointer to the command line
+ * string that will be used to properly start QEMU when executing
+ * the test, and the remaining elements represent the actual objects
+ * that will be allocated.
+ */
+void *allocate_objects(QTestState *qts, char **path, QGuestAllocator **p_alloc)
+{
+    int current = 0;
+    QGuestAllocator *alloc;
+    QOSGraphObject *parent = NULL;
+    QOSGraphEdge *edge;
+    QOSGraphNode *node;
+    void *edge_arg;
+    void *obj;
+
+    node = qos_graph_get_node(path[current]);
+    g_assert(node->type == QNODE_MACHINE);
+
+    obj = qos_machine_new(node, qts);
+    qos_object_queue_destroy(obj);
+
+    alloc = get_machine_allocator(obj);
+    if (p_alloc) {
+        *p_alloc = alloc;
+    }
+
+    for (;;) {
+        if (node->type != QNODE_INTERFACE) {
+            qos_object_start_hw(obj);
+            parent = obj;
+        }
+
+        /* follow edge and get object for next node constructor */
+        current++;
+        edge = qos_graph_get_edge(path[current - 1], path[current]);
+        node = qos_graph_get_node(path[current]);
+
+        if (node->type == QNODE_TEST) {
+            g_assert(qos_graph_edge_get_type(edge) == QEDGE_CONSUMED_BY);
+            return obj;
+        }
+
+        switch (qos_graph_edge_get_type(edge)) {
+        case QEDGE_PRODUCES:
+            obj = parent->get_driver(parent, path[current]);
+            break;
+
+        case QEDGE_CONSUMED_BY:
+            edge_arg = qos_graph_edge_get_arg(edge);
+            obj = qos_driver_new(node, obj, alloc, edge_arg);
+            qos_object_queue_destroy(obj);
+            break;
+
+        case QEDGE_CONTAINS:
+            obj = parent->get_device(parent, path[current]);
+            break;
+        }
+    }
+}
+
diff --git a/tests/libqos/qos_external.h b/tests/libqos/qos_external.h
new file mode 100644
index 0000000000..1c5bbae814
--- /dev/null
+++ b/tests/libqos/qos_external.h
@@ -0,0 +1,10 @@
+#ifndef QOS_EXTERNAL_H
+#define QOS_EXTERNAL_H
+#include "libqos/qgraph.h"
+
+void apply_to_node(const char *name, bool is_machine, bool is_abstract);
+void apply_to_qlist(QList *list, bool is_machine);
+QGuestAllocator *get_machine_allocator(QOSGraphObject *obj);
+void *allocate_objects(QTestState *qts, char **path, QGuestAllocator **p_alloc);
+
+#endif
diff --git a/tests/qos-test.c b/tests/qos-test.c
index fd70d73ea5..9d02b83e24 100644
--- a/tests/qos-test.c
+++ b/tests/qos-test.c
@@ -27,65 +27,11 @@
 #include "libqos/malloc.h"
 #include "libqos/qgraph.h"
 #include "libqos/qgraph_internal.h"
+#include "libqos/qos_external.h"
 
 static char *old_path;
 
-static void apply_to_node(const char *name, bool is_machine, bool is_abstract)
-{
-    char *machine_name = NULL;
-    if (is_machine) {
-        const char *arch = qtest_get_arch();
-        machine_name = g_strconcat(arch, "/", name, NULL);
-        name = machine_name;
-    }
-    qos_graph_node_set_availability(name, true);
-    if (is_abstract) {
-        qos_delete_cmd_line(name);
-    }
-    g_free(machine_name);
-}
 
-/**
- * apply_to_qlist(): using QMP queries QEMU for a list of
- * machines and devices available, and sets the respective node
- * as true. If a node is found, also all its produced and contained
- * child are marked available.
- *
- * See qos_graph_node_set_availability() for more info
- */
-static void apply_to_qlist(QList *list, bool is_machine)
-{
-    const QListEntry *p;
-    const char *name;
-    bool abstract;
-    QDict *minfo;
-    QObject *qobj;
-    QString *qstr;
-    QBool *qbool;
-
-    for (p = qlist_first(list); p; p = qlist_next(p)) {
-        minfo = qobject_to(QDict, qlist_entry_obj(p));
-        qobj = qdict_get(minfo, "name");
-        qstr = qobject_to(QString, qobj);
-        name = qstring_get_str(qstr);
-
-        qobj = qdict_get(minfo, "abstract");
-        if (qobj) {
-            qbool = qobject_to(QBool, qobj);
-            abstract = qbool_get_bool(qbool);
-        } else {
-            abstract = false;
-        }
-
-        apply_to_node(name, is_machine, abstract);
-        qobj = qdict_get(minfo, "alias");
-        if (qobj) {
-            qstr = qobject_to(QString, qobj);
-            name = qstring_get_str(qstr);
-            apply_to_node(name, is_machine, abstract);
-        }
-    }
-}
 
 /**
  * qos_set_machines_devices_available(): sets availability of qgraph
@@ -129,10 +75,6 @@ static void qos_set_machines_devices_available(void)
     qobject_unref(response);
 }
 
-static QGuestAllocator *get_machine_allocator(QOSGraphObject *obj)
-{
-    return obj->get_driver(obj, "memory");
-}
 
 static void restart_qemu_or_continue(char *path)
 {
@@ -159,78 +101,6 @@ void qos_invalidate_command_line(void)
     old_path = NULL;
 }
 
-/**
- * allocate_objects(): given an array of nodes @arg,
- * walks the path invoking all constructors and
- * passing the corresponding parameter in order to
- * continue the objects allocation.
- * Once the test is reached, return the object it consumes.
- *
- * Since the machine and QEDGE_CONSUMED_BY nodes allocate
- * memory in the constructor, g_test_queue_destroy is used so
- * that after execution they can be safely free'd.  (The test's
- * ->before callback is also welcome to use g_test_queue_destroy).
- *
- * Note: as specified in walk_path() too, @arg is an array of
- * char *, where arg[0] is a pointer to the command line
- * string that will be used to properly start QEMU when executing
- * the test, and the remaining elements represent the actual objects
- * that will be allocated.
- */
-static void *allocate_objects(QTestState *qts, char **path, QGuestAllocator **p_alloc)
-{
-    int current = 0;
-    QGuestAllocator *alloc;
-    QOSGraphObject *parent = NULL;
-    QOSGraphEdge *edge;
-    QOSGraphNode *node;
-    void *edge_arg;
-    void *obj;
-
-    node = qos_graph_get_node(path[current]);
-    g_assert(node->type == QNODE_MACHINE);
-
-    obj = qos_machine_new(node, qts);
-    qos_object_queue_destroy(obj);
-
-    alloc = get_machine_allocator(obj);
-    if (p_alloc) {
-        *p_alloc = alloc;
-    }
-
-    for (;;) {
-        if (node->type != QNODE_INTERFACE) {
-            qos_object_start_hw(obj);
-            parent = obj;
-        }
-
-        /* follow edge and get object for next node constructor */
-        current++;
-        edge = qos_graph_get_edge(path[current - 1], path[current]);
-        node = qos_graph_get_node(path[current]);
-
-        if (node->type == QNODE_TEST) {
-            g_assert(qos_graph_edge_get_type(edge) == QEDGE_CONSUMED_BY);
-            return obj;
-        }
-
-        switch (qos_graph_edge_get_type(edge)) {
-        case QEDGE_PRODUCES:
-            obj = parent->get_driver(parent, path[current]);
-            break;
-
-        case QEDGE_CONSUMED_BY:
-            edge_arg = qos_graph_edge_get_arg(edge);
-            obj = qos_driver_new(node, obj, alloc, edge_arg);
-            qos_object_queue_destroy(obj);
-            break;
-
-        case QEDGE_CONTAINS:
-            obj = parent->get_device(parent, path[current]);
-            break;
-        }
-    }
-}
 
 /* The argument to run_one_test, which is the test function that is registered
  * with GTest, is a vector of strings.  The first item is the initial command
@@ -239,14 +109,14 @@ static void *allocate_objects(QTestState *qts, char **path, QGuestAllocator **p_
  */
 static char **current_path;
 
-const char *qos_get_current_command_line(void)
+void *qos_allocate_objects(QTestState *qts, QGuestAllocator **p_alloc)
 {
-    return current_path[0];
+    return allocate_objects(qts, current_path + 1, p_alloc);
 }
 
-void *qos_allocate_objects(QTestState *qts, QGuestAllocator **p_alloc)
+const char *qos_get_current_command_line(void)
 {
-    return allocate_objects(qts, current_path + 1, p_alloc);
+    return current_path[0];
 }
 
 /**
-- 
2.23.0

Re: [Qemu-devel] [PATCH v3 12/22] libqos: move useful qos-test funcs to qos_external

[Stefan Hajnoczi]

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

On Wed, Sep 18, 2019 at 11:19:39PM +0000, Oleinik, Alexander wrote:
> The moved functions are not specific to qos-test and might be useful
> elsewhere. For example the virtual-device fuzzer makes use of them for
> qos-assisted fuzz-targets.
> 
> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> ---
>  tests/Makefile.include      |   1 +
>  tests/libqos/qos_external.c | 151 ++++++++++++++++++++++++++++++++++++
>  tests/libqos/qos_external.h |  10 +++
>  tests/qos-test.c            | 140 ++-------------------------------
>  4 files changed, 167 insertions(+), 135 deletions(-)
>  create mode 100644 tests/libqos/qos_external.c
>  create mode 100644 tests/libqos/qos_external.h

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

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

[Qemu-devel] [PATCH v3 13/22] libqtest: make qtest_bufwrite send "atomic"

[Oleinik, Alexander]

When using qtest "in-process" communication, qtest_sendf directly calls
a function in the server (qtest.c). Combining the contents of the
subsequent socket_sends into the qtest_sendf, makes it so the server can
immediately handle the command, without building a local buffer and
waiting for a newline.

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 tests/libqtest.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 19feea9e17..d770462869 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -1086,9 +1086,7 @@ void qtest_bufwrite(QTestState *s, uint64_t addr, const void *data, size_t size)
     gchar *bdata;
 
     bdata = g_base64_encode(data, size);
-    qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx ", addr, size);
-    socket_send(s->fd, bdata, strlen(bdata));
-    socket_send(s->fd, "\n", 1);
+    qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx %s\n", addr, size, bdata);
     qtest_rsp(s, 0);
     g_free(bdata);
 }
-- 
2.23.0

Re: [Qemu-devel] [PATCH v3 13/22] libqtest: make qtest_bufwrite send "atomic"

[Stefan Hajnoczi]

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

On Wed, Sep 18, 2019 at 11:19:40PM +0000, Oleinik, Alexander wrote:
> When using qtest "in-process" communication, qtest_sendf directly calls
> a function in the server (qtest.c). Combining the contents of the
> subsequent socket_sends into the qtest_sendf, makes it so the server can
> immediately handle the command, without building a local buffer and
> waiting for a newline.
> 
> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> ---
>  tests/libqtest.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 19feea9e17..d770462869 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -1086,9 +1086,7 @@ void qtest_bufwrite(QTestState *s, uint64_t addr, const void *data, size_t size)
>      gchar *bdata;
>  
>      bdata = g_base64_encode(data, size);
> -    qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx ", addr, size);
> -    socket_send(s->fd, bdata, strlen(bdata));
> -    socket_send(s->fd, "\n", 1);
> +    qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx %s\n", addr, size, bdata);
>      qtest_rsp(s, 0);
>      g_free(bdata);
>  }
> -- 
> 2.23.0

Cc John Snow, who added the b64write command.

The downside to doing this is that sprintf-formatting needs to be
performed on the entire base64 buffer.  This slows things down slightly
and a larger temporary buffer needs to be allocated, but I'm not sure it
matters.

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

Re: [PATCH v3 13/22] libqtest: make qtest_bufwrite send "atomic"

[John Snow]

On 9/19/19 6:37 AM, Stefan Hajnoczi wrote:
> On Wed, Sep 18, 2019 at 11:19:40PM +0000, Oleinik, Alexander wrote:
>> When using qtest "in-process" communication, qtest_sendf directly calls
>> a function in the server (qtest.c). Combining the contents of the
>> subsequent socket_sends into the qtest_sendf, makes it so the server can
>> immediately handle the command, without building a local buffer and
>> waiting for a newline.
>>
>> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
>> ---
>>  tests/libqtest.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/tests/libqtest.c b/tests/libqtest.c
>> index 19feea9e17..d770462869 100644
>> --- a/tests/libqtest.c
>> +++ b/tests/libqtest.c
>> @@ -1086,9 +1086,7 @@ void qtest_bufwrite(QTestState *s, uint64_t addr, const void *data, size_t size)
>>      gchar *bdata;
>>  
>>      bdata = g_base64_encode(data, size);
>> -    qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx ", addr, size);
>> -    socket_send(s->fd, bdata, strlen(bdata));
>> -    socket_send(s->fd, "\n", 1);
>> +    qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx %s\n", addr, size, bdata);
>>      qtest_rsp(s, 0);
>>      g_free(bdata);
>>  }
>> -- 
>> 2.23.0
> 
> Cc John Snow, who added the b64write command.
> 
> The downside to doing this is that sprintf-formatting needs to be
> performed on the entire base64 buffer.  This slows things down slightly
> and a larger temporary buffer needs to be allocated, but I'm not sure it
> matters.
> 

*struggles to remember*

I guess I wanted something that had some space savings while maintaining
some semblance of debuggability. This is almost certainly meant for AHCI
tests where it's writing various patterns to large blocks of memory.

I doubt I really measured the performance of it, but it seemed like the
way to go for transferring medium amounts of data at the time via the
qtest protocol.

Looks like I am the only user of it, still:

tests/ahci-test.c:    qtest_bufwrite(ahci->parent->qts, ptr, tx, bufsize);
tests/ahci-test.c:    qtest_bufwrite(ahci->parent->qts, ptr, tx, bufsize);
tests/libqos/ahci.c:        qtest_bufwrite(ahci->parent->qts, ptr,
buffer, bufsize);

The buffers can be quite large, so you might be re-buffering a decent
amount of data from the sender now.

1, Are large transfers like this guaranteed to be atomic anyway? What
kind of socket is it? we're probably eclipsing frame and packet sizes here.

2, I am not sure what being "atomic" affords us in terms of allowing the
server to not wait for newlines, how does this change help?

--js

Re: [PATCH v3 13/22] libqtest: make qtest_bufwrite send "atomic"

[Oleinik, Alexander]

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

On Thu, 2019-09-19 at 14:56 -0400, John Snow wrote:
>
> On 9/19/19 6:37 AM, Stefan Hajnoczi wrote:
> > On Wed, Sep 18, 2019 at 11:19:40PM +0000, Oleinik, Alexander wrote:
> > > When using qtest "in-process" communication, qtest_sendf directly
> > > calls
> > > a function in the server (qtest.c). Combining the contents of the
> > > subsequent socket_sends into the qtest_sendf, makes it so the
> > > server can
> > > immediately handle the command, without building a local buffer
> > > and
> > > waiting for a newline.
> > >
> > > Signed-off-by: Alexander Oleinik <alxndr@bu.edu<mailto:alxndr@bu.edu>>
> > > ---
> > >  tests/libqtest.c | 4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > >
> > > diff --git a/tests/libqtest.c b/tests/libqtest.c
> > > index 19feea9e17..d770462869 100644
> > > --- a/tests/libqtest.c
> > > +++ b/tests/libqtest.c
> > > @@ -1086,9 +1086,7 @@ void qtest_bufwrite(QTestState *s, uint64_t
> > > addr, const void *data, size_t size)
> > >      gchar *bdata;
> > >
> > >      bdata = g_base64_encode(data, size);
> > > -    qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx ", addr, size);
> > > -    socket_send(s->fd, bdata, strlen(bdata));
> > > -    socket_send(s->fd, "\n", 1);
> > > +    qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx %s\n", addr,
> > > size, bdata);
> > >      qtest_rsp(s, 0);
> > >      g_free(bdata);
> > >  }
> > > --
> > > 2.23.0
> >
> > Cc John Snow, who added the b64write command.
> >
> > The downside to doing this is that sprintf-formatting needs to be
> > performed on the entire base64 buffer.  This slows things down
> > slightly
> > and a larger temporary buffer needs to be allocated, but I'm not
> > sure it
> > matters.
> >
>
> *struggles to remember*
>
> I guess I wanted something that had some space savings while
> maintaining
> some semblance of debuggability. This is almost certainly meant for
> AHCI
> tests where it's writing various patterns to large blocks of memory.
>
> I doubt I really measured the performance of it, but it seemed like
> the
> way to go for transferring medium amounts of data at the time via the
> qtest protocol.
>
> Looks like I am the only user of it, still:
>
> tests/ahci-test.c:    qtest_bufwrite(ahci->parent->qts, ptr, tx,
> bufsize);
> tests/ahci-test.c:    qtest_bufwrite(ahci->parent->qts, ptr, tx,
> bufsize);
> tests/libqos/ahci.c:        qtest_bufwrite(ahci->parent->qts, ptr,
> buffer, bufsize);
>
> The buffers can be quite large, so you might be re-buffering a decent
> amount of data from the sender now.
>
> 1, Are large transfers like this guaranteed to be atomic anyway? What
> kind of socket is it? we're probably eclipsing frame and packet sizes
> here.
>
> 2, I am not sure what being "atomic" affords us in terms of allowing
> the server to not wait for newlines, how does this change help?
>
> --js

I'm modifying qtest to allow the server and client to co-exist within
the same process (facilitating coverage-guided fuzzing). One of the
modifications is making qtest_sendf directly call a function in
qtest.c. All the other qtest commands are sent with a single
qtest_sendf call, so the qtest.c function could immediately call
qtest_process_command. This breaks if the command is sent with
different qtest_send/socket_send calls, as in b64write.

It should be simple to change qtest_server_inproc_recv (the qtest.c receiver) to wait for an "\n" prior to qtest_process_command, so I will probably do that and then normal(socket) qtest will keep the memory-reduction benefits of the non-"atomic" approach.

As a side note, would qtest_memwrite, also benefit from splitting up the send command?

    for (i = 0; i < size; i++) {
        sprintf(&enc[i * 2], "%02x", ptr[i]);
    }

    qtest_sendf(s, "write 0x%" PRIx64 " 0x%zx 0x%s\n", addr, size, enc);
    qtest_rsp(s, 0);
    g_free(enc);

[-- Attachment #2: Type: text/html, Size: 28204 bytes --]

Re: [PATCH v3 13/22] libqtest: make qtest_bufwrite send "atomic"

[John Snow]

On 9/19/19 3:36 PM, Oleinik, Alexander wrote:
> On Thu, 2019-09-19 at 14:56 -0400, John Snow wrote:
>> >
>> > On 9/19/19 6:37 AM, Stefan Hajnoczi wrote:
>>> > > On Wed, Sep 18, 2019 at 11:19:40PM +0000, Oleinik, Alexander wrote:
>>>> > > > When using qtest "in-process" communication, qtest_sendf directly
>>>> > > > calls
>>>> > > > a function in the server (qtest.c). Combining the contents of the
>>>> > > > subsequent socket_sends into the qtest_sendf, makes it so the
>>>> > > > server can
>>>> > > > immediately handle the command, without building a local buffer
>>>> > > > and
>>>> > > > waiting for a newline.
>>>> > > >
>>>> > > > Signed-off-by: Alexander Oleinik <alxndr@bu.edu
>>>> <mailto:alxndr@bu.edu>>
>>>> > > > ---
>>>> > > >  tests/libqtest.c | 4 +---
>>>> > > >  1 file changed, 1 insertion(+), 3 deletions(-)
>>>> > > >
>>>> > > > diff --git a/tests/libqtest.c b/tests/libqtest.c
>>>> > > > index 19feea9e17..d770462869 100644
>>>> > > > --- a/tests/libqtest.c
>>>> > > > +++ b/tests/libqtest.c
>>>> > > > @@ -1086,9 +1086,7 @@ void qtest_bufwrite(QTestState *s, uint64_t
>>>> > > > addr, const void *data, size_t size)
>>>> > > >      gchar *bdata;
>>>> > > >  
>>>> > > >      bdata = g_base64_encode(data, size);
>>>> > > > -    qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx ", addr, size);
>>>> > > > -    socket_send(s->fd, bdata, strlen(bdata));
>>>> > > > -    socket_send(s->fd, "\n", 1);
>>>> > > > +    qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx %s\n", addr,
>>>> > > > size, bdata);
>>>> > > >      qtest_rsp(s, 0);
>>>> > > >      g_free(bdata);
>>>> > > >  }
>>>> > > > -- 
>>>> > > > 2.23.0
>>> > >
>>> > > Cc John Snow, who added the b64write command.
>>> > >
>>> > > The downside to doing this is that sprintf-formatting needs to be
>>> > > performed on the entire base64 buffer.  This slows things down
>>> > > slightly
>>> > > and a larger temporary buffer needs to be allocated, but I'm not
>>> > > sure it
>>> > > matters.
>>> > >
>> >
>> > *struggles to remember*
>> >
>> > I guess I wanted something that had some space savings while
>> > maintaining
>> > some semblance of debuggability. This is almost certainly meant for
>> > AHCI
>> > tests where it's writing various patterns to large blocks of memory.
>> >
>> > I doubt I really measured the performance of it, but it seemed like
>> > the
>> > way to go for transferring medium amounts of data at the time via the
>> > qtest protocol.
>> >
>> > Looks like I am the only user of it, still:
>> >
>> > tests/ahci-test.c:    qtest_bufwrite(ahci->parent->qts, ptr, tx,
>> > bufsize);
>> > tests/ahci-test.c:    qtest_bufwrite(ahci->parent->qts, ptr, tx,
>> > bufsize);
>> > tests/libqos/ahci.c:        qtest_bufwrite(ahci->parent->qts, ptr,
>> > buffer, bufsize);
>> >
>> > The buffers can be quite large, so you might be re-buffering a decent
>> > amount of data from the sender now.
>> >
>> > 1, Are large transfers like this guaranteed to be atomic anyway? What
>> > kind of socket is it? we're probably eclipsing frame and packet sizes
>> > here.
>> >
>> > 2, I am not sure what being "atomic" affords us in terms of allowing
>> > the server to not wait for newlines, how does this change help?
>> >
>> > --js
> 
> I'm modifying qtest to allow the server and client to co-exist within
> the same process (facilitating coverage-guided fuzzing). One of the
> modifications is making qtest_sendf directly call a function in
> qtest.c. All the other qtest commands are sent with a single
> qtest_sendf call, so the qtest.c function could immediately call
> qtest_process_command. This breaks if the command is sent with
> different qtest_send/socket_send calls, as in b64write.
> 
> It should be simple to change qtest_server_inproc_recv (the qtest.c
> receiver) to wait for an "\n" prior to qtest_process_command, so I will
> probably do that and then normal(socket) qtest will keep the
> memory-reduction benefits of the non-"atomic" approach.
> 

Before you spend the effort, just benchmark it if you can. I'd look at
the before and after of the AHCI tests with and without the pre-buffering.

> As a side note, would qtest_memwrite, also benefit from splitting up the
> send command?
> 
>     for (i = 0; i < size; i++) {
>         sprintf(&enc[i * 2], "%02x", ptr[i]);
>     }
> 
>     qtest_sendf(s, "write 0x%" PRIx64 " 0x%zx 0x%s\n", addr, size, enc);
>     qtest_rsp(s, 0);
>     g_free(enc);

Depends on the users. I think at the time I wrote bufwrite it was using
obviously more data. Some of the memwrite users might be writing pretty
small things.

Looks like users are:

tests/ahci-test
tests/ide-test
tests/libqos/e1000e.c
tests/libqos/pci-pc.c
tests/libqos/pci-spapr.c
tests/megasas-test.c
tests/tpm-util.c
tests/virtio-9p-test.c

the libqos ones are used by other tests -- you can give it a skim and
come up with a small list of tests to benchmark and see if it makes any
actual difference.

--js

Re: [PATCH v3 13/22] libqtest: make qtest_bufwrite send "atomic"

[Alexander Oleinik]

On 9/19/19 2:56 PM, John Snow wrote:
> 
> 
> On 9/19/19 6:37 AM, Stefan Hajnoczi wrote:
>> On Wed, Sep 18, 2019 at 11:19:40PM +0000, Oleinik, Alexander wrote:
>>> When using qtest "in-process" communication, qtest_sendf directly calls
>>> a function in the server (qtest.c). Combining the contents of the
>>> subsequent socket_sends into the qtest_sendf, makes it so the server can
>>> immediately handle the command, without building a local buffer and
>>> waiting for a newline.
>>>
>>> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
>>> ---
>>>   tests/libqtest.c | 4 +---
>>>   1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/tests/libqtest.c b/tests/libqtest.c
>>> index 19feea9e17..d770462869 100644
>>> --- a/tests/libqtest.c
>>> +++ b/tests/libqtest.c
>>> @@ -1086,9 +1086,7 @@ void qtest_bufwrite(QTestState *s, uint64_t addr, const void *data, size_t size)
>>>       gchar *bdata;
>>>   
>>>       bdata = g_base64_encode(data, size);
>>> -    qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx ", addr, size);
>>> -    socket_send(s->fd, bdata, strlen(bdata));
>>> -    socket_send(s->fd, "\n", 1);
>>> +    qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx %s\n", addr, size, bdata);
>>>       qtest_rsp(s, 0);
>>>       g_free(bdata);
>>>   }
>>> -- 
>>> 2.23.0
>>
>> Cc John Snow, who added the b64write command.
>>
>> The downside to doing this is that sprintf-formatting needs to be
>> performed on the entire base64 buffer.  This slows things down slightly
>> and a larger temporary buffer needs to be allocated, but I'm not sure it
>> matters.
>>
> 
> *struggles to remember*
> 
> I guess I wanted something that had some space savings while maintaining
> some semblance of debuggability. This is almost certainly meant for AHCI
> tests where it's writing various patterns to large blocks of memory.
> 
> I doubt I really measured the performance of it, but it seemed like the
> way to go for transferring medium amounts of data at the time via the
> qtest protocol.
> 
> Looks like I am the only user of it, still:
> 
> tests/ahci-test.c:    qtest_bufwrite(ahci->parent->qts, ptr, tx, bufsize);
> tests/ahci-test.c:    qtest_bufwrite(ahci->parent->qts, ptr, tx, bufsize);
> tests/libqos/ahci.c:        qtest_bufwrite(ahci->parent->qts, ptr,
> buffer, bufsize);
> 
> The buffers can be quite large, so you might be re-buffering a decent
> amount of data from the sender now.
> 
> 1, Are large transfers like this guaranteed to be atomic anyway? What
> kind of socket is it? we're probably eclipsing frame and packet sizes here.
> 
> 2, I am not sure what being "atomic" affords us in terms of allowing the
> server to not wait for newlines, how does this change help?
> 
> --js
> 
I'm modifying qtest to allow the server and client to co-exist within
the same process (facilitating coverage-guided fuzzing). One of the
modifications is making qtest_sendf directly call a function in
qtest.c. All the other qtest commands are sent with a single
qtest_sendf call, so the qtest.c function could immediately call
qtest_process_command. This breaks if the command is sent with
different qtest_send/socket_send calls, as in b64write.

It should be simple to change qtest_server_inproc_recv (the qtest.c 
receiver) to
wait for an "\n" prior to qtest_process_command, so I will probably do 
that and
then normal(socket) qtest will keep the memory-reduction benefits of the
non-"atomic" approach.

As a side note, would qtest_memwrite, also benefit from splitting up the 
send
command?

      for (i = 0; i < size; i++) {
          sprintf(&enc[i * 2], "%02x", ptr[i]);
      }

      qtest_sendf(s, "write 0x%" PRIx64 " 0x%zx 0x%s\n", addr, size, enc);
      qtest_rsp(s, 0);
      g_free(enc);

[Qemu-devel] [PATCH v3 14/22] libqtest: add in-process qtest.c tx/rx handlers

[Oleinik, Alexander]

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 tests/libqtest.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/libqtest.h |  5 +++++
 2 files changed, 51 insertions(+)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index d770462869..fc10322d52 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -60,6 +60,7 @@ struct QTestState
 static GHookList abrt_hooks;
 static struct sigaction sigact_old;
 static GString *recv_str;
+static const char *qtest_arch;
 
 static int qtest_query_target_endianness(QTestState *s);
 
@@ -490,6 +491,7 @@ static GString *qtest_client_socket_recv_line(void* opaque)
     return line;
 }
 
+
 static gchar **qtest_rsp(QTestState *s, int expected_args)
 {
     GString *line;
@@ -830,6 +832,9 @@ char *qtest_hmp(QTestState *s, const char *fmt, ...)
 
 const char *qtest_get_arch(void)
 {
+    if (qtest_arch) {
+        return qtest_arch;
+    }
     const char *qemu = qtest_qemu_binary();
     const char *end = strrchr(qemu, '/');
 
@@ -1367,3 +1372,44 @@ static void qtest_client_set_rx_handler(QTestState *s,
     s->ops.recv_line = recv;
     s->ops.recv_line_opaque = opaque;
 }
+
+static GString *qtest_client_inproc_recv_line(void* opaque)
+{
+    GString *line;
+    size_t offset;
+    char *eol;
+
+    eol = strchr(recv_str->str, '\n');
+    offset = eol - recv_str->str;
+    line = g_string_new_len(recv_str->str, offset);
+    g_string_erase(recv_str, 0, offset + 1);
+    return line;
+}
+
+QTestState *qtest_inproc_init(bool log, const char* arch,
+                    void (*send)(void*, const char*, size_t))
+{
+    QTestState *qts;
+    qts = g_new(QTestState, 1);
+    qts->wstatus = 0;
+    for (int i = 0; i < MAX_IRQ; i++) {
+        qts->irq_level[i] = false;
+    }
+
+    qtest_client_set_rx_handler(qts, qtest_client_inproc_recv_line, qts);
+    qtest_client_set_tx_handler(qts, send, NULL);
+
+    qts->big_endian = qtest_query_target_endianness(qts);
+    qtest_arch = arch;
+
+    return qts;
+}
+
+void qtest_client_inproc_recv(void *opaque, const char *str, size_t len)
+{
+    if (!recv_str) {
+        recv_str = g_string_new(NULL);
+    }
+    g_string_append_len(recv_str, str, len);
+    return;
+}
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 40fa235a52..453bd8998f 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -716,4 +716,9 @@ bool qtest_probe_child(QTestState *s);
  * Set expected exit status of the child.
  */
 void qtest_set_expected_status(QTestState *s, int status);
+
+
+QTestState *qtest_inproc_init(bool log, const char* arch,
+                    void (*send)(void*, const char*, size_t));
+void qtest_client_inproc_recv(void *opaque, const char *str, size_t len);
 #endif
-- 
2.23.0

Re: [Qemu-devel] [PATCH v3 14/22] libqtest: add in-process qtest.c tx/rx handlers

[Stefan Hajnoczi]

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

On Wed, Sep 18, 2019 at 11:19:41PM +0000, Oleinik, Alexander wrote:
> @@ -830,6 +832,9 @@ char *qtest_hmp(QTestState *s, const char *fmt, ...)
>  
>  const char *qtest_get_arch(void)

Maybe this should be per QTestState just like big_endian, but the global
qtest_arch variable is okay for now.

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

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

Re: [Qemu-devel] [PATCH v3 14/22] libqtest: add in-process qtest.c tx/rx handlers

[Oleinik, Alexander]

On Thu, 2019-09-19 at 11:42 +0100, Stefan Hajnoczi wrote:
> On Wed, Sep 18, 2019 at 11:19:41PM +0000, Oleinik, Alexander wrote:
> > @@ -830,6 +832,9 @@ char *qtest_hmp(QTestState *s, const char *fmt,
> > ...)
> >  
> >  const char *qtest_get_arch(void)
> 
> Maybe this should be per QTestState just like big_endian, but the
> global
> qtest_arch variable is okay for now.
> 

I was worried that the same QTestState may be reused to run tests for
different for qemu-system binaries, but I can see that this is not
possible. I'll make this change.

[Qemu-devel] [PATCH v3 15/22] fuzz: Add target/fuzz makefile rules

[Oleinik, Alexander]

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 Makefile        | 12 +++++++++++-
 Makefile.objs   |  6 +++++-
 Makefile.target |  1 +
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 3a43492340..5e5033a500 100644
--- a/Makefile
+++ b/Makefile
@@ -426,6 +426,7 @@ dummy := $(call unnest-vars,, \
                 qom-obj-y \
                 io-obj-y \
                 common-obj-y \
+                softmmu-obj-y \
                 common-obj-m \
                 ui-obj-y \
                 ui-obj-m \
@@ -463,7 +464,16 @@ $(SOFTMMU_ALL_RULES): $(crypto-obj-y)
 $(SOFTMMU_ALL_RULES): $(io-obj-y)
 $(SOFTMMU_ALL_RULES): config-all-devices.mak
 $(SOFTMMU_ALL_RULES): $(edk2-decompressed)
-$(SOFTMMU_ALL_RULES): $(softmmu-main-y)
+$(SOFTMMU_ALL_RULES): $(softmmu-obj-y)
+
+SOFTMMU_FUZZ_RULES=$(filter %-softmmu/fuzz, $(TARGET_DIRS_RULES))
+$(SOFTMMU_FUZZ_RULES): $(authz-obj-y)
+$(SOFTMMU_FUZZ_RULES): $(block-obj-y)
+$(SOFTMMU_FUZZ_RULES): $(chardev-obj-y)
+$(SOFTMMU_FUZZ_RULES): $(crypto-obj-y)
+$(SOFTMMU_FUZZ_RULES): $(io-obj-y)
+$(SOFTMMU_FUZZ_RULES): config-all-devices.mak
+$(SOFTMMU_FUZZ_RULES): $(edk2-decompressed)
 
 SOFTMMU_FUZZ_RULES=$(filter %-softmmu/fuzz, $(TARGET_DIRS_RULES))
 $(SOFTMMU_FUZZ_RULES): $(authz-obj-y)
diff --git a/Makefile.objs b/Makefile.objs
index bb1cfa05ef..99ea6b66de 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -87,9 +87,13 @@ common-obj-$(CONFIG_FDT) += device_tree.o
 
 common-obj-y += qapi/
 
-softmmu-main-y = main.o
 endif
 
+ifneq ($(CONFIG_FUZZ),y)
+softmmu-obj-y = main.o
+endif
+
+
 #######################################################################
 # Target-independent parts used in system and user emulation
 common-obj-y += cpus-common.o
diff --git a/Makefile.target b/Makefile.target
index 6065989964..f3efe3debd 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -194,6 +194,7 @@ all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y) $(chardev-obj-y)
 all-obj-$(CONFIG_USER_ONLY) += $(crypto-user-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(crypto-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(io-obj-y)
+all-obj-$(CONFIG_SOFTMMU) += $(softmmu-obj-y)
 
 ifdef CONFIG_SOFTMMU
 $(QEMU_PROG_BUILD): config-devices.mak
-- 
2.23.0

[Qemu-devel] [PATCH v3 16/22] fuzz: add fuzzer skeleton

[Oleinik, Alexander]

tests/fuzz/fuzz.c serves as the entry point for the virtual-device
fuzzer. Namely, libfuzzer invokes the LLVMFuzzerInitialize and
LLVMFuzzerTestOneInput functions, both of which are defined in this
file. This change adds a "FuzzTarget" struct, along with the
fuzz_add_target function, which should be used to define new fuzz
targets.

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 tests/fuzz/Makefile.include |   4 +-
 tests/fuzz/fuzz.c           | 179 ++++++++++++++++++++++++++++++++++++
 tests/fuzz/fuzz.h           |  30 ++++++
 3 files changed, 211 insertions(+), 2 deletions(-)
 create mode 100644 tests/fuzz/fuzz.c
 create mode 100644 tests/fuzz/fuzz.h

diff --git a/tests/fuzz/Makefile.include b/tests/fuzz/Makefile.include
index 324e6c1433..b415b056b0 100644
--- a/tests/fuzz/Makefile.include
+++ b/tests/fuzz/Makefile.include
@@ -1,4 +1,4 @@
-# QEMU_PROG_FUZZ=qemu-fuzz-$(TARGET_NAME)$(EXESUF)
+QEMU_PROG_FUZZ=qemu-fuzz-$(TARGET_NAME)$(EXESUF)
 fuzz-obj-y = $(libqos-obj-y)
 fuzz-obj-y += tests/libqtest.o
-
+fuzz-obj-y += tests/fuzz/fuzz.o
diff --git a/tests/fuzz/fuzz.c b/tests/fuzz/fuzz.c
new file mode 100644
index 0000000000..833f436731
--- /dev/null
+++ b/tests/fuzz/fuzz.c
@@ -0,0 +1,179 @@
+#include "qemu/osdep.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+
+
+#include "tests/libqtest.h"
+#include "sysemu/qtest.h"
+#include "fuzz.h"
+#include "tests/libqos/qgraph.h"
+#include "sysemu/runstate.h"
+#include "sysemu/sysemu.h"
+
+typedef struct FuzzTargetState {
+        FuzzTarget *target;
+        QSLIST_ENTRY(FuzzTargetState) target_list;
+} FuzzTargetState;
+
+typedef QSLIST_HEAD(, FuzzTargetState) FuzzTargetList;
+
+static const char *fuzz_arch = TARGET_NAME;
+
+static FuzzTargetList *fuzz_target_list;
+static FuzzTarget *fuzz_target;
+static QTestState *fuzz_qts;
+static bool trace;
+
+
+void set_fuzz_target_args(int argc, char **argv)
+{
+    if (fuzz_target) {
+        fuzz_target->main_argc = argc;
+        fuzz_target->main_argv = argv;
+    }
+}
+
+void reboot(QTestState *s)
+{
+    qemu_system_reset(SHUTDOWN_CAUSE_GUEST_RESET);
+}
+
+static QTestState *qtest_setup(void)
+{
+    qtest_server_set_tx_handler(&qtest_client_inproc_recv, NULL);
+    return qtest_inproc_init(trace, fuzz_arch, &qtest_server_inproc_recv);
+}
+
+void fuzz_add_target(const char *name, const char *description,
+        FuzzTarget *target)
+{
+    FuzzTargetState *tmp;
+    FuzzTargetState *target_state;
+    if (!fuzz_target_list) {
+        fuzz_target_list = g_new0(FuzzTargetList, 1);
+    }
+
+    QSLIST_FOREACH(tmp, fuzz_target_list, target_list) {
+        if (g_strcmp0(tmp->target->name->str, name) == 0) {
+            fprintf(stderr, "Error: Fuzz target name %s already in use\n",
+                    name);
+            abort();
+        }
+    }
+    target_state = g_new0(FuzzTargetState, 1);
+    target_state->target = g_new0(FuzzTarget, 1);
+    *(target_state->target) = *target;
+    target_state->target->name = g_string_new(name);
+    target_state->target->description = g_string_new(description);
+    QSLIST_INSERT_HEAD(fuzz_target_list, target_state, target_list);
+}
+
+
+static FuzzTarget *fuzz_get_target(char* name)
+{
+    FuzzTargetState *tmp;
+    if (!fuzz_target_list) {
+        fprintf(stderr, "Fuzz target list not initialized\n");
+        abort();
+    }
+
+    QSLIST_FOREACH(tmp, fuzz_target_list, target_list) {
+        if (g_strcmp0(tmp->target->name->str, name) == 0) {
+            break;
+        }
+    }
+    return tmp->target;
+}
+
+
+static void usage(char *path)
+{
+    printf("Usage: %s --FUZZ_TARGET [LIBFUZZER ARGUMENTS]\n", path);
+    printf("where --FUZZ_TARGET is one of:\n");
+    FuzzTargetState *tmp;
+    if (!fuzz_target_list) {
+        fprintf(stderr, "Fuzz target list not initialized\n");
+        abort();
+    }
+    QSLIST_FOREACH(tmp, fuzz_target_list, target_list) {
+        printf(" --%s  : %s\n", tmp->target->name->str,
+                tmp->target->description->str);
+    }
+    exit(0);
+}
+
+
+/* Executed for each fuzzing-input */
+int LLVMFuzzerTestOneInput(const unsigned char *Data, size_t Size)
+{
+    if (fuzz_target->fuzz) {
+        fuzz_target->fuzz(fuzz_qts, Data, Size);
+    }
+    return 0;
+}
+
+/* Executed once, prior to fuzzing */
+int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
+{
+
+    char *target_name, *trace_qtest;
+
+    /* --trace is useful for outputting a log of qtest commands that trigger
+     * a crash. The log can can then be replayed with a simple qtest script. */
+    if (*argc > 2) {
+        trace_qtest = (*argv)[2];
+        if (strcmp(trace_qtest, "--trace") == 0) {
+            trace = true;
+        }
+    }
+
+    /* Initialize qgraph and modules */
+    qos_graph_init();
+    module_call_init(MODULE_INIT_FUZZ_TARGET);
+    module_call_init(MODULE_INIT_QOM);
+    module_call_init(MODULE_INIT_LIBQOS);
+
+    if (*argc <= 1) {
+        usage(**argv);
+    }
+
+    /* Identify the fuzz target */
+    target_name = (*argv)[1];
+    target_name += 2;
+    fuzz_target = fuzz_get_target(target_name);
+
+    fuzz_qts = qtest_setup(void);
+
+    if (!fuzz_target) {
+        fprintf(stderr, "Error: Fuzz fuzz_target name %s not found\n",
+                target_name);
+        usage(**argv);
+    }
+
+    if (fuzz_target->pre_main) {
+        fuzz_target->pre_main();
+    }
+
+    if (trace) {
+        printf("### cmd_line: ");
+        for (int i = 0; i < (fuzz_target->main_argc); i++) {
+            printf("%s ", ((fuzz_target->main_argv))[i]);
+        }
+        printf("\n");
+    }
+
+    /* Run QEMU's softmmu main with the calculated arguments*/
+    qemu_init(fuzz_target->main_argc, fuzz_target->main_argv, NULL);
+
+    /* If configured, this is a good spot to set up snapshotting */
+    if (fuzz_target->pre_fuzz) {
+        fuzz_target->pre_fuzz(fuzz_qts);
+    }
+
+    if (trace) {
+        printf("### END INITIALIZATION\n");
+    }
+
+    return 0;
+}
diff --git a/tests/fuzz/fuzz.h b/tests/fuzz/fuzz.h
new file mode 100644
index 0000000000..73af029c82
--- /dev/null
+++ b/tests/fuzz/fuzz.h
@@ -0,0 +1,30 @@
+#ifndef FUZZER_H_
+#define FUZZER_H_
+
+#include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "qapi/error.h"
+#include "exec/memory.h"
+#include "tests/libqtest.h"
+
+
+typedef struct FuzzTarget {
+    GString *name;
+    GString *description;
+    void(*pre_main)(void);
+    void(*pre_fuzz)(QTestState *);
+    void(*fuzz)(QTestState *, const unsigned char *, size_t);
+    int main_argc;
+    char **main_argv;
+} FuzzTarget;
+
+void set_fuzz_target_args(int argc, char **argv);
+void reboot(QTestState *);
+void fuzz_add_target(const char *name, const char *description, FuzzTarget
+        *target);
+
+int LLVMFuzzerTestOneInput(const unsigned char *Data, size_t Size);
+int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp);
+
+#endif
+
-- 
2.23.0

Re: [Qemu-devel] [PATCH v3 16/22] fuzz: add fuzzer skeleton

[Stefan Hajnoczi]

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

On Wed, Sep 18, 2019 at 11:19:43PM +0000, Oleinik, Alexander wrote:
> +void set_fuzz_target_args(int argc, char **argv)
> +{
> +    if (fuzz_target) {
> +        fuzz_target->main_argc = argc;
> +        fuzz_target->main_argv = argv;
> +    }
> +}

Why calls this and why?

> +
> +void reboot(QTestState *s)
> +{
> +    qemu_system_reset(SHUTDOWN_CAUSE_GUEST_RESET);
> +}

Why does reboot() take an unused argument?

> +static void usage(char *path)
> +{
> +    printf("Usage: %s --FUZZ_TARGET [LIBFUZZER ARGUMENTS]\n", path);
> +    printf("where --FUZZ_TARGET is one of:\n");

Is the "--" prefix a libfuzzer requirement?  I would have expected
either FUZZ_TARGET by itself or --fuzz-target=FUZZ_TARGET (a properly
formatted long option) so that collisions with other command-line
options are not possible.

> +typedef struct FuzzTarget {
> +    GString *name;
> +    GString *description;
> +    void(*pre_main)(void);
> +    void(*pre_fuzz)(QTestState *);
> +    void(*fuzz)(QTestState *, const unsigned char *, size_t);
> +    int main_argc;
> +    char **main_argv;
> +} FuzzTarget;
> +
> +void set_fuzz_target_args(int argc, char **argv);
> +void reboot(QTestState *);
> +void fuzz_add_target(const char *name, const char *description, FuzzTarget
> +        *target);

This is a strange API.  I can't make sense of the struct,
fuzz_add_target(), and set_fuzz_target_args() without reading the
implementation:

1. set_fuzz_target_args() implies that there is global state but then
   FuzzTarget also has main_argc and main_argv fields.  I'm not sure
   what the relationship is.

2. fuzz_add_target() takes name and description as arguments but expects
   the caller to populate the struct arg's pre_main(), pre_fuzz(),
   fuzz() function pointers.  This is inconsistent and undocumented.

A cleaner API:

  /* Each fuzz target implements the following interface: */
  typedef struct {
      const char *name;        /* command-line option for this target */
      const char *description; /* human-readable help text */

      /* TODO documentation */
      void (*pre_main)(void);

      /* TODO documentation */
      void (*pre_fuzz)(QTestState *);

      /* TODO documentation */
      void (*fuzz)(QTestState *, const unsigned char *, size_t);
  } FuzzTarget;

  void fuzz_register_target(const FuzzTarget *target);

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

Re: [Qemu-devel] [PATCH v3 16/22] fuzz: add fuzzer skeleton

[Oleinik, Alexander]

On Thu, 2019-09-19 at 13:48 +0100, Stefan Hajnoczi wrote:

> > +
> > +void reboot(QTestState *s)
> > +{
> > +    qemu_system_reset(SHUTDOWN_CAUSE_GUEST_RESET);
> > +}
> 
> Why does reboot() take an unused argument?
It was needed when I had a reset_state(s) pointer which was separate
from fuzz(). Since fuzz() is responsible for initializing and resetting
state now, I'll remove it.

> 
> > +static void usage(char *path)
> > +{
> > +    printf("Usage: %s --FUZZ_TARGET [LIBFUZZER ARGUMENTS]\n",
> > path);
> > +    printf("where --FUZZ_TARGET is one of:\n");
> 
> Is the "--" prefix a libfuzzer requirement?  I would have expected
> either FUZZ_TARGET by itself or --fuzz-target=FUZZ_TARGET (a properly
> formatted long option) so that collisions with other command-line
> options are not possible.
Yes libfuzzer will only pass arguments that start with "--". I can
replace it with --fuzz-target=FUZZ_TARGET. Alternatively, I can try to
build separate binaries for each target. It might waste disk space, but
we wouldn't need arguments (--trace could be replace with TRACE=1 in
ENV). With this design, I'm not sure what to do with code such as
i440fx_fuzz.c which re-purposes some functions for multiple different
fuzz targets.

> > +typedef struct FuzzTarget {
> > +    GString *name;
> > +    GString *description;
> > +    void(*pre_main)(void);
> > +    void(*pre_fuzz)(QTestState *);
> > +    void(*fuzz)(QTestState *, const unsigned char *, size_t);
> > +    int main_argc;
> > +    char **main_argv;
> > +} FuzzTarget;
> > +
> > +void set_fuzz_target_args(int argc, char **argv);
> > +void reboot(QTestState *);
> > +void fuzz_add_target(const char *name, const char *description,
> > FuzzTarget
> > +        *target);
> 
> This is a strange API.  I can't make sense of the struct,
> fuzz_add_target(), and set_fuzz_target_args() without reading the
> implementation:
> 
> 1. set_fuzz_target_args() implies that there is global state but then
>    FuzzTarget also has main_argc and main_argv fields.  I'm not sure
>    what the relationship is.
This is essentially there for the QOS support. For QOS targets, when
running fuzz_add_qos_target(), we don't yet know argc and argv, since
that requires walking the QOSGraph. When we have populated the
FuzzTargetList, QOSGraph and parsed the --FUZZ_TARGET, we set global
FuzzTarget and check against it while walking the QOSGraph. When we
find the matching path, we then know the argc/argv and can set them for
the global fuzz_target. Since qos_fuzz.c still needs major work due to
all of the duplicated code, I'll take another stab at this. Maybe we
can identify the argc/argv immediately when we add the fuzz target node
to the QOSGraph. This is another case for "each target has its own
binary", since that could avoid much of this complexity, since we
wouldn't need the fuzz_target_list.

> 2. fuzz_add_target() takes name and description as arguments but
> expects
>    the caller to populate the struct arg's pre_main(), pre_fuzz(),
>    fuzz() function pointers.  This is inconsistent and undocumented.
> 
> A cleaner API:
> 
>   /* Each fuzz target implements the following interface: */
>   typedef struct {
>       const char *name;        /* command-line option for this target
> */
>       const char *description; /* human-readable help text */
> 
>       /* TODO documentation */
>       void (*pre_main)(void);
> 
>       /* TODO documentation */
>       void (*pre_fuzz)(QTestState *);
> 
>       /* TODO documentation */
>       void (*fuzz)(QTestState *, const unsigned char *, size_t);
>   } FuzzTarget;

Sounds good. Should there also be argc and argv here? 

>   void fuzz_register_target(const FuzzTarget *target);

Re: [Qemu-devel] [PATCH v3 16/22] fuzz: add fuzzer skeleton

[Stefan Hajnoczi]

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

On Thu, Sep 19, 2019 at 01:49:09PM +0000, Oleinik, Alexander wrote:
> On Thu, 2019-09-19 at 13:48 +0100, Stefan Hajnoczi wrote:
> > > +static void usage(char *path)
> > > +{
> > > +    printf("Usage: %s --FUZZ_TARGET [LIBFUZZER ARGUMENTS]\n",
> > > path);
> > > +    printf("where --FUZZ_TARGET is one of:\n");
> > 
> > Is the "--" prefix a libfuzzer requirement?  I would have expected
> > either FUZZ_TARGET by itself or --fuzz-target=FUZZ_TARGET (a properly
> > formatted long option) so that collisions with other command-line
> > options are not possible.
> Yes libfuzzer will only pass arguments that start with "--". I can
> replace it with --fuzz-target=FUZZ_TARGET. Alternatively, I can try to
> build separate binaries for each target. It might waste disk space, but
> we wouldn't need arguments (--trace could be replace with TRACE=1 in
> ENV). With this design, I'm not sure what to do with code such as
> i440fx_fuzz.c which re-purposes some functions for multiple different
> fuzz targets.

Building a single fuzzing binary with all targets feels natural.  Please
support the --fuzz-target=TARGET syntax though.

> > A cleaner API:
> > 
> >   /* Each fuzz target implements the following interface: */
> >   typedef struct {
> >       const char *name;        /* command-line option for this target
> > */
> >       const char *description; /* human-readable help text */
> > 
> >       /* TODO documentation */
> >       void (*pre_main)(void);
> > 
> >       /* TODO documentation */
> >       void (*pre_fuzz)(QTestState *);
> > 
> >       /* TODO documentation */
> >       void (*fuzz)(QTestState *, const unsigned char *, size_t);
> >   } FuzzTarget;
> 
> Sounds good. Should there also be argc and argv here? 

If they are read-only and provided by the FuzzTarget, then yes.  The
reason I consider this "cleaner" is because the FuzzTarget struct is
stateless and just captures the information about the fuzz target
instead of mixing it with runtime state.  But like I said, I didn't
really understand the design of the struct so maybe I don't understand
the full problem :).

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

Re: [Qemu-devel] [PATCH v3 16/22] fuzz: add fuzzer skeleton

[Darren Kenny]

On Wed, Sep 18, 2019 at 11:19:43PM +0000, Oleinik, Alexander wrote:
>tests/fuzz/fuzz.c serves as the entry point for the virtual-device
>fuzzer. Namely, libfuzzer invokes the LLVMFuzzerInitialize and
>LLVMFuzzerTestOneInput functions, both of which are defined in this
>file. This change adds a "FuzzTarget" struct, along with the
>fuzz_add_target function, which should be used to define new fuzz
>targets.
>
>Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
>---
> tests/fuzz/Makefile.include |   4 +-
> tests/fuzz/fuzz.c           | 179 ++++++++++++++++++++++++++++++++++++
> tests/fuzz/fuzz.h           |  30 ++++++
> 3 files changed, 211 insertions(+), 2 deletions(-)
> create mode 100644 tests/fuzz/fuzz.c
> create mode 100644 tests/fuzz/fuzz.h
>
>diff --git a/tests/fuzz/Makefile.include b/tests/fuzz/Makefile.include
>index 324e6c1433..b415b056b0 100644
>--- a/tests/fuzz/Makefile.include
>+++ b/tests/fuzz/Makefile.include
>@@ -1,4 +1,4 @@
>-# QEMU_PROG_FUZZ=qemu-fuzz-$(TARGET_NAME)$(EXESUF)
>+QEMU_PROG_FUZZ=qemu-fuzz-$(TARGET_NAME)$(EXESUF)
> fuzz-obj-y = $(libqos-obj-y)
> fuzz-obj-y += tests/libqtest.o
>-
>+fuzz-obj-y += tests/fuzz/fuzz.o
>diff --git a/tests/fuzz/fuzz.c b/tests/fuzz/fuzz.c
>new file mode 100644
>index 0000000000..833f436731
>--- /dev/null
>+++ b/tests/fuzz/fuzz.c
>@@ -0,0 +1,179 @@
>+#include "qemu/osdep.h"
>+
>+#include <stdio.h>
>+#include <stdlib.h>
>+
>+
>+#include "tests/libqtest.h"
>+#include "sysemu/qtest.h"
>+#include "fuzz.h"
>+#include "tests/libqos/qgraph.h"
>+#include "sysemu/runstate.h"
>+#include "sysemu/sysemu.h"
>+
>+typedef struct FuzzTargetState {
>+        FuzzTarget *target;
>+        QSLIST_ENTRY(FuzzTargetState) target_list;
>+} FuzzTargetState;
>+
>+typedef QSLIST_HEAD(, FuzzTargetState) FuzzTargetList;
>+
>+static const char *fuzz_arch = TARGET_NAME;
>+
>+static FuzzTargetList *fuzz_target_list;
>+static FuzzTarget *fuzz_target;
>+static QTestState *fuzz_qts;
>+static bool trace;
>+
>+
>+void set_fuzz_target_args(int argc, char **argv)
>+{
>+    if (fuzz_target) {
>+        fuzz_target->main_argc = argc;
>+        fuzz_target->main_argv = argv;
>+    }
>+}
>+
>+void reboot(QTestState *s)
>+{
>+    qemu_system_reset(SHUTDOWN_CAUSE_GUEST_RESET);
>+}
>+
>+static QTestState *qtest_setup(void)
>+{
>+    qtest_server_set_tx_handler(&qtest_client_inproc_recv, NULL);
>+    return qtest_inproc_init(trace, fuzz_arch, &qtest_server_inproc_recv);
>+}
>+
>+void fuzz_add_target(const char *name, const char *description,
>+        FuzzTarget *target)
>+{
>+    FuzzTargetState *tmp;
>+    FuzzTargetState *target_state;
>+    if (!fuzz_target_list) {
>+        fuzz_target_list = g_new0(FuzzTargetList, 1);
>+    }
>+
>+    QSLIST_FOREACH(tmp, fuzz_target_list, target_list) {
>+        if (g_strcmp0(tmp->target->name->str, name) == 0) {
>+            fprintf(stderr, "Error: Fuzz target name %s already in use\n",
>+                    name);
>+            abort();
>+        }
>+    }
>+    target_state = g_new0(FuzzTargetState, 1);
>+    target_state->target = g_new0(FuzzTarget, 1);
>+    *(target_state->target) = *target;
>+    target_state->target->name = g_string_new(name);
>+    target_state->target->description = g_string_new(description);
>+    QSLIST_INSERT_HEAD(fuzz_target_list, target_state, target_list);
>+}
>+
>+
>+static FuzzTarget *fuzz_get_target(char* name)
>+{
>+    FuzzTargetState *tmp;
>+    if (!fuzz_target_list) {
>+        fprintf(stderr, "Fuzz target list not initialized\n");
>+        abort();
>+    }
>+
>+    QSLIST_FOREACH(tmp, fuzz_target_list, target_list) {
>+        if (g_strcmp0(tmp->target->name->str, name) == 0) {
>+            break;
>+        }
>+    }
>+    return tmp->target;
>+}
>+
>+
>+static void usage(char *path)
>+{
>+    printf("Usage: %s --FUZZ_TARGET [LIBFUZZER ARGUMENTS]\n", path);
>+    printf("where --FUZZ_TARGET is one of:\n");
>+    FuzzTargetState *tmp;
>+    if (!fuzz_target_list) {
>+        fprintf(stderr, "Fuzz target list not initialized\n");
>+        abort();
>+    }
>+    QSLIST_FOREACH(tmp, fuzz_target_list, target_list) {
>+        printf(" --%s  : %s\n", tmp->target->name->str,
>+                tmp->target->description->str);
>+    }
>+    exit(0);
>+}
>+
>+
>+/* Executed for each fuzzing-input */
>+int LLVMFuzzerTestOneInput(const unsigned char *Data, size_t Size)
>+{
>+    if (fuzz_target->fuzz) {
>+        fuzz_target->fuzz(fuzz_qts, Data, Size);
>+    }
>+    return 0;
>+}
>+
>+/* Executed once, prior to fuzzing */
>+int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
>+{
>+
>+    char *target_name, *trace_qtest;
>+
>+    /* --trace is useful for outputting a log of qtest commands that trigger
>+     * a crash. The log can can then be replayed with a simple qtest script. */
>+    if (*argc > 2) {
>+        trace_qtest = (*argv)[2];
>+        if (strcmp(trace_qtest, "--trace") == 0) {
>+            trace = true;
>+        }
>+    }
>+
>+    /* Initialize qgraph and modules */
>+    qos_graph_init();
>+    module_call_init(MODULE_INIT_FUZZ_TARGET);
>+    module_call_init(MODULE_INIT_QOM);
>+    module_call_init(MODULE_INIT_LIBQOS);
>+
>+    if (*argc <= 1) {
>+        usage(**argv);
>+    }
>+
>+    /* Identify the fuzz target */
>+    target_name = (*argv)[1];
>+    target_name += 2;
>+    fuzz_target = fuzz_get_target(target_name);
>+
>+    fuzz_qts = qtest_setup(void);

Shouldn't be passing void as an argument here, causes a build
failure for me.

Thanks,

Darren.

[Qemu-devel] [PATCH v3 17/22] fuzz: add support for fork-based fuzzing.

[Oleinik, Alexander]

fork() is a simple way to ensure that state does not leak in between
fuzzing runs. Unfortunately, the fuzzer mutation engine relies on
bitmaps which contain coverage information for each fuzzing run, and
these bitmaps should be copied from the child to the parent(where the
mutation occurs). These bitmaps are created through compile-time
instrumentation and there seems to be no simple way to re-map them as
shared memory. As a workaround, we use a linker script modification to
place all of the bitmaps together and add some markers around them which
we can observe from our code. Then, we map shared memory and copy the
bimaps to the SHM (in the child) and out of the SHM(in the parent) after
each fuzzing run. Ram blocks are marked as DONTFORK in exec.c, which
breaks this approach. For now, avoid this with an #ifdef.

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 exec.c                      |  2 ++
 tests/fuzz/Makefile.include |  3 +++
 tests/fuzz/fork_fuzz.c      | 27 ++++++++++++++++++++++
 tests/fuzz/fork_fuzz.h      | 12 ++++++++++
 tests/fuzz/fork_fuzz.ld     | 46 +++++++++++++++++++++++++++++++++++++
 5 files changed, 90 insertions(+)
 create mode 100644 tests/fuzz/fork_fuzz.c
 create mode 100644 tests/fuzz/fork_fuzz.h
 create mode 100644 tests/fuzz/fork_fuzz.ld

diff --git a/exec.c b/exec.c
index 235d6bc883..d3838f4ea4 100644
--- a/exec.c
+++ b/exec.c
@@ -2295,7 +2295,9 @@ static void ram_block_add(RAMBlock *new_block, Error **errp, bool shared)
         qemu_ram_setup_dump(new_block->host, new_block->max_length);
         qemu_madvise(new_block->host, new_block->max_length, QEMU_MADV_HUGEPAGE);
         /* MADV_DONTFORK is also needed by KVM in absence of synchronous MMU */
+#ifndef CONFIG_FUZZ /* This conflicts with fork-based fuzzing */
         qemu_madvise(new_block->host, new_block->max_length, QEMU_MADV_DONTFORK);
+#endif
         ram_block_notify_add(new_block->host, new_block->max_length);
     }
 }
diff --git a/tests/fuzz/Makefile.include b/tests/fuzz/Makefile.include
index b415b056b0..687dacce04 100644
--- a/tests/fuzz/Makefile.include
+++ b/tests/fuzz/Makefile.include
@@ -2,3 +2,6 @@ QEMU_PROG_FUZZ=qemu-fuzz-$(TARGET_NAME)$(EXESUF)
 fuzz-obj-y = $(libqos-obj-y)
 fuzz-obj-y += tests/libqtest.o
 fuzz-obj-y += tests/fuzz/fuzz.o
+fuzz-obj-y += tests/fuzz/fork_fuzz.o
+
+FUZZ_LDFLAGS += -Xlinker -T$(SRC_PATH)/tests/fuzz/fork_fuzz.ld
diff --git a/tests/fuzz/fork_fuzz.c b/tests/fuzz/fork_fuzz.c
new file mode 100644
index 0000000000..26d0b4b42e
--- /dev/null
+++ b/tests/fuzz/fork_fuzz.c
@@ -0,0 +1,27 @@
+#include "qemu/osdep.h"
+#include "fork_fuzz.h"
+
+uintptr_t feature_shm;
+
+void counter_shm_init(void)
+{
+    feature_shm = (uintptr_t)mmap(NULL,
+            &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START,
+            PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+    return;
+}
+
+void counter_shm_store(void)
+{
+    memcpy((void *)feature_shm,
+            &__FUZZ_COUNTERS_START,
+            &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
+}
+
+void counter_shm_load(void)
+{
+    memcpy(&__FUZZ_COUNTERS_START,
+            (void *)feature_shm,
+            &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
+}
+
diff --git a/tests/fuzz/fork_fuzz.h b/tests/fuzz/fork_fuzz.h
new file mode 100644
index 0000000000..b5f8b35015
--- /dev/null
+++ b/tests/fuzz/fork_fuzz.h
@@ -0,0 +1,12 @@
+#ifndef FORK_FUZZ_H
+#define FORK_FUZZ_H
+
+extern uint8_t __FUZZ_COUNTERS_START;
+extern uint8_t __FUZZ_COUNTERS_END;
+
+void counter_shm_init(void);
+void counter_shm_store(void);
+void counter_shm_load(void);
+
+#endif
+
diff --git a/tests/fuzz/fork_fuzz.ld b/tests/fuzz/fork_fuzz.ld
new file mode 100644
index 0000000000..ba0ba79570
--- /dev/null
+++ b/tests/fuzz/fork_fuzz.ld
@@ -0,0 +1,46 @@
+/* We adjust linker script modification to place all of the stuff that needs to
+ * persist across fuzzing runs into a contiguous seciton of memory. Then, it is
+ * easy to copy it to and from shared memory.
+ *
+ * Total Size : A5A00
+ * Sancov counters: B26F
+ * Coverage counters: 56D60
+ * TracePC Object: 43C00
+*/
+
+SECTIONS
+{
+  .data.fuzz_start : ALIGN(4K)
+  {
+        __FUZZ_COUNTERS_START = .;
+  }
+  .data.fuzz_ordered :
+  {
+      /* Internal Libfuzzer TracePC object which contains the ValueProfileMap.
+       * Not optimal that we have to copy the rest of the TracePC object.
+       * */
+      __start___sancov_cntrs = .;
+      *(__sancov_cntrs*)
+      __stop___sancov_cntrs = .;
+  }
+  .data.fuzz_unordered :
+  {
+      /* Coverage counters. They're not necessary for fuzzing, but are useful
+       * for analyzing the fuzzing performance
+       * */
+      __start___llvm_prf_cnts = .;
+      *(*llvm_prf_cnts);
+      __stop___llvm_prf_cnts = .;
+
+      /* Lowest stack counter */
+      *(__sancov_lowest_stack);
+      /* Internal Libfuzzer TracePC object which contains the ValueProfileMap.
+       * Not optimal that we have to copy the rest of the TracePC object.
+       * */
+      *FuzzerTracePC*(.bss._ZN6fuzzer*)
+      __FUZZ_COUNTERS_END = .;
+  }
+}
+/* Dont overwrite the SECTIONS in the default linker script. Instead insert the
+ * above into the default script */
+INSERT AFTER .data;
-- 
2.23.0

Re: [Qemu-devel] [PATCH v3 17/22] fuzz: add support for fork-based fuzzing.

[Stefan Hajnoczi]

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

On Wed, Sep 18, 2019 at 11:19:44PM +0000, Oleinik, Alexander wrote:
> diff --git a/exec.c b/exec.c
> index 235d6bc883..d3838f4ea4 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2295,7 +2295,9 @@ static void ram_block_add(RAMBlock *new_block, Error **errp, bool shared)
>          qemu_ram_setup_dump(new_block->host, new_block->max_length);
>          qemu_madvise(new_block->host, new_block->max_length, QEMU_MADV_HUGEPAGE);
>          /* MADV_DONTFORK is also needed by KVM in absence of synchronous MMU */
> +#ifndef CONFIG_FUZZ /* This conflicts with fork-based fuzzing */
>          qemu_madvise(new_block->host, new_block->max_length, QEMU_MADV_DONTFORK);
> +#endif
>          ram_block_notify_add(new_block->host, new_block->max_length);
>      }
>  }

I didn't check Makefile changes but regular softmmu binaries should
continue to work with --enable-fuzzing so we cannot use #ifdef here.

Perhaps this should be a runtime check similar to qtest_enabled():

  /* The fuzzer's fork child requires access to guest RAM */
  if (!fuzz_enabled()) {
      qemu_madvise(new_block->host, new_block->max_length, QEMU_MADV_DONTFORK);
  }

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

Re: [Qemu-devel] [PATCH v3 17/22] fuzz: add support for fork-based fuzzing.

[Oleinik, Alexander]

On Thu, 2019-09-19 at 13:54 +0100, Stefan Hajnoczi wrote:
> On Wed, Sep 18, 2019 at 11:19:44PM +0000, Oleinik, Alexander wrote:
> > diff --git a/exec.c b/exec.c
> > index 235d6bc883..d3838f4ea4 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -2295,7 +2295,9 @@ static void ram_block_add(RAMBlock
> > *new_block, Error **errp, bool shared)
> >          qemu_ram_setup_dump(new_block->host, new_block-
> > >max_length);
> >          qemu_madvise(new_block->host, new_block->max_length,
> > QEMU_MADV_HUGEPAGE);
> >          /* MADV_DONTFORK is also needed by KVM in absence of
> > synchronous MMU */
> > +#ifndef CONFIG_FUZZ /* This conflicts with fork-based fuzzing */
> >          qemu_madvise(new_block->host, new_block->max_length,
> > QEMU_MADV_DONTFORK);
> > +#endif
> >          ram_block_notify_add(new_block->host, new_block-
> > >max_length);
> >      }
> >  }
> 
> I didn't check Makefile changes but regular softmmu binaries should
> continue to work with --enable-fuzzing so we cannot use #ifdef here.
> Perhaps this should be a runtime check similar to qtest_enabled()

Yes - I'll add a runtime check. The makefile requires a make clean
between softmmu and fuzzer builds, since the ".o"s for the fuzzer build
are compiled with fsanitize=fuzzer(must be linked against libfuzzer). 
I can see that the #ifndef CONFIG_FUZZ is a proper softmmu builds with
--enable-fuzzer are currently broken due to the ifndef CONFIG_FUZZ
check.

Re: [Qemu-devel] [PATCH v3 17/22] fuzz: add support for fork-based fuzzing.

[Stefan Hajnoczi]

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

On Thu, Sep 19, 2019 at 02:01:53PM +0000, Oleinik, Alexander wrote:
> On Thu, 2019-09-19 at 13:54 +0100, Stefan Hajnoczi wrote:
> > On Wed, Sep 18, 2019 at 11:19:44PM +0000, Oleinik, Alexander wrote:
> > > diff --git a/exec.c b/exec.c
> > > index 235d6bc883..d3838f4ea4 100644
> > > --- a/exec.c
> > > +++ b/exec.c
> > > @@ -2295,7 +2295,9 @@ static void ram_block_add(RAMBlock
> > > *new_block, Error **errp, bool shared)
> > >          qemu_ram_setup_dump(new_block->host, new_block-
> > > >max_length);
> > >          qemu_madvise(new_block->host, new_block->max_length,
> > > QEMU_MADV_HUGEPAGE);
> > >          /* MADV_DONTFORK is also needed by KVM in absence of
> > > synchronous MMU */
> > > +#ifndef CONFIG_FUZZ /* This conflicts with fork-based fuzzing */
> > >          qemu_madvise(new_block->host, new_block->max_length,
> > > QEMU_MADV_DONTFORK);
> > > +#endif
> > >          ram_block_notify_add(new_block->host, new_block-
> > > >max_length);
> > >      }
> > >  }
> > 
> > I didn't check Makefile changes but regular softmmu binaries should
> > continue to work with --enable-fuzzing so we cannot use #ifdef here.
> > Perhaps this should be a runtime check similar to qtest_enabled()
> 
> Yes - I'll add a runtime check. The makefile requires a make clean
> between softmmu and fuzzer builds, since the ".o"s for the fuzzer build
> are compiled with fsanitize=fuzzer(must be linked against libfuzzer). 
> I can see that the #ifndef CONFIG_FUZZ is a proper softmmu builds with
> --enable-fuzzer are currently broken due to the ifndef CONFIG_FUZZ
> check.

In that case the fuzzer is a whole different make target and shouldn't
share .o files with *-softmmu/ build directories.

Stefan

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

Re: [PATCH v3 17/22] fuzz: add support for fork-based fuzzing.

[Alexander Oleinik]

On 9/18/19 7:19 PM, Oleinik, Alexander wrote:
> fork() is a simple way to ensure that state does not leak in between
> fuzzing runs. Unfortunately, the fuzzer mutation engine relies on
> bitmaps which contain coverage information for each fuzzing run, and
> these bitmaps should be copied from the child to the parent(where the
> mutation occurs). These bitmaps are created through compile-time
> instrumentation and there seems to be no simple way to re-map them as
> shared memory. As a workaround, we use a linker script modification to
> place all of the bitmaps together and add some markers around them which
> we can observe from our code. Then, we map shared memory and copy the
> bimaps to the SHM (in the child) and out of the SHM(in the parent) after
> each fuzzing run. Ram blocks are marked as DONTFORK in exec.c, which
> breaks this approach. For now, avoid this with an #ifdef.
> 
> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> ---
>   exec.c                      |  2 ++
>   tests/fuzz/Makefile.include |  3 +++
>   tests/fuzz/fork_fuzz.c      | 27 ++++++++++++++++++++++
>   tests/fuzz/fork_fuzz.h      | 12 ++++++++++
>   tests/fuzz/fork_fuzz.ld     | 46 +++++++++++++++++++++++++++++++++++++
>   5 files changed, 90 insertions(+)
>   create mode 100644 tests/fuzz/fork_fuzz.c
>   create mode 100644 tests/fuzz/fork_fuzz.h
>   create mode 100644 tests/fuzz/fork_fuzz.ld
> 
> diff --git a/exec.c b/exec.c
> index 235d6bc883..d3838f4ea4 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2295,7 +2295,9 @@ static void ram_block_add(RAMBlock *new_block, Error **errp, bool shared)
>           qemu_ram_setup_dump(new_block->host, new_block->max_length);
>           qemu_madvise(new_block->host, new_block->max_length, QEMU_MADV_HUGEPAGE);
>           /* MADV_DONTFORK is also needed by KVM in absence of synchronous MMU */
> +#ifndef CONFIG_FUZZ /* This conflicts with fork-based fuzzing */
>           qemu_madvise(new_block->host, new_block->max_length, QEMU_MADV_DONTFORK);
> +#endif
>           ram_block_notify_add(new_block->host, new_block->max_length);
>       }
>   }
> diff --git a/tests/fuzz/Makefile.include b/tests/fuzz/Makefile.include
> index b415b056b0..687dacce04 100644
> --- a/tests/fuzz/Makefile.include
> +++ b/tests/fuzz/Makefile.include
> @@ -2,3 +2,6 @@ QEMU_PROG_FUZZ=qemu-fuzz-$(TARGET_NAME)$(EXESUF)
>   fuzz-obj-y = $(libqos-obj-y)
>   fuzz-obj-y += tests/libqtest.o
>   fuzz-obj-y += tests/fuzz/fuzz.o
> +fuzz-obj-y += tests/fuzz/fork_fuzz.o
> +
> +FUZZ_LDFLAGS += -Xlinker -T$(SRC_PATH)/tests/fuzz/fork_fuzz.ld
> diff --git a/tests/fuzz/fork_fuzz.c b/tests/fuzz/fork_fuzz.c
> new file mode 100644
> index 0000000000..26d0b4b42e
> --- /dev/null
> +++ b/tests/fuzz/fork_fuzz.c
> @@ -0,0 +1,27 @@
> +#include "qemu/osdep.h"
> +#include "fork_fuzz.h"
> +
> +uintptr_t feature_shm;
> +
> +void counter_shm_init(void)
> +{
> +    feature_shm = (uintptr_t)mmap(NULL,
> +            &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START,
> +            PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> +    return;
> +}
> +
> +void counter_shm_store(void)
> +{
> +    memcpy((void *)feature_shm,
> +            &__FUZZ_COUNTERS_START,
> +            &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
> +}
> +
> +void counter_shm_load(void)
> +{
> +    memcpy(&__FUZZ_COUNTERS_START,
> +            (void *)feature_shm,
> +            &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
> +}
> +
> diff --git a/tests/fuzz/fork_fuzz.h b/tests/fuzz/fork_fuzz.h
> new file mode 100644
> index 0000000000..b5f8b35015
> --- /dev/null
> +++ b/tests/fuzz/fork_fuzz.h
> @@ -0,0 +1,12 @@
> +#ifndef FORK_FUZZ_H
> +#define FORK_FUZZ_H
> +
> +extern uint8_t __FUZZ_COUNTERS_START;
> +extern uint8_t __FUZZ_COUNTERS_END;
> +
> +void counter_shm_init(void);
> +void counter_shm_store(void);
> +void counter_shm_load(void);
> +
> +#endif
> +
> diff --git a/tests/fuzz/fork_fuzz.ld b/tests/fuzz/fork_fuzz.ld
> new file mode 100644
> index 0000000000..ba0ba79570
> --- /dev/null
> +++ b/tests/fuzz/fork_fuzz.ld
> @@ -0,0 +1,46 @@
> +/* We adjust linker script modification to place all of the stuff that needs to
> + * persist across fuzzing runs into a contiguous seciton of memory. Then, it is
> + * easy to copy it to and from shared memory.
> + *
> + * Total Size : A5A00
> + * Sancov counters: B26F
> + * Coverage counters: 56D60
> + * TracePC Object: 43C00
> +*/
> +
> +SECTIONS
> +{
> +  .data.fuzz_start : ALIGN(4K)
> +  {
> +        __FUZZ_COUNTERS_START = .;
> +  }
> +  .data.fuzz_ordered :
> +  {
> +      /* Internal Libfuzzer TracePC object which contains the ValueProfileMap.
> +       * Not optimal that we have to copy the rest of the TracePC object.
> +       * */
> +      __start___sancov_cntrs = .;
> +      *(__sancov_cntrs*)
> +      __stop___sancov_cntrs = .;
> +  }
> +  .data.fuzz_unordered :
> +  {
> +      /* Coverage counters. They're not necessary for fuzzing, but are useful
> +       * for analyzing the fuzzing performance
> +       * */
> +      __start___llvm_prf_cnts = .;
> +      *(*llvm_prf_cnts);
> +      __stop___llvm_prf_cnts = .;
> +
> +      /* Lowest stack counter */
> +      *(__sancov_lowest_stack);
> +      /* Internal Libfuzzer TracePC object which contains the ValueProfileMap.
> +       * Not optimal that we have to copy the rest of the TracePC object.
> +       * */
> +      *FuzzerTracePC*(.bss._ZN6fuzzer*)
> +      __FUZZ_COUNTERS_END = .;
> +  }
> +}
> +/* Dont overwrite the SECTIONS in the default linker script. Instead insert the
> + * above into the default script */
> +INSERT AFTER .data;
> 
The obvious issue with this are that we lose the threads created prior 
to fork(). Over the past days I've been coverage of existing virtio-net 
tests with and without fork(). With fork(), virtio_net_tx_bh doesn't get 
scheduled, and never runs. Could this be due to the missing RCU thread?
This is probably an even bigger problem for iothread devices, such as 
virtio-blk...

Does anyone have suggestions for how to approach the thread problem, or 
at least the RCU part of it?
I know Paolo made some fork-related changes in 
21b7cf9e07e5991c57b461181cfb5bbb6fe7a9d6 rcu: handle forks safely
and
2a96a552f9502ac34c29da2f3a39788db5ee5692 ,
but from what I can tell, this is mostly for qemu-user.

-Alex

[Qemu-devel] [PATCH v3 18/22] fuzz: expose fuzz target name

[Oleinik, Alexander]

This is needed for the qos-assisted fuzzers which walk the qos tree and
need a way to check if the current path matches the name of the fuzz
target.

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 tests/fuzz/fuzz.c | 3 +++
 tests/fuzz/fuzz.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/tests/fuzz/fuzz.c b/tests/fuzz/fuzz.c
index 833f436731..d44dab7360 100644
--- a/tests/fuzz/fuzz.c
+++ b/tests/fuzz/fuzz.c
@@ -33,6 +33,9 @@ void set_fuzz_target_args(int argc, char **argv)
         fuzz_target->main_argv = argv;
     }
 }
+char *get_fuzz_target_name(void){
+    return fuzz_target->name->str;
+}
 
 void reboot(QTestState *s)
 {
diff --git a/tests/fuzz/fuzz.h b/tests/fuzz/fuzz.h
index 73af029c82..d9d0bf11a8 100644
--- a/tests/fuzz/fuzz.h
+++ b/tests/fuzz/fuzz.h
@@ -19,6 +19,7 @@ typedef struct FuzzTarget {
 } FuzzTarget;
 
 void set_fuzz_target_args(int argc, char **argv);
+char *get_fuzz_target_name(void);
 void reboot(QTestState *);
 void fuzz_add_target(const char *name, const char *description, FuzzTarget
         *target);
-- 
2.23.0

Re: [Qemu-devel] [PATCH v3 18/22] fuzz: expose fuzz target name

[Darren Kenny]

On Wed, Sep 18, 2019 at 11:19:45PM +0000, Oleinik, Alexander wrote:
>This is needed for the qos-assisted fuzzers which walk the qos tree and
>need a way to check if the current path matches the name of the fuzz
>target.
>
>Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
>---
> tests/fuzz/fuzz.c | 3 +++
> tests/fuzz/fuzz.h | 1 +
> 2 files changed, 4 insertions(+)
>
>diff --git a/tests/fuzz/fuzz.c b/tests/fuzz/fuzz.c
>index 833f436731..d44dab7360 100644
>--- a/tests/fuzz/fuzz.c
>+++ b/tests/fuzz/fuzz.c
>@@ -33,6 +33,9 @@ void set_fuzz_target_args(int argc, char **argv)
>         fuzz_target->main_argv = argv;
>     }
> }
>+char *get_fuzz_target_name(void){

To avoid inadvertent changes you might want to make this 'const
char*' as the return type.

Thanks,

Darren.

>+    return fuzz_target->name->str;
>+}
>
> void reboot(QTestState *s)
> {
>diff --git a/tests/fuzz/fuzz.h b/tests/fuzz/fuzz.h
>index 73af029c82..d9d0bf11a8 100644
>--- a/tests/fuzz/fuzz.h
>+++ b/tests/fuzz/fuzz.h
>@@ -19,6 +19,7 @@ typedef struct FuzzTarget {
> } FuzzTarget;
>
> void set_fuzz_target_args(int argc, char **argv);
>+char *get_fuzz_target_name(void);
> void reboot(QTestState *);
> void fuzz_add_target(const char *name, const char *description, FuzzTarget
>         *target);
>-- 
>2.23.0
>
>

[Qemu-devel] [PATCH v3 19/22] fuzz: add support for qos-assisted fuzz targets

[Oleinik, Alexander]

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 tests/fuzz/qos_fuzz.c | 212 ++++++++++++++++++++++++++++++++++++++++++
 tests/fuzz/qos_fuzz.h |  19 ++++
 2 files changed, 231 insertions(+)
 create mode 100644 tests/fuzz/qos_fuzz.c
 create mode 100644 tests/fuzz/qos_fuzz.h

diff --git a/tests/fuzz/qos_fuzz.c b/tests/fuzz/qos_fuzz.c
new file mode 100644
index 0000000000..b08c8b37ba
--- /dev/null
+++ b/tests/fuzz/qos_fuzz.c
@@ -0,0 +1,212 @@
+#include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "exec/memory.h"
+#include "exec/address-spaces.h"
+#include "sysemu/sysemu.h"
+#include "qemu/main-loop.h"
+
+#include <wordexp.h>
+
+#include "libqos/malloc.h"
+#include "libqos/qgraph.h"
+#include "libqos/qgraph_internal.h"
+
+#include "fuzz.h"
+#include "qos_fuzz.h"
+#include "tests/libqos/qgraph.h"
+#include "tests/libqos/qos_external.h"
+#include "tests/libqtest.h"
+
+#include "qapi/qapi-commands-machine.h"
+#include "qapi/qapi-commands-qom.h"
+#include "qapi/qmp/qlist.h"
+
+/*
+ * Replaced the qmp commands with direct qmp_marshal calls.
+ * Probably there is a better way to do this
+ */
+static void qos_set_machines_devices_available(void)
+{
+    QDict *req = qdict_new();
+    QObject *response;
+    QDict *args = qdict_new();
+    QList *lst;
+    Error *err = NULL;
+
+    qmp_marshal_query_machines(NULL, &response, &err);
+    assert(!err);
+    lst = qobject_to(QList, response);
+    apply_to_qlist(lst, true);
+
+    qobject_unref(response);
+
+
+    qdict_put_str(req, "execute", "qom-list-types");
+    qdict_put_str(args, "implements", "device");
+    qdict_put_bool(args, "abstract", true);
+    qdict_put_obj(req, "arguments", (QObject *) args);
+
+    qmp_marshal_qom_list_types(args, &response, &err);
+    assert(!err);
+    lst = qobject_to(QList, response);
+    apply_to_qlist(lst, false);
+    qobject_unref(response);
+    qobject_unref(req);
+}
+
+void *fuzz_qos_obj;
+QGuestAllocator *fuzz_qos_alloc;
+
+static char **fuzz_path_vec;
+static char **current_path;
+
+
+void *qos_allocate_objects(QTestState *qts, QGuestAllocator **p_alloc)
+{
+    return allocate_objects(qts, current_path + 1, p_alloc);
+}
+
+
+static void qos_build_main_args()
+{
+    char **path = fuzz_path_vec;
+    QOSGraphNode *test_node;
+    GString *cmd_line = g_string_new(path[0]);
+    void *test_arg;
+
+    /* Before test */
+    current_path = path;
+    test_node = qos_graph_get_node(path[(g_strv_length(path) - 1)]);
+    test_arg = test_node->u.test.arg;
+    if (test_node->u.test.before) {
+        test_arg = test_node->u.test.before(cmd_line, test_arg);
+    }
+    /* Prepend the arguments that we need */
+    g_string_prepend(cmd_line,
+            "qemu-system-i386 -display none -machine accel=qtest -m 128 ");
+    wordexp_t result;
+    wordexp(cmd_line->str, &result, 0);
+    set_fuzz_target_args(result.we_wordc, result.we_wordv);
+
+    g_string_free(cmd_line, true);
+}
+
+/*
+ * This function is largely a copy of qos-test.c:walk_path. Since walk_path
+ * is itself a callback, its a little annoying to add another argument/layer of
+ * indirection
+ */
+static void walk_path(QOSGraphNode *orig_path, int len)
+{
+    QOSGraphNode *path;
+    QOSGraphEdge *edge;
+
+    /* etype set to QEDGE_CONSUMED_BY so that machine can add to the command line */
+    QOSEdgeType etype = QEDGE_CONSUMED_BY;
+
+    /* twice QOS_PATH_MAX_ELEMENT_SIZE since each edge can have its arg */
+    char **path_vec = g_new0(char *, (QOS_PATH_MAX_ELEMENT_SIZE * 2));
+    int path_vec_size = 0;
+
+    char *after_cmd, *before_cmd, *after_device;
+    GString *after_device_str = g_string_new("");
+    char *node_name = orig_path->name, *path_str;
+
+    GString *cmd_line = g_string_new("");
+    GString *cmd_line2 = g_string_new("");
+
+    path = qos_graph_get_node(node_name); /* root */
+    node_name = qos_graph_edge_get_dest(path->path_edge); /* machine name */
+
+    path_vec[path_vec_size++] = node_name;
+    path_vec[path_vec_size++] = qos_get_machine_type(node_name);
+
+    for (;;) {
+        path = qos_graph_get_node(node_name);
+        if (!path->path_edge) {
+            break;
+        }
+
+        node_name = qos_graph_edge_get_dest(path->path_edge);
+
+        /* append node command line + previous edge command line */
+        if (path->command_line && etype == QEDGE_CONSUMED_BY) {
+            g_string_append(cmd_line, path->command_line);
+            g_string_append(cmd_line, after_device_str->str);
+            g_string_truncate(after_device_str, 0);
+        }
+
+        path_vec[path_vec_size++] = qos_graph_edge_get_name(path->path_edge);
+        /* detect if edge has command line args */
+        after_cmd = qos_graph_edge_get_after_cmd_line(path->path_edge);
+        after_device = qos_graph_edge_get_extra_device_opts(path->path_edge);
+        before_cmd = qos_graph_edge_get_before_cmd_line(path->path_edge);
+        edge = qos_graph_get_edge(path->name, node_name);
+        etype = qos_graph_edge_get_type(edge);
+
+        if (before_cmd) {
+            g_string_append(cmd_line, before_cmd);
+        }
+        if (after_cmd) {
+            g_string_append(cmd_line2, after_cmd);
+        }
+        if (after_device) {
+            g_string_append(after_device_str, after_device);
+        }
+    }
+
+    path_vec[path_vec_size++] = NULL;
+    g_string_append(cmd_line, after_device_str->str);
+    g_string_free(after_device_str, true);
+
+    g_string_append(cmd_line, cmd_line2->str);
+    g_string_free(cmd_line2, true);
+
+    /*
+     * here position 0 has <arch>/<machine>, position 1 has <machine>.
+     * The path must not have the <arch>, qtest_add_data_func adds it.
+     */
+    path_str = g_strjoinv("/", path_vec + 1);
+
+    /* Check that this is the test we care about: */
+    char *test_name = strrchr(path_str, '/') + 1;
+    if (strcmp(test_name, get_fuzz_target_name()) == 0) {
+        /*
+         * put arch/machine in position 1 so run_one_test can do its work
+         * and add the command line at position 0.
+         */
+        path_vec[1] = path_vec[0];
+        path_vec[0] = g_string_free(cmd_line, false);
+
+        fuzz_path_vec = path_vec;
+    } else {
+        g_free(path_vec);
+    }
+
+    g_free(path_str);
+}
+
+void fuzz_add_qos_target(const char *name,
+        const char *description,
+        const char *interface,
+        QOSGraphTestOptions *opts,
+        FuzzTarget *fuzz_opts
+        )
+{
+    qos_add_test(name, interface, NULL, opts);
+    fuzz_add_target(name, description, fuzz_opts);
+}
+
+/* Do what is normally done in qos_test.c:main */
+void qos_setup(void){
+    qos_set_machines_devices_available();
+    qos_graph_foreach_test_path(walk_path);
+    qos_build_main_args();
+}
+
+void qos_init_path(QTestState *s)
+{
+    fuzz_qos_obj = qos_allocate_objects(s , &fuzz_qos_alloc);
+}
diff --git a/tests/fuzz/qos_fuzz.h b/tests/fuzz/qos_fuzz.h
new file mode 100644
index 0000000000..4ae775972a
--- /dev/null
+++ b/tests/fuzz/qos_fuzz.h
@@ -0,0 +1,19 @@
+#ifndef _QOS_FUZZ_H_
+#define _QOS_FUZZ_H_
+
+#include "tests/libqos/qgraph.h"
+
+int qos_fuzz(const unsigned char *Data, size_t Size);
+void qos_setup(void);
+
+extern void *fuzz_qos_obj;
+extern QGuestAllocator *fuzz_qos_alloc;
+
+void fuzz_add_qos_target(const char *name,
+        const char *description,
+        const char *interface,
+        QOSGraphTestOptions *opts,
+        FuzzTarget *fuzz_opts);
+
+void qos_init_path(QTestState *);
+#endif
-- 
2.23.0

[Qemu-devel] [PATCH v3 20/22] fuzz: add i440fx fuzz targets

[Oleinik, Alexander]

These three targets should simply fuzz reads/writes to a couple ioports,
but they mostly serve as examples of different ways to write targets.
They demonstrate using qtest and qos for fuzzing, as well as using
rebooting and forking to reset state, or not resetting it at all.

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 tests/fuzz/Makefile.include |   3 +
 tests/fuzz/i440fx_fuzz.c    | 158 ++++++++++++++++++++++++++++++++++++
 2 files changed, 161 insertions(+)
 create mode 100644 tests/fuzz/i440fx_fuzz.c

diff --git a/tests/fuzz/Makefile.include b/tests/fuzz/Makefile.include
index 687dacce04..37d6821bee 100644
--- a/tests/fuzz/Makefile.include
+++ b/tests/fuzz/Makefile.include
@@ -3,5 +3,8 @@ fuzz-obj-y = $(libqos-obj-y)
 fuzz-obj-y += tests/libqtest.o
 fuzz-obj-y += tests/fuzz/fuzz.o
 fuzz-obj-y += tests/fuzz/fork_fuzz.o
+fuzz-obj-y += tests/fuzz/qos_fuzz.o
+
+fuzz-obj-y += tests/fuzz/i440fx_fuzz.o
 
 FUZZ_LDFLAGS += -Xlinker -T$(SRC_PATH)/tests/fuzz/fork_fuzz.ld
diff --git a/tests/fuzz/i440fx_fuzz.c b/tests/fuzz/i440fx_fuzz.c
new file mode 100644
index 0000000000..9079c40f55
--- /dev/null
+++ b/tests/fuzz/i440fx_fuzz.c
@@ -0,0 +1,158 @@
+#include "qemu/osdep.h"
+
+#include "fuzz.h"
+#include "tests/libqtest.h"
+#include "fuzz/qos_fuzz.h"
+#include "fuzz/fork_fuzz.h"
+#include "qemu/main-loop.h"
+#include "tests/libqos/pci.h"
+#include "tests/libqos/pci-pc.h"
+
+enum action_id {
+    WRITEB,
+    WRITEW,
+    WRITEL,
+    READB,
+    READW,
+    READL,
+};
+
+static void i440fx_fuzz_qtest(QTestState *s,
+        const unsigned char *Data, size_t Size) {
+
+    typedef struct QTestFuzzAction {
+        uint8_t id;
+        uint8_t addr;
+        uint32_t value;
+    } QTestFuzzAction;
+    QTestFuzzAction *a = (QTestFuzzAction *)Data;
+    while (Size >= sizeof(QTestFuzzAction)) {
+        uint16_t addr = a->addr % 2 ? 0xcf8 : 0xcfc;
+        switch (a->id) {
+        case WRITEB:
+            qtest_outb(s, addr, (uint8_t)a->value);
+            break;
+        case WRITEW:
+            qtest_outw(s, addr, (uint16_t)a->value);
+            break;
+        case WRITEL:
+            qtest_outl(s, addr, (uint32_t)a->value);
+            break;
+        case READB:
+            qtest_inb(s, addr);
+            break;
+        case READW:
+            qtest_inw(s, addr);
+            break;
+        case READL:
+            qtest_inl(s, addr);
+            break;
+        }
+        a++;
+        Size -= sizeof(QTestFuzzAction);
+    }
+    qtest_clock_step_next(s);
+    main_loop_wait(true);
+    reboot(s);
+}
+
+static void i440fx_fuzz_qos(QTestState *s,
+        const unsigned char *Data, size_t Size) {
+
+    typedef struct QOSFuzzAction {
+        uint8_t id;
+        int devfn;
+        uint8_t offset;
+        uint32_t value;
+    } QOSFuzzAction;
+
+    QOSFuzzAction *a = (QOSFuzzAction *)Data;
+    static QPCIBus *bus;
+    if (!bus) {
+        bus = qpci_new_pc(s, fuzz_qos_alloc);
+    }
+
+    while (Size >= sizeof(QOSFuzzAction)) {
+        switch (a->id) {
+        case WRITEB:
+            bus->config_writeb(bus, a->devfn, a->offset, (uint8_t)a->value);
+            break;
+        case WRITEW:
+            bus->config_writew(bus, a->devfn, a->offset, (uint16_t)a->value);
+            break;
+        case WRITEL:
+            bus->config_writel(bus, a->devfn, a->offset, (uint32_t)a->value);
+            break;
+        case READB:
+            bus->config_readb(bus, a->devfn, a->offset);
+            break;
+        case READW:
+            bus->config_readw(bus, a->devfn, a->offset);
+            break;
+        case READL:
+            bus->config_readl(bus, a->devfn, a->offset);
+            break;
+        }
+        a++;
+        Size -= sizeof(QOSFuzzAction);
+    }
+    qtest_clock_step_next(s);
+    main_loop_wait(true);
+}
+
+static void i440fx_fuzz_qos_fork(QTestState *s,
+        const unsigned char *Data, size_t Size) {
+    if (fork() == 0) {
+        i440fx_fuzz_qos(s, Data, Size);
+        counter_shm_store();
+        _Exit(0);
+    } else {
+        wait(NULL);
+        counter_shm_load();
+    }
+}
+
+static void fork_init(QTestState *s)
+{
+    counter_shm_init();
+}
+static const char *i440fx_qtest_argv[] = {"qemu_system_i386", "-machine", "accel=qtest"};
+
+static void register_pci_fuzz_targets(void)
+{
+    /* Uses simple qtest commands and reboots to reset state */
+    fuzz_add_target("i440fx-qtest-reboot-fuzz",
+            "Fuzz the i440fx using raw qtest commands and rebooting"
+            "after each run",
+            &(FuzzTarget){
+                .fuzz = i440fx_fuzz_qtest,
+                .main_argc = 3,
+                .main_argv = (char **)i440fx_qtest_argv,
+                });
+
+    /* Uses libqos and forks to prevent state leakage */
+    fuzz_add_qos_target("i440fx-qos-fork-fuzz",
+            "Fuzz the i440fx using qos helpers and forking"
+            "for each run",
+            "i440FX-pcihost",
+            &(QOSGraphTestOptions){},
+            &(FuzzTarget){
+            .pre_main = &qos_setup,
+            .pre_fuzz = &fork_init,
+            .fuzz = &i440fx_fuzz_qos_fork
+            });
+
+    /* Uses libqos. Doesn't do anything to reset state. Note that if we were to
+     reboot after each run, we would also have to redo the qos-related
+     initialization (qos_init_path) */
+    fuzz_add_qos_target("i440fx-qos-nocleanup-fuzz",
+            "Fuzz the i440fx using qos helpers. No cleanup done after each run",
+            "i440FX-pcihost",
+            &(QOSGraphTestOptions){},
+            &(FuzzTarget){
+            .pre_main = &qos_setup,
+            .fuzz = &i440fx_fuzz_qos
+            });
+}
+
+fuzz_target_init(register_pci_fuzz_targets);
-- 
2.23.0

Re: [Qemu-devel] [PATCH v3 20/22] fuzz: add i440fx fuzz targets

[Stefan Hajnoczi]

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

On Wed, Sep 18, 2019 at 11:19:47PM +0000, Oleinik, Alexander wrote:
> +static void i440fx_fuzz_qtest(QTestState *s,
> +        const unsigned char *Data, size_t Size) {
> +
> +    typedef struct QTestFuzzAction {
> +        uint8_t id;
> +        uint8_t addr;
> +        uint32_t value;
> +    } QTestFuzzAction;

I'm concerned about padding within the struct and struct alignment
causing us to skip some bytes in Data[].  Also, on some architectures
unaligned memory accesses are unsupported so memcpy() is safer than
casting a pointer directly into Data[].

The question is whether skipping bytes in Data[] matters.
Feedback-directed fuzzing should realize that certain offsets in Data[]
are unused and do not affect the input space.  Still, I think we should
arrange things so that every bit in Data[] matters as much as possible.

The struct can be arranged to avoid struct field padding:

  uint32_t value;
  uint8_t id;
  uint8_t addr;

> +    QTestFuzzAction *a = (QTestFuzzAction *)Data;
> +    while (Size >= sizeof(QTestFuzzAction)) {

To avoid struct alignment issues:

  QTestFuzzAction a;
  while (Size >= sizeof(a)) {
      memcpy(&a, Data, sizeof(a));
      ...
      Data += sizeof(a);
  }

> +        uint16_t addr = a->addr % 2 ? 0xcf8 : 0xcfc;

Please define constants for these magic numbers (e.g.
I440FX_PCI_HOST_BRIDGE_CFG, I440FX_PCI_HOST_BRIDGE_DATA).

> +        switch (a->id) {

How about:

  switch (a->id % ACTION_MAX) {

This way we always exercise a useful code path instead of just skipping
the input.

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

[Qemu-devel] [PATCH v3 21/22] fuzz: add virtio-net fuzz target

[Oleinik, Alexander]

The virtio-net fuzz target feeds inputs to all three virtio-net
virtqueues, and uses forking to avoid leaking state between fuzz runs.

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 tests/fuzz/Makefile.include  |   1 +
 tests/fuzz/virtio_net_fuzz.c | 120 +++++++++++++++++++++++++++++++++++
 2 files changed, 121 insertions(+)
 create mode 100644 tests/fuzz/virtio_net_fuzz.c

diff --git a/tests/fuzz/Makefile.include b/tests/fuzz/Makefile.include
index 37d6821bee..f1d9b46b1c 100644
--- a/tests/fuzz/Makefile.include
+++ b/tests/fuzz/Makefile.include
@@ -6,5 +6,6 @@ fuzz-obj-y += tests/fuzz/fork_fuzz.o
 fuzz-obj-y += tests/fuzz/qos_fuzz.o
 
 fuzz-obj-y += tests/fuzz/i440fx_fuzz.o
+fuzz-obj-y += tests/fuzz/virtio_net_fuzz.o
 
 FUZZ_LDFLAGS += -Xlinker -T$(SRC_PATH)/tests/fuzz/fork_fuzz.ld
diff --git a/tests/fuzz/virtio_net_fuzz.c b/tests/fuzz/virtio_net_fuzz.c
new file mode 100644
index 0000000000..2f6e14b06d
--- /dev/null
+++ b/tests/fuzz/virtio_net_fuzz.c
@@ -0,0 +1,120 @@
+#include "qemu/osdep.h"
+
+#include "qemu/main-loop.h"
+
+#include "fuzz.h"
+#include "fork_fuzz.h"
+#include "qos_fuzz.h"
+#include "tests/libqtest.h"
+#include "tests/libqos/virtio-net.h"
+
+
+
+static void virtio_net_ctrl_fuzz_multi(QTestState *s,
+        const unsigned char *Data, size_t Size)
+{
+    typedef struct vq_action {
+        uint8_t queue;
+        uint8_t length;
+        uint8_t write;
+        uint8_t next;
+        bool kick;
+    } vq_action;
+
+    uint64_t req_addr[10];
+    int reqi = 0;
+    uint32_t free_head = 0;
+
+    QGuestAllocator *t_alloc = fuzz_qos_alloc;
+
+    QVirtioNet *net_if = fuzz_qos_obj;
+    QVirtioDevice *dev = net_if->vdev;
+    QVirtQueue *q;
+    vq_action vqa;
+    int iters = 0;
+    while (true) {
+        if (Size < sizeof(vqa)) {
+            break;
+        }
+        vqa = *((vq_action *)Data);
+        Data += sizeof(vqa);
+        Size -= sizeof(vqa);
+
+        q = net_if->queues[vqa.queue % 3];
+
+        vqa.length = vqa.length >= Size ? Size :  vqa.length;
+
+        req_addr[reqi] = guest_alloc(t_alloc, vqa.length);
+        qtest_memwrite(s, req_addr[reqi], Data, vqa.length);
+        if (iters == 0) {
+            free_head = qvirtqueue_add(s, q, req_addr[reqi], vqa.length,
+                    vqa.write, vqa.next);
+        } else {
+            qvirtqueue_add(s, q,
+                    req_addr[reqi], vqa.length, vqa.write , vqa.next);
+        }
+        iters++;
+        reqi++;
+        if (iters == 10) {
+            break;
+        }
+        Data += vqa.length;
+        Size -= vqa.length;
+    }
+    if (iters) {
+        qvirtqueue_kick(s, dev, q, free_head);
+        qtest_clock_step_next(s);
+        main_loop_wait(false);
+        for (int i = 0; i < reqi; i++) {
+            guest_free(t_alloc, req_addr[i]);
+        }
+    }
+}
+
+static int *sv;
+
+static void *virtio_net_test_setup_socket(GString *cmd_line, void *arg)
+{
+    if (!sv) {
+        sv = g_new(int, 2);
+        int ret = socketpair(PF_UNIX, SOCK_STREAM, 0, sv);
+        fcntl(sv[0], F_SETFL, O_NONBLOCK);
+        g_assert_cmpint(ret, !=, -1);
+    }
+    g_string_append_printf(cmd_line, " -netdev socket,fd=%d,id=hs0 ", sv[1]);
+    return arg;
+}
+
+static void virtio_net_fork_fuzz(QTestState *s,
+        const unsigned char *Data, size_t Size)
+{
+    if (fork() == 0) {
+        virtio_net_ctrl_fuzz_multi(s, Data, Size);
+        counter_shm_store();
+        _Exit(0);
+    } else {
+        wait(NULL);
+        counter_shm_load();
+    }
+}
+
+static void fork_init(QTestState *s)
+{
+    qos_init_path(s);
+    counter_shm_init();
+}
+
+static void register_virtio_net_fuzz_targets(void)
+{
+    fuzz_add_qos_target("virtio-net-fork-fuzz",
+            "Fuzz the virtio-net virtual queues, forking for each fuzz run",
+            "virtio-net",
+            &(QOSGraphTestOptions){.before = virtio_net_test_setup_socket},
+            &(FuzzTarget){
+            .pre_main = &qos_setup,
+            .pre_fuzz = &fork_init,
+            .fuzz = &virtio_net_fork_fuzz
+            });
+}
+
+fuzz_target_init(register_virtio_net_fuzz_targets);
-- 
2.23.0

[Qemu-devel] [PATCH v3 22/22] fuzz: add documentation to docs/devel/

[Oleinik, Alexander]

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 docs/devel/fuzzing.txt | 114 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 114 insertions(+)
 create mode 100644 docs/devel/fuzzing.txt

diff --git a/docs/devel/fuzzing.txt b/docs/devel/fuzzing.txt
new file mode 100644
index 0000000000..53a1f858f5
--- /dev/null
+++ b/docs/devel/fuzzing.txt
@@ -0,0 +1,114 @@
+= Fuzzing =
+
+== Introduction ==
+
+This document describes the virtual-device fuzzing infrastructure in QEMU and
+how to use it to implement additional fuzzers.
+
+== Basics ==
+
+Fuzzing operates by passing inputs to an entry point/target function. The
+fuzzer tracks the code coverage triggered by the input. Based on these
+findings, the fuzzer mutates the input and repeats the fuzzing. 
+
+To fuzz QEMU, we rely on libfuzzer. Unlike other fuzzers such as AFL, libfuzzer
+is an _in-process_ fuzzer. For the developer, this means that it is their
+responsibility to ensure that state is reset between fuzzing-runs.
+
+== Building the fuzzers ==
+
+NOTE: If possible, build a 32-bit binary. When forking, the page map ends up
+being much smaller. On 64-bit AddressSanitizer consumes 20 Terabytes of virtual
+memory.
+
+To build the fuzzers, install a recent version of clang:
+Configure with (substitute the clang binaries with the version you installed):
+
+    CC=clang-8 CXX=clang++-8 /path/to/configure --enable-fuzzing
+
+Fuzz targets are built similarily to system/softmmu:
+
+    make i386-softmmu/fuzz
+
+This builds ./i386-softmmu/qemu-fuzz-i386
+
+The first option to this command is: --FUZZER_NAME
+To list all of the available fuzzers run qemu-fuzz-i386 with no arguments.
+
+Libfuzzer parses all arguments that do not begin with "--". Information about
+these is available by passing -help=1
+
+Now the only thing left to do is wait for the fuzzer to trigger potential
+crashes.
+
+== Adding a new fuzzer ==
+Coverage over virtual devices can be improved by adding additional fuzzers. 
+Fuzzers are kept in tests/fuzz/ and should be added to
+tests/fuzz/Makefile.include
+
+Fuzzers can rely on both qtest and libqos to communicate with virtual devices.
+
+1. Create a new source file. For example ``tests/fuzz/fuzz-foo-device.c``.
+
+2. Write the fuzzing code using the libqtest/libqos API. See existing fuzzers
+for reference.
+
+3. Register the fuzzer in ``tests/fuzz/Makefile.include`` by appending the
+corresponding object to fuzz-obj-y
+
+Fuzzers can be more-or-less thought of as special qtest programs which can
+modify the qtest commands and/or qtest command arguments based on inputs
+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.
+
+
+= Implmentation Details =
+
+== The Fuzzer's Lifecycle ==
+
+The fuzzer has two entrypoints that libfuzzer calls.
+
+LLVMFuzzerInitialize: called prior to fuzzing. Used to initialize all of the
+necessary state
+
+LLVMFuzzerTestOneInput: called for each fuzzing run. Processes the input and
+resets the state at the end of each run.
+
+In more detail:
+
+LLVMFuzzerInitialize parses the arguments to the fuzzer (must start with two
+dashes, so they are ignored by libfuzzer main()). Currently, the arguments
+select the fuzz target. Then, the qtest client is initialized. If the target
+requires qos, qgraph is set up and the QOM/LIBQOS modules are initailized.
+Then the QGraph is walked and the QEMU cmd_line is determined and saved.
+
+After this, the vl.c:real_main is called to set up the guest. After this, the
+fuzzer saves the initial vm/device state to ram, after which the initilization
+is complete.
+
+LLVMFuzzerTestOneInput: Uses qtest/qos functions to act based on the fuzz
+input. It is also responsible for manually calling the main loop/main_loop_wait
+to ensure that bottom halves are executed. Finally, it calls reset() which
+restores state from the ramfile and/or resets the guest.
+
+
+Since the same process is reused for many fuzzing runs, QEMU state needs to
+be reset at the end of each run. There are currently two implemented
+options for resetting state: 
+1. Reboot the guest between runs.
+   Pros: Straightforward and fast for simple fuzz targets. 
+   Cons: Depending on the device, does not reset all device state. If the
+   device requires some initialization prior to being ready for fuzzing
+   (common for QOS-based targets), this initialization needs to be done after
+   each reboot.
+   Example target: --virtio-net-ctrl-fuzz
+2. Run each test case in a separate forked process and copy the coverage
+   information back to the parent. This is fairly similar to AFL's "deferred"
+   fork-server mode [3]
+   Pros: Relatively fast. Devices only need to be initialized once. No need
+   to do slow reboots or vmloads.
+   Cons: Not officially supported by libfuzzer and the implementation is very
+   flimsy. Does not work well for devices that rely on dedicated threads.
+   Example target: --qtest-fork-fuzz
+
-- 
2.23.0

Re: [Qemu-devel] [PATCH v3 22/22] fuzz: add documentation to docs/devel/

[Darren Kenny]

Hi Alexander,

Some comments, and questions below...

On Wed, Sep 18, 2019 at 11:19:48PM +0000, Oleinik, Alexander wrote:
>Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
>---
> docs/devel/fuzzing.txt | 114 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 114 insertions(+)
> create mode 100644 docs/devel/fuzzing.txt
>
>diff --git a/docs/devel/fuzzing.txt b/docs/devel/fuzzing.txt
>new file mode 100644
>index 0000000000..53a1f858f5
>--- /dev/null
>+++ b/docs/devel/fuzzing.txt
>@@ -0,0 +1,114 @@
>+= Fuzzing =
>+
>+== Introduction ==
>+
>+This document describes the virtual-device fuzzing infrastructure in QEMU and
>+how to use it to implement additional fuzzers.
>+
>+== Basics ==
>+
>+Fuzzing operates by passing inputs to an entry point/target function. The
>+fuzzer tracks the code coverage triggered by the input. Based on these
>+findings, the fuzzer mutates the input and repeats the fuzzing.
>+
>+To fuzz QEMU, we rely on libfuzzer. Unlike other fuzzers such as AFL, libfuzzer
>+is an _in-process_ fuzzer. For the developer, this means that it is their
>+responsibility to ensure that state is reset between fuzzing-runs.
>+
>+== Building the fuzzers ==
>+
>+NOTE: If possible, build a 32-bit binary. When forking, the page map ends up
>+being much smaller. On 64-bit AddressSanitizer consumes 20 Terabytes of virtual
>+memory.

It might be worth having a little more on this, since I would
imagine most people would be more interested in 64-bit these days,
but are unlikely to have 20Tb available :)

But if there is little to be gained by the 64-bit vs 32-bit if
fuzzing a device, then it might be worth explaining why.

>+
>+To build the fuzzers, install a recent version of clang:
>+Configure with (substitute the clang binaries with the version you installed):
>+
>+    CC=clang-8 CXX=clang++-8 /path/to/configure --enable-fuzzing
>+
>+Fuzz targets are built similarily to system/softmmu:

Typo: s/similarily/similarly/

>+
>+    make i386-softmmu/fuzz
>+
>+This builds ./i386-softmmu/qemu-fuzz-i386

Reading this, it kind of implies that it should be also possible to
build i386-softmmu/all still - which from what I can see it isn't,
the build fails with an undefined 'main' - most likely the main.o
object is not being linked in for this target?

Unfortunately, also doing the above build command is also failing
for me after applying this set of patches:

  qemu-upstream-libfuzz/tests/fuzz/fuzz.c:149:28: error: expected expression
      fuzz_qts = qtest_setup(void);
                             ^
  1 error generated.
  make[1]: *** [tests/fuzz/fuzz.o] Error 1

This is due to the (void) in the calling of the function, will comment on the
patch separately.

>+
>+The first option to this command is: --FUZZER_NAME
>+To list all of the available fuzzers run qemu-fuzz-i386 with no arguments.
>+
>+Libfuzzer parses all arguments that do not begin with "--". Information about
>+these is available by passing -help=1
>+
>+Now the only thing left to do is wait for the fuzzer to trigger potential
>+crashes.

I would suggest adding an example of doing a run for this here.

>+
>+== Adding a new fuzzer ==
>+Coverage over virtual devices can be improved by adding additional fuzzers.
>+Fuzzers are kept in tests/fuzz/ and should be added to
>+tests/fuzz/Makefile.include
>+
>+Fuzzers can rely on both qtest and libqos to communicate with virtual devices.
>+
>+1. Create a new source file. For example ``tests/fuzz/fuzz-foo-device.c``.
>+
>+2. Write the fuzzing code using the libqtest/libqos API. See existing fuzzers
>+for reference.
>+
>+3. Register the fuzzer in ``tests/fuzz/Makefile.include`` by appending the
>+corresponding object to fuzz-obj-y
>+
>+Fuzzers can be more-or-less thought of as special qtest programs which can
>+modify the qtest commands and/or qtest command arguments based on inputs
>+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.
>+
>+
>+= Implmentation Details =
>+
>+== The Fuzzer's Lifecycle ==
>+
>+The fuzzer has two entrypoints that libfuzzer calls.
>+
>+LLVMFuzzerInitialize: called prior to fuzzing. Used to initialize all of the
>+necessary state
>+
>+LLVMFuzzerTestOneInput: called for each fuzzing run. Processes the input and
>+resets the state at the end of each run.

It might be worth explicitly mentioning that Libfuzzer provides it's
own main() function which calls these.

>+
>+In more detail:
>+
>+LLVMFuzzerInitialize parses the arguments to the fuzzer (must start with two
>+dashes, so they are ignored by libfuzzer main()). Currently, the arguments
>+select the fuzz target. Then, the qtest client is initialized. If the target
>+requires qos, qgraph is set up and the QOM/LIBQOS modules are initailized.
>+Then the QGraph is walked and the QEMU cmd_line is determined and saved.
>+
>+After this, the vl.c:real_main is called to set up the guest. After this, the
>+fuzzer saves the initial vm/device state to ram, after which the initilization
>+is complete.

Is this still true with this patchset? I don't see real_main being
defined any more in vl.c.

>+
>+LLVMFuzzerTestOneInput: Uses qtest/qos functions to act based on the fuzz
>+input. It is also responsible for manually calling the main loop/main_loop_wait
>+to ensure that bottom halves are executed. Finally, it calls reset() which
>+restores state from the ramfile and/or resets the guest.
>+
>+
>+Since the same process is reused for many fuzzing runs, QEMU state needs to
>+be reset at the end of each run. There are currently two implemented
>+options for resetting state:
>+1. Reboot the guest between runs.
>+   Pros: Straightforward and fast for simple fuzz targets.
>+   Cons: Depending on the device, does not reset all device state. If the
>+   device requires some initialization prior to being ready for fuzzing
>+   (common for QOS-based targets), this initialization needs to be done after
>+   each reboot.
>+   Example target: --virtio-net-ctrl-fuzz
>+2. Run each test case in a separate forked process and copy the coverage
>+   information back to the parent. This is fairly similar to AFL's "deferred"
>+   fork-server mode [3]
>+   Pros: Relatively fast. Devices only need to be initialized once. No need
>+   to do slow reboots or vmloads.
>+   Cons: Not officially supported by libfuzzer and the implementation is very
>+   flimsy. Does not work well for devices that rely on dedicated threads.
>+   Example target: --qtest-fork-fuzz
>+

Thanks,

Darren.

Re: [Qemu-devel] [PATCH v3 00/22] Add virtual device fuzzing support

[Stefan Hajnoczi]

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

On Wed, Sep 18, 2019 at 11:19:27PM +0000, Oleinik, Alexander wrote:
>  create mode 100644 docs/devel/fuzzing.txt
>  create mode 100644 main.c
>  create mode 100644 tests/fuzz/Makefile.include
>  create mode 100644 tests/fuzz/fork_fuzz.c
>  create mode 100644 tests/fuzz/fork_fuzz.h
>  create mode 100644 tests/fuzz/fork_fuzz.ld
>  create mode 100644 tests/fuzz/fuzz.c
>  create mode 100644 tests/fuzz/fuzz.h
>  create mode 100644 tests/fuzz/i440fx_fuzz.c
>  create mode 100644 tests/fuzz/qos_fuzz.c
>  create mode 100644 tests/fuzz/qos_fuzz.h
>  create mode 100644 tests/fuzz/virtio_net_fuzz.c
>  create mode 100644 tests/libqos/qos_external.c
>  create mode 100644 tests/libqos/qos_external.h

Please ensure that all new files have copyright/license headers.  When
code was moved from an old file, use the old file's copyright/license.

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

Re: [Qemu-devel] [PATCH v3 00/22] Add virtual device fuzzing support

[Stefan Hajnoczi]

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

On Wed, Sep 18, 2019 at 11:19:27PM +0000, Oleinik, Alexander wrote:
> This series adds a framework for coverage-guided fuzzing of
> virtual-devices. Fuzzing targets are based on qtest and can make use of
> the libqos abstractions.
> 
> Build instructions in docs/devel/fuzzing.txt
> 
> V3:
>  * Rebased onto v4.1.0+
>  * Add the fuzzer as a new build-target type in the build-system
>  * Add indirection to qtest client/server communication functions
>  * Remove ramfile and snapshot-based fuzzing support
>  * Add i440fx fuzz-target as a reference for developers.
>  * Add linker-script to assist with fork-based fuzzer

I have done an initial review, mostly skipping Makefile changes.
Thanks!

Stefan

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

Re: [Qemu-devel] [PATCH v3 00/22] Add virtual device fuzzing support

[no-reply]

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



Hi,

This series failed the asan 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-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

clang -iquote /tmp/qemu-test/build/tests -iquote tests -iquote /tmp/qemu-test/src/tcg -iquote /tmp/qemu-test/src/tcg/i386 -I/tmp/qemu-test/src/linux-headers -I/tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -I/usr/include/pixman-1  -I/tmp/qemu-test/src/dtc/libfdt -Werror  -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include  -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99  -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong  -I/usr/include/p11-kit-1     -I/usr/include/libpng16  -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1   -I/tmp/qemu-test/src/tests -MMD -MP -MT tests/test-authz-simple.o -MF tests/test-authz-simple.d -fsanitize=undefined -fsanitize=address -g   -c -o tests/test-authz-simple.o /tmp/qemu-test/src/tests/test-authz-simple.c
clang -iquote /tmp/qemu-test/build/tests -iquote tests -iquote /tmp/qemu-test/src/tcg -iquote /tmp/qemu-test/src/tcg/i386 -I/tmp/qemu-test/src/linux-headers -I/tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -I/usr/include/pixman-1  -I/tmp/qemu-test/src/dtc/libfdt -Werror  -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include  -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99  -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong  -I/usr/include/p11-kit-1     -I/usr/include/libpng16  -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1   -I/tmp/qemu-test/src/tests -MMD -MP -MT tests/test-authz-list.o -MF tests/test-authz-list.d -fsanitize=undefined -fsanitize=address -g   -c -o tests/test-authz-list.o /tmp/qemu-test/src/tests/test-authz-list.c
clang -iquote /tmp/qemu-test/build/tests -iquote tests -iquote /tmp/qemu-test/src/tcg -iquote /tmp/qemu-test/src/tcg/i386 -I/tmp/qemu-test/src/linux-headers -I/tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -I/usr/include/pixman-1  -I/tmp/qemu-test/src/dtc/libfdt -Werror  -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include  -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99  -Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits -fstack-protector-strong  -I/usr/include/p11-kit-1     -I/usr/include/libpng16  -I/usr/include/spice-1 -I/usr/include/spice-server -I/usr/include/cacard -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/nss3 -I/usr/include/nspr4 -pthread -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/uuid -I/usr/include/pixman-1   -I/tmp/qemu-test/src/tests -MMD -MP -MT tests/test-authz-listfile.o -MF tests/test-authz-listfile.d -fsanitize=undefined -fsanitize=address -g   -c -o tests/test-authz-listfile.o /tmp/qemu-test/src/tests/test-authz-listfile.c
/tmp/qemu-test/src/tests/test-char.c:31:13: error: static declaration of 'main_loop' follows non-static declaration
static void main_loop(void)
            ^
/tmp/qemu-test/src/include/sysemu/sysemu.h:117:6: note: previous declaration is here


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

Re: [Qemu-devel] [PATCH v3 00/22] Add virtual device fuzzing support

[no-reply]

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



Hi,

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

Message-id: 20190918231846.22538-1-alxndr@bu.edu
Subject: [Qemu-devel] [PATCH v3 00/22] Add virtual device fuzzing support
Type: series

=== 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 ===

Switched to a new branch 'test'
5ad94a8 fuzz: add documentation to docs/devel/
042e172 fuzz: add virtio-net fuzz target
621539a fuzz: add i440fx fuzz targets
dae5ad1 fuzz: add support for qos-assisted fuzz targets
ab8dc3a fuzz: expose fuzz target name
79fdc72 fuzz: add support for fork-based fuzzing.
4b1ba3f fuzz: add fuzzer skeleton
fe16e71 fuzz: Add target/fuzz makefile rules
f95cc25 libqtest: add in-process qtest.c tx/rx handlers
6d9d7dc libqtest: make qtest_bufwrite send "atomic"
918fbde libqos: move useful qos-test funcs to qos_external
fd767d6 libqos: split qos-test and libqos makefile vars
5b787c6 tests: provide test variables to other targets
ddcaa47 qtest: add in-process incoming command handler
8eef87c module: check module wasn't already initialized
3d76929 fuzz: Add target/fuzz makefile rules
72f81a2 fuzz: add configure flag --enable-fuzzing
c8a9bd8 libqtest: Add a layer of abstraciton to send/recv
ccb41d8 qtest: add qtest_server_send abstraction
b556f51 fuzz: Add FUZZ_TARGET module type
4644c02 libqos: Rename i2c_send and i2c_recv
63655ea softmmu: split off vl.c:main() into main.c

=== OUTPUT BEGIN ===
1/22 Checking commit 63655ea72e20 (softmmu: split off vl.c:main() into main.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#59: 
new file mode 100644

WARNING: architecture specific defines should be avoided
#68: FILE: main.c:5:
+#if defined(__APPLE__) || defined(main)

ERROR: externs should be avoided in .c files
#70: FILE: main.c:7:
+int qemu_main(int argc, char **argv, char **envp);

total: 1 errors, 2 warnings, 114 lines checked

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

2/22 Checking commit 4644c02486b6 (libqos: Rename i2c_send and i2c_recv)
3/22 Checking commit b556f5132021 (fuzz: Add FUZZ_TARGET module type)
4/22 Checking commit ccb41d8c3bd1 (qtest: add qtest_server_send abstraction)
WARNING: line over 80 characters
#71: FILE: qtest.c:795:
+void qtest_server_set_tx_handler(void (*send)(void*, const char*, size_t), void *opaque)

total: 0 errors, 1 warnings, 49 lines checked

Patch 4/22 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/22 Checking commit c8a9bd8d9dd9 (libqtest: Add a layer of abstraciton to send/recv)
6/22 Checking commit 72f81a2600dc (fuzz: add configure flag --enable-fuzzing)
7/22 Checking commit 3d769292ca2e (fuzz: Add target/fuzz makefile rules)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#85: 
new file mode 100644

total: 0 errors, 1 warnings, 65 lines checked

Patch 7/22 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/22 Checking commit 8eef87cc2d23 (module: check module wasn't already initialized)
9/22 Checking commit ddcaa4751604 (qtest: add in-process incoming command handler)
10/22 Checking commit 5b787c6a0543 (tests: provide test variables to other targets)
11/22 Checking commit fd767d613cc4 (libqos: split qos-test and libqos makefile vars)
12/22 Checking commit 918fbde8252a (libqos: move useful qos-test funcs to qos_external)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#27: 
new file mode 100644

total: 0 errors, 1 warnings, 343 lines checked

Patch 12/22 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
13/22 Checking commit 6d9d7dc7f758 (libqtest: make qtest_bufwrite send "atomic")
14/22 Checking commit f95cc251da3e (libqtest: add in-process qtest.c tx/rx handlers)
15/22 Checking commit fe16e710bff0 (fuzz: Add target/fuzz makefile rules)
16/22 Checking commit 4b1ba3f2594e (fuzz: add fuzzer skeleton)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29: 
new file mode 100644

WARNING: Block comments use a leading /* on a separate line
#155: FILE: tests/fuzz/fuzz.c:122:
+    /* --trace is useful for outputting a log of qtest commands that trigger

WARNING: Block comments use a trailing */ on a separate line
#156: FILE: tests/fuzz/fuzz.c:123:
+     * a crash. The log can can then be replayed with a simple qtest script. */

total: 0 errors, 3 warnings, 215 lines checked

Patch 16/22 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
17/22 Checking commit 79fdc7221b97 (fuzz: add support for fork-based fuzzing.)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#49: 
new file mode 100644

total: 0 errors, 1 warnings, 100 lines checked

Patch 17/22 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
18/22 Checking commit ab8dc3a2c256 (fuzz: expose fuzz target name)
19/22 Checking commit dae5ad183400 (fuzz: add support for qos-assisted fuzz targets)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#11: 
new file mode 100644

WARNING: line over 80 characters
#121: FILE: tests/fuzz/qos_fuzz.c:106:
+    /* etype set to QEDGE_CONSUMED_BY so that machine can add to the command line */

total: 0 errors, 2 warnings, 231 lines checked

Patch 19/22 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
20/22 Checking commit 621539aa3b08 (fuzz: add i440fx fuzz targets)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29: 
new file mode 100644

WARNING: line over 80 characters
#152: FILE: tests/fuzz/i440fx_fuzz.c:119:
+static const char *i440fx_qtest_argv[] = {"qemu_system_i386", "-machine", "accel=qtest"};

WARNING: Block comments use a leading /* on a separate line
#178: FILE: tests/fuzz/i440fx_fuzz.c:145:
+    /* Uses libqos. Doesn't do anything to reset state. Note that if we were to

WARNING: Block comments use * on subsequent lines
#179: FILE: tests/fuzz/i440fx_fuzz.c:146:
+    /* Uses libqos. Doesn't do anything to reset state. Note that if we were to
+     reboot after each run, we would also have to redo the qos-related

WARNING: Block comments use a trailing */ on a separate line
#180: FILE: tests/fuzz/i440fx_fuzz.c:147:
+     initialization (qos_init_path) */

total: 0 errors, 5 warnings, 166 lines checked

Patch 20/22 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
21/22 Checking commit 042e172ef433 (fuzz: add virtio-net fuzz target)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#25: 
new file mode 100644

total: 0 errors, 1 warnings, 126 lines checked

Patch 21/22 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
22/22 Checking commit 5ad94a85737a (fuzz: add documentation to docs/devel/)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#11: 
new file mode 100644

total: 0 errors, 1 warnings, 114 lines checked

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

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190918231846.22538-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

Re: [Qemu-devel] [PATCH v3 00/22] Add virtual device fuzzing support

[no-reply]

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



Hi,

This series failed the docker-quick@centos7 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
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

default devices   yes
fuzzing support   no

warning: Python 2 support is deprecated
warning: Python 3 will be required for building future versions of QEMU
cross containers  no

NOTE: guest cross-compilers enabled: cc
---
  CC      backends/hostmem-ram.o
  CC      backends/hostmem-file.o
/tmp/qemu-test/src/vl.c: In function 'qemu_cleanup':
/tmp/qemu-test/src/vl.c:4479:6: error: old-style function definition [-Werror=old-style-definition]
 void qemu_cleanup()
      ^
cc1: all warnings being treated as errors


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

Re: [Qemu-devel] [PATCH v3 00/22] Add virtual device fuzzing support

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190918231846.22538-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 ===

  CC      hw/acpi/cpu_hotplug.o
  CC      hw/acpi/memory_hotplug.o
/tmp/qemu-test/src/vl.c: In function 'qemu_cleanup':
/tmp/qemu-test/src/vl.c:4479:6: error: old-style function definition [-Werror=old-style-definition]
 void qemu_cleanup()
      ^~~~~~~~~~~~
cc1: all warnings being treated as errors


The full log is available at
http://patchew.org/logs/20190918231846.22538-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