Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate coverage to trace-pc-guard #84

Closed
mikea opened this issue Nov 14, 2016 · 9 comments
Closed

Migrate coverage to trace-pc-guard #84

mikea opened this issue Nov 14, 2016 · 9 comments
Assignees

Comments

@mikea
Copy link
Contributor

mikea commented Nov 14, 2016

Replace COV_FLAGS with -fsanitize-coverage=trace-pc-guard.

Easy test:

docker run -ti -e COV_FLAGS="-fsanitize-coverage=trace-pc-guard" ossfuzz/expat test
@mikea
Copy link
Contributor Author

mikea commented Nov 14, 2016

expat fuzzer crashes with:

==3296==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000000a136f4 at pc 0x0000005cf95a bp 0x7fff09d70a70 sp 0x7fff09d70a68
WRITE of size 4 at 0x000000a136f4 thread T0
    #0 0x5cf959 in fuzzer::TracePC::HandleInit(unsigned int*, unsigned int*) /src/libfuzzer/FuzzerTracePC.cpp:49:8
    #1 0x5de349 in __sanitizer_cov_trace_pc_guard_init /src/libfuzzer/FuzzerTracePC.cpp:286:15
    #2 0x511a7b in sancov.module_ctor (/out/parse_fuzzer+0x511a7b)
    #3 0x772b4c in __libc_csu_init (/out/parse_fuzzer+0x772b4c)
    #4 0x7faf2bee77be in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x207be)
    #5 0x41c1a8 in _start (/out/parse_fuzzer+0x41c1a8)

0x000000a136f4 is located 44 bytes to the left of global variable '__sancov_guard.9' defined in '/src/parse_fuzzer.cc' (0xa13720) of size 12
0x000000a136f4 is located 0 bytes to the right of global variable '__sancov_guard' defined in '/src/parse_fuzzer.cc' (0xa136e0) of size 20
SUMMARY: AddressSanitizer: global-buffer-overflow /src/libfuzzer/FuzzerTracePC.cpp:49:8 in fuzzer::TracePC::HandleInit(unsigned int*, unsigned int*)
Shadow bytes around the buggy address:
  0x00008013a680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00008013a690: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00008013a6a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00008013a6b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x00008013a6c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x00008013a6d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00[04]f9
  0x00008013a6e0: f9 f9 f9 f9 00 04 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x00008013a6f0: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  0x00008013a700: 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
  0x00008013a710: 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
  0x00008013a720: f9 f9 f9 f9 00 00 00 f9 f9 f9 f9 f9 00 f9 f9 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==3296==ABORTING

@kcc
Copy link
Contributor

kcc commented Nov 15, 2016

how can I reproduce it? (w/o docker, please)

@mikea
Copy link
Contributor Author

mikea commented Nov 15, 2016

