This adds two new options WITH_ASAN/WITH_UBSAN that can be set to
enable instrumentation of all binaries with AddressSanitizer and/or
UndefinedBehaviourSanitizer. This current patch is almost sufficient
to get a complete buildworld with sanitizer instrumentation but in
order to actually build and boot a system it depends on a few more
follow-up commits.
Details
Details
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 40349 Build 37238: arc lint + arc unit
Event Timeline
| libexec/rtld-elf/Makefile | ||
|---|---|---|
| 17 | I do not think rtld would be ever compatible. | |
| share/mk/bsd.lib.mk | ||
| 9 | What does this mean, and what it does? | |
| 154 | What does this mean? Are you hacking on the SHARED_CFLAGS value, removing -fstack-protector when sanitizers are enabled? Can it be done clean, for instance by not adding -fstack-protector at all to the variable? If my guess is right, such fight over the options should not be done. | |
| libexec/rtld-elf/Makefile | ||
|---|---|---|
| 17 | I was thinking that minimal UBSAN might work, but yeah will update the comment. | |
| share/mk/bsd.lib.mk | ||
| 9 | This just creates an empty target so that bsd.sanitizer.mk can check whether it was included from bsd.lib.mk or bsd.prog.mk. Other makefiles use this pattern as an "include guard". | |
| 154 | This is here to ensure that the .nosspico files don't have sanitizer instrumentation since they are used by RTLD. | |
| lib/csu/Makefile.inc | ||
|---|---|---|
| 9–10 | This is a related but independent change? | |
| lib/csu/Makefile.inc | ||
|---|---|---|
| 9–10 | I originally had this as a separate commit but looks like I squashed it while rebasing. Will factor it out. | |
| share/mk/bsd.sanitizer.mk | ||
|---|---|---|
| 12 | This ensures that clang adds the require libraries to the linker command line: the ASAN runtime needs to intercept many libc functions (even when libc is built with ASAN instrumentation). And this runtime is linked as a static library if you pass -fsanitize=address. | |
| share/mk/bsd.lib.mk | ||
|---|---|---|
| 154 | RTLD pulls in some of these files from libc (and libc is built with ASAN instrumentation), so unless we remove -fsanitize=address from build command for the .nosspico files, rtld will end up using some object files that call __asan_foo functions. | |
| lib/libclang_rt/Makefile.inc | ||
|---|---|---|
| 16 | Is it necessary to use different assignment operators here for MK_PROFILE vs. MK_(A|UB)SAN? | |
| share/mk/bsd.lib.mk | ||
| 154 | Ah, I didn't realize that the rtld-libc build reuses libc object files. You might consider amending the description of .nossppico files in share/mk/bsd.README then. | |
| share/mk/bsd.sanitizer.mk | ||
| 12 | I see, I was relying on the fact that we pass CFLAGS when linking. And I see now that -fsanitize* gets stripped from _LDFLAGS. | |
Comment Actions
Fix running kyua by not instrumenting libunwind functions called during C++ exception unwinding. This results in false-positive ASAN reports.
| lib/csu/Makefile.inc | ||
|---|---|---|
| 9–10 | Just noticed there is already a SSP_CFLAGS= above which should no longer be needed with MK_SSP=no. | |
| lib/libclang_rt/Makefile.inc | ||
| 16 | Most other places use := for MK_* assignments, I guess both work fine here. | |
| share/mk/bsd.sanitizer.mk | ||
| 12 | It does seem like most (all?) linker invocation will get the CFLAGS but I feel that adding it to LDFLAGS as well is safer. | |
Comment Actions
Thanks, this looks reasonable to me. The change is missing src.conf.5 updates for the new knobs.
| lib/libgcc_eh/Makefile.inc | ||
|---|---|---|
| 24 | I haven't had time to debug this, my naive assumption is that asan expects functions that do stack unwinding to be uninstrumented. | |
| lib/libgcc_eh/Makefile.inc | ||
|---|---|---|
| 24 | https://github.com/llvm/llvm-project/commit/adf1561d6ce8af057127c65af863b3f0e1c77e60 seems relevant, don't know if there are other issues though, but it does look like upstream intends libunwind to work with asan | |
| share/mk/bsd.sanitizer.mk | ||
| 4 | Tabs after your ='s | |
| 4 | Style is probably to use yes/no not 1/0? | |
| stand/Makefile.inc | ||
| 4–5 | Missing a tab after the = | |
| lib/libgcc_eh/Makefile.inc | ||
|---|---|---|
| 24 | Thanks, will try applying that tomorrow. | |
| Makefile.inc1 | ||
|---|---|---|
| 720–721 | No, I added this as an experimental flag to try and add asan to the bootstrap tools. It won't work yet since there are still quite a few UBSan warnings. Happy to drop it from this commit and just hardcode it to no? | |
| lib/libgcc_eh/Makefile.inc | ||
|---|---|---|
| 24 | I tried applying that patch but it seems it's not sufficient to allow instrumenting libunwind (at least the version currently in contrib/). root@freebsd-amd64:~ # kyua --help
=================================================================
==808==ERROR: AddressSanitizer: stack-buffer-underflow on address 0x7fffffffb500 at pc 0x0000019ae27c bp 0x7fffffffb2c0 sp 0x7fffffffaa80
WRITE of size 552 at 0x7fffffffb500 thread T0
#0 0x19ae27b in memset /local/scratch/alr48/cheri/freebsd/contrib/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc:782:3
#1 0x8029930a5 in libunwind::CFI_Parser<libunwind::LocalAddressSpace>::PrologInfo::PrologInfo(libunwind::CFI_Parser<libunwind::LocalAddressSpace>::PrologInfo::InitializeTime) /local/scratch/alr48/cheri/freebsd/contrib/llvm-project/libunwind/src/DwarfParser.hpp:96:9
#2 0x8029930a5 in libunwind::UnwindCursor<libunwind::LocalAddressSpace, libunwind::Registers_x86_64>::getInfoFromFdeCie(libunwind::CFI_Parser<libunwind::LocalAddressSpace>::FDE_Info const&, libunwind::CFI_Parser<libunwind::LocalAddressSpace>::CIE_Info const&, unsigned long, unsigned long) /local/scratch/alr48/cheri/freebsd/contrib/llvm-project/libunwind/src/UnwindCursor.hpp:1510:38
#3 0x8029930a5 in libunwind::UnwindCursor<libunwind::LocalAddressSpace, libunwind::Registers_x86_64>::getInfoFromDwarfSection(unsigned long, libunwind::UnwindInfoSections const&, unsigned int) /local/scratch/alr48/cheri/freebsd/contrib/llvm-project/libunwind/src/UnwindCursor.hpp:1571:9
#4 0x80298ff4f in libunwind::UnwindCursor<libunwind::LocalAddressSpace, libunwind::Registers_x86_64>::setInfoBasedOnIPRegister(bool) /local/scratch/alr48/cheri/freebsd/contrib/llvm-project/libunwind/src/UnwindCursor.hpp:1958:17
#5 0x80298ea39 in unw_init_local /local/scratch/alr48/cheri/freebsd/contrib/llvm-project/libunwind/src/libunwind.cpp:79:7
#6 0x80298cc5c in unwind_phase2 /local/scratch/alr48/cheri/freebsd/contrib/llvm-project/libunwind/src/UnwindLevel1.c:133:3
#7 0x80298d5ba in _Unwind_Resume /local/scratch/alr48/cheri/freebsd/contrib/llvm-project/libunwind/src/UnwindLevel1.c:394:5
#8 0x1a7ae39 in utils::cmdline::parse(int, char const* const*, std::__1::vector<utils::cmdline::base_option const*, std::__1::allocator<utils::cmdline::base_option const*> > const&) /local/scratch/alr48/cheri/freebsd/contrib/kyua/utils/cmdline/parser.cpp
#9 0x1cefab6 in (anonymous namespace)::safe_main(utils::cmdline::ui*, int, char const* const*, std::__1::auto_ptr<utils::cmdline::base_command<utils::config::tree> >) /local/scratch/alr48/cheri/freebsd/contrib/kyua/cli/main.cpp:203:45
#10 0x1ced089 in cli::main(utils::cmdline::ui*, int, char const* const*, std::__1::auto_ptr<utils::cmdline::base_command<utils::config::tree> >) /local/scratch/alr48/cheri/freebsd/contrib/kyua/cli/main.cpp:280:31
#11 0x1cf1670 in cli::main(int, char const* const*) /local/scratch/alr48/cheri/freebsd/contrib/kyua/cli/main.cpp:353:27
Address 0x7fffffffb500 is located in stack of thread T0 at offset 0 in frame
#0 0x27 (<unknown module>)
AddressSanitizer can't parse the stack frame descriptor: ||
SUMMARY: AddressSanitizer: stack-buffer-underflow /local/scratch/alr48/cheri/freebsd/contrib/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc:782:3 in memset
Shadow bytes around the buggy address:
0x4ffffffff650: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x4ffffffff660: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x4ffffffff670: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x4ffffffff680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x4ffffffff690: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x4ffffffff6a0:[f1]f1 f1 f1 00 f2 f2 f2 00 00 00 00 00 00 00 00
0x4ffffffff6b0: 00 f2 f2 f2 f2 f2 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
0x4ffffffff6c0: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
0x4ffffffff6d0: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
0x4ffffffff6e0: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
0x4ffffffff6f0: f8 f8 f8 f8 f8 f8 f2 f2 f2 f2 f2 f2 f2 f2 f8 f2
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
Shadow gap: cc
==808==ABORTING | |
Comment Actions
Style fixes + drop BOOTSTRAP_TOOLS_ASAN/BOOTSTRAP_TOOLS_UBSAN
Will commit this tomorrow after a final round of testing.