robot created this revision.
robot added a reviewer: cryptoad.
robot added a project: Sanitizers.
Herald added subscribers: Sanitizers, llvm-commits, hintonda, mgorny, 
kubamracek.

Fixes Bug 32434
See https://bugs.llvm.org/show_bug.cgi?id=32434

Short summary:
std::rethrow_exception does not use __cxa_throw to rethrow the exception, so if
it is called from uninstrumented code, it will leave the stack poisoned. This
can lead to false positives.

Long description:

For functions which don't return normally (e.g. via exceptions), asan needs to
unpoison the entire stack. It is not known before a call to such a function
where execution will continue, some function which don't contain cleanup code
like destructors might be skipped. After stack unwinding, execution might
continue in uninstrumented code.

If the stack has been poisoned before such a function is called, but the stack
is unwound during the unconventional return, then zombie redzones (entries) for
no longer existing stack variables can remain in the shadow memory. Normally,
this is avoided by asan generating a call to __asan_handle_no_return before all
functions marked as [[noreturn]]. This __asan_handle_no_return unpoisons the
entire stack. Since these [[noreturn]] functions can be called from
uninstrumented code, asan also introduces interceptor functions which call
__asan_handle_no_return before running the original [[noreturn]] function;
for example, __cxa_throw is intercepted.

If a [[noreturn]] function is called from uninstrumented code (so the stack is
left poisoned) and additionally, execution continues in uninstrumented code, new
stack variables might be introduced and overlap with the stack variables
which have been removed during stack unwinding. Since the redzones are not
cleared nor overwritten by uninstrumented code, they remain but now contain
invalid data.

Now, if the redzones are checked against the new stack variables, false
positive reports can occur. This can happen for example by the uninstrumented
code calling an intercepted function such as memcpy, or an instrumented
function.

Intercepting std::rethrow_exception directly is not easily possible since it
depends on the C++ standard library implementation (e.g. libcxx vs libstdc++)
and the mangled name it produces for this function. As a rather simple
workaround, we're intercepting _Unwind_RaiseException for libstdc++. For
libcxxabi, we can intercept the ABI function __cxa_rethrow_primary_exception.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D42644

Files:
  lib/asan/asan_interceptors.cc
  lib/asan/asan_interceptors.h
  lib/asan/tests/CMakeLists.txt
  lib/asan/tests/asan_rethrow_exception_test.cc

Index: lib/asan/tests/asan_rethrow_exception_test.cc
===================================================================
--- /dev/null
+++ lib/asan/tests/asan_rethrow_exception_test.cc
@@ -0,0 +1,94 @@
+#if ASAN_HAS_EXCEPTIONS
+
+
+#include "asan_test_utils.h"
+
+#include <cstring>
+#include <exception>
+
+
+namespace {
+
+
+// Not instrumented because std::rethrow_exception is a [[noreturn]] function, for which the
+// compiler would emit a call to __asan_handle_no_return which unpoisons the stack.
+// We emulate here some code not compiled with asan. This function is not [[noreturn]] because the
+// scenario we're emulating doesn't always throw. If it were [[noreturn]], the calling code would
+// emit a call to __asan_handle_no_return.
+void NOINLINE ATTRIBUTE_NO_SANITIZE_ADDRESS
+uninstrumented_rethrow_exception(std::exception_ptr const& exc_ptr)
+{
+  // if we just call rethrow_exception, the compiler knows this function is noreturn, even
+  // though it's not inlined
+  if(Ident(true))
+    std::rethrow_exception(exc_ptr);
+}
+
+// access stack, instrumented by asan
+// Asan checks pass if either the stack is entirely unpoisoned around dst and src, or redzones
+// exist but don't overlap with src nor dst.
+// We want this function to emit asan checks (checking the shadow memory). This works either by
+// calling std::memcpy which is intercepted by asan, or by generating instrumented code (if memcpy
+// is resolved via a builtin). To make sure it works in either case, we place the memcpy outside of
+// uninstrumented_fill_and_use_stack and into this separate function, which is instrumented.
+void NOINLINE
+instrumented_use_stack(void* dst, void const* src, int size)
+{
+  std::memcpy(dst, src, size);
+}
+
+// Create variables on the stack without touching the shadow memory, and use them to check if there
+// are redzones remaining (from prior stack operations) on the stack.
+void ATTRIBUTE_NO_SANITIZE_ADDRESS
+uninstrumented_fill_and_use_stack()
+{
+  // memcpy is intercepted by asan which performs checks on src and dst
+  using T = int[1000];
+  T x{};
+  T y{};
+  instrumented_use_stack(&x, &y, sizeof(T));
+}
+
+
+// Create redzones for stack variables in shadow memory and call std::rethrow_exception which
+// should unpoison the entire stack.
+void NOINLINE
+create_redzones_and_throw(std::exception_ptr const& exc_ptr)
+{
+  int i[100];
+  break_optimization(i); // force creation of redzones even though variable is unused
+
+  uninstrumented_rethrow_exception(exc_ptr);
+}
+
+
+} // namespace
+
+
+// Check that std::rethrow_exception is intercepted by asan and the interception unpoisons the
+// stack.
+// If std::rethrow_exception is NOT intercepted, then calls to this function from instrumented code
+// will still unpoison the stack because std::rethrow_exception is a [[noreturn]] function and any
+// [[noreturn]] function call will be instrumented with __asan_handle_no_return. However, calls to
+// std::rethrow_exception from UNinstrumented code will not unpoison the stack, so we need to
+// intercept std::rethrow_exception to unpoison the stack.
+TEST(AddressSanitizer, RethrowException) {
+  // In some implementations of std::make_exception_ptr, e.g. libstdc++ prior to gcc 7, this
+  // function calls __cxa_throw. The __cxa_throw is intercepted by asan to unpoison the entire
+  // stack; since this test essentially tests that the stack is unpoisoned by a call to
+  // std::rethrow_exception, we need to generate the exception_ptr BEFORE we have the local
+  // variables poison the stack.
+  std::exception_ptr my_exception_ptr = std::make_exception_ptr("up");
+
+  try
+  {
+    create_redzones_and_throw(my_exception_ptr);
+  }catch(char const*)
+  {
+    uninstrumented_fill_and_use_stack();
+    // ignore exception such that test is successful unless asan reports above
+  }
+}
+
+
+#endif // ASAN_HAS_EXCEPTIONS
Index: lib/asan/tests/CMakeLists.txt
===================================================================
--- lib/asan/tests/CMakeLists.txt
+++ lib/asan/tests/CMakeLists.txt
@@ -155,6 +155,7 @@
   asan_oob_test.cc
   asan_mem_test.cc
   asan_str_test.cc
