All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 00/20] Add virtual device fuzzing support
@ 2019-10-30 14:49 Oleinik, Alexander
  2019-10-30 14:49 ` [PATCH v4 01/20] softmmu: split off vl.c:main() into main.c Oleinik, Alexander
                   ` (22 more replies)
  0 siblings, 23 replies; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: 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.

V4:
 * add/transfer license headers to new files
 * restructure the added QTestClientTransportOps struct
 * restructure the FuzzTarget struct and fuzzer skeleton
 * fork-based fuzzer now directly mmaps shm over the coverage bitmaps
 * fixes to i440 and virtio-net fuzz targets
 * undo the changes to qtest_memwrite
 * possible to build /fuzz and /all in the same build-dir
 * misc fixes to address V3 comments

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 (20):
  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
  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 configure flag --enable-fuzzing
  fuzz: Add target/fuzz makefile rules
  fuzz: add fuzzer skeleton
  fuzz: add support for fork-based fuzzing.
  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                     |  16 ++-
 Makefile.objs                |   4 +
 Makefile.target              |  18 ++-
 configure                    |  39 ++++++
 docs/devel/fuzzing.txt       | 119 ++++++++++++++++++
 exec.c                       |  12 +-
 include/qemu/module.h        |   4 +-
 include/sysemu/qtest.h       |   4 +
 include/sysemu/sysemu.h      |   4 +
 main.c                       |  52 ++++++++
 qtest.c                      |  30 ++++-
 tests/Makefile.include       |  75 +++++------
 tests/fuzz/Makefile.include  |  11 ++
 tests/fuzz/fork_fuzz.c       |  51 ++++++++
 tests/fuzz/fork_fuzz.h       |  23 ++++
 tests/fuzz/fork_fuzz.ld      |  37 ++++++
 tests/fuzz/fuzz.c            | 177 ++++++++++++++++++++++++++
 tests/fuzz/fuzz.h            |  66 ++++++++++
 tests/fuzz/i440fx_fuzz.c     | 176 ++++++++++++++++++++++++++
 tests/fuzz/qos_fuzz.c        | 232 +++++++++++++++++++++++++++++++++++
 tests/fuzz/qos_fuzz.h        |  33 +++++
 tests/fuzz/virtio_net_fuzz.c | 123 +++++++++++++++++++
 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  | 168 +++++++++++++++++++++++++
 tests/libqos/qos_external.h  |  28 +++++
 tests/libqtest.c             | 109 ++++++++++++++--
 tests/libqtest.h             |   4 +
 tests/pca9552-test.c         |  10 +-
 tests/qos-test.c             | 140 +--------------------
 util/module.c                |   7 ++
 vl.c                         |  36 ++----
 34 files changed, 1601 insertions(+), 237 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



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

* [PATCH v4 01/20] softmmu: split off vl.c:main() into main.c
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
@ 2019-10-30 14:49 ` Oleinik, Alexander
  2019-11-05 16:41   ` Darren Kenny
  2019-11-06 15:01   ` Stefan Hajnoczi
  2019-10-30 14:49 ` [PATCH v4 02/20] libqos: Rename i2c_send and i2c_recv Oleinik, Alexander
                   ` (21 subsequent siblings)
  22 siblings, 2 replies; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Oleinik, Alexander, Paolo Bonzini

From: Alexander Oleinik <alxndr@bu.edu>

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                  | 52 +++++++++++++++++++++++++++++++++++++++++
 vl.c                    | 36 +++++++---------------------
 5 files changed, 68 insertions(+), 27 deletions(-)
 create mode 100644 main.c

diff --git a/Makefile b/Makefile
index 0e994a275d..d2b2ecd3c4 100644
--- a/Makefile
+++ b/Makefile
@@ -474,6 +474,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 11ba1a36bd..9ff9b0c6f9 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..03f9838b81 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);
+void 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..ecd6389424
--- /dev/null
+++ b/main.c
@@ -0,0 +1,52 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/sysemu.h"
+
+#ifdef CONFIG_SDL
+#if defined(__APPLE__) || defined(main)
+#include <SDL.h>
+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
+#define main qemu_main
+#endif /* CONFIG_COCOA */
+
+int main(int argc, char **argv, char **envp)
+{
+    qemu_init(argc, argv, envp);
+    main_loop();
+    qemu_cleanup();
+
+    return 0;
+}
diff --git a/vl.c b/vl.c
index c389d24b2c..472f09e12a 100644
--- a/vl.c
+++ b/vl.c
@@ -36,25 +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
-#define main qemu_main
-#endif /* CONFIG_COCOA */
-
-
 #include "qemu/error-report.h"
 #include "qemu/sockets.h"
 #include "sysemu/accel.h"
@@ -1797,7 +1778,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;
@@ -2824,7 +2805,7 @@ static void user_register_global_props(void)
                       global_init_func, NULL, NULL);
 }
 
-int main(int argc, char **argv, char **envp)
+void qemu_init(int argc, char **argv, char **envp)
 {
     int i;
     int snapshot, linux_boot;
@@ -3404,7 +3385,7 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_watchdog:
                 if (watchdog) {
                     error_report("only one watchdog option may be given");
-                    return 1;
+                    exit(1);
                 }
                 watchdog = optarg;
                 break;
@@ -4440,7 +4421,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) {
@@ -4457,8 +4438,11 @@ int main(int argc, char **argv, char **envp)
     accel_setup_post(current_machine);
     os_setup_post();
 
-    main_loop();
+    return;
+}
 
+void qemu_cleanup(void)
+{
     gdbserver_cleanup();
 
     /*
@@ -4495,6 +4479,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



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

* [PATCH v4 02/20] libqos: Rename i2c_send and i2c_recv
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
  2019-10-30 14:49 ` [PATCH v4 01/20] softmmu: split off vl.c:main() into main.c Oleinik, Alexander
@ 2019-10-30 14:49 ` Oleinik, Alexander
  2019-11-06 15:17   ` Stefan Hajnoczi
  2019-10-30 14:49 ` [PATCH v4 03/20] fuzz: Add FUZZ_TARGET module type Oleinik, Alexander
                   ` (20 subsequent siblings)
  22 siblings, 1 reply; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Oleinik, Alexander, Thomas Huth, Paolo Bonzini

From: Alexander Oleinik <alxndr@bu.edu>

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..42ebf8ba3a 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 qimx_i2c_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 qimx_i2c_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 = qimx_i2c_send;
+    s->parent.recv = qimx_i2c_recv;
     s->parent.qts = qts;
 }
 
diff --git a/tests/libqos/i2c-omap.c b/tests/libqos/i2c-omap.c
index 9ae8214fa8..5f4d79f87c 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 qomap_i2c_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 qomap_i2c_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 = qomap_i2c_send;
+    s->parent.recv = qomap_i2c_recv;
     s->parent.qts = qts;
 }
 
diff --git a/tests/libqos/i2c.c b/tests/libqos/i2c.c
index 156114e745..38f800dbab 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 qi2c_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 qi2c_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);
+    qi2c_send(i2cdev, &reg, 1);
+    qi2c_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);
+    qi2c_send(i2cdev, cmd, len + 1);
     g_free(cmd);
 }
 
diff --git a/tests/libqos/i2c.h b/tests/libqos/i2c.h
index 945b65b34c..c65f087834 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 qi2c_send(QI2CDevice *dev, const uint8_t *buf, uint16_t len);
+void qi2c_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..d80ed93cd3 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);
+    qi2c_send(i2cdev, &reg, 1);
 
     /* PCA9552_LS0 */
-    i2c_recv(i2cdev, &resp, 1);
+    qi2c_recv(i2cdev, &resp, 1);
     g_assert_cmphex(resp, ==, 0x54);
 
     /* PCA9552_LS1 */
-    i2c_recv(i2cdev, &resp, 1);
+    qi2c_recv(i2cdev, &resp, 1);
     g_assert_cmphex(resp, ==, 0x55);
 
     /* PCA9552_LS2 */
-    i2c_recv(i2cdev, &resp, 1);
+    qi2c_recv(i2cdev, &resp, 1);
     g_assert_cmphex(resp, ==, 0x55);
 
     /* PCA9552_LS3 */
-    i2c_recv(i2cdev, &resp, 1);
+    qi2c_recv(i2cdev, &resp, 1);
     g_assert_cmphex(resp, ==, 0x54);
 }
 
-- 
2.23.0



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

* [PATCH v4 03/20] fuzz: Add FUZZ_TARGET module type
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
  2019-10-30 14:49 ` [PATCH v4 01/20] softmmu: split off vl.c:main() into main.c Oleinik, Alexander
  2019-10-30 14:49 ` [PATCH v4 02/20] libqos: Rename i2c_send and i2c_recv Oleinik, Alexander
@ 2019-10-30 14:49 ` Oleinik, Alexander
  2019-11-06 13:17   ` Darren Kenny
  2019-11-06 15:18   ` Stefan Hajnoczi
  2019-10-30 14:49 ` [PATCH v4 04/20] qtest: add qtest_server_send abstraction Oleinik, Alexander
                   ` (19 subsequent siblings)
  22 siblings, 2 replies; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Oleinik, Alexander

From: Alexander Oleinik <alxndr@bu.edu>

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



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

* [PATCH v4 04/20] qtest: add qtest_server_send abstraction
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (2 preceding siblings ...)
  2019-10-30 14:49 ` [PATCH v4 03/20] fuzz: Add FUZZ_TARGET module type Oleinik, Alexander
@ 2019-10-30 14:49 ` Oleinik, Alexander
  2019-11-06 13:29   ` Darren Kenny
  2019-11-06 15:19   ` Stefan Hajnoczi
  2019-10-30 14:49 ` [PATCH v4 06/20] module: check module wasn't already initialized Oleinik, Alexander
                   ` (18 subsequent siblings)
  22 siblings, 2 replies; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Oleinik, Alexander, Thomas Huth, Paolo Bonzini

From: Alexander Oleinik <alxndr@bu.edu>

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



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

* [PATCH v4 06/20] module: check module wasn't already initialized
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (3 preceding siblings ...)
  2019-10-30 14:49 ` [PATCH v4 04/20] qtest: add qtest_server_send abstraction Oleinik, Alexander
@ 2019-10-30 14:49 ` Oleinik, Alexander
  2019-11-06 15:26   ` Stefan Hajnoczi
  2019-11-06 17:40   ` Darren Kenny
  2019-10-30 14:49 ` [PATCH v4 05/20] libqtest: Add a layer of abstraciton to send/recv Oleinik, Alexander
                   ` (17 subsequent siblings)
  22 siblings, 2 replies; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Oleinik, Alexander

From: Alexander Oleinik <alxndr@bu.edu>

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



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

* [PATCH v4 05/20] libqtest: Add a layer of abstraciton to send/recv
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (4 preceding siblings ...)
  2019-10-30 14:49 ` [PATCH v4 06/20] module: check module wasn't already initialized Oleinik, Alexander
@ 2019-10-30 14:49 ` Oleinik, Alexander
  2019-11-06 16:22   ` Stefan Hajnoczi
  2019-10-30 14:49 ` [PATCH v4 07/20] qtest: add in-process incoming command handler Oleinik, Alexander
                   ` (16 subsequent siblings)
  22 siblings, 1 reply; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Oleinik, Alexander, Thomas Huth, Paolo Bonzini

From: Alexander Oleinik <alxndr@bu.edu>

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 | 56 +++++++++++++++++++++++++++++++++++++++---------
 tests/libqtest.h |  1 -
 2 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 3706bccd8d..822bfe208b 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -35,6 +35,13 @@
 #define SOCKET_TIMEOUT 50
 #define SOCKET_MAX_FDS 16
 
+
+typedef struct QTestClientTransportOps {
+    void     (*send)(QTestState* , const char*, size_t);
+
+    GString* (*recv_line)(QTestState *);
+} QTestTransportOps;
+
 struct QTestState
 {
     int fd;
@@ -45,6 +52,7 @@ struct QTestState
     bool big_endian;
     bool irq_level[MAX_IRQ];
     GString *rx;
+    QTestTransportOps ops;
 };
 
 static GHookList abrt_hooks;
@@ -52,6 +60,18 @@ static struct sigaction sigact_old;
 
 static int qtest_query_target_endianness(QTestState *s);
 
