search

staticanalysis / address-sanitizer

Automatically exported from code.google.com/p/address-sanitizer
0 stars 0 forks source link

CHECK failed: ... "((trace)) != (0)" (0x0, 0x0) #321

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago 
$ bin/clang++ ../projects/compiler-rt/test/asan/TestCases/time_interceptor.cc 
-fsanitize=address
 ./a.out 
=================================================================
==27297==ERROR: AddressSanitizer: heap-use-after-free on address 0x60200000eff0 
at pc 0x43cb36 bp 0x7fff3688ed10 sp 0x7fff3688e4d0
WRITE of size 8 at 0x60200000eff0 thread T0
    #0 0x43cb35 in __interceptor_time.part.54 /usr/local/google/ssd/llvm/projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:544
    #1 0x4b34c5 in main (/usr/local/google/ssd/llvm/llvm_cmake_build/a.out+0x4b34c5)
    #2 0x7fb9d32d976c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226
    #3 0x4b325c in _start (/usr/local/google/ssd/llvm/llvm_cmake_build/a.out+0x4b325c)

0x60200000eff0 is located 0 bytes inside of 8-byte region 
[0x60200000eff0,0x60200000eff8)
freed by thread T0 here:
==27297==AddressSanitizer CHECK failed: 
/usr/local/google/ssd/llvm/projects/compiler-rt/lib/asan/asan_allocator2.cc:205 
"((trace)) != (0)" (0x0, 0x0)
    #0 0x49b98e in __asan::AsanCheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) /usr/local/google/ssd/llvm/projects/compiler-rt/lib/asan/asan_rtl.cc:70
    #1 0x49ff23 in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) /usr/local/google/ssd/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_common.cc:76
    #2 0x41c6ab in GetStackTraceFromId /usr/local/google/ssd/llvm/projects/compiler-rt/lib/asan/asan_allocator2.cc:205
    #3 0x41c6ab in __asan::AsanChunkView::GetFreeStack(__sanitizer::StackTrace*) /usr/local/google/ssd/llvm/projects/compiler-rt/lib/asan/asan_allocator2.cc:214
    #4 0x4989bf in __asan::DescribeHeapAddress(unsigned long, unsigned long) /usr/local/google/ssd/llvm/projects/compiler-rt/lib/asan/asan_report.cc:462
    #5 0x49a617 in __asan_report_error /usr/local/google/ssd/llvm/projects/compiler-rt/lib/asan/asan_report.cc:888
    #6 0x43cb56 in __interceptor_time.part.54 /usr/local/google/ssd/llvm/projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:544
    #7 0x4b34c5 in main (/usr/local/google/ssd/llvm/llvm_cmake_build/a.out+0x4b34c5)
    #8 0x7fb9d32d976c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226
    #9 0x4b325c in _start (/usr/local/google/ssd/llvm/llvm_cmake_build/a.out+0x4b325c)

Original issue reported on code.google.com by ramosian.glider@gmail.com on 17 Jun 2014 at 3:14

GoogleCodeExporter commented 9 years ago 
This happens because a call to REAL(time) overwrites the stack ID in the freed 
memory region:

 539 INTERCEPTOR(unsigned long, time, unsigned long *t) {
 540   void *ctx;
 541   COMMON_INTERCEPTOR_ENTER(ctx, time, t);
 542   unsigned long res = REAL(time)(t);
 543   if (t && res != (unsigned long)-1) {
 544     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, t, sizeof(*t));
 545   }
 546   return res;
 547 }

There is a plenty of other places in sanitizer_common_interceptors.inc where an 
interceptor calls the real functions before checking the pointers for being 
addressable.
Is it possible to store the stack ID somewhere else?

If not, we must fix all the interceptors that may write to heap-allocated 
memory:
1. Upon INTERCEPTOR(fname) entry, copy the chunk header of every pointer that 
fname() can potentially overwrite (most of the time there's only one such 
pointer);
2. Call REAL(fname)(ptr)
3. Check the return value of the function to determine which pointers were 
overwritten.
4. For each such pointer check whether it was addressable. If it was not, 
restore the chunk header and bail out.

It's still theoretically possible that the memory passed to INTERCEPTOR(fname) 
is unmapped already (although ASan allocator doesn't do that), and we get a 
SEGV before knowing that it's unaddressable.

Original comment by ramosian.glider@gmail.com on 17 Jun 2014 at 4:31

GoogleCodeExporter commented 9 years ago 
Another important conclusion is that our tests must be less tolerant to CHECK 
failures. I'd suggest somehow inserting a 'CHECK-NOT: CHECK failed' into each 
lit test by default.

Original comment by ramosian.glider@gmail.com on 17 Jun 2014 at 4:34

GoogleCodeExporter commented 9 years ago 
Re: the CHECK-failed problems - we can make ASan optionally call _exit(0) on 
CHECK-failure, instead of _exit(1).
Then "not /path/to/binary" in lit-tests will fail on CHECK-failure, and we will 
notice the error.

Original comment by samso...@google.com on 17 Jun 2014 at 4:46

GoogleCodeExporter commented 9 years ago 
The first solution sounds cleaner...

Original comment by tetra2...@gmail.com on 17 Jun 2014 at 4:57

GoogleCodeExporter commented 9 years ago 
Why not exit with a signal code on CHECK failure?  The 'not' utility will 
return non-zero if the subprocess exits due to a signal instead of a normal 
non-zero exit code.  This makes it so that mainline LLVM tests using not fail 
properly when an assertion fails.  Calling abort() is probably inappropriate in 
the sanitizer, but maybe some other approach is OK.

Original comment by rnk@google.com on 17 Jun 2014 at 6:32

GoogleCodeExporter commented 9 years ago 
>> Another important conclusion is that our tests must be less tolerant to 
CHECK failures
Agree. I would add a check for SUMMARY

Original comment by konstant...@gmail.com on 17 Jun 2014 at 6:45

GoogleCodeExporter commented 9 years ago 
>> Is it possible to store the stack ID somewhere else?
We can use allocator's metadata (currently, not used in asan at all), 
but it will consume extra RAM == bad. 

>> 1. Upon INTERCEPTOR(fname) entry, copy the chunk header of every pointer 
that fname() can potentially overwrite

Too expensive. 
You will have to check if this is a heap (what if not?)
then if it is small heap or larger, then get the header.

Original comment by konstant...@gmail.com on 17 Jun 2014 at 6:48

GoogleCodeExporter commented 9 years ago 
I would fix these issues lazily one-by-one for each interceptor. 
This one, e.g. like this: 

 INTERCEPTOR(unsigned long, time, unsigned long *t) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, time, t);
   long tmp;
   unsigned long res = REAL(time)(&tmp);
   if (t && res != (unsigned long)-1) {
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, t, sizeof(*t));
   }
   if (t)
      *t = tmp;
   return res;
 }

I admit this may not be possible for all the cases.

Original comment by konstant...@gmail.com on 17 Jun 2014 at 6:54

GoogleCodeExporter commented 9 years ago 
Per an offline discussion we've decided to act in a lazy fashion.
I've fixed the interceptors for time() and frexp() in r211153 and added 
comments to other interceptors that may potentially corrupt the stack IDs.
Kostya talked me out of raising a signal on CHECK(), because this is subtle and 
may break someone somewhere.

Original comment by ramosian.glider@gmail.com on 18 Jun 2014 at 9:38