ASan: fix potential use-after-free in backtrace interceptor

[thurston]

Summary

Various ASan interceptors may corrupt memory if passed a
pointer to freed memory (https://github.com/google/sanitizers/issues/321).
This patch fixes the issue for the backtrace interceptor,
by calling REAL(backtrace) with a known-good scratch buffer,
and performing an addressability check on the user-provided
buffer prior to writing to it.

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

4408

why we can't check with COMMON_INTERCEPTOR_WRITE_RANGE before real and after?
before for asan, after for msan

4414

internal_memcpy?

[thurston]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

4408

That would be a stricter check than currently done, because it would have to performed with 'size' rather than 'res'.

For example, if the user calls backtrace with a buffer of 100 elements but a size parameter of 1000000, COMMON_INTERCEPTOR_WRITE_RANGE before REAL would complain because the buffer is not large enough.

However, if the REAL(backtrace) function only ends up returning 50 items, then the buffer is large enough in practice, and it doesn't really matter that the size parameter was too large. The current implementation only checks that the buffer was large enough for the actual number of items written by REAL(backtrace).

4414

Will do.

[thurston]

Use internal_memcpy as suggested by Vitaly

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

 #else
 #define INIT_PTHREAD_SIGMASK
 #endif
 
 #if SANITIZER_INTERCEPT_BACKTRACE
 INTERCEPTOR(int, backtrace, void **buffer, int size) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, backtrace, buffer, size);
-  // FIXME: under ASan the call below may write to freed memory and corrupt
-  // its metadata. See
-  // https://github.com/google/sanitizers/issues/321.
-  int res = REAL(backtrace)(buffer, size);
-  if (res && buffer)
+  // 'buffer' might be freed memory, hence it is unsafe to directly call
+  // REAL(backtrace)(buffer, size). Instead, we use our own known-good
+  // scratch buffer.
+  void **scratch = (void**)InternalAlloc(sizeof(void*) * size);
+  int res = REAL(backtrace)(scratch, size);
+  if (res && buffer) {
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buffer, res * sizeof(*buffer));
+    internal_memcpy(buffer, scratch, res * sizeof(*buffer));
+  }
+  InternalFree(scratch);
   return res;
 }
 
 INTERCEPTOR(char **, backtrace_symbols, void **buffer, int size) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, backtrace_symbols, buffer, size);
   if (buffer && size)
     COMMON_INTERCEPTOR_READ_RANGE(ctx, buffer, size * sizeof(*buffer));

compiler-rt/test/asan/TestCases/backtrace_interceptor.cpp

 // RUN: %clangxx_asan -O0 %s -o %t && not %run %t 2>&1 | FileCheck %s
 
 // Windows does not have execinfo.h. For now, be conservative and
 // restrict the test to glibc.
 // REQUIRES: glibc-2.27
 
-// Interceptor can cause use-after-free
-// (https://github.com/google/sanitizers/issues/321)
-// XFAIL: *
-
 // Test the backtrace() interceptor.
 
 #include <assert.h>
 #include <execinfo.h>
 #include <math.h>
 #include <stdio.h>
 #include <stdlib.h>
 
 #define MAX_BT 100
 
 int main() {
   void **buffer = (void **)malloc(sizeof(void *) * MAX_BT);
   assert(buffer != NULL);
   free(buffer);
 
+  // Deliberate use-after-free of 'buffer'. We expect ASan to
+  // catch this, without triggering internal sanitizer errors.
   int numEntries = backtrace(buffer, MAX_BT);
   printf("backtrace returned %d entries\n", numEntries);
 
   // CHECK: use-after-free
   // CHECK: SUMMARY
   return 0;
 }