+  asan_rethrow_exception_test.cc
   asan_test_main.cc)
 if(APPLE)
   list(APPEND ASAN_INST_TEST_SOURCES asan_mac_test.cc asan_mac_test_helpers.mm)
Index: lib/asan/asan_interceptors.h
===================================================================
--- lib/asan/asan_interceptors.h
+++ lib/asan/asan_interceptors.h
@@ -85,8 +85,17 @@
     !(SANITIZER_ANDROID && defined(__i386)) && \
     !SANITIZER_SOLARIS
 # define ASAN_INTERCEPT___CXA_THROW 1
+# define ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION 1
+# ifdef _GLIBCXX_SJLJ_EXCEPTIONS
+#  define ASAN_INTERCEPT__UNWIND_SJLJ_RAISEEXCEPTION 1
+# else
+#  define ASAN_INTERCEPT__UNWIND_RAISEEXCEPTION 1
+# endif
 #else
 # define ASAN_INTERCEPT___CXA_THROW 0
+# define ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION 0
+# define ASAN_INTERCEPT__UNWIND_RAISEEXCEPTION 0
+# define ASAN_INTERCEPT__UNWIND_SJLJ_RAISEEXCEPTION 0
 #endif
 
 #if !SANITIZER_WINDOWS
Index: lib/asan/asan_interceptors.cc
===================================================================
--- lib/asan/asan_interceptors.cc
+++ lib/asan/asan_interceptors.cc
@@ -33,6 +33,10 @@
 #include "sanitizer_common/sanitizer_posix.h"
 #endif
 
+#if ASAN_INTERCEPT__UNWIND_RAISEEXCEPTION || ASAN_INTERCEPT__SJLJ_UNWIND_RAISEEXCEPTION
+#include <unwind.h>
+#endif
+
 #if defined(__i386) && SANITIZER_LINUX
 #define ASAN_PTHREAD_CREATE_VERSION "GLIBC_2.1"
 #elif defined(__mips__) && SANITIZER_LINUX
@@ -318,6 +322,30 @@
 }
 #endif
 
+#if ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION
+INTERCEPTOR(void, __cxa_rethrow_primary_exception, void *a) {
+  CHECK(REAL(__cxa_throw));
+  __asan_handle_no_return();
+  REAL(__cxa_rethrow_primary_exception)(a);
+}
+#endif
+
+#if ASAN_INTERCEPT__UNWIND_RAISEEXCEPTION
+INTERCEPTOR(_Unwind_Reason_Code, _Unwind_RaiseException, struct _Unwind_Exception* object) {
+  CHECK(REAL(_Unwind_RaiseException));
+  __asan_handle_no_return();
+  return REAL(_Unwind_RaiseException)(object);
+}
+#endif
+
+#if ASAN_INTERCEPT__SJLJ_UNWIND_RAISEEXCEPTION
+INTERCEPTOR(_Unwind_Reason_Code, _Unwind_SjLj_RaiseException, struct _Unwind_Exception* object) {
+  CHECK(REAL(_Unwind_SjLj_RaiseException));
+  __asan_handle_no_return();
+  return REAL(_Unwind_SjLj_RaiseException)(object);
+}
+#endif
+
 #if ASAN_INTERCEPT_INDEX
 # if ASAN_USE_ALIAS_ATTRIBUTE_FOR_INDEX
 INTERCEPTOR(char*, index, const char *string, int c)
@@ -598,6 +626,17 @@
 #if ASAN_INTERCEPT___CXA_THROW
   ASAN_INTERCEPT_FUNC(__cxa_throw);
 #endif
+#if ASAN_INTERCEPT___CXA_RETHROW_PRIMARY_EXCEPTION
+  ASAN_INTERCEPT_FUNC(__cxa_rethrow_primary_exception);
+#endif
+  // Indirectly intercept std::rethrow_exception.
+#if ASAN_INTERCEPT__UNWIND_RAISEEXCEPTION
+  INTERCEPT_FUNCTION(_Unwind_RaiseException);
+#endif
+  // Indirectly intercept std::rethrow_exception.
+#if ASAN_INTERCEPT__UNWIND_SJLJ_RAISEEXCEPTION
+  INTERCEPT_FUNCTION(_Unwind_SjLj_RaiseException);
+#endif
 
   // Intercept threading-related functions
 #if ASAN_INTERCEPT_PTHREAD_CREATE
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to