+static void qtest_client_socket_send(QTestState*,
+        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(QTestState *);
+
+static void qtest_client_set_tx_handler(QTestState *s,
+        void (*send)(QTestState*, const char *, size_t));
+static void qtest_client_set_rx_handler(QTestState *s,
+        GString * (*recv)(QTestState *));
+
+
 static int init_socket(const char *socket_path)
 {
     struct sockaddr_un addr;
@@ -234,6 +254,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);
+    qtest_client_set_tx_handler(s, qtest_client_socket_send);
+
     qtest_add_abrt_handler(kill_qemu_hook_func, s);
 
     command = g_strdup_printf("exec %s "
@@ -360,6 +383,7 @@ void qtest_quit(QTestState *s)
     g_free(s);
 }
 
+
 static void socket_send(int fd, const char *buf, size_t size)
 {
     size_t offset;
@@ -379,22 +403,23 @@ 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 qtest_client_socket_send(QTestState *s,
+                                     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);
+    socket_send(s->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,7 +456,7 @@ 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(QTestState *s)
 {
     GString *line;
     size_t offset;
@@ -468,7 +493,7 @@ static gchar **qtest_rsp(QTestState *s, int expected_args)
     int i;
 
 redo:
-    line = qtest_recv_line(s);
+    line = s->ops.recv_line(s);
     words = g_strsplit(line->str, " ", 0);
     g_string_free(line, TRUE);
 
@@ -1336,3 +1361,14 @@ void qmp_assert_error_class(QDict *rsp, const char *class)
 
     qobject_unref(rsp);
 }
+
+static void qtest_client_set_tx_handler(QTestState *s,
+                    void (*send)(QTestState*, const char*, size_t))
+{
+    s->ops.send = send;
+}
+static void qtest_client_set_rx_handler(QTestState *s,
+                    GString* (*recv)(QTestState *))
+{
+    s->ops.recv_line = recv;
+}
diff --git a/tests/libqtest.h b/tests/libqtest.h
index c9e21e05b3..31267fc915 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -728,5 +728,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



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

* [PATCH v4 07/20] qtest: add in-process incoming command handler
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (5 preceding siblings ...)
  2019-10-30 14:49 ` [PATCH v4 05/20] libqtest: Add a layer of abstraciton to send/recv Oleinik, Alexander
@ 2019-10-30 14:49 ` Oleinik, Alexander
  2019-11-06 16:33   ` Stefan Hajnoczi
  2019-10-30 14:49 ` [PATCH v4 08/20] tests: provide test variables to other targets Oleinik, Alexander
                   ` (15 subsequent siblings)
  22 siblings, 1 reply; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Oleinik, Alexander, Thomas Huth, Paolo Bonzini

From: Alexander Oleinik <alxndr@bu.edu>

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                | 13 +++++++++++++
 2 files changed, 14 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..9fbfa0f08f 100644
--- a/qtest.c
+++ b/qtest.c
@@ -802,3 +802,16 @@ bool qtest_driver(void)
 {
     return qtest_chr.chr != NULL;
 }
+
+void qtest_server_inproc_recv(void *dummy, const char *buf, size_t size)
+{
+    static GString *gstr;
+    if (!gstr) {
+        gstr = g_string_new(NULL);
+    }
+    g_string_append(gstr, buf);
+    if (gstr->str[gstr->len - 1] == '\n') {
+        qtest_process_inbuf(NULL, gstr);
+        g_string_free(gstr, true);
+    }
+}
-- 
2.23.0



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

* [PATCH v4 08/20] tests: provide test variables to other targets
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (6 preceding siblings ...)
  2019-10-30 14:49 ` [PATCH v4 07/20] qtest: add in-process incoming command handler Oleinik, Alexander
@ 2019-10-30 14:49 ` Oleinik, Alexander
  2019-11-07 14:32   ` Darren Kenny
  2019-10-30 14:49 ` [PATCH v4 09/20] libqos: split qos-test and libqos makefile vars Oleinik, Alexander
                   ` (14 subsequent siblings)
  22 siblings, 1 reply; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Oleinik, Alexander

From: Alexander Oleinik <alxndr@bu.edu>

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 34ec03391c..67853d10c3 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
@@ -873,6 +872,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



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

* [PATCH v4 09/20] libqos: split qos-test and libqos makefile vars
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (7 preceding siblings ...)
  2019-10-30 14:49 ` [PATCH v4 08/20] tests: provide test variables to other targets Oleinik, Alexander
@ 2019-10-30 14:49 ` Oleinik, Alexander
  2019-11-07 14:03   ` Darren Kenny
  2019-10-30 14:49 ` [PATCH v4 10/20] libqos: move useful qos-test funcs to qos_external Oleinik, Alexander
                   ` (13 subsequent siblings)
  22 siblings, 1 reply; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Oleinik, Alexander

From: Alexander Oleinik <alxndr@bu.edu>

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 | 71 +++++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 35 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 67853d10c3..1517c4817e 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -699,52 +699,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-pci-modern.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 = $(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-pci-modern.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
@@ -776,7 +777,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



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

* [PATCH v4 10/20] libqos: move useful qos-test funcs to qos_external
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (8 preceding siblings ...)
  2019-10-30 14:49 ` [PATCH v4 09/20] libqos: split qos-test and libqos makefile vars Oleinik, Alexander
@ 2019-10-30 14:49 ` Oleinik, Alexander
  2019-11-06 16:41   ` Stefan Hajnoczi
  2019-10-30 14:49 ` [PATCH v4 11/20] libqtest: make qtest_bufwrite send "atomic" Oleinik, Alexander
                   ` (12 subsequent siblings)
  22 siblings, 1 reply; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Oleinik, Alexander, Thomas Huth, Paolo Bonzini

From: Alexander Oleinik <alxndr@bu.edu>

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 | 168 ++++++++++++++++++++++++++++++++++++
 tests/libqos/qos_external.h |  28 ++++++
 tests/qos-test.c            | 140 ++----------------------------
 4 files changed, 202 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 1517c4817e..2cccc05ae1 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -714,6 +714,7 @@ libqos-usb-obj-y = $(libqos-spapr-obj-y) $(libqos-pc-obj-y) tests/libqos/usb.o
 # Devices
 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..398556dde0
--- /dev/null
+++ b/tests/libqos/qos_external.c
@@ -0,0 +1,168 @@
+/*
+ * libqos driver framework
+ *
+ * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#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..7b44930c55
--- /dev/null
+++ b/tests/libqos/qos_external.h
@@ -0,0 +1,28 @@
+/*
+ * libqos driver framework
+ *
+ * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#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



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

* [PATCH v4 11/20] libqtest: make qtest_bufwrite send "atomic"
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (9 preceding siblings ...)
  2019-10-30 14:49 ` [PATCH v4 10/20] libqos: move useful qos-test funcs to qos_external Oleinik, Alexander
@ 2019-10-30 14:49 ` Oleinik, Alexander
  2019-11-06 16:44   ` Stefan Hajnoczi
  2019-10-30 14:49 ` [PATCH v4 12/20] libqtest: add in-process qtest.c tx/rx handlers Oleinik, Alexander
                   ` (11 subsequent siblings)
  22 siblings, 1 reply; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Oleinik, Alexander, Thomas Huth, Paolo Bonzini

From: Alexander Oleinik <alxndr@bu.edu>

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, 2 insertions(+), 2 deletions(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 822bfe208b..ff3153daf2 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -1083,8 +1083,8 @@ void qtest_bufwrite(QTestState *s, uint64_t addr, const void *data, size_t size)
 
     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);
+    s->ops.send(s, bdata, strlen(bdata));
+    s->ops.send(s, "\n", 1);
     qtest_rsp(s, 0);
     g_free(bdata);
 }
-- 
2.23.0



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

* [PATCH v4 12/20] libqtest: add in-process qtest.c tx/rx handlers
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (10 preceding siblings ...)
  2019-10-30 14:49 ` [PATCH v4 11/20] libqtest: make qtest_bufwrite send "atomic" Oleinik, Alexander
@ 2019-10-30 14:49 ` Oleinik, Alexander
  2019-11-06 16:56   ` Stefan Hajnoczi
  2019-10-30 14:49 ` [PATCH v4 13/20] fuzz: add configure flag --enable-fuzzing Oleinik, Alexander
                   ` (10 subsequent siblings)
  22 siblings, 1 reply; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier, Oleinik, Alexander, Thomas Huth, Paolo Bonzini

From: Alexander Oleinik <alxndr@bu.edu>

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
There's a particularily ugly line here:
qtest_client_set_tx_handler(qts,
        (void (*)(QTestState *s, const char*, size_t)) send);

Since qtest.c has no knowledge of the QTestState, I'm not sure how to
avoid doing this, without adding back the *opaque that was present in
v3.

 qtest.c          |  2 +-
 tests/libqtest.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
 tests/libqtest.h |  5 +++++
 3 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/qtest.c b/qtest.c
index 9fbfa0f08f..f817a5d789 100644
--- a/qtest.c
+++ b/qtest.c
@@ -812,6 +812,6 @@ void qtest_server_inproc_recv(void *dummy, const char *buf, size_t size)
     g_string_append(gstr, buf);
     if (gstr->str[gstr->len - 1] == '\n') {
         qtest_process_inbuf(NULL, gstr);
-        g_string_free(gstr, true);
+        g_string_truncate(gstr, 0);
     }
 }
diff --git a/tests/libqtest.c b/tests/libqtest.c
index ff3153daf2..6143af33da 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -71,6 +71,7 @@ static void qtest_client_set_tx_handler(QTestState *s,
 static void qtest_client_set_rx_handler(QTestState *s,
         GString * (*recv)(QTestState *));
 
+static GString *recv_str;
 
 static int init_socket(const char *socket_path)
 {
@@ -486,6 +487,7 @@ static GString *qtest_client_socket_recv_line(QTestState *s)
     return line;
 }
 
+
 static gchar **qtest_rsp(QTestState *s, int expected_args)
 {
     GString *line;
@@ -1372,3 +1374,50 @@ static void qtest_client_set_rx_handler(QTestState *s,
 {
     s->ops.recv_line = recv;
 }
+
+static GString *qtest_client_inproc_recv_line(QTestState *s)
+{
+    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);
+    /* Re-cast the  send pointer, since qtest.c should need to know about
+     * QTestState
+     */
+    qtest_client_set_tx_handler(qts,
+            (void (*)(QTestState *s, const char*, size_t)) send);
+
+    qts->big_endian = qtest_query_target_endianness(qts);
+    gchar *bin_path = g_strconcat("/qemu-system-", arch, NULL);
+    setenv("QTEST_QEMU_BINARY", bin_path, 0);
+    g_free(bin_path);
+
+    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 31267fc915..7251de4ba9 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -728,4 +728,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



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

* [PATCH v4 13/20] fuzz: add configure flag --enable-fuzzing
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (11 preceding siblings ...)
  2019-10-30 14:49 ` [PATCH v4 12/20] libqtest: add in-process qtest.c tx/rx handlers Oleinik, Alexander
@ 2019-10-30 14:49 ` Oleinik, Alexander
  2019-11-06 16:57   ` Stefan Hajnoczi
  2019-10-30 14:50 ` [PATCH v4 15/20] fuzz: add fuzzer skeleton Oleinik, Alexander
                   ` (9 subsequent siblings)
  22 siblings, 1 reply; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Oleinik, Alexander

From: Alexander Oleinik <alxndr@bu.edu>

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

diff --git a/configure b/configure
index 3be9e92a24..aeca632dd9 100755
--- a/configure
+++ b/configure
@@ -501,6 +501,7 @@ libxml2=""
 debug_mutex="no"
 libpmem=""
 default_devices="yes"
+fuzzing="no"
 
 supported_cpu="no"
 supported_os="no"
@@ -630,6 +631,15 @@ int main(void) { return 0; }
 EOF
 }
 
+write_c_fuzzer_skeleton() {
+    cat > $TMPC <<EOF
+#include <stdint.h>
+#include <sys/types.h>
+int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size);
+int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { return 0; }
+EOF
+}
+
 if check_define __linux__ ; then
   targetos="Linux"
 elif check_define _WIN32 ; then
@@ -1532,6 +1542,10 @@ for opt do
   ;;
   --disable-xkbcommon) xkbcommon=no
   ;;
+  --enable-fuzzing) fuzzing=yes
+  ;;
+  --disable-fuzzing) fuzzing=no
+  ;;
   *)
       echo "ERROR: unknown option $opt"
       echo "Try '$0 --help' for more information"
@@ -5911,6 +5925,15 @@ EOF
   fi
 fi
 
+##########################################
+# checks for fuzzer
+if test "$fuzzing" = "yes" ; then
+  write_c_fuzzer_skeleton
+  if compile_prog "$CPU_CFLAGS -Werror -fsanitize=address,fuzzer" ""; then
+      have_fuzzer=yes
+  fi
+fi
+
 ##########################################
 # check for libpmem
 
@@ -6491,6 +6514,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
@@ -7327,6 +7351,16 @@ fi
 if test "$sheepdog" = "yes" ; then
   echo "CONFIG_SHEEPDOG=y" >> $config_host_mak
 fi
+if test "$fuzzing" = "yes" ; then
+  if test "$have_fuzzer" = "yes"; then
+    FUZZ_LDFLAGS=" -fsanitize=address,fuzzer"
+    FUZZ_CFLAGS=" -fsanitize=address,fuzzer"
+    CFLAGS=" -fsanitize=address"
+  else
+    error_exit "Your compiler doesn't support -fsanitize=address,fuzzer"
+    exit 1
+  fi
+fi
 
 if test "$tcg_interpreter" = "yes"; then
   QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
@@ -7409,6 +7443,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



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

* [PATCH v4 15/20] fuzz: add fuzzer skeleton
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (12 preceding siblings ...)
  2019-10-30 14:49 ` [PATCH v4 13/20] fuzz: add configure flag --enable-fuzzing Oleinik, Alexander
@ 2019-10-30 14:50 ` Oleinik, Alexander
  2019-11-07 12:55   ` Stefan Hajnoczi
  2019-10-30 14:50 ` [PATCH v4 14/20] fuzz: Add target/fuzz makefile rules Oleinik, Alexander
                   ` (8 subsequent siblings)
  22 siblings, 1 reply; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: Oleinik, Alexander

From: Alexander Oleinik <alxndr@bu.edu>

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           | 177 ++++++++++++++++++++++++++++++++++++
 tests/fuzz/fuzz.h           |  66 ++++++++++++++
 3 files changed, 245 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..0e38f81c48
--- /dev/null
+++ b/tests/fuzz/fuzz.c
@@ -0,0 +1,177 @@
+/*
+ * fuzzing driver
+ *
+ * Copyright Red Hat Inc., 2019
+ *
+ * Authors:
+ *  Alexander Bulekov   <alxndr@bu.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <wordexp.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"
+#include "qemu/main-loop.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;
+
+
+
+void flush_events(QTestState *s)
+{
+    int i = 10;
+    while (g_main_context_pending(NULL) && i-- > 0) {
+        main_loop_wait(false);
+    }
+}
+
+static QTestState *qtest_setup(void)
+{
+    qtest_server_set_tx_handler(&qtest_client_inproc_recv, NULL);
+    return qtest_inproc_init(false, fuzz_arch, &qtest_server_inproc_recv);
+}
+
+void fuzz_add_target(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, target->name) == 0) {
+            fprintf(stderr, "Error: Fuzz target name %s already in use\n",
+                    target->name);
+            abort();
+        }
+    }
+    target_state = g_new0(FuzzTargetState, 1);
+    target_state->target = g_new0(FuzzTarget, 1);
+    *(target_state->target) = *target;
+    QSLIST_INSERT_HEAD(fuzz_target_list, target_state, target_list);
+}
+
+
+
+static void usage(char *path)
+{
+    printf("Usage: %s --fuzz-target=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,
+                tmp->target->description);
+    }
+    exit(0);
+}
+
+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 (strcmp(tmp->target->name, name) == 0) {
+            return tmp->target;
+        }
+    }
+    return NULL;
+}
+
+
+/* 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;
+
+    /* 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];
+    if (!strstr(target_name, "--fuzz-target=")) {
+        usage(**argv);
+    }
+
+    target_name += strlen("--fuzz-target=");
+
+    fuzz_target = fuzz_get_target(target_name);
+    if (!fuzz_target) {
+        usage(**argv);
+    }
+
+    fuzz_qts = qtest_setup();
+
+    if (!fuzz_target) {
+        fprintf(stderr, "Error: Fuzz fuzz_target name %s not found\n",
+                target_name);
+        usage(**argv);
+    }
+
+    if (fuzz_target->pre_vm_init) {
+        fuzz_target->pre_vm_init();
+    }
+
+    /* Run QEMU's softmmu main with the fuzz-target dependent arguments */
+    char *init_cmdline = fuzz_target->get_init_cmdline(fuzz_target);
+
+    wordexp_t result;
+    wordexp(init_cmdline, &result, 0);
+
+    qemu_init(result.we_wordc, result.we_wordv, NULL);
+
+    if (fuzz_target->pre_fuzz) {
+        fuzz_target->pre_fuzz(fuzz_qts);
+    }
+
+    return 0;
+}
diff --git a/tests/fuzz/fuzz.h b/tests/fuzz/fuzz.h
new file mode 100644
index 0000000000..b569b622d7
--- /dev/null
+++ b/tests/fuzz/fuzz.h
@@ -0,0 +1,66 @@
+/*
+ * fuzzing driver
+ *
+ * Copyright Red Hat Inc., 2019
+ *
+ * Authors:
+ *  Alexander Bulekov   <alxndr@bu.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef 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 {
+    const char *name;         /* command-line option(FUZZ_TARGET) for the target */
+    const char *description;  /* help text */
+
+
+    /* returns the arg-list that is passed to qemu/softmmu init() */
+    char* (*get_init_cmdline)(struct FuzzTarget *);
+
+    /*
+     * will run once, prior to running qemu/softmmu init.
+     * eg: set up shared-memory for communication with the child-process
+     */
+    void(*pre_vm_init)(void);
+
+    /*
+     * will run once, prior to to the fuzz-loop.
+     * eg: detect the memory map
+     */
+    void(*pre_fuzz)(QTestState *);
+
+    /*
+     * accepts and executes an input from libfuzzer. this is repeatedly
+     * executed during the fuzzing loop. Its should handle setup, input
+     * execution and cleanup
+     */
+    void(*fuzz)(QTestState *, const unsigned char *, size_t);
+
+} FuzzTarget;
+
+void flush_events(QTestState *);
+void reboot(QTestState *);
+
+/*
+ * makes a copy of *target and adds it to the target-list.
+ * i.e. fine to set up target on the caller's stack
+ */
+void fuzz_add_target(FuzzTarget *target);
+
+int LLVMFuzzerTestOneInput(const unsigned char *Data, size_t Size);
+int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp);
+
+#endif
+
-- 
2.23.0



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

* [PATCH v4 14/20] fuzz: Add target/fuzz makefile rules
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (13 preceding siblings ...)
  2019-10-30 14:50 ` [PATCH v4 15/20] fuzz: add fuzzer skeleton Oleinik, Alexander
@ 2019-10-30 14:50 ` Oleinik, Alexander
  2019-11-07 14:31   ` Darren Kenny
  2019-10-30 14:50 ` [PATCH v4 16/20] fuzz: add support for fork-based fuzzing Oleinik, Alexander
                   ` (7 subsequent siblings)
  22 siblings, 1 reply; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: Oleinik, Alexander

From: Alexander Oleinik <alxndr@bu.edu>

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 Makefile                    | 15 ++++++++++++++-
 Makefile.objs               |  4 +++-
 Makefile.target             | 18 +++++++++++++++++-
 tests/fuzz/Makefile.include |  4 ++++
 4 files changed, 38 insertions(+), 3 deletions(-)
 create mode 100644 tests/fuzz/Makefile.include

diff --git a/Makefile b/Makefile
index d2b2ecd3c4..571f5562c9 100644
--- a/Makefile
+++ b/Makefile
@@ -464,7 +464,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)
@@ -476,6 +476,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
@@ -526,6 +535,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
@@ -535,6 +547,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.objs b/Makefile.objs
index 9ff9b0c6f9..5478a554f6 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -86,10 +86,12 @@ common-obj-$(CONFIG_FDT) += device_tree.o
 # qapi
 
 common-obj-y += qapi/
+softmmu-obj-y = main.o
 
-softmmu-main-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 ca3d14efe1..cddc8e4306 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -202,7 +202,7 @@ endif
 COMMON_LDADDS = ../libqemuutil.a
 
 # build either PROG or PROGW
-$(QEMU_PROG_BUILD): $(all-obj-y) $(COMMON_LDADDS)
+$(QEMU_PROG_BUILD): $(all-obj-y) $(COMMON_LDADDS) $(softmmu-obj-y)
 	$(call LINK, $(filter-out %.mak, $^))
 ifdef CONFIG_DARWIN
 	$(call quiet-command,Rez -append $(SRC_PATH)/pc-bios/qemu.rsrc -o $@,"REZ","$(TARGET_DIR)$@")
@@ -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



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

* [PATCH v4 16/20] fuzz: add support for fork-based fuzzing.
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (14 preceding siblings ...)
  2019-10-30 14:50 ` [PATCH v4 14/20] fuzz: Add target/fuzz makefile rules Oleinik, Alexander