Sure. From the full output of the above (https://gist.github.com/mikea/90a119db04ab54aef09b26ec9ce5aa38) and expat build scrtipt (https://github.com/google/oss-fuzz/blob/master/targets/expat/build.sh):

# build libfuzzer
clang++ -g -stdlib=libc++ -std=c++11 -fsanitize=address -c /src/libfuzzer/FuzzerCrossOver.cpp /src/libfuzzer/FuzzerDriver.cpp /src/libfuzzer/FuzzerExtFunctionsDlsym.cpp /src/libfuzzer/FuzzerExtFunctionsWeak.cpp /src/libfuzzer/FuzzerIO.cpp /src/libfuzzer/FuzzerLoop.cpp /src/libfuzzer/FuzzerMain.cpp /src/libfuzzer/FuzzerMutate.cpp /src/libfuzzer/FuzzerSHA1.cpp /src/libfuzzer/FuzzerTracePC.cpp /src/libfuzzer/FuzzerTraceState.cpp /src/libfuzzer/FuzzerUtil.cpp /src/libfuzzer/FuzzerUtilDarwin.cpp /src/libfuzzer/FuzzerUtilLinux.cpp -I/src/libfuzzer

ar ruv /usr/lib/libfuzzer.a /work/libfuzzer/FuzzerCrossOver.o /work/libfuzzer/FuzzerDriver.o /work/libfuzzer/FuzzerExtFunctionsDlsym.o /work/libfuzzer/FuzzerExtFunctionsWeak.o /work/libfuzzer/FuzzerIO.o /work/libfuzzer/FuzzerLoop.o /work/libfuzzer/FuzzerMain.o /work/libfuzzer/FuzzerMutate.o /work/libfuzzer/FuzzerSHA1.o /work/libfuzzer/FuzzerTracePC.o /work/libfuzzer/FuzzerTraceState.o /work/libfuzzer/FuzzerUtil.o /work/libfuzzer/FuzzerUtilDarwin.o /work/libfuzzer/FuzzerUtilLinux.o

export 'CFLAGS=-g -fsanitize=address -fsanitize-coverage=trace-pc-guard'
export 'CXXFLAGS=-g -fsanitize=address -fsanitize-coverage=trace-pc-guard -stdlib=libc++'
export 'CC=clang'
export 'CXX=clang++'
export 'FUZZER_LDFLAGS=-Wl,-whole-archive /usr/local/lib/libc++.a /usr/local/lib/libc++abi.a -Wl,-no-whole-archive'

# build expat
cd /src/expat/expat

./buildconf.sh
./configure
make clean
make -j$(nproc) all

$CXX $CXXFLAGS -std=c++11 -Ilib/ \
    /src/parse_fuzzer.cc -o /out/parse_fuzzer \
    -lfuzzer .libs/libexpat.a $FUZZER_LDFLAGS

@kcc
Copy link
Contributor

kcc commented Nov 15, 2016

fixed in LLVM r287030.
The problem was that asan instrumented the coverage guards
emitted by -fsanitize-coverage=trace-pc-guard
which caused trouble if libFuzzer was asan-ified.
It caused inefficiency otherwise as well, as the guard section was longer. Good catch!

You'll need to update the compiler now...

earl pushed a commit to earl/llvm-mirror that referenced this issue Nov 15, 2016
dylanmckay pushed a commit to avr-llvm/llvm that referenced this issue Nov 16, 2016
llvm-beanz pushed a commit to llvm-beanz/llvm-submodules that referenced this issue Nov 24, 2016
@kcc
Copy link
Contributor

kcc commented Dec 10, 2016

also don't forget to add trace-cmp

@kcc kcc changed the title Migrate coverage to trace-pc-guard Migrate coverage to trace-pc-guard,trace-cmp Dec 10, 2016
@kcc
Copy link
Contributor

kcc commented Dec 12, 2016

Just to clarify: trace-pc-guard actually requires some work on the sanitizers' side and I want us first to switch to trace-pc-guard.

Then, separately (in a few days), I want to also enable trace-cmp.
I don't want to enable trace-cmp w/o trace-pc-guard because this configuration is not tested.

@oliverchang
Copy link
Collaborator

(moving discussion from email thread).

Latest status:

It looks like the coverage dumped by -dump_coverage=1 doesn't work with the sancov tool.

$ ./parse_msgpack_fuzzer -runs=1000 -dump_coverage=1

...


SanitizerCoverage: ./parse_msgpack_fuzzer.20585.sancov 151 PCs written

$ sancov -symbolize parse_msgpack_fuzzer parse_msgpack_fuzzer.20585.sancov

Error: Coverage points in binary and .sancov file do not match.

@mikea
Copy link
Contributor Author

mikea commented Jan 17, 2017

The last problem should be fixed with llvm-mirror/llvm@5699207

Let's recheck it tomorrow.

@oliverchang
Copy link
Collaborator

Thanks Mike! Looks like sancov is working now with the coverage:

https://clusterfuzz-external.appspot.com/coverage_report/fuzzer/libFuzzer_json_parse_afl_fuzzer/2017-01-18

@mikea mikea changed the title Migrate coverage to trace-pc-guard,trace-cmp Migrate coverage to trace-pc-guard Jan 18, 2017
@mikea mikea closed this as completed Jan 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants