Re: [PATCH 1/4] fuzz: refine crash detection mechanism

[Li Qiuhao]

On Tue, 2020-12-22 at 11:47 -0500, Alexander Bulekov wrote:
> Oops let me try to resend this..
> 
> Qiuhao Li <Qiuhao.Li@outlook.com> writes:
> 
> > The original crash detection method is to fork a process to test
> > our new
> > trace input. If the child process exits in time and the second-to-
> > last line
> > is the same as the first crash, we think it is a crash triggered by
> > the same
> > bug. However, in some situations, it doesn't work since it is a
> > hardcoded-offset string comparison.
> > 
> > For example, suppose an assertion failure makes the crash. In that
> > case, the
> > second-to-last line will be 'timeout: the monitored command dumped
> > core',
> > 
> 
> Ah - I have not encountered this message. Are you running an
> --enable-sanitizers build? I believe ASAN disables coredumps, by
> default. I have to turn them on with:
> ASAN_OPTIONS=abort_on_error=1:disable_coredump=0:unmap_shadow_on_exit
> =1
> 
> Maybe this is a matter of setting the correct env variables/disabling
> coredumps?

Yes, I built emu-system-i386 with --enable-fuzzing --enable-sanitizers.

I tested a program built with ASAN, but it did try to dump core:

---

qiuhao@XPS-13-9360:~/tmp$ cat test.c
#include <signal.h>
int main (void) {
raise(SIGABRT);
return(0);
}

qiuhao@XPS-13-9360:~/tmp$ clang --version
clang version 10.0.0-4ubuntu1 
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

qiuhao@XPS-13-9360:~/tmp$ clang -g -O0 -fsanitize=address test.c && ./a.out
Aborted (core dumped)

---

Only when I test a program built with "-fanitizer=address,fuzzer" will it 
disable the core dump and print the stack backtrace and info deadly signal:

---

qiuhao@XPS-13-9360:~/tmp$ cat fuzz.cc 
#include <stdint.h>
#include <signal.h>

extern "C" int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) {
  raise(SIGABRT);
  return 0;
}

qiuhao@XPS-13-9360:~/tmp$ clang++ -fsanitize=address,fuzzer fuzz.cc && ./a.out 
INFO: Seed: 3533057472
INFO: Loaded 1 modules   (1 inline 8-bit counters): 1 [0x5a6ec0, 0x5a6ec1), 
INFO: Loaded 1 PC tables (1 PCs): 1 [0x56b140,0x56b150), 
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 
4096 bytes
==38847== ERROR: libFuzzer: deadly signal
    #0 0x526d21 in __sanitizer_print_stack_trace 
(/home/qiuhao/tmp/a.out+0x526d21)
    #1 0x471e78 in fuzzer::PrintStackTrace() (/home/qiuhao/tmp/a.out+0x471e78)
    #2 0x456fc3 in fuzzer::Fuzzer::CrashCallback() 
(/home/qiuhao/tmp/a.out+0x456fc3)
    #3 0x7fe3df4b63bf  (/lib/x86_64-linux-gnu/libpthread.so.0+0x153bf)
    #4 0x7fe3df4b624a in __libc_signal_restore_set 
/build/glibc-ZN95T4/glibc-2.31/nptl/../sysdeps/unix/sysv/linux/internal-signals.h:86:3
    #5 0x7fe3df4b624a in raise 
/build/glibc-ZN95T4/glibc-2.31/nptl/../sysdeps/unix/sysv/linux/raise.c:48:3
    #6 0x550291 in LLVMFuzzerTestOneInput (/home/qiuhao/tmp/a.out+0x550291)
    #7 0x458681 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, 
unsigned long) (/home/qiuhao/tmp/a.out+0x458681)
    #8 0x45a3ba in 
fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile,
 fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) 
(/home/qiuhao/tmp/a.out+0x45a3ba)
    #9 0x45aa49 in 
fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, 
fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) 
(/home/qiuhao/tmp/a.out+0x45aa49)
    #10 0x44971e in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char 
const*, unsigned long)) (/home/qiuhao/tmp/a.out+0x44971e)
    #11 0x472562 in main (/home/qiuhao/tmp/a.out+0x472562)
    #12 0x7fe3df2a80b2 in __libc_start_main 
/build/glibc-ZN95T4/glibc-2.31/csu/../csu/libc-start.c:308:16
    #13 0x41e4bd in _start (/home/qiuhao/tmp/a.out+0x41e4bd)

NOTE: libFuzzer has rudimentary signal handlers.
      Combine libFuzzer with AddressSanitizer or similar for better crash 
reports.
SUMMARY: libFuzzer: deadly signal
MS: 0 ; base unit: 0000000000000000000000000000000000000000


artifact_prefix='./'; Test unit written to 
./crash-da39a3ee5e6b4b0d3255bfef95601890afd80709
Base64: 

---

Did I miss something? Or I misused ASAN?

> I like the idea of switching out CRASH_TOKEN for a regex, however I
> am
> not sure about using the hardcoded crash_patterns to perform
> matching:
> 
> 1.) You risk missing some crash pattern. E.g. I don't think
> abort()/hw_error() are covered right now.

I reversed your fallback in Patch, just renamed it :)

-    CRASH_TOKEN = os.getenv("CRASH_TOKEN")
+    crash_pattern = os.getenv("CRASH_PATTERN")

> 2.) At some point ASAN/compiler-rt might change the way it outputs
> crashes.

You are right, but the lines[-2] has the same problem.

> I think the current lines[-2] approach is ugly, but it is small,
> works
> in most cases (when coredumps are disabled), and has a simple
> CRASH_TOKEN fallback mechanism. We should fix the coredump problem.
> 
> Is there any way to do this without hardcoding patterns (or at least
> fall-back to something if you don't find a pattern)?
> 
> -Alex

The major reason I choose to use regex pattern is that we can keep trimming the 
input trace when removing a instruction triggers a different crash output, 
especially caused by a changed leaf stack frame. For example, Bug 1907497, 
https://lists.nongnu.org/archive/html/qemu-devel/2020-12/msg02380.html. Suppose 
there are duplicate writes which can still crash:

outl 0xcf8 0x80000804
outw 0xcfc 0xffff
write 0x0 0x1 0x12
write 0x2 0x1 0x2f
outl 0xcf8 0x80000811
outl 0xcfc 0x5a6a4406
write 0x6a44005a 0x1 0x11  <-- The only one we really need
write 0x6a44005a 0x1 0x11  <-- useless
write 0x6a44005a 0x1 0x11  <-- useless
write 0x6a44005c 0x1 0x3f
write 0x6a442050 0x4 0x0000446a
write 0x6a44204a 0x1 0xf3
write 0x6a44204c 0x1 0xff
writeq 0x6a44005a 0x17b3f0011
write 0x6a442050 0x4 0x0000446a
write 0x6a44204a 0x1 0xf3
write 0x6a44204c 0x1 0xff

For this fake input, The lines[-2] would be:

SUMMARY: AddressSanitizer: stack-overflow 
(/home/qiuhao/hack/qemu/build/qemu-system-i386+0x27ca049) in __asan_memcpy

But after we removed a useless write, the lines[-2] **has a chance (around 1 
out of 5)** to be:

SUMMARY: AddressSanitizer: stack-overflow 
/home/qiuhao/hack/qemu/build/../softmmu/physmem.c:309:27 in phys_page_find

or

SUMMARY: AddressSanitizer: stack-overflow 
/home/qiuhao/hack/qemu/build/../softmmu/physmem.c:354 in 
address_space_translate_internal

or

SUMMARY: AddressSanitizer: stack-overflow 
/home/qiuhao/hack/qemu/build/../softmmu/physmem.c:298 in section_covers_addr

This will stop us from trimming the useless writes.

---

In all, how about we try this:

1. If CRASH_PATTERN defined (fallback), use it.
2. Else if we can find a pre-defined pattern in the crash output, use it.
3. Use the second-to-end line as the string for comparison.

P.S.
There are some sophisticated crash case deduplication methods like stack 
backtrace hashing or coverage-based comparison, semantics-aware 
Deduplication[1]. But for now, I think our solution can cover most cases.

[1] https://arxiv.org/abs/1812.00140 6.3.1 Deduplication

> > which doesn't contain any information about the assertion failure
> > like where
> > it happened or the assertion statement. This may lead to a
> > minimized input
> > triggers assertion failure but may indicate another bug. As for
> > some
> > sanitizers' crashes, the direct string comparison may stop us from
> > getting a
> > smaller input, since they may have a different leaf stack frame.
> > 
> > Perhaps we can detect crashes using both precise output string
> > comparison
> > and rough pattern string match and info the user when the trace
> > input
> > triggers different but a seminar output.
> > 
> > Tested:
> > Assertion failure, https://bugs.launchpad.net/qemu/+bug/1908062
> > AddressSanitizer, https://bugs.launchpad.net/qemu/+bug/1907497
> > Trace input that doesn't crash
> > Trace input that crashes Qtest
> > 
> > Signed-off-by: Qiuhao Li <Qiuhao.Li@outlook.com>
> > ---
> >  scripts/oss-fuzz/minimize_qtest_trace.py | 59 ++++++++++++++++++
> > ------
> >  1 file changed, 46 insertions(+), 13 deletions(-)
> > 
> > diff --git a/scripts/oss-fuzz/minimize_qtest_trace.py
> > b/scripts/oss-fuzz/minimize_qtest_trace.py
> > index 5e405a0d5f..d3b09e6567 100755
> > --- a/scripts/oss-fuzz/minimize_qtest_trace.py
> > +++ b/scripts/oss-fuzz/minimize_qtest_trace.py
> > @@ -10,11 +10,16 @@ import os
> >  import subprocess
> >  import time
> >  import struct
> > +import re
> > 
> >  QEMU_ARGS = None
> >  QEMU_PATH = None
> >  TIMEOUT = 5
> > -CRASH_TOKEN = None
> > +
> > +crash_patterns = ("Assertion.+failed",
> > +                  "SUMMARY.+Sanitizer")
> > +crash_pattern = None
> > +crash_string = None
> > 
> >  write_suffix_lookup = {"b": (1, "B"),
> >                         "w": (2, "H"),
> > @@ -24,13 +29,12 @@ write_suffix_lookup = {"b": (1, "B"),
> >  def usage():
> >      sys.exit("""\
> >  Usage: QEMU_PATH="/path/to/qemu" QEMU_ARGS="args" {} input_trace
> > output_trace
> > -By default, will try to use the second-to-last line in the output
> > to identify
> > -whether the crash occred. Optionally, manually set a string that
> > idenitifes the
> > -crash by setting CRASH_TOKEN=
> > +By default, we will try to search predefined crash patterns
> > through the
> > +tracing output to see whether the crash occred. Optionally,
> > manually set a
> > +string that idenitifes the crash by setting CRASH_PATTERN=
> >  """.format((sys.argv[0])))
> > 
> >  def check_if_trace_crashes(trace, path):
> > -    global CRASH_TOKEN
> >      with open(path, "w") as tracefile:
> >          tracefile.write("".join(trace))
> > 
> > @@ -42,17 +46,47 @@ def check_if_trace_crashes(trace, path):
> >                            shell=True,
> >                            stdin=subprocess.PIPE,
> >                            stdout=subprocess.PIPE)
> > +    if rc.returncode == 137:    # Timed Out
> > +        return False
> > +
> >      stdo = rc.communicate()[0]
> >      output = stdo.decode('unicode_escape')
> >      > -    if rc.returncode == 137:    # Timed Out
> > -        return False
> > -    if len(output.splitlines()) < 2:
> > +    output_lines = output.splitlines()
> > +    # Usually we care about the summary info in the last few
> > lines, reverse.
> > +    output_lines.reverse()
> > +
> > +    global crash_pattern, crash_patterns, crash_string
> > +    if crash_pattern is None: # Initialization
> > +        for line in output_lines:
> > +            for c in crash_patterns:
> > +                if re.search(c, line) is not None:
> > +                    crash_pattern = c
> > +                    crash_string = line
> > +                    print("Identifying crash pattern by this
> > string: ",\
> > +                          crash_string)
> > +                    print("Using regex pattern: ", crash_pattern)
> > +                    return True
> > +        print("Failed to initialize crash pattern: no match.")
> >          return False
> > 
> > -    if CRASH_TOKEN is None:
> > -        CRASH_TOKEN = output.splitlines()[-2]
> > +    # First, we search exactly the previous crash string.
> > +    for line in output_lines:
> > +        if crash_string == line:
> > +            return True
> > +
> > +    # Then we decide whether a similar (same pattern) crash
> > happened.
> > +    # Slower now :(
> > +    for line in output_lines:
> > +        if re.search(crash_pattern, line) is not None:
> > +            print("\nINFO: The crash string changed during our
> > minimization process.")
> > +            print("Before: ", crash_string)
> > +            print("After: ", line)
> > +            print("The original regex pattern can still match,
> > updated the crash string.")
> > +            crash_string = line
> > +            return True
> > 
> > -    return CRASH_TOKEN in output
> > +    # The input did not trigger (the same type) bug.
> > +    return False
> > 
> > 
> >  def minimize_trace(inpath, outpath):
> > @@ -66,7 +100,6 @@ def minimize_trace(inpath, outpath):
> >      print("Crashed in {} seconds".format(end-start))
> >      TIMEOUT = (end-start)*5
> >      print("Setting the timeout for {} seconds".format(TIMEOUT))
> > -    print("Identifying Crashes by this string:
> > {}".format(CRASH_TOKEN))
> > 
> >      i = 0
> >      newtrace = trace[:]
> > @@ -152,6 +185,6 @@ if __name__ == '__main__':
> >          usage()
> >      # if "accel" not in QEMU_ARGS:
> >      #     QEMU_ARGS += " -accel qtest"
> > -    CRASH_TOKEN = os.getenv("CRASH_TOKEN")
> > +    crash_pattern = os.getenv("CRASH_PATTERN")
> >      QEMU_ARGS += " -qtest stdio -monitor none -serial none "
> >      minimize_trace(sys.argv[1], sys.argv[2])
> > --
> > 2.25.1