Re: [PATCH 09/10] fuzz: remove fork-fuzzing scaffolding

[Darren Kenny <darren.kenny@oracle.com>]

On Saturday, 2023-02-04 at 23:29:50 -05, Alexander Bulekov wrote:
> Fork-fuzzing provides a few pros, but our implementation prevents us
> from using fuzzers other than libFuzzer, and may be causing issues such
> as coverage-failure builds on OSS-Fuzz. It is not a great long-term
> solution as it depends on internal implementation details of libFuzzer
> (which is no longer in active development). Remove it in favor of other
> methods of resetting state between inputs.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>

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

Thanks,

Darren.

> ---
>  meson.build                   |  4 ---
>  tests/qtest/fuzz/fork_fuzz.c  | 41 -------------------------
>  tests/qtest/fuzz/fork_fuzz.h  | 23 --------------
>  tests/qtest/fuzz/fork_fuzz.ld | 56 -----------------------------------
>  tests/qtest/fuzz/meson.build  |  6 ++--
>  5 files changed, 3 insertions(+), 127 deletions(-)
>  delete mode 100644 tests/qtest/fuzz/fork_fuzz.c
>  delete mode 100644 tests/qtest/fuzz/fork_fuzz.h
>  delete mode 100644 tests/qtest/fuzz/fork_fuzz.ld
>
> diff --git a/meson.build b/meson.build
> index 6d3b665629..8be27c2408 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -215,10 +215,6 @@ endif
>  # Specify linker-script with add_project_link_arguments so that it is not placed
>  # within a linker --start-group/--end-group pair
>  if get_option('fuzzing')
> -  add_project_link_arguments(['-Wl,-T,',
> -                              (meson.current_source_dir() / 'tests/qtest/fuzz/fork_fuzz.ld')],
> -                             native: false, language: all_languages)
> -
>    # Specify a filter to only instrument code that is directly related to
>    # virtual-devices.
>    configure_file(output: 'instrumentation-filter',
> diff --git a/tests/qtest/fuzz/fork_fuzz.c b/tests/qtest/fuzz/fork_fuzz.c
> deleted file mode 100644
> index 6ffb2a7937..0000000000
> --- a/tests/qtest/fuzz/fork_fuzz.c
> +++ /dev/null
> @@ -1,41 +0,0 @@
> -/*
> - * 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"
> -
> -
> -void counter_shm_init(void)
> -{
> -    /* Copy what's in the counter region to a temporary buffer.. */
> -    void *copy = malloc(&__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
> -    memcpy(copy,
> -           &__FUZZ_COUNTERS_START,
> -           &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
> -
> -    /* Map a shared region over the counter region */
> -    if (mmap(&__FUZZ_COUNTERS_START,
> -             &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START,
> -             PROT_READ | PROT_WRITE, MAP_SHARED | MAP_FIXED | MAP_ANONYMOUS,
> -             0, 0) == MAP_FAILED) {
> -        perror("Error: ");
> -        exit(1);
> -    }
> -
> -    /* Copy the original data back to the counter-region */
> -    memcpy(&__FUZZ_COUNTERS_START, copy,
> -           &__FUZZ_COUNTERS_END - &__FUZZ_COUNTERS_START);
> -    free(copy);
> -}
> -
> -
> diff --git a/tests/qtest/fuzz/fork_fuzz.h b/tests/qtest/fuzz/fork_fuzz.h
> deleted file mode 100644
> index 9ecb8b58ef..0000000000
> --- a/tests/qtest/fuzz/fork_fuzz.h
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/*
> - * 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/qtest/fuzz/fork_fuzz.ld b/tests/qtest/fuzz/fork_fuzz.ld
> deleted file mode 100644
> index cfb88b7fdb..0000000000
> --- a/tests/qtest/fuzz/fork_fuzz.ld
> +++ /dev/null
> @@ -1,56 +0,0 @@
> -/*
> - * We adjust linker script modification to place all of the stuff that needs to
> - * persist across fuzzing runs into a contiguous section 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);
> -  }
> -}
> -INSERT AFTER .data;
> -
> -SECTIONS
> -{
> -  .data.fuzz_ordered :
> -  {
> -      /*
> -       * 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 = .;
> -
> -      /* Internal Libfuzzer TracePC object which contains the ValueProfileMap */
> -      FuzzerTracePC*(.bss*);
> -      /*
> -       * In case the above line fails, explicitly specify the (mangled) name of
> -       * the object we care about
> -       */
> -       *(.bss._ZN6fuzzer3TPCE);
> -  }
> -}
> -INSERT AFTER .data.fuzz_start;
> -
> -SECTIONS
> -{
> -  .data.fuzz_end : ALIGN(4K)
> -  {
> -      __FUZZ_COUNTERS_END = .;
> -  }
> -}
> -/*
> - * Don't overwrite the SECTIONS in the default linker script. Instead insert the
> - * above into the default script
> - */
> -INSERT AFTER .data.fuzz_ordered;
> diff --git a/tests/qtest/fuzz/meson.build b/tests/qtest/fuzz/meson.build
> index 189901d4a2..4d10b47b8f 100644
> --- a/tests/qtest/fuzz/meson.build
> +++ b/tests/qtest/fuzz/meson.build
> @@ -2,7 +2,7 @@ if not get_option('fuzzing')
>    subdir_done()
>  endif
>  
> -specific_fuzz_ss.add(files('fuzz.c', 'fork_fuzz.c', 'qos_fuzz.c',
> +specific_fuzz_ss.add(files('fuzz.c', 'qos_fuzz.c',
>                             'qtest_wrappers.c'), qos)
>  
>  # Targets
> @@ -12,7 +12,7 @@ specific_fuzz_ss.add(when: 'CONFIG_VIRTIO_SCSI', if_true: files('virtio_scsi_fuz
>  specific_fuzz_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio_blk_fuzz.c'))
>  specific_fuzz_ss.add(files('generic_fuzz.c'))
>  
> -fork_fuzz = declare_dependency(
> +fuzz_ld = declare_dependency(
>    link_args: fuzz_exe_ldflags +
>               ['-Wl,-wrap,qtest_inb',
>                '-Wl,-wrap,qtest_inw',
> @@ -35,4 +35,4 @@ fork_fuzz = declare_dependency(
>                '-Wl,-wrap,qtest_memset']
>  )
>  
> -specific_fuzz_ss.add(fork_fuzz)
> +specific_fuzz_ss.add(fuzz_ld)
> -- 
> 2.39.0