[compiler-rt] bd1170d - ASan: fix potential use-after-free in backtrace interceptor

[Thurston Dang via llvm-commits]

Author: Thurston Dang
Date: 2023-05-13T23:03:14Z
New Revision: bd1170d2c371283447555bda6057f10e4cb0d25a

URL: https://github.com/llvm/llvm-project/commit/bd1170d2c371283447555bda6057f10e4cb0d25a
DIFF: https://github.com/llvm/llvm-project/commit/bd1170d2c371283447555bda6057f10e4cb0d25a.diff

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

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.

Differential Revision: https://reviews.llvm.org/D150496

Added: 
    

Modified: 
    compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
    compiler-rt/test/asan/TestCases/backtrace_interceptor.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
index 1c315a5183c6..bc31627ccca5 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
@@ -4404,12 +4404,16 @@ INTERCEPTOR(int, pthread_sigmask, int how, __sanitizer_sigset_t *set,
 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;
 }
 

diff  --git a/compiler-rt/test/asan/TestCases/backtrace_interceptor.cpp b/compiler-rt/test/asan/TestCases/backtrace_interceptor.cpp
index bd9da879fe4d..f1ce7d18c0b8 100644
--- a/compiler-rt/test/asan/TestCases/backtrace_interceptor.cpp
+++ b/compiler-rt/test/asan/TestCases/backtrace_interceptor.cpp
@@ -4,10 +4,6 @@
 // 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>
@@ -23,6 +19,8 @@ int main() {
   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);