qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 00/10] qtest / fuzzer patches
@ 2020-07-21  8:10 Thomas Huth
  2020-07-21  8:10 ` [PULL 01/10] scripts/oss-fuzz: Limit target list to i386-softmmu Thomas Huth
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Thomas Huth @ 2020-07-21  8:10 UTC (permalink / raw)
  To: peter.maydell, qemu-devel

 Hi Peter,

the following changes since commit af3d69058e09bede9900f266a618ed11f76f49f3:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200720' into staging (2020-07-20 15:58:07 +0100)

are available in the Git repository at:

  https://gitlab.com/huth/qemu.git tags/pull-request-2020-07-21

for you to fetch changes up to 7ad36e2e241bd924f774a1f9fb208c102da58e50:

  hw: Mark nd_table[] misuse in realize methods FIXME (2020-07-21 08:41:15 +0200)

----------------------------------------------------------------
* Fix memory leak in fuzzer
* Fuzzer documentation updates
* Some other minor fuzzer updates
* Fix "make check-qtest SPEED=slow" (bug in msf2 instance_init)
----------------------------------------------------------------

Alexander Bulekov (6):
      fuzz: Fix leak when assembling datadir path string
      gitlab-ci.yml: Add oss-fuzz build tests
      fuzz: build without AddressSanitizer, by default
      docs/fuzz: describe building fuzzers with enable-sanitizers
      docs/fuzz: add information about useful libFuzzer flags
      docs/fuzz: add instructions for generating a coverage report

Markus Armbruster (2):
      msf2: Unbreak device-list-properties for "msf-soc"
      hw: Mark nd_table[] misuse in realize methods FIXME

Thomas Huth (2):
      scripts/oss-fuzz: Limit target list to i386-softmmu
      MAINTAINERS: Extend the device fuzzing section

 .gitlab-ci.yml            | 22 ++++++++---------
 MAINTAINERS               |  2 ++
 configure                 | 10 ++++----
 docs/devel/fuzzing.txt    | 63 +++++++++++++++++++++++++++++++++++++++++++++--
 hw/arm/allwinner-h3.c     |  1 +
 hw/arm/msf2-soc.c         |  9 ++++---
 hw/arm/xlnx-versal.c      |  1 +
 hw/arm/xlnx-zynqmp.c      |  1 +
 hw/dma/sparc32_dma.c      |  1 +
 hw/riscv/sifive_u.c       |  1 +
 scripts/oss-fuzz/build.sh |  2 +-
 tests/qtest/fuzz/fuzz.c   | 12 +++++----
 12 files changed, 96 insertions(+), 29 deletions(-)



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

* [PULL 01/10] scripts/oss-fuzz: Limit target list to i386-softmmu
  2020-07-21  8:10 [PULL 00/10] qtest / fuzzer patches Thomas Huth
@ 2020-07-21  8:10 ` Thomas Huth
  2020-07-21  8:10 ` [PULL 02/10] fuzz: Fix leak when assembling datadir path string Thomas Huth
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2020-07-21  8:10 UTC (permalink / raw)
  To: peter.maydell, qemu-devel

The build.sh script only copies qemu-fuzz-i386 to the destination folder,
so we can speed up the compilation step quite a bit by not compiling the
other targets here.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 scripts/oss-fuzz/build.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
index f5cee3d67e..a07b3022e8 100755
--- a/scripts/oss-fuzz/build.sh
+++ b/scripts/oss-fuzz/build.sh
@@ -68,7 +68,7 @@ mkdir -p "$DEST_DIR/lib/"  # Copy the shared libraries here
 
 # Build once to get the list of dynamic lib paths, and copy them over
 ../configure --disable-werror --cc="$CC" --cxx="$CXX" \
-    --extra-cflags="$EXTRA_CFLAGS"
+    --extra-cflags="$EXTRA_CFLAGS" --target-list="i386-softmmu"
 
 if ! make CONFIG_FUZZ=y CFLAGS="$LIB_FUZZING_ENGINE" "-j$(nproc)" \
     i386-softmmu/fuzz; then
-- 
2.18.1



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

* [PULL 02/10] fuzz: Fix leak when assembling datadir path string
  2020-07-21  8:10 [PULL 00/10] qtest / fuzzer patches Thomas Huth
  2020-07-21  8:10 ` [PULL 01/10] scripts/oss-fuzz: Limit target list to i386-softmmu Thomas Huth
