Project does not build with AddressSanitizer (ASan)

[practicalswift]

Hello fellow c-lightning developers!

Our project does not appear to build with AddressSanitizer (ASan):

$ git diff
diff --git a/Makefile b/Makefile
index fdbeff25..93dc4c35 100644
--- a/Makefile
+++ b/Makefile
@@ -179,7 +179,7 @@ ALL_PROGRAMS =

 CPPFLAGS = -DBINTOPKGLIBEXECDIR='"'$(shell sh tools/rel.sh $(bindir) $(pkglibexecdir))'"'
 CWARNFLAGS := -Werror -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition
-CDEBUGFLAGS := -std=gnu11 -g -fstack-protector
+CDEBUGFLAGS := -std=gnu11 -g -fstack-protector -fsanitize=address
 CFLAGS = $(CPPFLAGS) $(CWARNFLAGS) $(CDEBUGFLAGS) -I $(CCANDIR) $(EXTERNAL_INCLUDE_FLAGS) -I . -I/usr/local/include $(FEATURES) $(COVFLAGS) $(DEV_CFLAGS) -DSHACHAIN_BITS=48 -DJSMN_PARENT_LINKS $(PIE_CFLAGS) $(COMPAT_CFLAGS)

 # We can get configurator to run a different compile cmd to cross-configure.
diff --git a/configure b/configure
index c968d744..68f8ef57 100755
--- a/configure
+++ b/configure
@@ -7,7 +7,7 @@ set -e
 PREFIX=${PREFIX:-/usr/local}
 CC=${CC:-cc}
 CWARNFLAGS=${CWARNFLAGS:--Werror -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition}
-CDEBUGFLAGS=${CDEBUGFLAGS:--std=gnu11 -g -fstack-protector}
+CDEBUGFLAGS=${CDEBUGFLAGS:--std=gnu11 -g -fstack-protector -fsanitize=address}
 DEVELOPER=${DEVELOPER:-0}
 EXPERIMENTAL_FEATURES=${EXPERIMENTAL_FEATURES:-0}
 COMPAT=${COMPAT:-1}
$ CC="clang-7" ./configure
$ make
…
tools/headerversions.o: In function `main':
/.../lightning/tools/headerversions.c:52: undefined reference to `__asan_report_load8'

Is this a known issue?

[cdecker]

I tried getting asan to work a while ago, and got it to build. However it always ran into problems with the ccan/autodata module, which we rely on for JSON-RPC and some other things.

I'll see what if I still have a branch laying around that compiled with ASan.

[cdecker]

Ok, got it. You're simply missing the addition of -fsanitize=address to the LDFLAGS variable in the makefile (CDEBUGFLAGS is used when compiling only, not linking):

diff --git a/Makefile b/Makefile
index fdbeff25..354c890a 100644
--- a/Makefile
+++ b/Makefile
@@ -179,13 +179,13 @@ ALL_PROGRAMS =

 CPPFLAGS = -DBINTOPKGLIBEXECDIR='"'$(shell sh tools/rel.sh $(bindir) $(pkglibexecdir))'"'
 CWARNFLAGS := -Werror -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition
-CDEBUGFLAGS := -std=gnu11 -g -fstack-protector
+CDEBUGFLAGS := -std=gnu11 -g -fstack-protector -fsanitize=address
 CFLAGS = $(CPPFLAGS) $(CWARNFLAGS) $(CDEBUGFLAGS) -I $(CCANDIR) $(EXTERNAL_INCLUDE_FLAGS) -I . -I/usr/local/include $(FEATURES) $(COVFLAGS) $(DEV_CFLAGS) -DSHACHAIN_BITS=48 -DJSMN_PARENT_LINKS $(PIE_CFLAGS) $(COMPAT_CFLAGS)

 # We can get configurator to run a different compile cmd to cross-configure.
 CONFIGURATOR_CC := $(CC)

-LDFLAGS = $(PIE_LDFLAGS)
+LDFLAGS = $(PIE_LDFLAGS) -fsanitize=address
 ifeq ($(STATIC),1)
 LDLIBS = -L/usr/local/lib -Wl,-dn -lgmp -lsqlite3 -lz -Wl,-dy -lm -lpthread -ldl $(COVFLAGS)
 else

With this you should be able to see the issue I was talking about earlier about ccan/autodata being problematic:

$ lightningd/lightningd
=================================================================
==7335==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000000aedd08 at pc 0x000000553267 bp 0x7ffe88dbaec0 sp 0x7ffe88dbaeb8
READ of size 8 at 0x000000aedd08 thread T0
    #0 0x553266 in jsonrpc_new /home/cdecker/dev/lightning/lightningd/jsonrpc.c:798:32
    #1 0x55dd06 in new_lightningd /home/cdecker/dev/lightning/lightningd/lightningd.c:210:16
    #2 0x55b572 in main /home/cdecker/dev/lightning/lightningd/lightningd.c:644:7
    #3 0x7fedff204b96 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:310
    #4 0x41b859 in _start (/home/cdecker/dev/lightning/lightningd/lightningd+0x41b859)

0x000000aedd08 is located 56 bytes to the left of global variable 'autodata_json_command_182' defined in 'lightningd/connect_control.c:182:1' (0xaedd40) of size 8
0x000000aedd08 is located 0 bytes to the right of global variable 'autodata_json_command_532' defined in 'lightningd/chaintopology.c:532:1' (0xaedd00) of size 8
SUMMARY: AddressSanitizer: global-buffer-overflow /home/cdecker/dev/lightning/lightningd/jsonrpc.c:798:32 in jsonrpc_new
Shadow bytes around the buggy address:
  0x000080155b50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080155b60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080155b70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080155b80: 00 00 00 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9 f9
  0x000080155b90: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9
=>0x000080155ba0: 00[f9]f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9
  0x000080155bb0: 00 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9
  0x000080155bc0: 00 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9
  0x000080155bd0: 00 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9
  0x000080155be0: 00 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9
  0x000080155bf0: 00 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 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
==7335==ABORTING

There are ways to blacklist some parts of the code from being instrumented, however I could not figure out the syntax and how to pass these in correctly, and I got frustrated. Maybe you have more luck 😉

[cdecker]

One additional data point: it seems that this is accessing the data in a user defined section. The address sanitizer injects padding to detect access to uninitialized regions, which according to my understanding makes the usual assumption that elements in these sections are back to back invalid, hence the access to uninitialized memory.

More information in the bug I filed with asan: google/sanitizers#1028

[cdecker]

Directly after writing this I realized that this is for clang only, with gcc it should work fine. Switching to gcc I was able to introduce a use-after-free bug and have asan catch it. So we can indeed already use asan when building with gcc. Would still be good to also support clang + sanitizers eventually since gcc doesn't have the memory sanitizer.

Now running a speed comparison between asan and valgrind 😉

[practicalswift]

@cdecker Thanks! My guess is that ASan will be much faster :-)

[cdecker]

It is 😄 I had to tweak the tests a bit to make an apple to apple comparison (some tests reduce the number of iterations for valgrind), and I had to disable the leak check on exit (export ASAN_OPTIONS='leak_check_at_exit=false'), since we seem to have a leak on exit that we don't currently find.

Instrumentation	Runtime (pytest) [s]
valgrind	5970
asan	2226
None	2152

[cdecker]

Ok, cleaned things up a bit and submitted PR #2285 to get at least address sanitizer in, will continue working on the memory sanitizer.

We might want to add an additional test-target that exercises asan and if we notice that asan catches all of valgrinds detected errors, we might want to switch over completely.