Page MenuHomeFreeBSD

Add build system support for ASAN+UBSAN instrumentation
ClosedPublic

Authored by arichardson on Jul 5 2021, 10:45 AM.

Details

Summary

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.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

libexec/rtld-elf/Makefile
16

I do not think rtld would be ever compatible.

share/mk/bsd.lib.mk
8

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
16

I was thinking that minimal UBSAN might work, but yeah will update the comment.

share/mk/bsd.lib.mk
8

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.
I reused the existing logic to remove -fstack-protector here, not sure if there is a cleaner solution.

lib/csu/Makefile.inc
8–9

This is a related but independent change?

lib/csu/Makefile.inc
8–9

I originally had this as a separate commit but looks like I squashed it while rebasing. Will factor it out.
I don't think it really matters since none of the functions *should* end up with SSP checks but in theory it could be a problem.

share/mk/bsd.lib.mk
154

But we explicitly disable instrumentation for rtld, so this change is just for safety? I believe rtld is the only place where we generate this flavour of object files.

share/mk/bsd.sanitizer.mk
11

What does the linker do with -fsanitize? In my experiments this wasn't needed.

share/mk/bsd.sanitizer.mk
11

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
15

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
11

I see, I was relying on the fact that we pass CFLAGS when linking. And I see now that -fsanitize* gets stripped from _LDFLAGS.

Fix running kyua by not instrumenting libunwind functions called during C++ exception unwinding. This results in false-positive ASAN reports.

arichardson added inline comments.
lib/csu/Makefile.inc
8–9

Just noticed there is already a SSP_CFLAGS= above which should no longer be needed with MK_SSP=no.

lib/libclang_rt/Makefile.inc
15

Most other places use := for MK_* assignments, I guess both work fine here.

share/mk/bsd.sanitizer.mk
11

It does seem like most (all?) linker invocation will get the CFLAGS but I feel that adding it to LDFLAGS as well is safer.

Thanks, this looks reasonable to me. The change is missing src.conf.5 updates for the new knobs.

This revision is now accepted and ready to land.Jul 6 2021, 2:20 PM
Makefile.inc1
720–721

Do we have any documentation for BOOTSTRAP_TOOLS_ASAN/BOOTSTRAP_TOOLS_UBSAN?

lib/libgcc_eh/Makefile.inc
23

Is a bug, or a known limitation of asan?

lib/libgcc_eh/Makefile.inc
23

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
23

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 ↗(On Diff #91874)

Missing a tab after the =

share/mk/bsd.sanitizer.mk
6
15

Ditto

lib/libgcc_eh/Makefile.inc
23

Thanks, will try applying that tomorrow.

arichardson marked 4 inline comments as done.
This revision now requires review to proceed.Jul 19 2021, 12:00 PM
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
23

I tried applying that patch but it seems it's not sufficient to allow instrumenting libunwind (at least the version currently in contrib/).
I still get the following error with that patch applied:

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
Makefile.inc1
720–721

Yeah I'd leave them out of this

lib/csu/Makefile.inc
6–7

Tabs

lib/libgcc_s/Makefile
7

Tab

share/mk/bsd.sanitizer.mk
38

Tab

arichardson marked 4 inline comments as done.

Style fixes + drop BOOTSTRAP_TOOLS_ASAN/BOOTSTRAP_TOOLS_UBSAN

Will commit this tomorrow after a final round of testing.

This revision was not accepted when it landed; it landed in state Needs Review.Aug 2 2021, 1:35 PM
This revision was automatically updated to reflect the committed changes.