@ 2020-07-21  8:10 ` Thomas Huth
  2020-07-21  8:10 ` [PULL 03/10] gitlab-ci.yml: Add oss-fuzz build tests Thomas Huth
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2020-07-21  8:10 UTC (permalink / raw)
  To: peter.maydell, qemu-devel; +Cc: Alexander Bulekov

From: Alexander Bulekov <alxndr@bu.edu>

We freed the string containing the final datadir path, but did not free
the path to the executable's directory that we get from
g_path_get_dirname(). Fix that.

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20200717163523.1591-1-alxndr@bu.edu>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/fuzz/fuzz.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
index 6bc17ef313..031594a686 100644
--- a/tests/qtest/fuzz/fuzz.c
+++ b/tests/qtest/fuzz/fuzz.c
@@ -143,7 +143,7 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
 {
 
     char *target_name;
-    char *dir;
+    char *bindir, *datadir;
     bool serialize = false;
 
     /* Initialize qgraph and modules */
@@ -164,11 +164,13 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
          * location of the executable. Using this we add exec_dir/pc-bios to
          * the datadirs.
          */
-        dir = g_build_filename(g_path_get_dirname(**argv), "pc-bios", NULL);
-        if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
-            qemu_add_data_dir(dir);
+        bindir = g_path_get_dirname(**argv);
+        datadir = g_build_filename(bindir, "pc-bios", NULL);
+        g_free(bindir);
+        if (g_file_test(datadir, G_FILE_TEST_IS_DIR)) {
+            qemu_add_data_dir(datadir);
         }
-        g_free(dir);
+        g_free(datadir);
     } else if (*argc > 1) {  /* The target is specified as an argument */
         target_name = (*argv)[1];
         if (!strstr(target_name, "--fuzz-target=")) {
-- 
2.18.1



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

* [PULL 03/10] gitlab-ci.yml: Add oss-fuzz build tests
  2020-07-21  8:10 [PULL 00/10] qtest / fuzzer patches Thomas Huth
  2020-07-21  8:10 ` [PULL 01/10] scripts/oss-fuzz: Limit target list to i386-softmmu Thomas Huth
  2020-07-21  8:10 ` [PULL 02/10] fuzz: Fix leak when assembling datadir path string Thomas Huth
@ 2020-07-21  8:10 ` Thomas Huth
  2020-07-21  8:10 ` [PULL 04/10] fuzz: build without AddressSanitizer, by default Thomas Huth
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2020-07-21  8:10 UTC (permalink / raw)
  To: peter.maydell, qemu-devel; +Cc: Alexander Bulekov

From: Alexander Bulekov <alxndr@bu.edu>

This tries to build and run the fuzzers with the same build-script used
by oss-fuzz. This doesn't guarantee that the builds on oss-fuzz will
also succeed, since oss-fuzz provides its own compiler and fuzzer vars,
but it can catch changes that are not compatible with the the
./scripts/oss-fuzz/build.sh script.
The strange way of finding fuzzer binaries stems from the method used by
oss-fuzz:
https://github.com/google/oss-fuzz/blob/master/infra/base-images/base-runner/targets_list

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20200720073223.22945-1-thuth@redhat.com>
[thuth: Tweak the "script" to make it work, exclude slirp test, etc.]
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .gitlab-ci.yml | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 41597c3603..362e5ee755 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -164,22 +164,20 @@ build-clang:
       ppc-softmmu s390x-softmmu arm-linux-user
     MAKE_CHECK_ARGS: check
 
-build-fuzzer:
+build-oss-fuzz:
   <<: *native_build_job_definition
   variables:
     IMAGE: fedora
   script:
-    - mkdir build
-    - cd build
-    - ../configure --cc=clang --cxx=clang++ --enable-fuzzing
-                   --enable-sanitizers --target-list=x86_64-softmmu
-    - make -j"$JOBS" all check-build x86_64-softmmu/fuzz
-    - make check
-    - for fuzzer in i440fx-qos-fork-fuzz i440fx-qos-noreset-fuzz
-        i440fx-qtest-reboot-fuzz virtio-scsi-flags-fuzz virtio-scsi-fuzz ; do
-          echo Testing ${fuzzer} ... ;
-          x86_64-softmmu/qemu-fuzz-x86_64 --fuzz-target=${fuzzer} -runs=1000
-            || exit 1 ;
+    - mkdir build-oss-fuzz
+    - CC="clang" CXX="clang++" CFLAGS="-fsanitize=address"
+      ./scripts/oss-fuzz/build.sh
+    - for fuzzer in $(find ./build-oss-fuzz/DEST_DIR/ -executable -type f
+                      | grep -v slirp); do
+        grep "LLVMFuzzerTestOneInput" ${fuzzer} > /dev/null 2>&1 || continue ;
+        echo Testing ${fuzzer} ... ;
+        ASAN_OPTIONS="fast_unwind_on_malloc=0"
+         "${fuzzer}" -runs=1000 -seed=1 || exit 1 ;
       done
 
 build-tci:
-- 
2.18.1



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

* [PULL 04/10] fuzz: build without AddressSanitizer, by default
  2020-07-21  8:10 [PULL 00/10] qtest / fuzzer patches Thomas Huth
                   ` (2 preceding siblings ...)
  2020-07-21  8:10 ` [PULL 03/10] gitlab-ci.yml: Add oss-fuzz build tests Thomas Huth
@ 2020-07-21  8:10 ` Thomas Huth
  2020-07-21  8:10 ` [PULL 05/10] docs/fuzz: describe building fuzzers with enable-sanitizers Thomas Huth
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2020-07-21  8:10 UTC (permalink / raw)
  To: peter.maydell, qemu-devel; +Cc: Alexander Bulekov

From: Alexander Bulekov <alxndr@bu.edu>

We already have a nice --enable-sanitizers option to enable
AddressSanitizer. There is no reason to duplicate and force this
functionality in --enable-fuzzing. In the future, if more sanitizers are
added to --enable-sanitizers, it might be impossible to build with both
--enable-sanitizers and --enable-fuzzing, since not all sanitizers are
compatible with libFuzzer. In that case, we could enable ASAN with
--extra-cflags="-fsanitize=address"

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20200706195534.14962-2-alxndr@bu.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
[thuth: Added missing $CFLAGS]
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 configure | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index 33cee41f9c..4bd80ed507 100755
--- a/configure
+++ b/configure
@@ -6337,7 +6337,7 @@ fi
 # checks for fuzzer
 if test "$fuzzing" = "yes" ; then
   write_c_fuzzer_skeleton
-  if compile_prog "$CPU_CFLAGS -Werror -fsanitize=address,fuzzer" ""; then
+  if compile_prog "$CPU_CFLAGS -Werror -fsanitize=fuzzer" ""; then
       have_fuzzer=yes
   fi
 fi
@@ -7893,11 +7893,11 @@ if test "$have_mlockall" = "yes" ; then
 fi
 if test "$fuzzing" = "yes" ; then
   if test "$have_fuzzer" = "yes"; then
-    FUZZ_LDFLAGS=" -fsanitize=address,fuzzer"
-    FUZZ_CFLAGS=" -fsanitize=address,fuzzer"
-    CFLAGS="$CFLAGS -fsanitize=address,fuzzer-no-link"
+    FUZZ_LDFLAGS=" -fsanitize=fuzzer"
+    FUZZ_CFLAGS=" -fsanitize=fuzzer"
+    CFLAGS="$CFLAGS -fsanitize=fuzzer-no-link"
   else
-    error_exit "Your compiler doesn't support -fsanitize=address,fuzzer"
+    error_exit "Your compiler doesn't support -fsanitize=fuzzer"
     exit 1
   fi
 fi
-- 
2.18.1



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

* [PULL 05/10] docs/fuzz: describe building fuzzers with enable-sanitizers
  2020-07-21  8:10 [PULL 00/10] qtest / fuzzer patches Thomas Huth
                   ` (3 preceding siblings ...)
  2020-07-21  8:10 ` [PULL 04/10] fuzz: build without AddressSanitizer, by default Thomas Huth
@ 2020-07-21  8:10 ` Thomas Huth
  2020-07-21  8:10 ` [PULL 06/10] docs/fuzz: add information about useful libFuzzer flags Thomas Huth
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2020-07-21  8:10 UTC (permalink / raw)
  To: peter.maydell, qemu-devel; +Cc: Alexander Bulekov

From: Alexander Bulekov <alxndr@bu.edu>

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20200706195534.14962-3-alxndr@bu.edu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 docs/devel/fuzzing.txt | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/docs/devel/fuzzing.txt b/docs/devel/fuzzing.txt
index db5641de74..12bf6aa0ca 100644
--- a/docs/devel/fuzzing.txt
+++ b/docs/devel/fuzzing.txt
@@ -23,9 +23,12 @@ AddressSanitizer mmaps ~20TB of memory, as part of its detection. This results
 in a large page-map, and a much slower fork().
 
 To build the fuzzers, install a recent version of clang:
-Configure with (substitute the clang binaries with the version you installed):
+Configure with (substitute the clang binaries with the version you installed).
+Here, enable-sanitizers, is optional but it allows us to reliably detect bugs
+such as out-of-bounds accesses, use-after-frees, double-frees etc.
 
-    CC=clang-8 CXX=clang++-8 /path/to/configure --enable-fuzzing
+    CC=clang-8 CXX=clang++-8 /path/to/configure --enable-fuzzing \
+                                                --enable-sanitizers
 
 Fuzz targets are built similarly to system/softmmu:
 
-- 
2.18.1



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

* [PULL 06/10] docs/fuzz: add information about useful libFuzzer flags
  2020-07-21  8:10 [PULL 00/10] qtest / fuzzer patches Thomas Huth
                   ` (4 preceding siblings ...)
  2020-07-21  8:10 ` [PULL 05/10] docs/fuzz: describe building fuzzers with enable-sanitizers Thomas Huth
@ 2020-07-21  8:10 ` Thomas Huth
  2020-07-21  8:10 ` [PULL 07/10] docs/fuzz: add instructions for generating a coverage report Thomas Huth
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2020-07-21  8:10 UTC (permalink / raw)
  To: peter.maydell, qemu-devel; +Cc: Alexander Bulekov

From: Alexander Bulekov <alxndr@bu.edu>

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20200706195534.14962-4-alxndr@bu.edu>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 docs/devel/fuzzing.txt | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/docs/devel/fuzzing.txt b/docs/devel/fuzzing.txt
index 12bf6aa0ca..6d18115239 100644
--- a/docs/devel/fuzzing.txt
+++ b/docs/devel/fuzzing.txt
@@ -48,6 +48,43 @@ 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.
 
+== Useful libFuzzer flags ==
+
+As mentioned above, libFuzzer accepts some arguments. Passing -help=1 will list
+the available arguments. In particular, these arguments might be helpful:
+
+$CORPUS_DIR/ : Specify a directory as the last argument to libFuzzer. libFuzzer
+stores each "interesting" input in this corpus directory. The next time you run
+libFuzzer, it will read all of the inputs from the corpus, and continue fuzzing
+from there. You can also specify multiple directories. libFuzzer loads existing
+inputs from all specified directories, but will only write new ones to the
+first one specified.
+
+-max_len=4096 : specify the maximum byte-length of the inputs libFuzzer will
+generate.
+
+-close_fd_mask={1,2,3} : close, stderr, or both. Useful for targets that
+trigger many debug/error messages, or create output on the serial console.
+
+-jobs=4 -workers=4 : These arguments configure libFuzzer to run 4 fuzzers in
+parallel (4 fuzzing jobs in 4 worker processes). Alternatively, with only
+-jobs=N, libFuzzer automatically spawns a number of workers less than or equal
+to half the available CPU cores. Replace 4 with a number appropriate for your
+machine. Make sure to specify a $CORPUS_DIR, which will allow the parallel
+fuzzers to share information about the interesting inputs they find.
+
+-use_value_profile=1 : For each comparison operation, libFuzzer computes 
+(caller_pc&4095) | (popcnt(Arg1 ^ Arg2) << 12) and places this in the coverage
+table. Useful for targets with "magic" constants. If Arg1 came from the fuzzer's
+input and Arg2 is a magic constant, then each time the Hamming distance
+between Arg1 and Arg2 decreases, libFuzzer adds the input to the corpus.
+
+-shrink=1 : Tries to make elements of the corpus "smaller". Might lead to
+better coverage performance, depending on the target.
+
+Note that libFuzzer's exact behavior will depend on the version of
+clang and libFuzzer used to build the device fuzzers.
+
 == Adding a new fuzzer ==
 Coverage over virtual devices can be improved by adding additional fuzzers.
 Fuzzers are kept in tests/qtest/fuzz/ and should be added to
-- 
2.18.1



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

* [PULL 07/10] docs/fuzz: add instructions for generating a coverage report
  2020-07-21  8:10 [PULL 00/10] qtest / fuzzer patches Thomas Huth
                   ` (5 preceding siblings ...)
  2020-07-21  8:10 ` [PULL 06/10] docs/fuzz: add information about useful libFuzzer flags Thomas Huth
@ 2020-07-21  8:10 ` Thomas Huth
  2020-07-21  8:10 ` [PULL 08/10] MAINTAINERS: Extend the device fuzzing section Thomas Huth
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2020-07-21  8:10 UTC (permalink / raw)
  To: peter.maydell, qemu-devel; +Cc: Alexander Bulekov

From: Alexander Bulekov <alxndr@bu.edu>

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20200706195534.14962-5-alxndr@bu.edu>
[thuth: Replaced --enable-sanitizers with --enable-fuzzing]
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 docs/devel/fuzzing.txt | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/docs/devel/fuzzing.txt b/docs/devel/fuzzing.txt
index 6d18115239..96d71c94d7 100644
--- a/docs/devel/fuzzing.txt
+++ b/docs/devel/fuzzing.txt
@@ -85,6 +85,25 @@ better coverage performance, depending on the target.
 Note that libFuzzer's exact behavior will depend on the version of
 clang and libFuzzer used to build the device fuzzers.
 
+== Generating Coverage Reports ==
+Code coverage is a crucial metric for evaluating a fuzzer's performance.
+libFuzzer's output provides a "cov: " column that provides a total number of
+unique blocks/edges covered. To examine coverage on a line-by-line basis we
+can use Clang coverage:
+
+ 1. Configure libFuzzer to store a corpus of all interesting inputs (see
+    CORPUS_DIR above)
+ 2. ./configure the QEMU build with:
+    --enable-fuzzing \
+    --extra-cflags="-fprofile-instr-generate -fcoverage-mapping"
+ 3. Re-run the fuzzer. Specify $CORPUS_DIR/* as an argument, telling libfuzzer
+    to execute all of the inputs in $CORPUS_DIR and exit. Once the process
+    exits, you should find a file, "default.profraw" in the working directory.
+ 4. Execute these commands to generate a detailed HTML coverage-report:
+ llvm-profdata merge -output=default.profdata default.profraw
+ llvm-cov show ./path/to/qemu-fuzz-i386 -instr-profile=default.profdata \
+ --format html -output-dir=/path/to/output/report
+
 == Adding a new fuzzer ==
 Coverage over virtual devices can be improved by adding additional fuzzers.
 Fuzzers are kept in tests/qtest/fuzz/ and should be added to
-- 
2.18.1



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

* [PULL 08/10] MAINTAINERS: Extend the device fuzzing section
  2020-07-21  8:10 [PULL 00/10] qtest / fuzzer patches Thomas Huth
                   ` (6 preceding siblings ...)
  2020-07-21  8:10 ` [PULL 07/10] docs/fuzz: add instructions for generating a coverage report Thomas Huth
@ 2020-07-21  8:10 ` Thomas Huth
  2020-07-21  8:10 ` [PULL 09/10] msf2: Unbreak device-list-properties for "msf-soc" Thomas Huth
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2020-07-21  8:10 UTC (permalink / raw)
  To: peter.maydell, qemu-devel

The file docs/devel/fuzzing.txt should be in this section, too, and add
myself as a reviewer (since I often take the fuzzer patches through the
qtest-next tree, I should be notified on patches, too).

Message-Id: <20200721053926.17197-1-thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5e8616821a..3395abd4e1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2449,9 +2449,11 @@ M: Alexander Bulekov <alxndr@bu.edu>
 R: Paolo Bonzini <pbonzini@redhat.com>
 R: Bandan Das <bsd@redhat.com>
 R: Stefan Hajnoczi <stefanha@redhat.com>
+R: Thomas Huth <thuth@redhat.com>
 S: Maintained
 F: tests/qtest/fuzz/
 F: scripts/oss-fuzz/
+F: docs/devel/fuzzing.txt
 
 Register API
 M: Alistair Francis <alistair@alistair23.me>
-- 
2.18.1



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

* [PULL 09/10] msf2: Unbreak device-list-properties for "msf-soc"
  2020-07-21  8:10 [PULL 00/10] qtest / fuzzer patches Thomas Huth
                   ` (7 preceding siblings ...)
  2020-07-21  8:10 ` [PULL 08/10] MAINTAINERS: Extend the device fuzzing section Thomas Huth
@ 2020-07-21  8:10 ` Thomas Huth
  2020-07-21  8:10 ` [PULL 10/10] hw: Mark nd_table[] misuse in realize methods FIXME Thomas Huth
  2020-07-21 13:03 ` [PULL 00/10] qtest / fuzzer patches Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2020-07-21  8:10 UTC (permalink / raw)
  To: peter.maydell, qemu-devel; +Cc: Markus Armbruster

From: Markus Armbruster <armbru@redhat.com>

Watch this:

    $ qemu-system-aarch64 -M ast2600-evb -S -display none -qmp stdio
    {"QMP": {"version": {"qemu": {"micro": 50, "minor": 0, "major": 5}, "package": "v5.0.0-2464-g3a9163af4e"}, "capabilities": ["oob"]}}
    {"execute": "qmp_capabilities"}
    {"return": {}}
    {"execute": "device-list-properties", "arguments": {"typename": "msf2-soc"}}
    Unsupported NIC model: ftgmac100
    armbru@dusky:~/work/images$ echo $?
    1

This is what breaks "make check SPEED=slow".

Root cause is m2sxxx_soc_initfn()'s messing with nd_table[] via
qemu_check_nic_model().  That's wrong.

We fixed the exact same bug for device "allwinner-a10" in commit
8aabc5437b "hw/arm/allwinner-a10: Do not use nd_table in instance_init
function".  Fix this instance the same way: move the offending code to
m2sxxx_soc_realize(), where it's less wrong, and add a FIXME comment.

Fixes: 05b7374a58 ("msf2: Add EMAC block to SmartFusion2 SoC")
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200715140440.3540942-2-armbru@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/arm/msf2-soc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
index 16bb7c9916..33ea7df342 100644
--- a/hw/arm/msf2-soc.c
+++ b/hw/arm/msf2-soc.c
@@ -82,10 +82,6 @@ static void m2sxxx_soc_initfn(Object *obj)
     }
 
     object_initialize_child(obj, "emac", &s->emac, TYPE_MSS_EMAC);
-    if (nd_table[0].used) {
-        qemu_check_nic_model(&nd_table[0], TYPE_MSS_EMAC);
-        qdev_set_nic_properties(DEVICE(&s->emac), &nd_table[0]);
-    }
 }
 
 static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp)
@@ -187,6 +183,11 @@ static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp)
         g_free(bus_name);
     }
 
+    /* FIXME use qdev NIC properties instead of nd_table[] */
+    if (nd_table[0].used) {
+        qemu_check_nic_model(&nd_table[0], TYPE_MSS_EMAC);
+        qdev_set_nic_properties(DEVICE(&s->emac), &nd_table[0]);
+    }
     dev = DEVICE(&s->emac);
     object_property_set_link(OBJECT(&s->emac), "ahb-bus",
                              OBJECT(get_system_memory()), &error_abort);
-- 
2.18.1



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

* [PULL 10/10] hw: Mark nd_table[] misuse in realize methods FIXME
  2020-07-21  8:10 [PULL 00/10] qtest / fuzzer patches Thomas Huth
                   ` (8 preceding siblings ...)
  2020-07-21  8:10 ` [PULL 09/10] msf2: Unbreak device-list-properties for "msf-soc" Thomas Huth
@ 2020-07-21  8:10 ` Thomas Huth
  2020-07-21 13:03 ` [PULL 00/10] qtest / fuzzer patches Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2020-07-21  8:10 UTC (permalink / raw)
  To: peter.maydell, qemu-devel; +Cc: Markus Armbruster

From: Markus Armbruster <armbru@redhat.com>

nd_table[] contains NIC configuration for boards to pick up.  Device
code has no business looking there.  Several devices do it anyway.
Two of them already have a suitable FIXME comment: "allwinner-a10" and
"msf2-soc".  Copy it to the others: "allwinner-h3", "xlnx-versal",
"xlnx,zynqmp", "sparc32-ledma", "riscv.sifive.u.soc".

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200715140440.3540942-3-armbru@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Niek Linnenbank <nieklinnenbank@gmail.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/arm/allwinner-h3.c | 1 +
 hw/arm/xlnx-versal.c  | 1 +
 hw/arm/xlnx-zynqmp.c  | 1 +
 hw/dma/sparc32_dma.c  | 1 +
 hw/riscv/sifive_u.c   | 1 +
 5 files changed, 5 insertions(+)

diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
index 8e09468e86..ff92ded82c 100644
--- a/hw/arm/allwinner-h3.c
+++ b/hw/arm/allwinner-h3.c
@@ -358,6 +358,7 @@ static void allwinner_h3_realize(DeviceState *dev, Error **errp)
                               "sd-bus");
 
     /* EMAC */
+    /* FIXME use qdev NIC properties instead of nd_table[] */
     if (nd_table[0].used) {
         qemu_check_nic_model(&nd_table[0], TYPE_AW_SUN8I_EMAC);
         qdev_set_nic_properties(DEVICE(&s->emac), &nd_table[0]);
diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
index ead038b971..e3aa4bd1e5 100644
--- a/hw/arm/xlnx-versal.c
+++ b/hw/arm/xlnx-versal.c
@@ -160,6 +160,7 @@ static void versal_create_gems(Versal *s, qemu_irq *pic)
         object_initialize_child(OBJECT(s), name, &s->lpd.iou.gem[i],
                                 TYPE_CADENCE_GEM);
         dev = DEVICE(&s->lpd.iou.gem[i]);
+        /* FIXME use qdev NIC properties instead of nd_table[] */
         if (nd->used) {
             qemu_check_nic_model(nd, "cadence_gem");
             qdev_set_nic_properties(dev, nd);
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 772cfa3771..5855e5d5bf 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -455,6 +455,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     for (i = 0; i < XLNX_ZYNQMP_NUM_GEMS; i++) {
         NICInfo *nd = &nd_table[i];
 
+        /* FIXME use qdev NIC properties instead of nd_table[] */
         if (nd->used) {
             qemu_check_nic_model(nd, TYPE_CADENCE_GEM);
             qdev_set_nic_properties(DEVICE(&s->gem[i]), nd);
diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index 9459178866..bcd1626fbd 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -341,6 +341,7 @@ static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
     DeviceState *d;
     NICInfo *nd = &nd_table[0];
 
+    /* FIXME use qdev NIC properties instead of nd_table[] */
     qemu_check_nic_model(nd, TYPE_LANCE);
 
     d = qdev_new(TYPE_LANCE);
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 19a976c9a6..e5682c38a9 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -714,6 +714,7 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
     }
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->otp), 0, memmap[SIFIVE_U_OTP].base);
 
+    /* FIXME use qdev NIC properties instead of nd_table[] */
     if (nd->used) {
         qemu_check_nic_model(nd, TYPE_CADENCE_GEM);
         qdev_set_nic_properties(DEVICE(&s->gem), nd);
-- 
2.18.1



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

* Re: [PULL 00/10] qtest / fuzzer patches
  2020-07-21  8:10 [PULL 00/10] qtest / fuzzer patches Thomas Huth
                   ` (9 preceding siblings ...)
  2020-07-21  8:10 ` [PULL 10/10] hw: Mark nd_table[] misuse in realize methods FIXME Thomas Huth
@ 2020-07-21 13:03 ` Peter Maydell
  10 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2020-07-21 13:03 UTC (permalink / raw)
  To: Thomas Huth; +Cc: QEMU Developers

On Tue, 21 Jul 2020 at 09:11, Thomas Huth <thuth@redhat.com> wrote:
>
>  Hi Peter,
>
> the following changes since commit af3d69058e09bede9900f266a618ed11f76f49f3:
>
>   Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200720' into staging (2020-07-20 15:58:07 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/huth/qemu.git tags/pull-request-2020-07-21
>
> for you to fetch changes up to 7ad36e2e241bd924f774a1f9fb208c102da58e50:
>
>   hw: Mark nd_table[] misuse in realize methods FIXME (2020-07-21 08:41:15 +0200)
>
> ----------------------------------------------------------------
> * Fix memory leak in fuzzer
> * Fuzzer documentation updates
> * Some other minor fuzzer updates
> * Fix "make check-qtest SPEED=slow" (bug in msf2 instance_init)
> ----------------------------------------------------------------


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.

-- PMM


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

end of thread, other threads:[~2020-07-21 13:04 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-21  8:10 [PULL 00/10] qtest / fuzzer patches Thomas Huth
2020-07-21  8:10 ` [PULL 01/10] scripts/oss-fuzz: Limit target list to i386-softmmu Thomas Huth
2020-07-21  8:10 ` [PULL 02/10] fuzz: Fix leak when assembling datadir path string Thomas Huth
2020-07-21  8:10 ` [PULL 03/10] gitlab-ci.yml: Add oss-fuzz build tests Thomas Huth
2020-07-21  8:10 ` [PULL 04/10] fuzz: build without AddressSanitizer, by default Thomas Huth
2020-07-21  8:10 ` [PULL 05/10] docs/fuzz: describe building fuzzers with enable-sanitizers Thomas Huth
2020-07-21  8:10 ` [PULL 06/10] docs/fuzz: add information about useful libFuzzer flags Thomas Huth
2020-07-21  8:10 ` [PULL 07/10] docs/fuzz: add instructions for generating a coverage report Thomas Huth
2020-07-21  8:10 ` [PULL 08/10] MAINTAINERS: Extend the device fuzzing section Thomas Huth
2020-07-21  8:10 ` [PULL 09/10] msf2: Unbreak device-list-properties for "msf-soc" Thomas Huth
2020-07-21  8:10 ` [PULL 10/10] hw: Mark nd_table[] misuse in realize methods FIXME Thomas Huth
2020-07-21 13:03 ` [PULL 00/10] qtest / fuzzer patches Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).