@ 2019-10-30 14:50 ` Oleinik, Alexander
  2019-11-07 13:17   ` Stefan Hajnoczi
  2019-10-30 14:50 ` [PATCH v4 17/20] fuzz: add support for qos-assisted fuzz targets Oleinik, Alexander
                   ` (6 subsequent siblings)
  22 siblings, 1 reply; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: Oleinik, Alexander, Richard Henderson, Paolo Bonzini

From: Alexander Oleinik <alxndr@bu.edu>

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 they are not shared with fork()-ed processes, by
default. To address this, we create a shared memory region, adjust its
size and map it _over_ the counter region. Furthermore, libfuzzer
doesn't generally expose the globals that specify the location of the
counters/coverage bitmap. As a workaround, we rely on a custom linker
script which forces all of the bitmaps we care about to be placed in a
contiguous region, which is easy to locate and mmap over.

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 exec.c                      | 12 +++++++--
 tests/fuzz/Makefile.include |  3 +++
 tests/fuzz/fork_fuzz.c      | 51 +++++++++++++++++++++++++++++++++++++
 tests/fuzz/fork_fuzz.h      | 23 +++++++++++++++++
 tests/fuzz/fork_fuzz.ld     | 37 +++++++++++++++++++++++++++
 5 files changed, 124 insertions(+), 2 deletions(-)
 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 91c8b79656..b15207b00c 100644
--- a/exec.c
+++ b/exec.c
@@ -35,6 +35,7 @@
 #include "sysemu/kvm.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/tcg.h"
+#include "sysemu/qtest.h"
 #include "qemu/timer.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
