[PATCH 0/3] Fix oss-fuzz builds post-meson integration

[PATCH 0/3] Fix oss-fuzz builds post-meson integration

[Alexander Bulekov]

QEMU stopped building on oss-fuzz, after the meson integration, due to
some linking issues:

https://oss-fuzz-build-logs.storage.googleapis.com/log-3eaddfbd-7e05-4ddd-9d86-ee4b16c0fac6.txt

Those problems should be partially fixed by:

Depends-on: meson: fix libqos linking
(https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg00802.html)

These patches:
1. Build on the above patch to fix the way we specify the linker script,
   to ensure that it is not specified within start-group/end-group linker
   pairs
2. Add support for running --enable-fuzzing with a custom LIB_FUZZING_ENGINE
3. Fix a problem with how we specify custom rpath in the oss-fuzz
   build-script

Alexander Bulekov (3):
  meson: specify fuzz linker script as a project arg
  fuzz: Add support for custom fuzzing library
  scripts/oss-fuzz/build.sh: fix rpath

 configure                    | 12 ++++++++++--
 meson.build                  |  9 ++++++++-
 scripts/oss-fuzz/build.sh    |  2 +-
 tests/qtest/fuzz/meson.build |  5 ++---
 4 files changed, 21 insertions(+), 7 deletions(-)

-- 
2.28.0

[PATCH 1/3] meson: specify fuzz linker script as a project arg

[Alexander Bulekov]

With this change, the fuzzer-linker script should be specified outside
any --start-group/--end-group pairs. We need this on oss-fuzz, where
partially applying the linker-script results in a linker failure

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 meson.build                  | 9 ++++++++-
 tests/qtest/fuzz/meson.build | 1 -
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 74f8ea0c2e..a0407ce050 100644
--- a/meson.build
+++ b/meson.build
@@ -31,6 +31,14 @@ endforeach
 have_tools = 'CONFIG_TOOLS' in config_host
 have_block = have_system or have_tools
 
+# Specify linker-script with add_project_link_arguments so that it is not placed
+# within a linker --start-group/--end-group pair
+if 'CONFIG_FUZZ' in config_host
+   config_host += {'QEMU_LDFLAGS': config_host['QEMU_LDFLAGS'] +
+            ' -Wl,-T,' + (meson.current_source_dir() / 'tests/qtest/fuzz/fork_fuzz.ld')}
+endif
+
+
 add_project_arguments(config_host['QEMU_CFLAGS'].split(),
                       native: false, language: ['c', 'objc'])
 add_project_arguments(config_host['QEMU_CXXFLAGS'].split(),
@@ -1019,7 +1027,6 @@ foreach target : target_dirs
         'gui': false,
         'sources': specific_fuzz.sources(),
         'dependencies': specific_fuzz.dependencies(),
-        'link_depends': [files('tests/qtest/fuzz/fork_fuzz.ld')],
       }]
     endif
   else
