[ASAN] Add interceptor for __longjmp_chk

[Lekensteyn]

Summary

glibc on Linux calls __longjmp_chk instead of longjmp (or _longjmp) when
_FORTIFY_SOURCE is defined. Ensure that an ASAN-instrumented program intercepts
this function when a system library calls it, otherwise the stack might remain
poisoned and result in CHECK failures and false positives.

Fixes https://github.com/google/sanitizers/issues/721

[vitalybuka]

LGTM

[eugenis]

lib/asan/asan_interceptors.cc

442 ↗

(On Diff #96328)

Why not REAL(__longjmp_chk) ?

lib/asan/asan_interceptors.h

61 ↗

(On Diff #96328)

&& !SANITIZER_ANDROID

test/asan/TestCases/Linux/longjmp_chk.c

8 ↗

(On Diff #96328)

Why do you split the test in two translation units?
If it is to prevent "msg" from being optimized out, a call to __asan_address_is_poisoned would have the same effect.

[Lekensteyn]

lib/asan/asan_interceptors.cc

442 ↗

(On Diff #96328)

I was concerned that this would result in linking issues since __longjmp_chk is not a standard symbol (it is specific to glib). Furthermore, if an external application somehow tries to use the __longjmp_chk symbol when available (while we don't have glibc as C library), then it would call into a nullptr.

It is possible that this concern is invalid though, what do you think?

test/asan/TestCases/Linux/longjmp_chk.c

8 ↗

(On Diff #96328)

The essence is that one application is built with ASAN and another library is built without ASAN, but with _FORTIFY_SOURCE. Only this combination is causing issues, if both are built with ASAN or if both are built without FS, then there is no problem.

[eugenis]

lib/asan/asan_interceptors.cc

442 ↗

(On Diff #96328)

I think it is still better to call __longjmp_chk to avoid weakening _FORTIFY_SOURCE. We already have quite a few dependencies like this, for example see the obstack interceptor in sanitizer_common - it is glibc-specific and can not be realistically implemented without REAL(obstack).

test/asan/TestCases/Linux/longjmp_chk.c

8 ↗

(On Diff #96328)

Why is there no problem when both are built with fortify? I see that asan disables fortify on darwin, but probably not on linux.

[Lekensteyn]

lib/asan/asan_interceptors.cc

442 ↗

(On Diff #96328)

Ok, using __longjmp_chk seems to work, and if the symbol does not exist in the libc it still seems to build (I tried adding an interceptor for a non-existing __longjmp_chk2). Will fix this in the next revision.

test/asan/TestCases/Linux/longjmp_chk.c

8 ↗

(On Diff #96328)

There can still be a problem even if both are built with fortify. Most important is that the non-ASAN code does a __longjmp_chk through the stack frame of the ASAN code.

That's why two builds are needed, one with ASAN and one without.

[eugenis]

test/asan/TestCases/Linux/longjmp_chk.c

8 ↗

(On Diff #96328)

Oh I get it, ASan instrumentation adds __asan_handle_no_return() before any call to longjmp, that's why the test needs to call longjmp from an uninstrumented code.

[Lekensteyn]

Changes:

• Clarified why "msg" is passed in "callback"
• Clarified why the "external library" is built without ASAN
• Added !SANITIZER_ANDROID as suggested

Can I commit it now?

[eugenis]

LGTM

compiler-rt/trunk/lib/asan/asan_interceptors.h

 #endif
 
 #if !SANITIZER_WINDOWS
 # define ASAN_INTERCEPT_SIGLONGJMP 1
 #else
 # define ASAN_INTERCEPT_SIGLONGJMP 0
 #endif
 
+#if SANITIZER_LINUX && !SANITIZER_ANDROID
+# define ASAN_INTERCEPT___LONGJMP_CHK 1
+#else
+# define ASAN_INTERCEPT___LONGJMP_CHK 0
+#endif
+
 // Android bug: https://code.google.com/p/android/issues/detail?id=61799
 #if ASAN_HAS_EXCEPTIONS && !SANITIZER_WINDOWS && \
     !(SANITIZER_ANDROID && defined(__i386))
 # define ASAN_INTERCEPT___CXA_THROW 1
 #else
 # define ASAN_INTERCEPT___CXA_THROW 0
 #endif
 

compiler-rt/trunk/lib/asan/asan_interceptors.cc

 
 #if ASAN_INTERCEPT__LONGJMP
 INTERCEPTOR(void, _longjmp, void *env, int val) {
   __asan_handle_no_return();
   REAL(_longjmp)(env, val);
 }
 #endif
 
+#if ASAN_INTERCEPT___LONGJMP_CHK
+INTERCEPTOR(void, __longjmp_chk, void *env, int val) {
+  __asan_handle_no_return();
+  REAL(__longjmp_chk)(env, val);
+}
+#endif
+
 #if ASAN_INTERCEPT_SIGLONGJMP
 INTERCEPTOR(void, siglongjmp, void *env, int val) {
   __asan_handle_no_return();
   REAL(siglongjmp)(env, val);
 }
 #endif
 
 #if ASAN_INTERCEPT___CXA_THROW
#endif
   ASAN_INTERCEPT_FUNC(signal);
 #endif
 #if ASAN_INTERCEPT_SWAPCONTEXT
   ASAN_INTERCEPT_FUNC(swapcontext);
 #endif
 #if ASAN_INTERCEPT__LONGJMP
   ASAN_INTERCEPT_FUNC(_longjmp);
 #endif
+#if ASAN_INTERCEPT___LONGJMP_CHK
+  ASAN_INTERCEPT_FUNC(__longjmp_chk);
+#endif
 #if ASAN_INTERCEPT_SIGLONGJMP
   ASAN_INTERCEPT_FUNC(siglongjmp);
 #endif
 
   // Intercept exception handling functions.
 #if ASAN_INTERCEPT___CXA_THROW
   ASAN_INTERCEPT_FUNC(__cxa_throw);
 #endif

compiler-rt/trunk/test/asan/TestCases/Linux/longjmp_chk.c

+// Verify that use of longjmp() in a _FORTIFY_SOURCE'd library (without ASAN)
+// is correctly intercepted such that the stack is unpoisoned.
+// Note: it is essential that the external library is not built with ASAN,
+// otherwise it would be able to unpoison the stack before use.
+//
+// RUN: %clang -DIS_LIBRARY -D_FORTIFY_SOURCE=2 -O2 %s -c -o %t.o
+// RUN: %clang_asan -O2 %s %t.o -o %t
+// RUN: %run %t
+
+#ifdef IS_LIBRARY
+/* the library */
+#include <setjmp.h>
+#include <assert.h>
+#include <sanitizer/asan_interface.h>
+
+static jmp_buf jenv;
+
+void external_callme(void (*callback)(void)) {
+  if (setjmp(jenv) == 0) {
+    callback();
+  }
+}
+
+void external_longjmp(char *msg) {
+  longjmp(jenv, 1);
+}
+
+void external_check_stack(void) {
+  char buf[256] = "";
+  for (int i = 0; i < 256; i++) {
+    assert(!__asan_address_is_poisoned(buf + i));
+  }
+}
+#else
+/* main program */
+extern void external_callme(void (*callback)(void));
+extern void external_longjmp(char *msg);
+extern void external_check_stack(void);
+
+static void callback(void) {
+  char msg[16];   /* Note: this triggers addition of a redzone. */
+  /* Note: msg is passed to prevent compiler optimization from removing it. */
+  external_longjmp(msg);
+}
+
+int main() {
+  external_callme(callback);
+  external_check_stack();
+  return 0;
+}
+#endif