| Message ID | 20230205042951.3570008-10-alxndr@bu.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Retire Fork-Based Fuzzing | expand |
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
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)
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> --- 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