@@ -2266,8 +2267,15 @@ static void ram_block_add(RAMBlock *new_block, Error **errp, bool shared)
     if (new_block->host) {
         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 */
-        qemu_madvise(new_block->host, new_block->max_length, QEMU_MADV_DONTFORK);
+        /*
+         * MADV_DONTFORK is also needed by KVM in absence of synchronous MMU
+         * Configure it unless the machine is a qtest server, in which case it
+         * may be forked, for fuzzing purposes
+         */
+        if (!qtest_enabled()) {
+            qemu_madvise(new_block->host, new_block->max_length,
+                         QEMU_MADV_DONTFORK);
+        }
         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..4c4d00b034
--- /dev/null
+++ b/tests/fuzz/fork_fuzz.c
@@ -0,0 +1,51 @@
+/*
+ * Fork-based fuzzing helpers
+ *
+ * Copyright Red Hat Inc., 2019
+ *
+ * Authors:
+ *  Alexander Bulekov   <alxndr@bu.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "fork_fuzz.h"
+
+uintptr_t feature_shm;
+
+void counter_shm_init(void)
+{
+    int fd = shm_open("/qemu-fuzz-cntrs", O_CREAT | O_RDWR, S_IRUSR | S_IWUSR);
+    if (fd == -1) {
+        perror("Error: ");
+        exit(1);
+    }
+    if (ftruncate(fd, &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START) == -1) {
+        perror("Error: ");
+        exit(1);
+    }
+    /* Copy what's in the counter region to the shm.. */
+    void *rptr = mmap(NULL ,
+            &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START,
+            PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+    memcpy(rptr,
+           &__FUZZ_COUNTERS_START,
+           &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
+
+    munmap(rptr, &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
+
+    /* And map the shm over the counter region */
+    rptr = mmap(&__FUZZ_COUNTERS_START,
+            &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START,
+            PROT_READ | PROT_WRITE, MAP_SHARED | MAP_FIXED, fd, 0);
+    if (!rptr) {
+        perror("Error: ");
+        exit(1);
+    }
+    return;
+}
+
+
diff --git a/tests/fuzz/fork_fuzz.h b/tests/fuzz/fork_fuzz.h
new file mode 100644
index 0000000000..9ecb8b58ef
--- /dev/null
+++ b/tests/fuzz/fork_fuzz.h
@@ -0,0 +1,23 @@
+/*
+ * Fork-based fuzzing helpers
+ *
+ * Copyright Red Hat Inc., 2019
+ *
+ * Authors:
+ *  Alexander Bulekov   <alxndr@bu.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef FORK_FUZZ_H
+#define FORK_FUZZ_H
+
+extern uint8_t __FUZZ_COUNTERS_START;
+extern uint8_t __FUZZ_COUNTERS_END;
+
+void counter_shm_init(void);
+
+#endif
+
diff --git a/tests/fuzz/fork_fuzz.ld b/tests/fuzz/fork_fuzz.ld
new file mode 100644
index 0000000000..51ba3717c4
--- /dev/null
+++ b/tests/fuzz/fork_fuzz.ld
@@ -0,0 +1,37 @@
+/* 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 re-map the counter-related memory as shared.
+*/
+
+SECTIONS
+{
+  .data.fuzz_start : ALIGN(4K)
+  {
+      __FUZZ_COUNTERS_START = .;
+      __start___sancov_cntrs = .;
+      *(_*sancov_cntrs);
+      __stop___sancov_cntrs = .;
+
+      /* Lowest stack counter */
+      *(__sancov_lowest_stack);
+
+      /* 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 = .;
+  }
+  .data.fuzz_ordered :
+  {
+      /* Internal Libfuzzer TracePC object which contains the ValueProfileMap */
+      FuzzerTracePC*(.bss);
+  }
+  .data.fuzz_end : ALIGN(4K)
+  {
+      __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



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

* [PATCH v4 17/20] fuzz: add support for qos-assisted fuzz targets
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (15 preceding siblings ...)
  2019-10-30 14:50 ` [PATCH v4 16/20] fuzz: add support for fork-based fuzzing Oleinik, Alexander
@ 2019-10-30 14:50 ` Oleinik, Alexander
  2019-11-07 13:22   ` Stefan Hajnoczi
  2019-10-30 14:50 ` [PATCH v4 18/20] fuzz: add i440fx " Oleinik, Alexander
                   ` (5 subsequent siblings)
  22 siblings, 1 reply; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: Oleinik, Alexander

From: Alexander Oleinik <alxndr@bu.edu>

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 tests/fuzz/qos_fuzz.c | 232 ++++++++++++++++++++++++++++++++++++++++++
 tests/fuzz/qos_fuzz.h |  33 ++++++
 2 files changed, 265 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..07015da4ca
--- /dev/null
+++ b/tests/fuzz/qos_fuzz.c
@@ -0,0 +1,232 @@
+/*
+ * QOS-assisted fuzzing helpers
+ *
+ * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#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"
+
+
+void *fuzz_qos_obj;
+QGuestAllocator *fuzz_qos_alloc;
+
+static const char *fuzz_target_name;
+static char **fuzz_path_vec;
+
+/*
+ * 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);
+}
+
+static char **current_path;
+
+void *qos_allocate_objects(QTestState *qts, QGuestAllocator **p_alloc)
+{
+    return allocate_objects(qts, current_path + 1, p_alloc);
+}
+
+static char *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 64 ");
+    return cmd_line->str;
+}
+
+/*
+ * 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, 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);
+}
+
+static char *qos_get_cmdline(FuzzTarget *t)
+{
+    /*
+     * Set a global variable that we use to identify the qos_path for our
+     * fuzz_target
+     */
+    fuzz_target_name = t->name;
+    qos_set_machines_devices_available();
+    qos_graph_foreach_test_path(walk_path);
+    return qos_build_main_args();
+}
+
+void fuzz_add_qos_target(
+        FuzzTarget *fuzz_opts,
+        const char *interface,
+        QOSGraphTestOptions *opts
+        )
+{
+    qos_add_test(fuzz_opts->name, interface, NULL, opts);
+    fuzz_opts->get_init_cmdline = qos_get_cmdline;
+    fuzz_add_target(fuzz_opts);
+}
+
+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..6c280f07c5
--- /dev/null
+++ b/tests/fuzz/qos_fuzz.h
@@ -0,0 +1,33 @@
+/*
+ * QOS-assisted fuzzing helpers
+ *
+ * Copyright Red Hat Inc., 2019
+ *
+ * Authors:
+ *  Alexander Bulekov   <alxndr@bu.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef _QOS_FUZZ_H_
+#define _QOS_FUZZ_H_
+
+#include "tests/fuzz/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(
+        FuzzTarget *fuzz_opts,
+        const char *interface,
+        QOSGraphTestOptions *opts
+        );
+
+void qos_init_path(QTestState *);
+
+#endif
-- 
2.23.0



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

* [PATCH v4 18/20] fuzz: add i440fx fuzz targets
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (16 preceding siblings ...)
  2019-10-30 14:50 ` [PATCH v4 17/20] fuzz: add support for qos-assisted fuzz targets Oleinik, Alexander
@ 2019-10-30 14:50 ` Oleinik, Alexander
  2019-11-07 13:26   ` Stefan Hajnoczi
  2019-10-30 14:50 ` [PATCH v4 19/20] fuzz: add virtio-net fuzz target Oleinik, Alexander
                   ` (4 subsequent siblings)
  22 siblings, 1 reply; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: Oleinik, Alexander

From: Alexander Oleinik <alxndr@bu.edu>

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    | 176 ++++++++++++++++++++++++++++++++++++
 2 files changed, 179 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..7304465b42
--- /dev/null
+++ b/tests/fuzz/i440fx_fuzz.c
@@ -0,0 +1,176 @@
+/*
+ * I440FX Fuzzing Target
+ *
+ * Copyright Red Hat Inc., 2019
+ *
+ * Authors:
+ *  Alexander Bulekov   <alxndr@bu.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+
+#include "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"
+
+
+#define I440FX_PCI_HOST_BRIDGE_CFG 0xcf8
+#define I440FX_PCI_HOST_BRIDGE_DATA 0xcfc
+
+enum action_id {
+    WRITEB,
+    WRITEW,
+    WRITEL,
+    READB,
+    READW,
+    READL,
+    ACTION_MAX
+};
+
+static void i440fx_fuzz_qtest(QTestState *s,
+        const unsigned char *Data, size_t Size) {
+    typedef struct QTestFuzzAction {
+        uint32_t value;
+        uint8_t id;
+        uint8_t addr;
+    } QTestFuzzAction;
+    QTestFuzzAction a;
+
+    while (Size >= sizeof(a)) {
+        memcpy(&a, Data, sizeof(a));
+        uint16_t addr = a.addr % 2 ? I440FX_PCI_HOST_BRIDGE_CFG :
+                                      I440FX_PCI_HOST_BRIDGE_DATA;
+        switch (a.id % ACTION_MAX) {
+        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;
+        }
+        Size -= sizeof(a);
+        Data += sizeof(a);
+    }
+    flush_events(s);
+}
+
+static void i440fx_fuzz_qos(QTestState *s,
+        const unsigned char *Data, size_t Size) {
+
+    typedef struct QOSFuzzAction {
+        uint32_t value;
+        int devfn;
+        uint8_t offset;
+        uint8_t id;
+    } QOSFuzzAction;
+
+    static QPCIBus *bus;
+    if (!bus) {
+        bus = qpci_new_pc(s, fuzz_qos_alloc);
+    }
+
+    QOSFuzzAction a;
+    while (Size >= sizeof(a)) {
+        memcpy(&a, Data, sizeof(a));
+        switch (a.id % ACTION_MAX) {
+        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;
+        }
+        Size -= sizeof(a);
+        Data += sizeof(a);
+    }
+    flush_events(s);
+}
+
+static void i440fx_fuzz_qos_fork(QTestState *s,
+        const unsigned char *Data, size_t Size) {
+    if (fork() == 0) {
+        i440fx_fuzz_qos(s, Data, Size);
+        _Exit(0);
+    } else {
+        wait(NULL);
+    }
+}
+
+static const char *i440fx_qtest_argv = "qemu_system_i386 -machine accel=qtest"
+                                       "-m 0 -display none";
+static char *i440fx_argv(FuzzTarget *t)
+{
+    return (char *)i440fx_qtest_argv;
+}
+
+static void fork_init(void)
+{
+    counter_shm_init();
+}
+
+static void register_pci_fuzz_targets(void)
+{
+    /* Uses simple qtest commands and reboots to reset state */
+    fuzz_add_target(&(FuzzTarget){
+                .name = "i440fx-qtest-reboot-fuzz",
+                .description = "Fuzz the i440fx using raw qtest commands and"
+                               "rebooting after each run",
+                .get_init_cmdline = i440fx_argv,
+                .fuzz = i440fx_fuzz_qtest});
+
+    /* Uses libqos and forks to prevent state leakage */
+    fuzz_add_qos_target(&(FuzzTarget){
+                .name = "i440fx-qos-fork-fuzz",
+                .description = "Fuzz the i440fx using raw qtest commands and"
+                               "rebooting after each run",
+                .pre_vm_init = &fork_init,
+                .fuzz = i440fx_fuzz_qos_fork,},
+                "i440FX-pcihost",
+                &(QOSGraphTestOptions){}
+                );
+
+    /* Uses libqos. Doesn't do anything to reset state. Note that if we were to
+     reboot after each run, we would also have to redo the qos-related
+     initialization (qos_init_path) */
+    fuzz_add_qos_target(&(FuzzTarget){
+                .name = "i440fx-qos-noreset-fuzz",
+                .description = "Fuzz the i440fx using raw qtest commands and"
+                               "rebooting after each run",
+                .fuzz = i440fx_fuzz_qos,},
+                "i440FX-pcihost",
+                &(QOSGraphTestOptions){}
+                );
+}
+
+fuzz_target_init(register_pci_fuzz_targets);
-- 
2.23.0



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

* [PATCH v4 19/20] fuzz: add virtio-net fuzz target
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (17 preceding siblings ...)
  2019-10-30 14:50 ` [PATCH v4 18/20] fuzz: add i440fx " Oleinik, Alexander
@ 2019-10-30 14:50 ` Oleinik, Alexander
  2019-11-07 13:36   ` Stefan Hajnoczi
  2019-11-07 13:42   ` Jason Wang
  2019-10-30 14:50 ` [PATCH v4 20/20] fuzz: add documentation to docs/devel/ Oleinik, Alexander
                   ` (3 subsequent siblings)
  22 siblings, 2 replies; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: Oleinik, Alexander

From: Alexander Oleinik <alxndr@bu.edu>

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 | 123 +++++++++++++++++++++++++++++++++++
 2 files changed, 124 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..0543cfd32a
--- /dev/null
+++ b/tests/fuzz/virtio_net_fuzz.c
@@ -0,0 +1,123 @@
+/*
+ * virtio-net Fuzzing Target
+ *
+ * Copyright Red Hat Inc., 2019
+ *
+ * Authors:
+ *  Alexander Bulekov   <alxndr@bu.edu>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+
+#include "fuzz.h"
+#include "fork_fuzz.h"
+#include "qos_fuzz.h"
+#include "tests/libqtest.h"
+#include "tests/libqos/virtio-net.h"
+
+
+static void virtio_net_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;
+        }
+        memcpy(&vqa, Data, sizeof(vqa));
+        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);
+        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_fuzz_multi(s, Data, Size);
+        flush_events(s);
+        _Exit(0);
+    } else {
+        wait(NULL);
+    }
+}
+
+static void register_virtio_net_fuzz_targets(void)
+{
+    fuzz_add_qos_target(&(FuzzTarget){
+                .name = "virtio-net-fork-fuzz",
+                .description = "Fuzz the virtio-net virtual queues, forking"
+                                "for each fuzz run",
+                .pre_vm_init = &counter_shm_init,
+                .pre_fuzz = &qos_init_path,
+                .fuzz = virtio_net_fork_fuzz,},
+                "virtio-net",
+                &(QOSGraphTestOptions){.before = virtio_net_test_setup_socket}
+                );
+}
+
+fuzz_target_init(register_virtio_net_fuzz_targets);
-- 
2.23.0



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

* [PATCH v4 20/20] fuzz: add documentation to docs/devel/
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (18 preceding siblings ...)
  2019-10-30 14:50 ` [PATCH v4 19/20] fuzz: add virtio-net fuzz target Oleinik, Alexander
@ 2019-10-30 14:50 ` Oleinik, Alexander
  2019-11-07 13:40   ` Stefan Hajnoczi
  2019-10-30 15:23 ` [PATCH v4 00/20] Add virtual device fuzzing support no-reply
                   ` (2 subsequent siblings)
  22 siblings, 1 reply; 57+ messages in thread
From: Oleinik, Alexander @ 2019-10-30 14:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: Oleinik, Alexander

From: Alexander Oleinik <alxndr@bu.edu>

Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
---
 docs/devel/fuzzing.txt | 119 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 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..825ff0af51
--- /dev/null
+++ b/docs/devel/fuzzing.txt
@@ -0,0 +1,119 @@
+= 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 32-bit fuzzer is
+much faster, since the page-map has a smaller size. This is due to the fact that
+AddressSanitizer mmaps ~20TB of memory, as part of its detection. This results
+in a large page-map, and a much slower fork(). O
+
+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 similarly to system/softmmu:
+
+    make i386-softmmu/fuzz
+
+This builds ./i386-softmmu/qemu-fuzz-i386
+
+The first option to this command is: --fuzz_taget=FUZZ_NAME
+To list all of the available fuzzers run qemu-fuzz-i386 with no arguments.
+
+eg:
+    ./i386-softmmu/qemu-fuzz-i386 --fuzz-target=virtio-net-fork-fuzz
+
+Internally, 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. libfuzzer provides it's
+own main(), which performs some setup, and calls the entrypoints:
+
+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: i440fx-qtest-reboot-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. Does not work well for devices
+   that rely on dedicated threads.
+   Example target: virtio-net-fork-fuzz
+
-- 
2.23.0



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

* Re: [PATCH v4 00/20] Add virtual device fuzzing support
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (19 preceding siblings ...)
  2019-10-30 14:50 ` [PATCH v4 20/20] fuzz: add documentation to docs/devel/ Oleinik, Alexander
@ 2019-10-30 15:23 ` no-reply
  2019-11-06 15:27   ` Stefan Hajnoczi
  2019-11-05 13:57 ` Darren Kenny
  2019-11-07 13:41 ` Stefan Hajnoczi
  22 siblings, 1 reply; 57+ messages in thread
From: no-reply @ 2019-10-30 15:23 UTC (permalink / raw)
  To: alxndr; +Cc: alxndr, qemu-devel

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

  CC      tests/test-qapi-types-sub-sub-module.o
  CC      tests/test-qapi-visit.o
  CC      tests/include/test-qapi-visit-sub-module.o
/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)
             ^
In file included from /tmp/qemu-test/src/tests/test-char.c:10:0:
/tmp/qemu-test/src/include/sysemu/sysemu.h:117:6: note: previous declaration of 'main_loop' was here
 void main_loop(void);
      ^
make: *** [tests/test-char.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 662, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=24c0cf5ab5a24bf99c3e0023063c73e0', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-9qdq_pm7/src/docker-src.2019-10-30-11.19.26.32505:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=24c0cf5ab5a24bf99c3e0023063c73e0
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-9qdq_pm7/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    4m30.750s
user    0m8.740s


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

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

* Re: [PATCH v4 00/20] Add virtual device fuzzing support
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (20 preceding siblings ...)
  2019-10-30 15:23 ` [PATCH v4 00/20] Add virtual device fuzzing support no-reply
@ 2019-11-05 13:57 ` Darren Kenny
  2019-11-05 16:28   ` Alexander Oleinik
  2019-11-07 13:41 ` Stefan Hajnoczi
  22 siblings, 1 reply; 57+ messages in thread
From: Darren Kenny @ 2019-11-05 13:57 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: qemu-devel

Hi Alexander,

I've been trying out these patches, and I'm seeing a high volume of
crashes - where for v3, there were none in a run of over 3 weeks -
so it was a bit of a surprise :)

The question is what may have changed that is causing that level of
crashes - are you seeing this for the virtio-net-fork-fuzz tests?

But also, I've been trying to debug some of these crashes - and the
expectation is that you pass the crash-XXXX file as an argument to
the qemu-fuzz-* binary - and when I do, I see the crash - but when I
try to debug it, it ends up running through and exiting.

My assumption is that because of the fork in the test, the crash is
in one of the children.

(ASIDE: I think it might be worth adding a debugging/analysing
section to the documentation you've added to help people debug such
crashes)

Setting follow-fork-mode to child does get me there, and each crash
seems, at least in the samples that I've taken, to be in iov_copy:

  #0  0x00007ffff4cff377 in raise () from /lib64/libc.so.6
  #1  0x00007ffff4d00a68 in abort () from /lib64/libc.so.6
  #2  0x00007ffff4cf8196 in __assert_fail_base () from
  /lib64/libc.so.6
  #3  0x00007ffff4cf8242 in __assert_fail () from /lib64/libc.so.6
  #4  0x00005555574d4026 in iov_copy ()
  #5  0x000055555640dbd8 in virtio_net_flush_tx ()
  #6  0x000055555640c8ef in virtio_net_tx_bh ()
  #7  0x00005555574a05bb in aio_bh_call ()
  #8  0x00005555574a0a34 in aio_bh_poll ()
  #9  0x00005555574b1687 in aio_dispatch ()
  #10 0x00005555574a35f9 in aio_ctx_dispatch ()
  #11 0x00007ffff5e5d099 in g_main_context_dispatch () from
  /lib64/libglib-2.0.so.0
  #12 0x00005555574ae9fd in glib_pollfds_poll ()
  #13 0x00005555574ad972 in os_host_main_loop_wait ()
  #14 0x00005555574ad62c in main_loop_wait ()
  #15 0x000055555736c653 in flush_events ()
  #16 0x00005555573710a4 in virtio_net_fork_fuzz ()
  #17 0x000055555736cb85 in LLVMFuzzerTestOneInput ()
  ...

Have you seen these kind of crashes, or is this just me?

Just wondering if I should dig into it as a real issue, or some
mis-merge I've done (not all the patches were cleanly applied for
me when I cloned from master).

Thanks,

Darren.

On Wed, Oct 30, 2019 at 02:49:47PM +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.
>
>V4:
> * add/transfer license headers to new files
> * restructure the added QTestClientTransportOps struct
> * restructure the FuzzTarget struct and fuzzer skeleton
> * fork-based fuzzer now directly mmaps shm over the coverage bitmaps
> * fixes to i440 and virtio-net fuzz targets
> * undo the changes to qtest_memwrite
> * possible to build /fuzz and /all in the same build-dir
> * misc fixes to address V3 comments
>
>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 (20):
>  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
>  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 configure flag --enable-fuzzing
>  fuzz: Add target/fuzz makefile rules
>  fuzz: add fuzzer skeleton
>  fuzz: add support for fork-based fuzzing.
>  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                     |  16 ++-
> Makefile.objs                |   4 +
> Makefile.target              |  18 ++-
> configure                    |  39 ++++++
> docs/devel/fuzzing.txt       | 119 ++++++++++++++++++
> exec.c                       |  12 +-
> include/qemu/module.h        |   4 +-
> include/sysemu/qtest.h       |   4 +
> include/sysemu/sysemu.h      |   4 +
> main.c                       |  52 ++++++++
> qtest.c                      |  30 ++++-
> tests/Makefile.include       |  75 +++++------
> tests/fuzz/Makefile.include  |  11 ++
> tests/fuzz/fork_fuzz.c       |  51 ++++++++
> tests/fuzz/fork_fuzz.h       |  23 ++++
> tests/fuzz/fork_fuzz.ld      |  37 ++++++
> tests/fuzz/fuzz.c            | 177 ++++++++++++++++++++++++++
> tests/fuzz/fuzz.h            |  66 ++++++++++
> tests/fuzz/i440fx_fuzz.c     | 176 ++++++++++++++++++++++++++
> tests/fuzz/qos_fuzz.c        | 232 +++++++++++++++++++++++++++++++++++
> tests/fuzz/qos_fuzz.h        |  33 +++++
> tests/fuzz/virtio_net_fuzz.c | 123 +++++++++++++++++++
> 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  | 168 +++++++++++++++++++++++++
> tests/libqos/qos_external.h  |  28 +++++
> tests/libqtest.c             | 109 ++++++++++++++--
> tests/libqtest.h             |   4 +
> tests/pca9552-test.c         |  10 +-
> tests/qos-test.c             | 140 +--------------------
> util/module.c                |   7 ++
> vl.c                         |  36 ++----
> 34 files changed, 1601 insertions(+), 237 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
>
>


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

* Re: [PATCH v4 00/20] Add virtual device fuzzing support
  2019-11-05 13:57 ` Darren Kenny
@ 2019-11-05 16:28   ` Alexander Oleinik
  2019-11-05 16:47     ` Darren Kenny
  0 siblings, 1 reply; 57+ messages in thread
From: Alexander Oleinik @ 2019-11-05 16:28 UTC (permalink / raw)
  To: Darren Kenny; +Cc: qemu-devel

On 11/5/19 8:57 AM, Darren Kenny wrote:
> Hi Alexander,
> 
> I've been trying out these patches, and I'm seeing a high volume of
> crashes - where for v3, there were none in a run of over 3 weeks -
> so it was a bit of a surprise :)
> 
> The question is what may have changed that is causing that level of
> crashes - are you seeing this for the virtio-net-fork-fuzz tests?
Good question - my guess is that it may have to do with the change in 
how we run the main loop. I have not looked into it in much detail, but 
the crash below is likely triggered only after running the main_loop 
several times (the events handled in the first loop, schedule additional 
BHs). In v3, I believe the main_loop only ran once before the forked 
process exited. There are also changes to the linker-script used to 
facilitate communication between the forked process and the parent, but 
I think that would only impact the coverage information passed back to 
the parent.

> But also, I've been trying to debug some of these crashes - and the
> expectation is that you pass the crash-XXXX file as an argument to
> the qemu-fuzz-* binary - and when I do, I see the crash - but when I
> try to debug it, it ends up running through and exiting
> My assumption is that because of the fork in the test, the crash is
> in one of the children.
Right! Seems you are already using the follow-fork-mode option.

> (ASIDE: I think it might be worth adding a debugging/analysing
> section to the documentation you've added to help people debug such
> crashes)
Will do. Although it did not make it into v4, I am also working on an 
option to dump a trace of the qtest commands leading to a crashing 
input, which can then be replayed with a standard qtest program 
"replay.c". This seems like a good way to provide a reproducer to the 
device developers who may not be familiar with the fuzzer, or have time 
to build it.

> Setting follow-fork-mode to child does get me there, and each crash
> seems, at least in the samples that I've taken, to be in iov_copy:
Yes - this is what I have been using as well.

>   #0  0x00007ffff4cff377 in raise () from /lib64/libc.so.6
>   #1  0x00007ffff4d00a68 in abort () from /lib64/libc.so.6
>   #2  0x00007ffff4cf8196 in __assert_fail_base () from
>   /lib64/libc.so.6
>   #3  0x00007ffff4cf8242 in __assert_fail () from /lib64/libc.so.6
>   #4  0x00005555574d4026 in iov_copy ()
>   #5  0x000055555640dbd8 in virtio_net_flush_tx ()
>   #6  0x000055555640c8ef in virtio_net_tx_bh ()
>   #7  0x00005555574a05bb in aio_bh_call ()
>   #8  0x00005555574a0a34 in aio_bh_poll ()
>   #9  0x00005555574b1687 in aio_dispatch ()
>   #10 0x00005555574a35f9 in aio_ctx_dispatch ()
>   #11 0x00007ffff5e5d099 in g_main_context_dispatch () from
>   /lib64/libglib-2.0.so.0
>   #12 0x00005555574ae9fd in glib_pollfds_poll ()
>   #13 0x00005555574ad972 in os_host_main_loop_wait ()
>   #14 0x00005555574ad62c in main_loop_wait ()
>   #15 0x000055555736c653 in flush_events ()
>   #16 0x00005555573710a4 in virtio_net_fork_fuzz ()
>   #17 0x000055555736cb85 in LLVMFuzzerTestOneInput ()
>   ...
> 
> Have you seen these kind of crashes, or is this just me?
Not just you :) I posted a fix for this, but it may have not been 
complete. I think the fuzzer found it before we added forking, just by 
doing reboots in between runs:
https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg04882.html

> Just wondering if I should dig into it as a real issue, or some
> mis-merge I've done (not all the patches were cleanly applied for
> me when I cloned from master).

> Thanks,
> 
> Darren.
> 
> On Wed, Oct 30, 2019 at 02:49:47PM +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.
>>
>> V4:
>> * add/transfer license headers to new files
>> * restructure the added QTestClientTransportOps struct
>> * restructure the FuzzTarget struct and fuzzer skeleton
>> * fork-based fuzzer now directly mmaps shm over the coverage bitmaps
>> * fixes to i440 and virtio-net fuzz targets
>> * undo the changes to qtest_memwrite
>> * possible to build /fuzz and /all in the same build-dir
>> * misc fixes to address V3 comments
>>
>> 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 (20):
>>  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
>>  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 configure flag --enable-fuzzing
>>  fuzz: Add target/fuzz makefile rules
>>  fuzz: add fuzzer skeleton
>>  fuzz: add support for fork-based fuzzing.
>>  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                     |  16 ++-
>> Makefile.objs                |   4 +
>> Makefile.target              |  18 ++-
>> configure                    |  39 ++++++
>> docs/devel/fuzzing.txt       | 119 ++++++++++++++++++
>> exec.c                       |  12 +-
>> include/qemu/module.h        |   4 +-
>> include/sysemu/qtest.h       |   4 +
>> include/sysemu/sysemu.h      |   4 +
>> main.c                       |  52 ++++++++
>> qtest.c                      |  30 ++++-
>> tests/Makefile.include       |  75 +++++------
>> tests/fuzz/Makefile.include  |  11 ++
>> tests/fuzz/fork_fuzz.c       |  51 ++++++++
>> tests/fuzz/fork_fuzz.h       |  23 ++++
>> tests/fuzz/fork_fuzz.ld      |  37 ++++++
>> tests/fuzz/fuzz.c            | 177 ++++++++++++++++++++++++++
>> tests/fuzz/fuzz.h            |  66 ++++++++++
>> tests/fuzz/i440fx_fuzz.c     | 176 ++++++++++++++++++++++++++
>> tests/fuzz/qos_fuzz.c        | 232 +++++++++++++++++++++++++++++++++++
>> tests/fuzz/qos_fuzz.h        |  33 +++++
>> tests/fuzz/virtio_net_fuzz.c | 123 +++++++++++++++++++
>> 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  | 168 +++++++++++++++++++++++++
>> tests/libqos/qos_external.h  |  28 +++++
>> tests/libqtest.c             | 109 ++++++++++++++--
>> tests/libqtest.h             |   4 +
>> tests/pca9552-test.c         |  10 +-
>> tests/qos-test.c             | 140 +--------------------
>> util/module.c                |   7 ++
>> vl.c                         |  36 ++----
>> 34 files changed, 1601 insertions(+), 237 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
>>
>>



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

* Re: [PATCH v4 01/20] softmmu: split off vl.c:main() into main.c
  2019-10-30 14:49 ` [PATCH v4 01/20] softmmu: split off vl.c:main() into main.c Oleinik, Alexander
@ 2019-11-05 16:41   ` Darren Kenny
  2019-11-12 16:46     ` Alexander Bulekov
  2019-11-06 15:01   ` Stefan Hajnoczi
  1 sibling, 1 reply; 57+ messages in thread
From: Darren Kenny @ 2019-11-05 16:41 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: Paolo Bonzini, qemu-devel

On Wed, Oct 30, 2019 at 02:49:48PM +0000, Oleinik, Alexander wrote:
>From: Alexander Oleinik <alxndr@bu.edu>
>
>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                  | 52 +++++++++++++++++++++++++++++++++++++++++
> vl.c                    | 36 +++++++---------------------
> 5 files changed, 68 insertions(+), 27 deletions(-)
> create mode 100644 main.c
>
>diff --git a/Makefile b/Makefile
>index 0e994a275d..d2b2ecd3c4 100644
>--- a/Makefile
>+++ b/Makefile
>@@ -474,6 +474,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 11ba1a36bd..9ff9b0c6f9 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..03f9838b81 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);
>+void 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..ecd6389424
>--- /dev/null
>+++ b/main.c
>@@ -0,0 +1,52 @@
>+/*
>+ * QEMU System Emulator
>+ *
>+ * Copyright (c) 2003-2008 Fabrice Bellard
>+ *
>+ * Permission is hereby granted, free of charge, to any person obtaining a copy
>+ * of this software and associated documentation files (the "Software"), to deal
>+ * in the Software without restriction, including without limitation the rights
>+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>+ * copies of the Software, and to permit persons to whom the Software is
>+ * furnished to do so, subject to the following conditions:
>+ *
>+ * The above copyright notice and this permission notice shall be included in
>+ * all copies or substantial portions of the Software.
>+ *
>+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
>+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>+ * THE SOFTWARE.
>+ */
>+
>+#include "qemu/osdep.h"
>+#include "sysemu/sysemu.h"
>+
>+#ifdef CONFIG_SDL
>+#if defined(__APPLE__) || defined(main)
>+#include <SDL.h>
>+int main(int argc, char **argv)
>+{
>+    return qemu_main(argc, argv, NULL);
>+}
>+#undef main
>+#define main qemu_main

This /looks/ wrong, you're defining a function main(), and then
immediately #undef and #define main again.

Maybe this could be written differently, or add a comment here as to
why you need to do this.

>+#endif
>+#endif /* CONFIG_SDL */
>+
>+#ifdef CONFIG_COCOA
>+#undef main
>+#define main qemu_main
>+#endif /* CONFIG_COCOA */

I don't really know the combinations that might exist, but it looks
like if CONFIG_SDL is not defined, then we're redefining main() to be
qemi_main() - so what main() function will actually be used here?

Thanks,

Darren.

>+
>+int main(int argc, char **argv, char **envp)
>+{
>+    qemu_init(argc, argv, envp);
>+    main_loop();
>+    qemu_cleanup();
>+
>+    return 0;
>+}
>diff --git a/vl.c b/vl.c
>index c389d24b2c..472f09e12a 100644
>--- a/vl.c
>+++ b/vl.c
>@@ -36,25 +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
>-#define main qemu_main
>-#endif /* CONFIG_COCOA */
>-
>-
> #include "qemu/error-report.h"
> #include "qemu/sockets.h"
> #include "sysemu/accel.h"
>@@ -1797,7 +1778,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;
>@@ -2824,7 +2805,7 @@ static void user_register_global_props(void)
>                       global_init_func, NULL, NULL);
> }
>
>-int main(int argc, char **argv, char **envp)
>+void qemu_init(int argc, char **argv, char **envp)
> {
>     int i;
>     int snapshot, linux_boot;
>@@ -3404,7 +3385,7 @@ int main(int argc, char **argv, char **envp)
>             case QEMU_OPTION_watchdog:
>                 if (watchdog) {
>                     error_report("only one watchdog option may be given");
>-                    return 1;
>+                    exit(1);
>                 }
>                 watchdog = optarg;
>                 break;
>@@ -4440,7 +4421,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) {
>@@ -4457,8 +4438,11 @@ int main(int argc, char **argv, char **envp)
>     accel_setup_post(current_machine);
>     os_setup_post();
>
>-    main_loop();
>+    return;
>+}
>
>+void qemu_cleanup(void)
>+{
>     gdbserver_cleanup();
>
>     /*
>@@ -4495,6 +4479,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
>
>


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

* Re: [PATCH v4 00/20] Add virtual device fuzzing support
  2019-11-05 16:28   ` Alexander Oleinik
@ 2019-11-05 16:47     ` Darren Kenny
  0 siblings, 0 replies; 57+ messages in thread
From: Darren Kenny @ 2019-11-05 16:47 UTC (permalink / raw)
  To: Alexander Oleinik; +Cc: qemu-devel

On Tue, Nov 05, 2019 at 11:28:59AM -0500, Alexander Oleinik wrote:
>On 11/5/19 8:57 AM, Darren Kenny wrote:
>>Hi Alexander,
>>
>>I've been trying out these patches, and I'm seeing a high volume of
>>crashes - where for v3, there were none in a run of over 3 weeks -
>>so it was a bit of a surprise :)
>>
>>The question is what may have changed that is causing that level of
>>crashes - are you seeing this for the virtio-net-fork-fuzz tests?
>Good question - my guess is that it may have to do with the change in 
>how we run the main loop. I have not looked into it in much detail, 
>but the crash below is likely triggered only after running the 
>main_loop several times (the events handled in the first loop, 
>schedule additional BHs). In v3, I believe the main_loop only ran once 
>before the forked process exited. There are also changes to the 
>linker-script used to facilitate communication between the forked 
>process and the parent, but I think that would only impact the 
>coverage information passed back to the parent.

OK, sounds like this is a genuine issue then - great :)

>>But also, I've been trying to debug some of these crashes - and the
>>expectation is that you pass the crash-XXXX file as an argument to
>>the qemu-fuzz-* binary - and when I do, I see the crash - but when I
>>try to debug it, it ends up running through and exiting
>>My assumption is that because of the fork in the test, the crash is
>>in one of the children.
>Right! Seems you are already using the follow-fork-mode option.

Yup

>
>>(ASIDE: I think it might be worth adding a debugging/analysing
>>section to the documentation you've added to help people debug such
>>crashes)
>Will do. Although it did not make it into v4, I am also working on an 
>option to dump a trace of the qtest commands leading to a crashing 
>input, which can then be replayed with a standard qtest program 
>"replay.c". This seems like a good way to provide a reproducer to the 
>device developers who may not be familiar with the fuzzer, or have 
>time to build it.

That would be great!

For me the biggest problem was that I didn't have a version of clang
new enough (OL7/RHEL7) to have Libfuzzer, so if people don't have
it, then building with it's support is a bit more involved since you
also need to build clang, etc.

>>Setting follow-fork-mode to child does get me there, and each crash
>>seems, at least in the samples that I've taken, to be in iov_copy:
>Yes - this is what I have been using as well.
>
>>  #0  0x00007ffff4cff377 in raise () from /lib64/libc.so.6
>>  #1  0x00007ffff4d00a68 in abort () from /lib64/libc.so.6
>>  #2  0x00007ffff4cf8196 in __assert_fail_base () from
>>  /lib64/libc.so.6
>>  #3  0x00007ffff4cf8242 in __assert_fail () from /lib64/libc.so.6
>>  #4  0x00005555574d4026 in iov_copy ()
>>  #5  0x000055555640dbd8 in virtio_net_flush_tx ()
>>  #6  0x000055555640c8ef in virtio_net_tx_bh ()
>>  #7  0x00005555574a05bb in aio_bh_call ()
>>  #8  0x00005555574a0a34 in aio_bh_poll ()
>>  #9  0x00005555574b1687 in aio_dispatch ()
>>  #10 0x00005555574a35f9 in aio_ctx_dispatch ()
>>  #11 0x00007ffff5e5d099 in g_main_context_dispatch () from
>>  /lib64/libglib-2.0.so.0
>>  #12 0x00005555574ae9fd in glib_pollfds_poll ()
>>  #13 0x00005555574ad972 in os_host_main_loop_wait ()
>>  #14 0x00005555574ad62c in main_loop_wait ()
>>  #15 0x000055555736c653 in flush_events ()
>>  #16 0x00005555573710a4 in virtio_net_fork_fuzz ()
>>  #17 0x000055555736cb85 in LLVMFuzzerTestOneInput ()
>>  ...
>>
>>Have you seen these kind of crashes, or is this just me?
>Not just you :) I posted a fix for this, but it may have not been 
>complete. I think the fuzzer found it before we added forking, just by 
>doing reboots in between runs:
>https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg04882.html

Ah! So I guess this hasn't been pulled into master yet (or at least
wasn't when I pulled from there yesterday.

Thanks,

Darren.

>
>>Just wondering if I should dig into it as a real issue, or some
>>mis-merge I've done (not all the patches were cleanly applied for
>>me when I cloned from master).
>
>>Thanks,
>>
>>Darren.
>>
>>On Wed, Oct 30, 2019 at 02:49:47PM +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.
>>>
>>>V4:
>>>* add/transfer license headers to new files
>>>* restructure the added QTestClientTransportOps struct
>>>* restructure the FuzzTarget struct and fuzzer skeleton
>>>* fork-based fuzzer now directly mmaps shm over the coverage bitmaps
>>>* fixes to i440 and virtio-net fuzz targets
>>>* undo the changes to qtest_memwrite
>>>* possible to build /fuzz and /all in the same build-dir
>>>* misc fixes to address V3 comments
>>>
>>>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 (20):
>>> 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
>>> 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 configure flag --enable-fuzzing
>>> fuzz: Add target/fuzz makefile rules
>>> fuzz: add fuzzer skeleton
>>> fuzz: add support for fork-based fuzzing.
>>> 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                     |  16 ++-
>>>Makefile.objs                |   4 +
>>>Makefile.target              |  18 ++-
>>>configure                    |  39 ++++++
>>>docs/devel/fuzzing.txt       | 119 ++++++++++++++++++
>>>exec.c                       |  12 +-
>>>include/qemu/module.h        |   4 +-
>>>include/sysemu/qtest.h       |   4 +
>>>include/sysemu/sysemu.h      |   4 +
>>>main.c                       |  52 ++++++++
>>>qtest.c                      |  30 ++++-
>>>tests/Makefile.include       |  75 +++++------
>>>tests/fuzz/Makefile.include  |  11 ++
>>>tests/fuzz/fork_fuzz.c       |  51 ++++++++
>>>tests/fuzz/fork_fuzz.h       |  23 ++++
>>>tests/fuzz/fork_fuzz.ld      |  37 ++++++
>>>tests/fuzz/fuzz.c            | 177 ++++++++++++++++++++++++++
>>>tests/fuzz/fuzz.h            |  66 ++++++++++
>>>tests/fuzz/i440fx_fuzz.c     | 176 ++++++++++++++++++++++++++
>>>tests/fuzz/qos_fuzz.c        | 232 +++++++++++++++++++++++++++++++++++
>>>tests/fuzz/qos_fuzz.h        |  33 +++++
>>>tests/fuzz/virtio_net_fuzz.c | 123 +++++++++++++++++++
>>>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  | 168 +++++++++++++++++++++++++
>>>tests/libqos/qos_external.h  |  28 +++++
>>>tests/libqtest.c             | 109 ++++++++++++++--
>>>tests/libqtest.h             |   4 +
>>>tests/pca9552-test.c         |  10 +-
>>>tests/qos-test.c             | 140 +--------------------
>>>util/module.c                |   7 ++
>>>vl.c                         |  36 ++----
>>>34 files changed, 1601 insertions(+), 237 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
>>>
>>>
>


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

* Re: [PATCH v4 03/20] fuzz: Add FUZZ_TARGET module type
  2019-10-30 14:49 ` [PATCH v4 03/20] fuzz: Add FUZZ_TARGET module type Oleinik, Alexander
@ 2019-11-06 13:17   ` Darren Kenny
  2019-11-06 15:18   ` Stefan Hajnoczi
  1 sibling, 0 replies; 57+ messages in thread
From: Darren Kenny @ 2019-11-06 13:17 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: qemu-devel

On Wed, Oct 30, 2019 at 02:49:50PM +0000, Oleinik, Alexander wrote:
>From: Alexander Oleinik <alxndr@bu.edu>
>
>Signed-off-by: Alexander Oleinik <alxndr@bu.edu>

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

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


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

* Re: [PATCH v4 04/20] qtest: add qtest_server_send abstraction
  2019-10-30 14:49 ` [PATCH v4 04/20] qtest: add qtest_server_send abstraction Oleinik, Alexander
@ 2019-11-06 13:29   ` Darren Kenny
  2019-11-06 15:19   ` Stefan Hajnoczi
  1 sibling, 0 replies; 57+ messages in thread
From: Darren Kenny @ 2019-11-06 13:29 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: Laurent Vivier, Paolo Bonzini, Thomas Huth, qemu-devel

On Wed, Oct 30, 2019 at 02:49:51PM +0000, Oleinik, Alexander wrote:
>From: Alexander Oleinik <alxndr@bu.edu>
>
>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));

Given that this is a function pointer, it may be worth asserting
that it is not NULL.

>+    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)

It's a nit, but it might be better to rename this to match the
variables actually being set, i.e. s/_tx_/_send_/.

Thanks,

Darren.

>+{
>+    qtest_server_send = send;
>+    qtest_server_send_opaque = opaque;
> }
>
> bool qtest_driver(void)
>-- 
>2.23.0
>
>


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

* Re: [PATCH v4 01/20] softmmu: split off vl.c:main() into main.c
  2019-10-30 14:49 ` [PATCH v4 01/20] softmmu: split off vl.c:main() into main.c Oleinik, Alexander
  2019-11-05 16:41   ` Darren Kenny
@ 2019-11-06 15:01   ` Stefan Hajnoczi
  1 sibling, 0 replies; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-06 15:01 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: Paolo Bonzini, qemu-devel

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

On Wed, Oct 30, 2019 at 02:49:48PM +0000, Oleinik, Alexander wrote:
> diff --git a/main.c b/main.c
> new file mode 100644
> index 0000000000..ecd6389424
> --- /dev/null
> +++ b/main.c
> @@ -0,0 +1,52 @@
> +/*
> + * QEMU System Emulator
> + *
> + * Copyright (c) 2003-2008 Fabrice Bellard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"

Perhaps this should be added:

  #include "qemu-common.h"

It has:

  /* main function, renamed */
  #if defined(CONFIG_COCOA)
  int qemu_main(int argc, char **argv, char **envp);
  #endif

This way the compiler can check prototypes.

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

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

* Re: [PATCH v4 02/20] libqos: Rename i2c_send and i2c_recv
  2019-10-30 14:49 ` [PATCH v4 02/20] libqos: Rename i2c_send and i2c_recv Oleinik, Alexander
@ 2019-11-06 15:17   ` Stefan Hajnoczi
  0 siblings, 0 replies; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-06 15:17 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: Laurent Vivier, Paolo Bonzini, Thomas Huth, qemu-devel

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

On Wed, Oct 30, 2019 at 02:49:49PM +0000, Oleinik, Alexander wrote:
> diff --git a/tests/libqos/i2c-imx.c b/tests/libqos/i2c-imx.c
> index f33ece55a3..42ebf8ba3a 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 qimx_i2c_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 qimx_i2c_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 = qimx_i2c_send;
> +    s->parent.recv = qimx_i2c_recv;

Why do the imx static functions need to be renamed?  Is this by
accident, maybe an aggressive sed command that touched all files?

>      s->parent.qts = qts;
>  }
>  
> diff --git a/tests/libqos/i2c-omap.c b/tests/libqos/i2c-omap.c
> index 9ae8214fa8..5f4d79f87c 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 qomap_i2c_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 qomap_i2c_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 = qomap_i2c_send;
> +    s->parent.recv = qomap_i2c_recv;

Same here.

Otherwise:

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

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

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

* Re: [PATCH v4 03/20] fuzz: Add FUZZ_TARGET module type
  2019-10-30 14:49 ` [PATCH v4 03/20] fuzz: Add FUZZ_TARGET module type Oleinik, Alexander
  2019-11-06 13:17   ` Darren Kenny
@ 2019-11-06 15:18   ` Stefan Hajnoczi
  1 sibling, 0 replies; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-06 15:18 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: qemu-devel

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

On Wed, Oct 30, 2019 at 02:49:50PM +0000, Oleinik, Alexander wrote:
> From: Alexander Oleinik <alxndr@bu.edu>
> 
> 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 --]

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

* Re: [PATCH v4 04/20] qtest: add qtest_server_send abstraction
  2019-10-30 14:49 ` [PATCH v4 04/20] qtest: add qtest_server_send abstraction Oleinik, Alexander
  2019-11-06 13:29   ` Darren Kenny
@ 2019-11-06 15:19   ` Stefan Hajnoczi
  1 sibling, 0 replies; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-06 15:19 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: Laurent Vivier, Paolo Bonzini, Thomas Huth, qemu-devel

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

On Wed, Oct 30, 2019 at 02:49:51PM +0000, Oleinik, Alexander wrote:
> From: Alexander Oleinik <alxndr@bu.edu>
> 
> 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 --]

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

* Re: [PATCH v4 06/20] module: check module wasn't already initialized
  2019-10-30 14:49 ` [PATCH v4 06/20] module: check module wasn't already initialized Oleinik, Alexander
@ 2019-11-06 15:26   ` Stefan Hajnoczi
  2019-11-06 17:40   ` Darren Kenny
  1 sibling, 0 replies; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-06 15:26 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: qemu-devel

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

On Wed, Oct 30, 2019 at 02:49:52PM +0000, Oleinik, Alexander wrote:
> From: Alexander Oleinik <alxndr@bu.edu>
> 
> 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 --]

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

* Re: [PATCH v4 00/20] Add virtual device fuzzing support
  2019-10-30 15:23 ` [PATCH v4 00/20] Add virtual device fuzzing support no-reply
@ 2019-11-06 15:27   ` Stefan Hajnoczi
  0 siblings, 0 replies; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-06 15:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: alxndr

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

On Wed, Oct 30, 2019 at 08:23:57AM -0700, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20191030144926.11873-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 ===
> 
>   CC      tests/test-qapi-types-sub-sub-module.o
>   CC      tests/test-qapi-visit.o
>   CC      tests/include/test-qapi-visit-sub-module.o
> /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)
>              ^
> In file included from /tmp/qemu-test/src/tests/test-char.c:10:0:
> /tmp/qemu-test/src/include/sysemu/sysemu.h:117:6: note: previous declaration of 'main_loop' was here
>  void main_loop(void);
>       ^
> make: *** [tests/test-char.o] Error 1
> make: *** Waiting for unfinished jobs....
> Traceback (most recent call last):
>   File "./tests/docker/docker.py", line 662, in <module>

Oops, the main_loop() change definitely broke tests/test-char.c.  Please
take a look.

Stefan

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

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

* Re: [PATCH v4 05/20] libqtest: Add a layer of abstraciton to send/recv
  2019-10-30 14:49 ` [PATCH v4 05/20] libqtest: Add a layer of abstraciton to send/recv Oleinik, Alexander
@ 2019-11-06 16:22   ` Stefan Hajnoczi
  0 siblings, 0 replies; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-06 16:22 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: Laurent Vivier, Paolo Bonzini, Thomas Huth, qemu-devel

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

On Wed, Oct 30, 2019 at 02:49:52PM +0000, Oleinik, Alexander wrote:
> @@ -360,6 +383,7 @@ void qtest_quit(QTestState *s)
>      g_free(s);
>  }
>  
> +
>  static void socket_send(int fd, const char *buf, size_t size)
>  {
>      size_t offset;
[...]
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index c9e21e05b3..31267fc915 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -728,5 +728,4 @@ bool qtest_probe_child(QTestState *s);
>   * Set expected exit status of the child.
>   */
>  void qtest_set_expected_status(QTestState *s, int status);
> -
>  #endif

Please avoid whitespace changes.

Otherwise:

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

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

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

* Re: [PATCH v4 07/20] qtest: add in-process incoming command handler
  2019-10-30 14:49 ` [PATCH v4 07/20] qtest: add in-process incoming command handler Oleinik, Alexander
@ 2019-11-06 16:33   ` Stefan Hajnoczi
  0 siblings, 0 replies; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-06 16:33 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: Laurent Vivier, Paolo Bonzini, Thomas Huth, qemu-devel

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

On Wed, Oct 30, 2019 at 02:49:53PM +0000, Oleinik, Alexander wrote:
> diff --git a/qtest.c b/qtest.c
> index ae7e6d779d..9fbfa0f08f 100644
> --- a/qtest.c
> +++ b/qtest.c
> @@ -802,3 +802,16 @@ bool qtest_driver(void)
>  {
>      return qtest_chr.chr != NULL;
>  }
> +
> +void qtest_server_inproc_recv(void *dummy, const char *buf, size_t size)
> +{
> +    static GString *gstr;
> +    if (!gstr) {
> +        gstr = g_string_new(NULL);
> +    }
> +    g_string_append(gstr, buf);
> +    if (gstr->str[gstr->len - 1] == '\n') {
> +        qtest_process_inbuf(NULL, gstr);
> +        g_string_free(gstr, true);

This double-frees gstr.  Please add:

  gstr = NULL;

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

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

* Re: [PATCH v4 10/20] libqos: move useful qos-test funcs to qos_external
  2019-10-30 14:49 ` [PATCH v4 10/20] libqos: move useful qos-test funcs to qos_external Oleinik, Alexander
@ 2019-11-06 16:41   ` Stefan Hajnoczi
  0 siblings, 0 replies; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-06 16:41 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: Laurent Vivier, Paolo Bonzini, Thomas Huth, qemu-devel

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

On Wed, Oct 30, 2019 at 02:49:56PM +0000, Oleinik, Alexander wrote:
> From: Alexander Oleinik <alxndr@bu.edu>
> 
> 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 | 168 ++++++++++++++++++++++++++++++++++++
>  tests/libqos/qos_external.h |  28 ++++++
>  tests/qos-test.c            | 140 ++----------------------------
>  4 files changed, 202 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 --]

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

* Re: [PATCH v4 11/20] libqtest: make qtest_bufwrite send "atomic"
  2019-10-30 14:49 ` [PATCH v4 11/20] libqtest: make qtest_bufwrite send "atomic" Oleinik, Alexander
@ 2019-11-06 16:44   ` Stefan Hajnoczi
  0 siblings, 0 replies; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-06 16:44 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: Laurent Vivier, Paolo Bonzini, Thomas Huth, qemu-devel

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

On Wed, Oct 30, 2019 at 02:49:57PM +0000, Oleinik, Alexander wrote:
> From: Alexander Oleinik <alxndr@bu.edu>
> 
> 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, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 822bfe208b..ff3153daf2 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -1083,8 +1083,8 @@ void qtest_bufwrite(QTestState *s, uint64_t addr, const void *data, size_t size)
>  
>      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);
> +    s->ops.send(s, bdata, strlen(bdata));
> +    s->ops.send(s, "\n", 1);
>      qtest_rsp(s, 0);
>      g_free(bdata);
>  }

Please update the commit message and description - they no longer seem
to match what the patch is doing.  The qtest_sendf() is not atomic, it
is still split into 3 send operations.

Stefan

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

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

* Re: [PATCH v4 12/20] libqtest: add in-process qtest.c tx/rx handlers
  2019-10-30 14:49 ` [PATCH v4 12/20] libqtest: add in-process qtest.c tx/rx handlers Oleinik, Alexander
@ 2019-11-06 16:56   ` Stefan Hajnoczi
  2019-11-12 17:38     ` Alexander Bulekov
  0 siblings, 1 reply; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-06 16:56 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: Laurent Vivier, Paolo Bonzini, Thomas Huth, qemu-devel

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

On Wed, Oct 30, 2019 at 02:49:58PM +0000, Oleinik, Alexander wrote:
> From: Alexander Oleinik <alxndr@bu.edu>
> 
> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> ---
> There's a particularily ugly line here:
> qtest_client_set_tx_handler(qts,
>         (void (*)(QTestState *s, const char*, size_t)) send);

Please typedef the function pointer to avoid repetition:

  typedef void (*QTestSendFn)(QTestState *s, const char *buf, size_t len);

And then introduce a wrapper function for type-safety:

  /* A type-safe wrapper for s->send() */
  static void send_wrapper(QTestState *s, const char *buf, size_t len)
  {
      s->send(s, buf, len);
  }

  ...

  qts->send = send;
  qtest_client_set_tx_handler(qts, send_wrapper);

Does this solve the issue?

By the way, I also wonder whether the size_t len arguments are necessary
since const char *buf is a NUL-terminated C string.  The string should
be enough since the length can be calculated from it.

> diff --git a/qtest.c b/qtest.c
> index 9fbfa0f08f..f817a5d789 100644
> --- a/qtest.c
> +++ b/qtest.c
> @@ -812,6 +812,6 @@ void qtest_server_inproc_recv(void *dummy, const char *buf, size_t size)
>      g_string_append(gstr, buf);
>      if (gstr->str[gstr->len - 1] == '\n') {
>          qtest_process_inbuf(NULL, gstr);
> -        g_string_free(gstr, true);
> +        g_string_truncate(gstr, 0);

Ah, a fix for the bug in an earlier commit.  Please squash it.

> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index ff3153daf2..6143af33da 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -71,6 +71,7 @@ static void qtest_client_set_tx_handler(QTestState *s,
>  static void qtest_client_set_rx_handler(QTestState *s,
>          GString * (*recv)(QTestState *));
>  
> +static GString *recv_str;

Can this be a QTestState field?

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

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

* Re: [PATCH v4 13/20] fuzz: add configure flag --enable-fuzzing
  2019-10-30 14:49 ` [PATCH v4 13/20] fuzz: add configure flag --enable-fuzzing Oleinik, Alexander
@ 2019-11-06 16:57   ` Stefan Hajnoczi
  0 siblings, 0 replies; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-06 16:57 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: qemu-devel

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

On Wed, Oct 30, 2019 at 02:49:59PM +0000, Oleinik, Alexander wrote:
> From: Alexander Oleinik <alxndr@bu.edu>
> 
> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> ---
>  configure | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)

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

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

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

* Re: [PATCH v4 06/20] module: check module wasn't already initialized
  2019-10-30 14:49 ` [PATCH v4 06/20] module: check module wasn't already initialized Oleinik, Alexander
  2019-11-06 15:26   ` Stefan Hajnoczi
@ 2019-11-06 17:40   ` Darren Kenny
  1 sibling, 0 replies; 57+ messages in thread
From: Darren Kenny @ 2019-11-06 17:40 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: qemu-devel

On Wed, Oct 30, 2019 at 02:49:52PM +0000, Oleinik, Alexander wrote:
>From: Alexander Oleinik <alxndr@bu.edu>
>
>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>

My only question here really is whether there is any possibility of
the list of any given module type being modified later, if so this
might break things if attempts are made to re-init modules.

In that case, this test might be more correctly belong in the
module's own init() function instead.

Assuming for now that it is the correct place to do it, unless
someone can say otherwise:

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

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


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

* Re: [PATCH v4 15/20] fuzz: add fuzzer skeleton
  2019-10-30 14:50 ` [PATCH v4 15/20] fuzz: add fuzzer skeleton Oleinik, Alexander
@ 2019-11-07 12:55   ` Stefan Hajnoczi
  2019-11-12 19:04     ` Alexander Bulekov
  0 siblings, 1 reply; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-07 12:55 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: qemu-devel

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

On Wed, Oct 30, 2019 at 02:50:00PM +0000, Oleinik, Alexander wrote:
> diff --git a/tests/fuzz/fuzz.c b/tests/fuzz/fuzz.c
> new file mode 100644
> index 0000000000..0e38f81c48
> --- /dev/null
> +++ b/tests/fuzz/fuzz.c
> @@ -0,0 +1,177 @@
> +/*
> + * fuzzing driver
> + *
> + * Copyright Red Hat Inc., 2019
> + *
> + * Authors:
> + *  Alexander Bulekov   <alxndr@bu.edu>

Bulekov instead of Oleinik?

> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include <stdio.h>
> +#include <stdlib.h>

stdio.h and stdlib.h are already included by qemu/osdep.h.

> +/* Executed for each fuzzing-input */
> +int LLVMFuzzerTestOneInput(const unsigned char *Data, size_t Size)
> +{
> +    if (fuzz_target->fuzz) {

Will this ever be NULL?

> +        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;
> +
> +    /* 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];
> +    if (!strstr(target_name, "--fuzz-target=")) {
> +        usage(**argv);
> +    }
> +
> +    target_name += strlen("--fuzz-target=");
> +
> +    fuzz_target = fuzz_get_target(target_name);
> +    if (!fuzz_target) {
> +        usage(**argv);
> +    }
> +
> +    fuzz_qts = qtest_setup();
> +
> +    if (!fuzz_target) {

This is dead code since fuzz_target was already checked above.  Please
remove this if statement.

> +        fprintf(stderr, "Error: Fuzz fuzz_target name %s not found\n",
> +                target_name);
> +        usage(**argv);
> +    }
> +
> +    if (fuzz_target->pre_vm_init) {
> +        fuzz_target->pre_vm_init();
> +    }
> +
> +    /* Run QEMU's softmmu main with the fuzz-target dependent arguments */
> +    char *init_cmdline = fuzz_target->get_init_cmdline(fuzz_target);

Where is init_cmdline freed or should this be const char *?

> +    wordexp_t result;
> +    wordexp(init_cmdline, &result, 0);

What is the purpose of word expansion here?

> +
> +    qemu_init(result.we_wordc, result.we_wordv, NULL);
> +
> +    if (fuzz_target->pre_fuzz) {
> +        fuzz_target->pre_fuzz(fuzz_qts);
> +    }
> +
> +    return 0;
> +}
> diff --git a/tests/fuzz/fuzz.h b/tests/fuzz/fuzz.h
> new file mode 100644
> index 0000000000..b569b622d7
> --- /dev/null
> +++ b/tests/fuzz/fuzz.h
> @@ -0,0 +1,66 @@
> +/*
> + * fuzzing driver
> + *
> + * Copyright Red Hat Inc., 2019
> + *
> + * Authors:
> + *  Alexander Bulekov   <alxndr@bu.edu>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef 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"
> +
> +

Some documentation would be nice:

/**
 * A libfuzzer fuzzing target
 *
 * The QEMU fuzzing binary is built with all available targets, each
 * with a unique @name that can be specified on the command-line to
 * select which target should run.
 *
 * A target must implement ->fuzz() to process a random input.  If QEMU
 * crashes in ->fuzz() then libfuzzer will record a failure.
 *
 * Fuzzing targets are registered with fuzz_add_target():
 *
 *   static const FuzzTarget fuzz_target = {
 *       .name = "my-device-fifo",
 *       .description = "Fuzz the FIFO buffer registers of my-device",
 *       ...
 *   };
 *
 *   static void register_fuzz_target(void)
 *   {
 *       fuzz_add_target(&fuzz_target);
 *   }
 *   fuzz_target_init(register_fuzz_target);
 */

> +typedef struct FuzzTarget {
> +    const char *name;         /* command-line option(FUZZ_TARGET) for the target */
> +    const char *description;  /* help text */
> +

If any of the function pointers can be NULL, please document this.

> +    /* returns the arg-list that is passed to qemu/softmmu init() */
> +    char* (*get_init_cmdline)(struct FuzzTarget *);

Does the caller need to call g_free() on the returned string?  Please
document this.

> +
> +    /*
> +     * will run once, prior to running qemu/softmmu init.
> +     * eg: set up shared-memory for communication with the child-process
> +     */
> +    void(*pre_vm_init)(void);
> +
> +    /*
> +     * will run once, prior to to the fuzz-loop.

s/to to/to/

> +     * eg: detect the memory map
> +     */
> +    void(*pre_fuzz)(QTestState *);

Please also mention that QEMU has been initialized at this point.

> +
> +    /*
> +     * accepts and executes an input from libfuzzer. this is repeatedly
> +     * executed during the fuzzing loop. Its should handle setup, input
> +     * execution and cleanup
> +     */
> +    void(*fuzz)(QTestState *, const unsigned char *, size_t);
> +
> +} FuzzTarget;
> +
> +void flush_events(QTestState *);
> +void reboot(QTestState *);
> +
> +/*
> + * makes a copy of *target and adds it to the target-list.
> + * i.e. fine to set up target on the caller's stack
> + */
> +void fuzz_add_target(FuzzTarget *target);

"makes a copy of *target" -> does this mean the argument type can be
const FuzzTarget *target?

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

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

* Re: [PATCH v4 16/20] fuzz: add support for fork-based fuzzing.
  2019-10-30 14:50 ` [PATCH v4 16/20] fuzz: add support for fork-based fuzzing Oleinik, Alexander
@ 2019-11-07 13:17   ` Stefan Hajnoczi
  0 siblings, 0 replies; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-07 13:17 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: Paolo Bonzini, qemu-devel, Richard Henderson

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

On Wed, Oct 30, 2019 at 02:50:01PM +0000, Oleinik, Alexander wrote:
> diff --git a/tests/fuzz/fork_fuzz.c b/tests/fuzz/fork_fuzz.c
> new file mode 100644
> index 0000000000..4c4d00b034
> --- /dev/null
> +++ b/tests/fuzz/fork_fuzz.c
> @@ -0,0 +1,51 @@
> +/*
> + * Fork-based fuzzing helpers
> + *
> + * Copyright Red Hat Inc., 2019
> + *
> + * Authors:
> + *  Alexander Bulekov   <alxndr@bu.edu>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "fork_fuzz.h"
> +
> +uintptr_t feature_shm;

Where is this variable used?

> +
> +void counter_shm_init(void)
> +{
> +    int fd = shm_open("/qemu-fuzz-cntrs", O_CREAT | O_RDWR, S_IRUSR | S_IWUSR);

It must be possible to run multiple fuzzer instances simultaneously on
one host.  Please use a unique shmem path for each parent process (e.g.
getpid() in the parent and getppid() in the child).

> +    if (fd == -1) {
> +        perror("Error: ");
> +        exit(1);
> +    }
> +    if (ftruncate(fd, &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START) == -1) {
> +        perror("Error: ");
> +        exit(1);
> +    }
> +    /* Copy what's in the counter region to the shm.. */
> +    void *rptr = mmap(NULL ,
> +            &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START,
> +            PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> +    memcpy(rptr,
> +           &__FUZZ_COUNTERS_START,
> +           &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
> +
> +    munmap(rptr, &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
> +
> +    /* And map the shm over the counter region */
> +    rptr = mmap(&__FUZZ_COUNTERS_START,
> +            &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START,
> +            PROT_READ | PROT_WRITE, MAP_SHARED | MAP_FIXED, fd, 0);

fd can be closed here to prevent leaking it.

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

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

* Re: [PATCH v4 17/20] fuzz: add support for qos-assisted fuzz targets
  2019-10-30 14:50 ` [PATCH v4 17/20] fuzz: add support for qos-assisted fuzz targets Oleinik, Alexander
@ 2019-11-07 13:22   ` Stefan Hajnoczi
  0 siblings, 0 replies; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-07 13:22 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: qemu-devel

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

On Wed, Oct 30, 2019 at 02:50:02PM +0000, Oleinik, Alexander wrote:
> +static char *qos_build_main_args()

Please use func(void) in C.  In C () functions have unspecified and
unchecked arguments whereas in C++ () means (void).  We want the
compiler to complain if arguments are passed to this function, so it
needs to be (void).

> +{
> +    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 64 ");

Does i386 need to be hardcoded?  An earlier patch declared a fuzz_arch
or similar variable (from TARGET_NAME).

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

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

* Re: [PATCH v4 18/20] fuzz: add i440fx fuzz targets
  2019-10-30 14:50 ` [PATCH v4 18/20] fuzz: add i440fx " Oleinik, Alexander
@ 2019-11-07 13:26   ` Stefan Hajnoczi
  0 siblings, 0 replies; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-07 13:26 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: qemu-devel

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

On Wed, Oct 30, 2019 at 02:50:03PM +0000, Oleinik, Alexander wrote:
> +static void i440fx_fuzz_qos_fork(QTestState *s,
> +        const unsigned char *Data, size_t Size) {
> +    if (fork() == 0) {
> +        i440fx_fuzz_qos(s, Data, Size);
> +        _Exit(0);
> +    } else {
> +        wait(NULL);
> +    }
> +}
> +
> +static const char *i440fx_qtest_argv = "qemu_system_i386 -machine accel=qtest"

Binaries are named qemu-system-TARGET.  I guess nothing looks at argv[0]
but it should use hyphens instead of underscores.

> +                                       "-m 0 -display none";
> +static char *i440fx_argv(FuzzTarget *t)
> +{
> +    return (char *)i440fx_qtest_argv;

.get_init_cmdline() should probably return const char *.

Otherwise:

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

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

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

* Re: [PATCH v4 19/20] fuzz: add virtio-net fuzz target
  2019-10-30 14:50 ` [PATCH v4 19/20] fuzz: add virtio-net fuzz target Oleinik, Alexander
@ 2019-11-07 13:36   ` Stefan Hajnoczi
  2019-11-07 13:42   ` Jason Wang
  1 sibling, 0 replies; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-07 13:36 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: qemu-devel

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

On Wed, Oct 30, 2019 at 02:50:03PM +0000, Oleinik, Alexander wrote:
> +static void virtio_net_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;
> +        }

More concisely:

  while (Size < sizeof(vqa)) {

> +        memcpy(&vqa, Data, sizeof(vqa));
> +        vqa = *((vq_action *)Data);

The assignment is redundant after the memcpy.

> +        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) {

What is the difference between iters and reqi?  The values of these
variables are always identical so I think only one of them is needed.

> +            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);

Here we could run the main loop until free_head appears in the used
ring.  That should allow the request processing code path to be explored
more fully, but this it's okay to leave it like this for now.

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

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

* Re: [PATCH v4 20/20] fuzz: add documentation to docs/devel/
  2019-10-30 14:50 ` [PATCH v4 20/20] fuzz: add documentation to docs/devel/ Oleinik, Alexander
@ 2019-11-07 13:40   ` Stefan Hajnoczi
  2019-11-07 15:02     ` Alexander Oleinik
  0 siblings, 1 reply; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-07 13:40 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: qemu-devel

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

On Wed, Oct 30, 2019 at 02:50:04PM +0000, Oleinik, Alexander wrote:
> +== Building the fuzzers ==
> +
> +NOTE: If possible, build a 32-bit binary. When forking, the 32-bit fuzzer is
> +much faster, since the page-map has a smaller size. This is due to the fact that
> +AddressSanitizer mmaps ~20TB of memory, as part of its detection. This results
> +in a large page-map, and a much slower fork(). O
> +
> +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 similarly to system/softmmu:
> +
> +    make i386-softmmu/fuzz
> +
> +This builds ./i386-softmmu/qemu-fuzz-i386

I'm surprised that "make i386-softmmu/fuzz" builds
i386-softmmu/qemu-fuzz-i386.  Should that be "make
i386-softmmu/qemu-fuzz-i386"?

> += Implmentation Details =

s/Implmentation/Implementation/

> +
> +== The Fuzzer's Lifecycle ==
> +
> +The fuzzer has two entrypoints that libfuzzer calls. libfuzzer provides it's
> +own main(), which performs some setup, and calls the entrypoints:
> +
> +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.

s/initailized/initialized/

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

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

* Re: [PATCH v4 00/20] Add virtual device fuzzing support
  2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
                   ` (21 preceding siblings ...)
  2019-11-05 13:57 ` Darren Kenny
@ 2019-11-07 13:41 ` Stefan Hajnoczi
  22 siblings, 0 replies; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-07 13:41 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: qemu-devel

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

On Wed, Oct 30, 2019 at 02:49:47PM +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.
> 
> V4:
>  * add/transfer license headers to new files
>  * restructure the added QTestClientTransportOps struct
>  * restructure the FuzzTarget struct and fuzzer skeleton
>  * fork-based fuzzer now directly mmaps shm over the coverage bitmaps
>  * fixes to i440 and virtio-net fuzz targets
>  * undo the changes to qtest_memwrite
>  * possible to build /fuzz and /all in the same build-dir
>  * misc fixes to address V3 comments

I have finished reviewing this series.  Please fold in my Reviewed-by
tags when you send the next series.  That way I'll know which patches to
skip :).

Thanks,
Stefan

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

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

* Re: [PATCH v4 19/20] fuzz: add virtio-net fuzz target
  2019-10-30 14:50 ` [PATCH v4 19/20] fuzz: add virtio-net fuzz target Oleinik, Alexander
  2019-11-07 13:36   ` Stefan Hajnoczi
@ 2019-11-07 13:42   ` Jason Wang
  2019-11-07 15:41     ` Stefan Hajnoczi
  1 sibling, 1 reply; 57+ messages in thread
From: Jason Wang @ 2019-11-07 13:42 UTC (permalink / raw)
  To: Oleinik, Alexander, qemu-devel


On 2019/10/30 下午10:50, Oleinik, Alexander wrote:
> From: Alexander Oleinik <alxndr@bu.edu>
>
> 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>


Can this fuzz vhost-net or vhost-user (I only see socket backend)? If 
it's not too hard, it would be even more interesting.

Thanks


> ---
>   tests/fuzz/Makefile.include  |   1 +
>   tests/fuzz/virtio_net_fuzz.c | 123 +++++++++++++++++++++++++++++++++++
>   2 files changed, 124 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..0543cfd32a
> --- /dev/null
> +++ b/tests/fuzz/virtio_net_fuzz.c
> @@ -0,0 +1,123 @@
> +/*
> + * virtio-net Fuzzing Target
> + *
> + * Copyright Red Hat Inc., 2019
> + *
> + * Authors:
> + *  Alexander Bulekov   <alxndr@bu.edu>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "fuzz.h"
> +#include "fork_fuzz.h"
> +#include "qos_fuzz.h"
> +#include "tests/libqtest.h"
> +#include "tests/libqos/virtio-net.h"
> +
> +
> +static void virtio_net_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;
> +        }
> +        memcpy(&vqa, Data, sizeof(vqa));
> +        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);
> +        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_fuzz_multi(s, Data, Size);
> +        flush_events(s);
> +        _Exit(0);
> +    } else {
> +        wait(NULL);
> +    }
> +}
> +
> +static void register_virtio_net_fuzz_targets(void)
> +{
> +    fuzz_add_qos_target(&(FuzzTarget){
> +                .name = "virtio-net-fork-fuzz",
> +                .description = "Fuzz the virtio-net virtual queues, forking"
> +                                "for each fuzz run",
> +                .pre_vm_init = &counter_shm_init,
> +                .pre_fuzz = &qos_init_path,
> +                .fuzz = virtio_net_fork_fuzz,},
> +                "virtio-net",
> +                &(QOSGraphTestOptions){.before = virtio_net_test_setup_socket}
> +                );
> +}
> +
> +fuzz_target_init(register_virtio_net_fuzz_targets);



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

* Re: [PATCH v4 09/20] libqos: split qos-test and libqos makefile vars
  2019-10-30 14:49 ` [PATCH v4 09/20] libqos: split qos-test and libqos makefile vars Oleinik, Alexander
@ 2019-11-07 14:03   ` Darren Kenny
  0 siblings, 0 replies; 57+ messages in thread
From: Darren Kenny @ 2019-11-07 14:03 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: qemu-devel

On Wed, Oct 30, 2019 at 02:49:55PM +0000, Oleinik, Alexander wrote:
>From: Alexander Oleinik <alxndr@bu.edu>
>
>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>

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

>---
> tests/Makefile.include | 71 +++++++++++++++++++++---------------------
> 1 file changed, 36 insertions(+), 35 deletions(-)
>
>diff --git a/tests/Makefile.include b/tests/Makefile.include
>index 67853d10c3..1517c4817e 100644
>--- a/tests/Makefile.include
>+++ b/tests/Makefile.include
>@@ -699,52 +699,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-pci-modern.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 = $(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-pci-modern.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
>@@ -776,7 +777,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
>
>


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

* Re: [PATCH v4 14/20] fuzz: Add target/fuzz makefile rules
  2019-10-30 14:50 ` [PATCH v4 14/20] fuzz: Add target/fuzz makefile rules Oleinik, Alexander
@ 2019-11-07 14:31   ` Darren Kenny
  0 siblings, 0 replies; 57+ messages in thread
From: Darren Kenny @ 2019-11-07 14:31 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: qemu-devel

On Wed, Oct 30, 2019 at 02:50:00PM +0000, Oleinik, Alexander wrote:
>From: Alexander Oleinik <alxndr@bu.edu>
>
>Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
>---
> Makefile                    | 15 ++++++++++++++-
> Makefile.objs               |  4 +++-
> Makefile.target             | 18 +++++++++++++++++-
> tests/fuzz/Makefile.include |  4 ++++
> 4 files changed, 38 insertions(+), 3 deletions(-)
> create mode 100644 tests/fuzz/Makefile.include
>
>diff --git a/Makefile b/Makefile
>index d2b2ecd3c4..571f5562c9 100644
>--- a/Makefile
>+++ b/Makefile
>@@ -464,7 +464,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)
>@@ -476,6 +476,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
>@@ -526,6 +535,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
>@@ -535,6 +547,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.objs b/Makefile.objs
>index 9ff9b0c6f9..5478a554f6 100644
>--- a/Makefile.objs
>+++ b/Makefile.objs
>@@ -86,10 +86,12 @@ common-obj-$(CONFIG_FDT) += device_tree.o
> # qapi
>
> common-obj-y += qapi/
>+softmmu-obj-y = main.o
>
>-softmmu-main-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 ca3d14efe1..cddc8e4306 100644
>--- a/Makefile.target
>+++ b/Makefile.target
>@@ -202,7 +202,7 @@ endif
> COMMON_LDADDS = ../libqemuutil.a
>
> # build either PROG or PROGW
>-$(QEMU_PROG_BUILD): $(all-obj-y) $(COMMON_LDADDS)
>+$(QEMU_PROG_BUILD): $(all-obj-y) $(COMMON_LDADDS) $(softmmu-obj-y)
> 	$(call LINK, $(filter-out %.mak, $^))
> ifdef CONFIG_DARWIN
> 	$(call quiet-command,Rez -append $(SRC_PATH)/pc-bios/qemu.rsrc -o $@,"REZ","$(TARGET_DIR)$@")
>@@ -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, $^))
>+

It may be useful to still handle the fuzz target here, and report
that fuzzing is disabled in this configuration, as it is, if I type
'make x86_64-softmmu/fuzz' I get the less useful output of:

   make[1]: *** No rule to make target `fuzz'.  Stop.

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

But otherwise, this seems to be cleaner in that it is not causing
rebuilds of objects between selecting target/all or target/fuzz,
assuming that is correct here.

So with that,

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

Thanks,

Darren.


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

* Re: [PATCH v4 08/20] tests: provide test variables to other targets
  2019-10-30 14:49 ` [PATCH v4 08/20] tests: provide test variables to other targets Oleinik, Alexander
@ 2019-11-07 14:32   ` Darren Kenny
  0 siblings, 0 replies; 57+ messages in thread
From: Darren Kenny @ 2019-11-07 14:32 UTC (permalink / raw)
  To: Oleinik, Alexander; +Cc: qemu-devel

On Wed, Oct 30, 2019 at 02:49:54PM +0000, Oleinik, Alexander wrote:
>From: Alexander Oleinik <alxndr@bu.edu>
>
>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>

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

>---
> tests/Makefile.include | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/tests/Makefile.include b/tests/Makefile.include
>index 34ec03391c..67853d10c3 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
>@@ -873,6 +872,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
>
>


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

* Re: [PATCH v4 20/20] fuzz: add documentation to docs/devel/
  2019-11-07 13:40   ` Stefan Hajnoczi
@ 2019-11-07 15:02     ` Alexander Oleinik
  0 siblings, 0 replies; 57+ messages in thread
From: Alexander Oleinik @ 2019-11-07 15:02 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: qemu-devel

On 11/7/19 8:40 AM, Stefan Hajnoczi wrote:
> On Wed, Oct 30, 2019 at 02:50:04PM +0000, Oleinik, Alexander wrote:
>> +== Building the fuzzers ==
>> +
>> +NOTE: If possible, build a 32-bit binary. When forking, the 32-bit fuzzer is
>> +much faster, since the page-map has a smaller size. This is due to the fact that
>> +AddressSanitizer mmaps ~20TB of memory, as part of its detection. This results
>> +in a large page-map, and a much slower fork(). O
>> +
>> +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 similarly to system/softmmu:
>> +
>> +    make i386-softmmu/fuzz
>> +
>> +This builds ./i386-softmmu/qemu-fuzz-i386
> 
> I'm surprised that "make i386-softmmu/fuzz" builds
> i386-softmmu/qemu-fuzz-i386.  Should that be "make
> i386-softmmu/qemu-fuzz-i386"
I tried to make the rule match the names for regular targets.
Ie:
make i386-softmmu/clean
make i386-softmmu/all
make i386-softmmu/install
Now there is an i386-softmmu/fuzz

>> += Implmentation Details =
> 
> s/Implmentation/Implementation/
> 
>> +
>> +== The Fuzzer's Lifecycle ==
>> +
>> +The fuzzer has two entrypoints that libfuzzer calls. libfuzzer provides it's
>> +own main(), which performs some setup, and calls the entrypoints:
>> +
>> +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.
> 
> s/initailized/initialized/
> 



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

* Re: [PATCH v4 19/20] fuzz: add virtio-net fuzz target
  2019-11-07 13:42   ` Jason Wang
@ 2019-11-07 15:41     ` Stefan Hajnoczi
  0 siblings, 0 replies; 57+ messages in thread
From: Stefan Hajnoczi @ 2019-11-07 15:41 UTC (permalink / raw)
  To: Jason Wang; +Cc: Oleinik, Alexander, qemu-devel

On Thu, Nov 7, 2019 at 2:44 PM Jason Wang <jasowang@redhat.com> wrote:
> On 2019/10/30 下午10:50, Oleinik, Alexander wrote:
> > From: Alexander Oleinik <alxndr@bu.edu>
> >
> > 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>
>
>
> Can this fuzz vhost-net or vhost-user (I only see socket backend)? If
> it's not too hard, it would be even more interesting.

Fuzzing vhost devices would be awesome but this patch series does not do that.

libfuzzer uses coverage-guided fuzzing.  It needs to instrument the
code.  vhost kernel modules or external vhost-user processes aren't
instrumented so the fuzzing engine has no code instrumentation
feedback.

It should be possible to solve those problems eventually.  You could
also run it as-is, but the fuzzer wouldn't make intelligent decisions
about mutating input data to explore new code paths in vhost kernel
modules.

Stefan


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

* Re: [PATCH v4 01/20] softmmu: split off vl.c:main() into main.c
  2019-11-05 16:41   ` Darren Kenny
@ 2019-11-12 16:46     ` Alexander Bulekov
  0 siblings, 0 replies; 57+ messages in thread
From: Alexander Bulekov @ 2019-11-12 16:46 UTC (permalink / raw)
  To: qemu-devel, Paolo Bonzini

On 11/5/19 11:41 AM, Darren Kenny wrote:
> On Wed, Oct 30, 2019 at 02:49:48PM +0000, Oleinik, Alexander wrote:
>> From: Alexander Oleinik <alxndr@bu.edu>
>>
>> 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                  | 52 +++++++++++++++++++++++++++++++++++++++++
>> vl.c                    | 36 +++++++---------------------
>> 5 files changed, 68 insertions(+), 27 deletions(-)
>> create mode 100644 main.c
>>
>> diff --git a/Makefile b/Makefile
>> index 0e994a275d..d2b2ecd3c4 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -474,6 +474,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 11ba1a36bd..9ff9b0c6f9 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..03f9838b81 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);
>> +void 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..ecd6389424
>> --- /dev/null
>> +++ b/main.c
>> @@ -0,0 +1,52 @@
>> +/*
>> + * QEMU System Emulator
>> + *
>> + * Copyright (c) 2003-2008 Fabrice Bellard
>> + *
>> + * Permission is hereby granted, free of charge, to any person 
>> obtaining a copy
>> + * of this software and associated documentation files (the 
>> "Software"), to deal
>> + * in the Software without restriction, including without limitation 
>> the rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, 
>> and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be 
>> included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT 
>> SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>> ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
>> DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "sysemu/sysemu.h"
>> +
>> +#ifdef CONFIG_SDL
>> +#if defined(__APPLE__) || defined(main)
>> +#include <SDL.h>
>> +int main(int argc, char **argv)
>> +{
>> +    return qemu_main(argc, argv, NULL);
>> +}
>> +#undef main
>> +#define main qemu_main
> 
> This /looks/ wrong, you're defining a function main(), and then
> immediately #undef and #define main again.
> 
> Maybe this could be written differently, or add a comment here as to
> why you need to do this.
> 
>> +#endif
>> +#endif /* CONFIG_SDL */
>> +
>> +#ifdef CONFIG_COCOA
>> +#undef main
>> +#define main qemu_main
>> +#endif /* CONFIG_COCOA */
> 
> I don't really know the combinations that might exist, but it looks
> like if CONFIG_SDL is not defined, then we're redefining main() to be
> qemi_main() - so what main() function will actually be used here?

I tried to copy this straight from vl.c. It seems that this was 
originally added for similar reasons that I added this patch - similarly 
to libfuzzer, SDL has its own main function, and I'm guessing its 
similar for cocoa. With some  preprocessor flags, the result looks like:
int SDL_main(int argc, char **argv)
{
     return qemu_main(argc, argv,
                                 ((void *)0)
                                     );
}
int qemu_main(int argc, char **argv, char **envp)
{
     qemu_init(argc, argv, envp);
     main_loop();
     qemu_cleanup();
     return 0;
}

So it looks like this is there since SDL expects main to have two args. 
Maybe this is something that can be solved by adding separate main-sdl.c 
and main-cocoa.c files and tweaks to the build system, which should be 
possible now that qemu_init is separate from main().

-Alex

> Thanks,
> 
> Darren.
> 
>> +
>> +int main(int argc, char **argv, char **envp)
>> +{
>> +    qemu_init(argc, argv, envp);
>> +    main_loop();
>> +    qemu_cleanup();
>> +
>> +    return 0;
>> +}
>> diff --git a/vl.c b/vl.c
>> index c389d24b2c..472f09e12a 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -36,25 +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
>> -#define main qemu_main
>> -#endif /* CONFIG_COCOA */
>> -
>> -
>> #include "qemu/error-report.h"
>> #include "qemu/sockets.h"
>> #include "sysemu/accel.h"
>> @@ -1797,7 +1778,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;
>> @@ -2824,7 +2805,7 @@ static void user_register_global_props(void)
>>                       global_init_func, NULL, NULL);
>> }
>>
>> -int main(int argc, char **argv, char **envp)
>> +void qemu_init(int argc, char **argv, char **envp)
>> {
>>     int i;
>>     int snapshot, linux_boot;
>> @@ -3404,7 +3385,7 @@ int main(int argc, char **argv, char **envp)
>>             case QEMU_OPTION_watchdog:
>>                 if (watchdog) {
>>                     error_report("only one watchdog option may be 
>> given");
>> -                    return 1;
>> +                    exit(1);
>>                 }
>>                 watchdog = optarg;
>>                 break;
>> @@ -4440,7 +4421,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) {
>> @@ -4457,8 +4438,11 @@ int main(int argc, char **argv, char **envp)
>>     accel_setup_post(current_machine);
>>     os_setup_post();
>>
>> -    main_loop();
>> +    return;
>> +}
>>
>> +void qemu_cleanup(void)
>> +{
>>     gdbserver_cleanup();
>>
>>     /*
>> @@ -4495,6 +4479,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
>>
>>


-- 
===
I recently changed my last name from Oleinik to Bulekov
===


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

* Re: [PATCH v4 12/20] libqtest: add in-process qtest.c tx/rx handlers
  2019-11-06 16:56   ` Stefan Hajnoczi
@ 2019-11-12 17:38     ` Alexander Bulekov
  0 siblings, 0 replies; 57+ messages in thread
From: Alexander Bulekov @ 2019-11-12 17:38 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: Laurent Vivier, Paolo Bonzini, Thomas Huth, qemu-devel

On 11/6/19 11:56 AM, Stefan Hajnoczi wrote:
> On Wed, Oct 30, 2019 at 02:49:58PM +0000, Oleinik, Alexander wrote:
>> From: Alexander Oleinik <alxndr@bu.edu>
>>
>> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
>> ---
>> There's a particularily ugly line here:
>> qtest_client_set_tx_handler(qts,
>>          (void (*)(QTestState *s, const char*, size_t)) send);
> 
> Please typedef the function pointer to avoid repetition:
> 
>    typedef void (*QTestSendFn)(QTestState *s, const char *buf, size_t len);
> 
> And then introduce a wrapper function for type-safety:
> 
>    /* A type-safe wrapper for s->send() */
>    static void send_wrapper(QTestState *s, const char *buf, size_t len)
>    {
>        s->send(s, buf, len);
>    }
> 
>    ...
> 
>    qts->send = send;
>    qtest_client_set_tx_handler(qts, send_wrapper);
> 
> Does this solve the issue?
So there should be two pointers qts->send and qts->ops->send? Otherwise 
qtest_client_set_tx_handler simply overwrites qts->send with the 
send_wrapper.

What I'm worried about is having to cast a
(void (*)(void *s, const char*, size_t) to a
(void (*)(QTestState *s, const char*, size_t)
I don't think this is defined according to the standard. If we add a 
secondary send function pointer to qts (void (*)(void *s, const char*, 
size_t)), then I think its no longer an issue, which I think is what you 
suggest above.

> By the way, I also wonder whether the size_t len arguments are necessary
> since const char *buf is a NUL-terminated C string.  The string should
> be enough since the length can be calculated from it.
I'll change it.

>> diff --git a/qtest.c b/qtest.c
>> index 9fbfa0f08f..f817a5d789 100644
>> --- a/qtest.c
>> +++ b/qtest.c
>> @@ -812,6 +812,6 @@ void qtest_server_inproc_recv(void *dummy, const char *buf, size_t size)
>>       g_string_append(gstr, buf);
>>       if (gstr->str[gstr->len - 1] == '\n') {
>>           qtest_process_inbuf(NULL, gstr);
>> -        g_string_free(gstr, true);
>> +        g_string_truncate(gstr, 0);
> 
> Ah, a fix for the bug in an earlier commit.  Please squash it.
> 
>> diff --git a/tests/libqtest.c b/tests/libqtest.c
>> index ff3153daf2..6143af33da 100644
>> --- a/tests/libqtest.c
>> +++ b/tests/libqtest.c
>> @@ -71,6 +71,7 @@ static void qtest_client_set_tx_handler(QTestState *s,
>>   static void qtest_client_set_rx_handler(QTestState *s,
>>           GString * (*recv)(QTestState *));
>>   
>> +static GString *recv_str;
> 
> Can this be a QTestState field?
> 


-- 
===
I recently changed my last name from Oleinik to Bulekov
===


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

* Re: [PATCH v4 15/20] fuzz: add fuzzer skeleton
  2019-11-07 12:55   ` Stefan Hajnoczi
@ 2019-11-12 19:04     ` Alexander Bulekov
  0 siblings, 0 replies; 57+ messages in thread
From: Alexander Bulekov @ 2019-11-12 19:04 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: qemu-devel

On 11/7/19 7:55 AM, Stefan Hajnoczi wrote:
> On Wed, Oct 30, 2019 at 02:50:00PM +0000, Oleinik, Alexander wrote:
>> diff --git a/tests/fuzz/fuzz.c b/tests/fuzz/fuzz.c
>> new file mode 100644
>> index 0000000000..0e38f81c48
>> --- /dev/null
>> +++ b/tests/fuzz/fuzz.c
>> @@ -0,0 +1,177 @@
>> +/*
>> + * fuzzing driver
>> + *
>> + * Copyright Red Hat Inc., 2019
>> + *
>> + * Authors:
>> + *  Alexander Bulekov   <alxndr@bu.edu>
> 
> Bulekov instead of Oleinik?
Yes I changed my last name and the approval from the court finally came 
through last week :)
I'll make sure its consistent across v5.

>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +
>> +#include <stdio.h>
>> +#include <stdlib.h>
> 
> stdio.h and stdlib.h are already included by qemu/osdep.h.
> 
>> +/* Executed for each fuzzing-input */
>> +int LLVMFuzzerTestOneInput(const unsigned char *Data, size_t Size)
>> +{
>> +    if (fuzz_target->fuzz) {
> 
> Will this ever be NULL?
I'll remove the check

>> +        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;
>> +
>> +    /* 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];
>> +    if (!strstr(target_name, "--fuzz-target=")) {
>> +        usage(**argv);
>> +    }
>> +
>> +    target_name += strlen("--fuzz-target=");
>> +
>> +    fuzz_target = fuzz_get_target(target_name);
>> +    if (!fuzz_target) {
>> +        usage(**argv);
>> +    }
>> +
>> +    fuzz_qts = qtest_setup();
>> +
>> +    if (!fuzz_target) {
> 
> This is dead code since fuzz_target was already checked above.  Please
> remove this if statement.
> 
>> +        fprintf(stderr, "Error: Fuzz fuzz_target name %s not found\n",
>> +                target_name);
>> +        usage(**argv);
>> +    }
>> +
>> +    if (fuzz_target->pre_vm_init) {
>> +        fuzz_target->pre_vm_init();
>> +    }
>> +
>> +    /* Run QEMU's softmmu main with the fuzz-target dependent arguments */
>> +    char *init_cmdline = fuzz_target->get_init_cmdline(fuzz_target);
> 
> Where is init_cmdline freed or should this be const char *?
> 
>> +    wordexp_t result;
>> +    wordexp(init_cmdline, &result, 0);
> 
> What is the purpose of word expansion here?
The fuzz target devs can specify arguments in a single string and not 
worry about calculating the argc and **argv - we take care of it for them.

>> +
>> +    qemu_init(result.we_wordc, result.we_wordv, NULL);
>> +
>> +    if (fuzz_target->pre_fuzz) {
>> +        fuzz_target->pre_fuzz(fuzz_qts);
>> +    }
>> +
>> +    return 0;
>> +}
>> diff --git a/tests/fuzz/fuzz.h b/tests/fuzz/fuzz.h
>> new file mode 100644
>> index 0000000000..b569b622d7
>> --- /dev/null
>> +++ b/tests/fuzz/fuzz.h
>> @@ -0,0 +1,66 @@
>> +/*
>> + * fuzzing driver
>> + *
>> + * Copyright Red Hat Inc., 2019
>> + *
>> + * Authors:
>> + *  Alexander Bulekov   <alxndr@bu.edu>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#ifndef 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"
>> +
>> +
> 
> Some documentation would be nice:
> 
...
> Does the caller need to call g_free() on the returned string?  Please
> document this.
...
> s/to to/to/
...
> Please also mention that QEMU has been initialized at this point.
> 
...
> "makes a copy of *target" -> does this mean the argument type can be
> const FuzzTarget *target?
> 

Thanks - I made changes to address these.
-Alex

-- 
===
I recently changed my last name from Oleinik to Bulekov
===


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

end of thread, other threads:[~2019-11-12 19:05 UTC | newest]

Thread overview: 57+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-30 14:49 [PATCH v4 00/20] Add virtual device fuzzing support Oleinik, Alexander
2019-10-30 14:49 ` [PATCH v4 01/20] softmmu: split off vl.c:main() into main.c Oleinik, Alexander
2019-11-05 16:41   ` Darren Kenny
2019-11-12 16:46     ` Alexander Bulekov
2019-11-06 15:01   ` Stefan Hajnoczi
2019-10-30 14:49 ` [PATCH v4 02/20] libqos: Rename i2c_send and i2c_recv Oleinik, Alexander
2019-11-06 15:17   ` Stefan Hajnoczi
2019-10-30 14:49 ` [PATCH v4 03/20] fuzz: Add FUZZ_TARGET module type Oleinik, Alexander
2019-11-06 13:17   ` Darren Kenny
2019-11-06 15:18   ` Stefan Hajnoczi
2019-10-30 14:49 ` [PATCH v4 04/20] qtest: add qtest_server_send abstraction Oleinik, Alexander
2019-11-06 13:29   ` Darren Kenny
2019-11-06 15:19   ` Stefan Hajnoczi
2019-10-30 14:49 ` [PATCH v4 06/20] module: check module wasn't already initialized Oleinik, Alexander
2019-11-06 15:26   ` Stefan Hajnoczi
2019-11-06 17:40   ` Darren Kenny
2019-10-30 14:49 ` [PATCH v4 05/20] libqtest: Add a layer of abstraciton to send/recv Oleinik, Alexander
2019-11-06 16:22   ` Stefan Hajnoczi
2019-10-30 14:49 ` [PATCH v4 07/20] qtest: add in-process incoming command handler Oleinik, Alexander
2019-11-06 16:33   ` Stefan Hajnoczi
2019-10-30 14:49 ` [PATCH v4 08/20] tests: provide test variables to other targets Oleinik, Alexander
2019-11-07 14:32   ` Darren Kenny
2019-10-30 14:49 ` [PATCH v4 09/20] libqos: split qos-test and libqos makefile vars Oleinik, Alexander
2019-11-07 14:03   ` Darren Kenny
2019-10-30 14:49 ` [PATCH v4 10/20] libqos: move useful qos-test funcs to qos_external Oleinik, Alexander
2019-11-06 16:41   ` Stefan Hajnoczi
2019-10-30 14:49 ` [PATCH v4 11/20] libqtest: make qtest_bufwrite send "atomic" Oleinik, Alexander
2019-11-06 16:44   ` Stefan Hajnoczi
2019-10-30 14:49 ` [PATCH v4 12/20] libqtest: add in-process qtest.c tx/rx handlers Oleinik, Alexander
2019-11-06 16:56   ` Stefan Hajnoczi
2019-11-12 17:38     ` Alexander Bulekov
2019-10-30 14:49 ` [PATCH v4 13/20] fuzz: add configure flag --enable-fuzzing Oleinik, Alexander
2019-11-06 16:57   ` Stefan Hajnoczi
2019-10-30 14:50 ` [PATCH v4 15/20] fuzz: add fuzzer skeleton Oleinik, Alexander
2019-11-07 12:55   ` Stefan Hajnoczi
2019-11-12 19:04     ` Alexander Bulekov
2019-10-30 14:50 ` [PATCH v4 14/20] fuzz: Add target/fuzz makefile rules Oleinik, Alexander
2019-11-07 14:31   ` Darren Kenny
2019-10-30 14:50 ` [PATCH v4 16/20] fuzz: add support for fork-based fuzzing Oleinik, Alexander
2019-11-07 13:17   ` Stefan Hajnoczi
2019-10-30 14:50 ` [PATCH v4 17/20] fuzz: add support for qos-assisted fuzz targets Oleinik, Alexander
2019-11-07 13:22   ` Stefan Hajnoczi
2019-10-30 14:50 ` [PATCH v4 18/20] fuzz: add i440fx " Oleinik, Alexander
2019-11-07 13:26   ` Stefan Hajnoczi
2019-10-30 14:50 ` [PATCH v4 19/20] fuzz: add virtio-net fuzz target Oleinik, Alexander
2019-11-07 13:36   ` Stefan Hajnoczi
2019-11-07 13:42   ` Jason Wang
2019-11-07 15:41     ` Stefan Hajnoczi
2019-10-30 14:50 ` [PATCH v4 20/20] fuzz: add documentation to docs/devel/ Oleinik, Alexander
2019-11-07 13:40   ` Stefan Hajnoczi
2019-11-07 15:02     ` Alexander Oleinik
2019-10-30 15:23 ` [PATCH v4 00/20] Add virtual device fuzzing support no-reply
2019-11-06 15:27   ` Stefan Hajnoczi
2019-11-05 13:57 ` Darren Kenny
2019-11-05 16:28   ` Alexander Oleinik
2019-11-05 16:47     ` Darren Kenny
2019-11-07 13:41 ` Stefan Hajnoczi

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