diff --git a/tests/qtest/fuzz/meson.build b/tests/qtest/fuzz/meson.build
index bb0a3f271d..3432c3e7c3 100644
--- a/tests/qtest/fuzz/meson.build
+++ b/tests/qtest/fuzz/meson.build
@@ -10,7 +10,6 @@ specific_fuzz_ss.add(when: 'CONFIG_VIRTIO_SCSI', if_true: files('virtio_scsi_fuz
 # this will be duplicated in meson.build
 fork_fuzz = declare_dependency(
   link_args: ['-fsanitize=fuzzer',
-              '-Wl,-T,' + (meson.current_source_dir() / 'fork_fuzz.ld'),
               '-Wl,-wrap,qtest_inb',
               '-Wl,-wrap,qtest_inw',
               '-Wl,-wrap,qtest_inl',
-- 
2.28.0

[PATCH 2/3] fuzz: Add support for custom fuzzing library

[Alexander Bulekov]

On oss-fuzz, we must use the LIB_FUZZING_ENGINE and CFLAGS environment
variables, rather than -fsanitize=fuzzer. With this change, when
LIB_FUZZING_ENGINE is set, the --enable-fuzzing configure option will
use that environment variable during the linking stage, rather than
-fsanitize=fuzzer

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 configure                    | 12 ++++++++++--
 tests/qtest/fuzz/meson.build |  4 ++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index 6ecaff429b..4182a88e75 100755
--- a/configure
+++ b/configure
@@ -6165,7 +6165,7 @@ fi
 
 ##########################################
 # checks for fuzzer
-if test "$fuzzing" = "yes" ; then
+if test "$fuzzing" = "yes" && test -z "${LIB_FUZZING_ENGINE+xxx}"; then
   write_c_fuzzer_skeleton
   if compile_prog "$CPU_CFLAGS -Werror -fsanitize=fuzzer" ""; then
     have_fuzzer=yes
@@ -7505,7 +7505,14 @@ if test "$have_mlockall" = "yes" ; then
   echo "HAVE_MLOCKALL=y" >> $config_host_mak
 fi
 if test "$fuzzing" = "yes" ; then
-  QEMU_CFLAGS="$QEMU_CFLAGS -fsanitize=fuzzer-no-link"
+  # If LIB_FUZZING_ENGINE is set, assume we are running on OSS-Fuzz, and the
+  # needed CFLAGS have already been provided
+  if test -z "${LIB_FUZZING_ENGINE+xxx}" ; then
+    QEMU_CFLAGS="$QEMU_CFLAGS -fsanitize=fuzzer-no-link"
+    FUZZ_LINK_COMMAND="-fsanitize=fuzzer"
+  else
+    FUZZ_LINK_COMMAND="$LIB_FUZZING_ENGINE"
+  fi
 fi
 
 if test "$plugins" = "yes" ; then
@@ -7620,6 +7627,7 @@ fi
 if test "$fuzzing" != "no"; then
     echo "CONFIG_FUZZ=y" >> $config_host_mak
 fi
+echo "FUZZ_LINK_COMMAND=$FUZZ_LINK_COMMAND" >> $config_host_mak
 
 if test "$edk2_blobs" = "yes" ; then
   echo "DECOMPRESS_EDK2_BLOBS=y" >> $config_host_mak
diff --git a/tests/qtest/fuzz/meson.build b/tests/qtest/fuzz/meson.build
index 3432c3e7c3..59a630802a 100644
--- a/tests/qtest/fuzz/meson.build
+++ b/tests/qtest/fuzz/meson.build
@@ -9,8 +9,8 @@ specific_fuzz_ss.add(when: 'CONFIG_VIRTIO_SCSI', if_true: files('virtio_scsi_fuz
 # unfortunately declare_dependency does not support link_depends, so
 # this will be duplicated in meson.build
 fork_fuzz = declare_dependency(
-  link_args: ['-fsanitize=fuzzer',
-              '-Wl,-wrap,qtest_inb',
+  link_args: config_host['FUZZ_LINK_COMMAND'].split() +
+             ['-Wl,-wrap,qtest_inb',
               '-Wl,-wrap,qtest_inw',
               '-Wl,-wrap,qtest_inl',
               '-Wl,-wrap,qtest_outb',
-- 
2.28.0

[PATCH 3/3] scripts/oss-fuzz/build.sh: fix rpath

[Alexander Bulekov]

Prior to this change,
readelf -d build/out/qemu/qemu-fuzz-i386-target-virtio-net-slirp
...
0x000000000000000f (RPATH)  Library rpath: ['$$ORIGIN/lib':$ORIGIN/migration:$ORIGIN/]

As of 1a4db552d8 ("ninjatool: quote dollars in variables"), we don't
need to manually double the dollars. Also, remove the single-quotes as
they are copied into the rpath.

After this change:
0x000000000000000f (RPATH)  Library rpath: [$ORIGIN/lib:$ORIGIN/migration:$ORIGIN/]

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 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 f0b7442c96..d16207eb67 100755
--- a/scripts/oss-fuzz/build.sh
+++ b/scripts/oss-fuzz/build.sh
@@ -81,7 +81,7 @@ rm qemu-fuzz-i386
 # Build a second time to build the final binary with correct rpath
 ../configure --disable-werror --cc="$CC" --cxx="$CXX" --enable-fuzzing \
     --prefix="$DEST_DIR" --bindir="$DEST_DIR" --datadir="$DEST_DIR/data/" \
-    --extra-cflags="$EXTRA_CFLAGS" --extra-ldflags="-Wl,-rpath,'\$\$ORIGIN/lib'" \
+    --extra-cflags="$EXTRA_CFLAGS" --extra-ldflags="-Wl,-rpath,\$ORIGIN/lib" \
     --target-list="i386-softmmu"
 make "-j$(nproc)" qemu-fuzz-i386 V=1
 
-- 
2.28.0

Re: [PATCH 1/3] meson: specify fuzz linker script as a project arg

[Paolo Bonzini]

On 02/09/20 16:37, Alexander Bulekov wrote:
> With this change, the fuzzer-linker script should be specified outside
> any --start-group/--end-group pairs. We need this on oss-fuzz, where
> partially applying the linker-script results in a linker failure

Is this okay also for targets that don't link to the fuzzing static library?

Paolo

Re: [PATCH 1/3] meson: specify fuzz linker script as a project arg

[Paolo Bonzini]

On 02/09/20 16:37, Alexander Bulekov wrote:
> +# Specify linker-script with add_project_link_arguments so that it is not placed
> +# within a linker --start-group/--end-group pair
> +if 'CONFIG_FUZZ' in config_host
> +   config_host += {'QEMU_LDFLAGS': config_host['QEMU_LDFLAGS'] +
> +            ' -Wl,-T,' + (meson.current_source_dir() / 'tests/qtest/fuzz/fork_fuzz.ld')}
> +endif
> +
> +

Also -- sorry -- please use a separate add_project_link_arguments
invocation.

Paolo

Re: [PATCH 2/3] fuzz: Add support for custom fuzzing library

[Paolo Bonzini]

On 02/09/20 16:37, Alexander Bulekov wrote:
> On oss-fuzz, we must use the LIB_FUZZING_ENGINE and CFLAGS environment
> variables, rather than -fsanitize=fuzzer. With this change, when
> LIB_FUZZING_ENGINE is set, the --enable-fuzzing configure option will
> use that environment variable during the linking stage, rather than
> -fsanitize=fuzzer
> 
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  configure                    | 12 ++++++++++--
>  tests/qtest/fuzz/meson.build |  4 ++--
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/configure b/configure
> index 6ecaff429b..4182a88e75 100755
> --- a/configure
> +++ b/configure
> @@ -6165,7 +6165,7 @@ fi
>  
>  ##########################################
>  # checks for fuzzer
> -if test "$fuzzing" = "yes" ; then
> +if test "$fuzzing" = "yes" && test -z "${LIB_FUZZING_ENGINE+xxx}"; then
>    write_c_fuzzer_skeleton
>    if compile_prog "$CPU_CFLAGS -Werror -fsanitize=fuzzer" ""; then
>      have_fuzzer=yes
> @@ -7505,7 +7505,14 @@ if test "$have_mlockall" = "yes" ; then
>    echo "HAVE_MLOCKALL=y" >> $config_host_mak
>  fi
>  if test "$fuzzing" = "yes" ; then
> -  QEMU_CFLAGS="$QEMU_CFLAGS -fsanitize=fuzzer-no-link"
> +  # If LIB_FUZZING_ENGINE is set, assume we are running on OSS-Fuzz, and the
> +  # needed CFLAGS have already been provided
> +  if test -z "${LIB_FUZZING_ENGINE+xxx}" ; then
> +    QEMU_CFLAGS="$QEMU_CFLAGS -fsanitize=fuzzer-no-link"
> +    FUZZ_LINK_COMMAND="-fsanitize=fuzzer"
> +  else
> +    FUZZ_LINK_COMMAND="$LIB_FUZZING_ENGINE"
> +  fi
>  fi

Can you name this FUZZ_EXE_LDFLAGS?

>  if test "$plugins" = "yes" ; then
> @@ -7620,6 +7627,7 @@ fi
>  if test "$fuzzing" != "no"; then
>      echo "CONFIG_FUZZ=y" >> $config_host_mak
>  fi
> +echo "FUZZ_LINK_COMMAND=$FUZZ_LINK_COMMAND" >> $config_host_mak
>  
>  if test "$edk2_blobs" = "yes" ; then
>    echo "DECOMPRESS_EDK2_BLOBS=y" >> $config_host_mak
> diff --git a/tests/qtest/fuzz/meson.build b/tests/qtest/fuzz/meson.build
> index 3432c3e7c3..59a630802a 100644
> --- a/tests/qtest/fuzz/meson.build
> +++ b/tests/qtest/fuzz/meson.build
> @@ -9,8 +9,8 @@ specific_fuzz_ss.add(when: 'CONFIG_VIRTIO_SCSI', if_true: files('virtio_scsi_fuz
>  # unfortunately declare_dependency does not support link_depends, so
>  # this will be duplicated in meson.build

Also for patch 1: the comment is now obsolete.

Paolo

>  fork_fuzz = declare_dependency(
> -  link_args: ['-fsanitize=fuzzer',
> -              '-Wl,-wrap,qtest_inb',
> +  link_args: config_host['FUZZ_LINK_COMMAND'].split() +
> +             ['-Wl,-wrap,qtest_inb',
>                '-Wl,-wrap,qtest_inw',
>                '-Wl,-wrap,qtest_inl',
>                '-Wl,-wrap,qtest_outb',
> 

Re: [PATCH 1/3] meson: specify fuzz linker script as a project arg

[Alexander Bulekov]

On 200902 1745, Paolo Bonzini wrote:
> On 02/09/20 16:37, Alexander Bulekov wrote:
> > With this change, the fuzzer-linker script should be specified outside
> > any --start-group/--end-group pairs. We need this on oss-fuzz, where
> > partially applying the linker-script results in a linker failure
> 
> Is this okay also for targets that don't link to the fuzzing static library?
> 
> Paolo
> 

To be honest, I still do not completely understand why there is a
different behavior when we specify the script within a group. The man
page for ld.bfd doesn't talk about linker arguments between
--start-group and --end-group that are not archives. If I understand the
purpose of these linker groups, the linker script should still apply to
everything (regardless where it is specified). I would expect there to
be no change in behavior, or a complaint about passing linker arguments
that are not archive paths.

I was also worried about what would happen to all the __wrap_qtest
arguments, since those are still in the group, and they can fail silently.
Disassembling the binary confirmed that all the calls to
qtest_{in*,out*} are still wrapped.

Anyways.. I tested this series with and without LIB_FUZZING_ENGINE, and
used nm to confirm that the layout of the symbols/data is correct in
both cases (at least on my machine and the oss-fuzz docker) ..

-Alex