* [PATCH v5 1/3] fuzz: adjust timeout to allow for longer inputs
2021-06-30 3:41 [PATCH v5 0/3] Fuzzer pattern-matching, timeouts, and instrumentation-filtering Alexander Bulekov
@ 2021-06-30 3:41 ` Alexander Bulekov
2021-06-30 3:41 ` [PATCH v5 2/3] fuzz: add an instrumentation filter Alexander Bulekov
2021-06-30 3:41 ` [PATCH v5 3/3] fuzz: make object-name matching case-insensitive Alexander Bulekov
2 siblings, 0 replies; 6+ messages in thread
From: Alexander Bulekov @ 2021-06-30 3:41 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Alexander Bulekov,
Philippe Mathieu-Daudé,
darren.kenny, Bandan Das, Stefan Hajnoczi, Paolo Bonzini
Using a custom timeout is useful to continue fuzzing complex devices,
even after we run into some slow code-path. However, simply adding a
fixed timeout to each input effectively caps the maximum input
length/number of operations at some artificial value. There are two
major problems with this:
1. Some code might only be reachable through long IO sequences.
2. Longer inputs can actually be _better_ for performance. While the
raw number of fuzzer executions decreases with larger inputs, the
number of MMIO/PIO/DMA operation/second actually increases, since
were are speding proportionately less time fork()ing.
With this change, we keep the custom-timeout, but we renew it, prior to
each MMIO/PIO/DMA operation. Thus, we time-out only when a specific
operation takes a long time.
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
tests/qtest/fuzz/generic_fuzz.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
index 6c67522717..43f9011626 100644
--- a/tests/qtest/fuzz/generic_fuzz.c
+++ b/tests/qtest/fuzz/generic_fuzz.c
@@ -661,15 +661,16 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
uint8_t op;
if (fork() == 0) {
+ struct sigaction sact;
+ struct itimerval timer;
/*
* Sometimes the fuzzer will find inputs that take quite a long time to
* process. Often times, these inputs do not result in new coverage.
* Even if these inputs might be interesting, they can slow down the
- * fuzzer, overall. Set a timeout to avoid hurting performance, too much
+ * fuzzer, overall. Set a timeout for each command to avoid hurting
+ * performance, too much
*/
if (timeout) {
- struct sigaction sact;
- struct itimerval timer;
sigemptyset(&sact.sa_mask);
sact.sa_flags = SA_NODEFER;
@@ -679,13 +680,17 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size)
memset(&timer, 0, sizeof(timer));
timer.it_value.tv_sec = timeout / USEC_IN_SEC;
timer.it_value.tv_usec = timeout % USEC_IN_SEC;
- setitimer(ITIMER_VIRTUAL, &timer, NULL);
}
op_clear_dma_patterns(s, NULL, 0);
pci_disabled = false;
while (cmd && Size) {
+ /* Reset the timeout, each time we run a new command */
+ if (timeout) {
+ setitimer(ITIMER_VIRTUAL, &timer, NULL);
+ }
+
/* Get the length until the next command or end of input */
nextcmd = memmem(cmd, Size, SEPARATOR, strlen(SEPARATOR));
cmd_len = nextcmd ? nextcmd - cmd : Size;
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v5 2/3] fuzz: add an instrumentation filter
2021-06-30 3:41 [PATCH v5 0/3] Fuzzer pattern-matching, timeouts, and instrumentation-filtering Alexander Bulekov
2021-06-30 3:41 ` [PATCH v5 1/3] fuzz: adjust timeout to allow for longer inputs Alexander Bulekov
@ 2021-06-30 3:41 ` Alexander Bulekov
2021-07-08 2:58 ` Alexander Bulekov
2021-07-08 9:30 ` Darren Kenny
2021-06-30 3:41 ` [PATCH v5 3/3] fuzz: make object-name matching case-insensitive Alexander Bulekov
2 siblings, 2 replies; 6+ messages in thread
From: Alexander Bulekov @ 2021-06-30 3:41 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Alexander Bulekov, Philippe Mathieu-Daudé,
darren.kenny, Bandan Das, Stefan Hajnoczi, Paolo Bonzini
By default, -fsanitize=fuzzer instruments all code with coverage
information. However, this means that libfuzzer will track coverage over
hundreds of source files that are unrelated to virtual-devices. This
means that libfuzzer will optimize inputs for coverage observed in timer
code, memory APIs etc. This slows down the fuzzer and stores many inputs
that are not relevant to the actual virtual-devices.
With this change, clang versions that support the
"-fsanitize-coverage-allowlist" will only instrument a subset of the
compiled code, that is directly related to virtual-devices.
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
configure | 13 +++++++++++++
scripts/oss-fuzz/instrumentation-filter-template | 14 ++++++++++++++
2 files changed, 27 insertions(+)
create mode 100644 scripts/oss-fuzz/instrumentation-filter-template
diff --git a/configure b/configure
index 38704b4e11..3b6ca054b9 100755
--- a/configure
+++ b/configure
@@ -5189,6 +5189,11 @@ if test "$fuzzing" = "yes" && test -z "${LIB_FUZZING_ENGINE+xxx}"; then
error_exit "Your compiler doesn't support -fsanitize=fuzzer"
exit 1
fi
+ have_clang_coverage_filter=no
+ echo > $TMPTXT
+ if compile_prog "$CPU_CFLAGS -Werror -fsanitize=fuzzer -fsanitize-coverage-allowlist=$TMPTXT" ""; then
+ have_clang_coverage_filter=yes
+ fi
fi
# Thread sanitizer is, for now, much noisier than the other sanitizers;
@@ -6120,6 +6125,14 @@ if test "$fuzzing" = "yes" ; then
# rule for the fuzzer adds these to the link_args. They need to be
# configurable, to support OSS-Fuzz
FUZZ_EXE_LDFLAGS="-fsanitize=fuzzer"
+
+ # Specify a filter to only instrument code that is directly related to
+ # virtual-devices.
+ if test "$have_clang_coverage_filter" = "yes" ; then
+ cp "$source_path/scripts/oss-fuzz/instrumentation-filter-template" \
+ instrumentation-filter
+ QEMU_CFLAGS="$QEMU_CFLAGS -fsanitize-coverage-allowlist=instrumentation-filter"
+ fi
else
FUZZ_EXE_LDFLAGS="$LIB_FUZZING_ENGINE"
fi
diff --git a/scripts/oss-fuzz/instrumentation-filter-template b/scripts/oss-fuzz/instrumentation-filter-template
new file mode 100644
index 0000000000..44e853159c
--- /dev/null
+++ b/scripts/oss-fuzz/instrumentation-filter-template
@@ -0,0 +1,14 @@
+# Code that we actually want the fuzzer to target
+# See: https://clang.llvm.org/docs/SanitizerCoverage.html#disabling-instrumentation-without-source-modification
+#
+src:*/hw/*
+src:*/include/hw/*
+src:*/slirp/*
+
+# We don't care about coverage over fuzzer-specific code, however we should
+# instrument the fuzzer entry-point so libFuzzer always sees at least some
+# coverage - otherwise it will exit after the first input
+src:*/tests/qtest/fuzz/fuzz.c
+
+# Enable instrumentation for all functions in those files
+fun:*
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v5 2/3] fuzz: add an instrumentation filter
2021-06-30 3:41 ` [PATCH v5 2/3] fuzz: add an instrumentation filter Alexander Bulekov
@ 2021-07-08 2:58 ` Alexander Bulekov
2021-07-08 9:30 ` Darren Kenny
1 sibling, 0 replies; 6+ messages in thread
From: Alexander Bulekov @ 2021-07-08 2:58 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Philippe Mathieu-Daudé,
darren.kenny, Bandan Das, Stefan Hajnoczi, Paolo Bonzini
On 210629 2341, Alexander Bulekov wrote:
> By default, -fsanitize=fuzzer instruments all code with coverage
> information. However, this means that libfuzzer will track coverage over
> hundreds of source files that are unrelated to virtual-devices. This
> means that libfuzzer will optimize inputs for coverage observed in timer
> code, memory APIs etc. This slows down the fuzzer and stores many inputs
> that are not relevant to the actual virtual-devices.
>
> With this change, clang versions that support the
> "-fsanitize-coverage-allowlist" will only instrument a subset of the
> compiled code, that is directly related to virtual-devices.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Ping?
Are there any other device-related subdirectories we should add to the
filter besides slirp? I'm considering net/ , since it do not seem to
generate much coverage noise, and there might be some tricky codepaths
there that benefit from the coverage guidance.
-Alex
> ---
> configure | 13 +++++++++++++
> scripts/oss-fuzz/instrumentation-filter-template | 14 ++++++++++++++
> 2 files changed, 27 insertions(+)
> create mode 100644 scripts/oss-fuzz/instrumentation-filter-template
>
> diff --git a/configure b/configure
> index 38704b4e11..3b6ca054b9 100755
> --- a/configure
> +++ b/configure
> @@ -5189,6 +5189,11 @@ if test "$fuzzing" = "yes" && test -z "${LIB_FUZZING_ENGINE+xxx}"; then
> error_exit "Your compiler doesn't support -fsanitize=fuzzer"
> exit 1
> fi
> + have_clang_coverage_filter=no
> + echo > $TMPTXT
> + if compile_prog "$CPU_CFLAGS -Werror -fsanitize=fuzzer -fsanitize-coverage-allowlist=$TMPTXT" ""; then
> + have_clang_coverage_filter=yes
> + fi
> fi
>
> # Thread sanitizer is, for now, much noisier than the other sanitizers;
> @@ -6120,6 +6125,14 @@ if test "$fuzzing" = "yes" ; then
> # rule for the fuzzer adds these to the link_args. They need to be
> # configurable, to support OSS-Fuzz
> FUZZ_EXE_LDFLAGS="-fsanitize=fuzzer"
> +
> + # Specify a filter to only instrument code that is directly related to
> + # virtual-devices.
> + if test "$have_clang_coverage_filter" = "yes" ; then
> + cp "$source_path/scripts/oss-fuzz/instrumentation-filter-template" \
> + instrumentation-filter
> + QEMU_CFLAGS="$QEMU_CFLAGS -fsanitize-coverage-allowlist=instrumentation-filter"
> + fi
> else
> FUZZ_EXE_LDFLAGS="$LIB_FUZZING_ENGINE"
> fi
> diff --git a/scripts/oss-fuzz/instrumentation-filter-template b/scripts/oss-fuzz/instrumentation-filter-template
> new file mode 100644
> index 0000000000..44e853159c
> --- /dev/null
> +++ b/scripts/oss-fuzz/instrumentation-filter-template
> @@ -0,0 +1,14 @@
> +# Code that we actually want the fuzzer to target
> +# See: https://clang.llvm.org/docs/SanitizerCoverage.html#disabling-instrumentation-without-source-modification
> +#
> +src:*/hw/*
> +src:*/include/hw/*
> +src:*/slirp/*
> +
> +# We don't care about coverage over fuzzer-specific code, however we should
> +# instrument the fuzzer entry-point so libFuzzer always sees at least some
> +# coverage - otherwise it will exit after the first input
> +src:*/tests/qtest/fuzz/fuzz.c
> +
> +# Enable instrumentation for all functions in those files
> +fun:*
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v5 2/3] fuzz: add an instrumentation filter
2021-06-30 3:41 ` [PATCH v5 2/3] fuzz: add an instrumentation filter Alexander Bulekov
2021-07-08 2:58 ` Alexander Bulekov
@ 2021-07-08 9:30 ` Darren Kenny
1 sibling, 0 replies; 6+ messages in thread
From: Darren Kenny @ 2021-07-08 9:30 UTC (permalink / raw)
To: Alexander Bulekov, qemu-devel
Cc: Thomas Huth, Philippe Mathieu-Daudé,
Alexander Bulekov, Bandan Das, Stefan Hajnoczi, Paolo Bonzini
Hi Alex,
Sorry, missed this one, apologies for the delay in responding.
On Tuesday, 2021-06-29 at 23:41:23 -04, Alexander Bulekov wrote:
> By default, -fsanitize=fuzzer instruments all code with coverage
> information. However, this means that libfuzzer will track coverage over
> hundreds of source files that are unrelated to virtual-devices. This
> means that libfuzzer will optimize inputs for coverage observed in timer
> code, memory APIs etc. This slows down the fuzzer and stores many inputs
> that are not relevant to the actual virtual-devices.
>
> With this change, clang versions that support the
> "-fsanitize-coverage-allowlist" will only instrument a subset of the
> compiled code, that is directly related to virtual-devices.
>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
> configure | 13 +++++++++++++
> scripts/oss-fuzz/instrumentation-filter-template | 14 ++++++++++++++
> 2 files changed, 27 insertions(+)
> create mode 100644 scripts/oss-fuzz/instrumentation-filter-template
>
> diff --git a/configure b/configure
> index 38704b4e11..3b6ca054b9 100755
> --- a/configure
> +++ b/configure
> @@ -5189,6 +5189,11 @@ if test "$fuzzing" = "yes" && test -z "${LIB_FUZZING_ENGINE+xxx}"; then
> error_exit "Your compiler doesn't support -fsanitize=fuzzer"
> exit 1
> fi
> + have_clang_coverage_filter=no
> + echo > $TMPTXT
> + if compile_prog "$CPU_CFLAGS -Werror -fsanitize=fuzzer -fsanitize-coverage-allowlist=$TMPTXT" ""; then
> + have_clang_coverage_filter=yes
> + fi
> fi
>
> # Thread sanitizer is, for now, much noisier than the other sanitizers;
> @@ -6120,6 +6125,14 @@ if test "$fuzzing" = "yes" ; then
> # rule for the fuzzer adds these to the link_args. They need to be
> # configurable, to support OSS-Fuzz
> FUZZ_EXE_LDFLAGS="-fsanitize=fuzzer"
> +
> + # Specify a filter to only instrument code that is directly related to
> + # virtual-devices.
> + if test "$have_clang_coverage_filter" = "yes" ; then
> + cp "$source_path/scripts/oss-fuzz/instrumentation-filter-template" \
> + instrumentation-filter
> + QEMU_CFLAGS="$QEMU_CFLAGS -fsanitize-coverage-allowlist=instrumentation-filter"
>
The only concern that I would have here is that the file
instrumentaion-filter is being sepcified without a full path, and
whether that is acceptable as a generic QEMU_CFLAGS element.
I couldn't find anything that suggests it needs the be a full-path, and
all examples seem to be a simple filename, so maybe it searches up the
directory tree, but I can't find anything to say that off-hand.
If that is acceptable, and is working, then I'm ok with it, just wanted
to be sure:
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Thanks,
Darren.
> + fi
> else
> FUZZ_EXE_LDFLAGS="$LIB_FUZZING_ENGINE"
> fi
> diff --git a/scripts/oss-fuzz/instrumentation-filter-template b/scripts/oss-fuzz/instrumentation-filter-template
> new file mode 100644
> index 0000000000..44e853159c
> --- /dev/null
> +++ b/scripts/oss-fuzz/instrumentation-filter-template
> @@ -0,0 +1,14 @@
> +# Code that we actually want the fuzzer to target
> +# See: https://clang.llvm.org/docs/SanitizerCoverage.html#disabling-instrumentation-without-source-modification
> +#
> +src:*/hw/*
> +src:*/include/hw/*
> +src:*/slirp/*
> +
> +# We don't care about coverage over fuzzer-specific code, however we should
> +# instrument the fuzzer entry-point so libFuzzer always sees at least some
> +# coverage - otherwise it will exit after the first input
> +src:*/tests/qtest/fuzz/fuzz.c
> +
> +# Enable instrumentation for all functions in those files
> +fun:*
> --
> 2.28.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v5 3/3] fuzz: make object-name matching case-insensitive
2021-06-30 3:41 [PATCH v5 0/3] Fuzzer pattern-matching, timeouts, and instrumentation-filtering Alexander Bulekov
2021-06-30 3:41 ` [PATCH v5 1/3] fuzz: adjust timeout to allow for longer inputs Alexander Bulekov
2021-06-30 3:41 ` [PATCH v5 2/3] fuzz: add an instrumentation filter Alexander Bulekov
@ 2021-06-30 3:41 ` Alexander Bulekov
2 siblings, 0 replies; 6+ messages in thread
From: Alexander Bulekov @ 2021-06-30 3:41 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Alexander Bulekov,
Philippe Mathieu-Daudé,
darren.kenny, Bandan Das, Stefan Hajnoczi, Paolo Bonzini
We have some configs for devices such as the AC97 and ES1370 that were
not matching memory-regions correctly, because the configs provided
lowercase names. To resolve these problems and prevent them from
occurring again in the future, convert both the pattern and names to
lower-case, prior to checking for a match.
Suggested-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
tests/qtest/fuzz/generic_fuzz.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
index 43f9011626..59be7219de 100644
--- a/tests/qtest/fuzz/generic_fuzz.c
+++ b/tests/qtest/fuzz/generic_fuzz.c
@@ -751,8 +751,13 @@ static int locate_fuzz_memory_regions(Object *child, void *opaque)
static int locate_fuzz_objects(Object *child, void *opaque)
{
+ GString *type_name;
+ GString *path_name;
char *pattern = opaque;
- if (g_pattern_match_simple(pattern, object_get_typename(child))) {
+
+ type_name = g_string_new(object_get_typename(child));
+ g_string_ascii_down(type_name);
+ if (g_pattern_match_simple(pattern, type_name->str)) {
/* Find and save ptrs to any child MemoryRegions */
object_child_foreach_recursive(child, locate_fuzz_memory_regions, NULL);
@@ -769,8 +774,9 @@ static int locate_fuzz_objects(Object *child, void *opaque)
g_ptr_array_add(fuzzable_pci_devices, PCI_DEVICE(child));
}
} else if (object_dynamic_cast(OBJECT(child), TYPE_MEMORY_REGION)) {
- if (g_pattern_match_simple(pattern,
- object_get_canonical_path_component(child))) {
+ path_name = g_string_new(object_get_canonical_path_component(child));
+ g_string_ascii_down(path_name);
+ if (g_pattern_match_simple(pattern, path_name->str)) {
MemoryRegion *mr;
mr = MEMORY_REGION(child);
if ((memory_region_is_ram(mr) ||
@@ -779,7 +785,9 @@ static int locate_fuzz_objects(Object *child, void *opaque)
g_hash_table_insert(fuzzable_memoryregions, mr, (gpointer)true);
}
}
+ g_string_free(path_name, true);
}
+ g_string_free(type_name, true);
return 0;
}
@@ -807,6 +815,7 @@ static void generic_pre_fuzz(QTestState *s)
MemoryRegion *mr;
QPCIBus *pcibus;
char **result;
+ GString *name_pattern;
if (!getenv("QEMU_FUZZ_OBJECTS")) {
usage();
@@ -836,10 +845,17 @@ static void generic_pre_fuzz(QTestState *s)
result = g_strsplit(getenv("QEMU_FUZZ_OBJECTS"), " ", -1);
for (int i = 0; result[i] != NULL; i++) {
+ name_pattern = g_string_new(result[i]);
+ /*
+ * Make the pattern lowercase. We do the same for all the MemoryRegion
+ * and Type names so the configs are case-insensitive.
+ */
+ g_string_ascii_down(name_pattern);
printf("Matching objects by name %s\n", result[i]);
object_child_foreach_recursive(qdev_get_machine(),
locate_fuzz_objects,
- result[i]);
+ name_pattern->str);
+ g_string_free(name_pattern, true);
}
g_strfreev(result);
printf("This process will try to fuzz the following MemoryRegions:\n");
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread