Bug 33206 - Sanitizer CHECK failed: ((allocated_for_dlsym)) < ((kDlsymAllocPoolSize)) (1036, 1024)) with preload

[denis13]

Summary

There might be a situation when ASan initializing later than shared library which has malloc in static constructor.
(rtld doesn't provide the order of initiazation) In this case ASan doesn't initialize interceptors but already intercepting malloc.
If malloc is too big to be handled by static local pool ASan will die with error:
Sanitizer CHECK failed: libsanitizer/asan/asan_malloc_linux.cc:40 ((allocated_for_dlsym)) < ((kDlsymAllocPoolSize)) (1036, 1024)

[kcc]

This is a rather intrusive change and I am not convinced we need it in trunk -- nobody has needed it before so it might be some very rare corner case.
Please explain in more detail why this situation happens and why it can't be avoided.
Why the library is not asan-instrumented?
Are you using dynamic asan run-time or static?

Finally, such change will need a test.

[m.ostapenko]

Let me share more background.
The problem occurs in such rare cases when we have one instrumented library and uninstrumented binary with lots of dependencies from other (uninstrumented) shared libs. In this case we can use LD_PRELOAD trick to preload shared ASan runtime to uninstrumented binary, but since rtld actually doesn't specify order of constructors execution, we can end up with overflow in static alloc_memory_for_dlsym pool if some constructor has malloc with some big size and this constructor is called before __asan_init. The right solution would be just build the executable with ASan, but unfortunately sometimes developers can't do this.

[eugenis]

Are you saying that LD_PRELOAD-ed library constructors are run after some other library constructors, and that other library is not a dependency of the LD_PRELOAD-ed one? I've never seen that. Is that on linux/glibc?

[m.ostapenko]

In D33784#770392, @eugenis wrote:

Are you saying that LD_PRELOAD-ed library constructors are run after some other library constructors, and that other library is not a dependency of the LD_PRELOAD-ed one? I've never seen that. Is that on linux/glibc?

Yeah, exactly. Here is an example from my host box:

$ LD_PRELOAD=/home/max/install/master/lib64/libasan.so.4  LD_DEBUG=libs  vim
...
     14209:     calling init: /lib/x86_64-linux-gnu/libz.so.1
     14209:
     14209:
     14209:     calling init: /lib/x86_64-linux-gnu/libattr.so.1
     14209:
     14209:
     14209:     calling init: /lib/x86_64-linux-gnu/libpcre.so.3
     14209:
     14209:
     14209:     calling init: /home/max/install/master/lib/../lib64/libgcc_s.so.1
     14209:
     14209:
     14209:     calling init: /lib/x86_64-linux-gnu/libm.so.6
     14209:
     14209:
     14209:     calling init: /home/max/install/master/lib/../lib64/libstdc++.so.6
     14209:
     14209:     /home/max/install/master/lib64/libasan.so.4: error: symbol lookup error: undefined symbol: swift_demangle (fatal)
     14209:
     14209:     calling init: /lib/x86_64-linux-gnu/librt.so.1
     14209:
     14209:
     14209:     calling init: /lib/x86_64-linux-gnu/libdl.so.2
     14209:
     14209:
     14209:     calling init: /usr/lib/x86_64-linux-gnu/libpython2.7.so.1.0
     14209:
     14209:
     14209:     calling init: /usr/lib/x86_64-linux-gnu/libgpm.so.2
     14209:
     14209:
     14209:     calling init: /lib/x86_64-linux-gnu/libacl.so.1
     14209:
     14209:
     14209:     calling init: /lib/x86_64-linux-gnu/libselinux.so.1
     14209:
     14209:
     14209:     calling init: /lib/x86_64-linux-gnu/libtinfo.so.5
     14209:
     14209:
     14209:     calling init: /home/max/install/master/lib64/libasan.so.4

[eugenis]

Yeah... that's weird.
Can we simply do ENSURE_ASAN_INITED(); thing in the malloc interceptor?

[denis13]

I forgot to provide full explanation, sorry for that.

This issue happens with LD_PRELOAD on Linux (x86_64 and armv7l)

When we set LD_PRELOAD to libasan and trying to execute binary which was not build with asan, we got following error:

12007:	
12007:	calling init: /lib/x86_64-linux-gnu/libpthread.so.0
12007:	
12007:	
12007:	calling init: /lib/x86_64-linux-gnu/libc.so.6
12007:	
12007:	
12007:	calling init: /usr/lib/x86_64-linux-gnu/libgraphite2.so.3
12007:	
12007:	
12007:	calling init: /usr/lib/x86_64-linux-gnu/libXdmcp.so.6
12007:	
12007:	
12007:	calling init: /usr/lib/x86_64-linux-gnu/libXau.so.6
12007:	
12007:	
12007:	calling init: /usr/lib/x86_64-linux-gnu/libdatrie.so.1
12007:	
12007:	
12007:	calling init: /lib/x86_64-linux-gnu/libexpat.so.1
12007:	
12007:	
12007:	calling init: /lib/x86_64-linux-gnu/libpcre.so.3
12007:	
12007:	
12007:	calling init: /lib/x86_64-linux-gnu/libglib-2.0.so.0
12007:	
12007:	
12007:	calling init: /lib/x86_64-linux-gnu/libz.so.1
12007:	
12007:	
12007:	calling init: /lib/x86_64-linux-gnu/libm.so.6
12007:	
12007:	
12007:	calling init: /lib/x86_64-linux-gnu/libpng12.so.0
12007:	
12007:	
12007:	calling init: /usr/lib/x86_64-linux-gnu/libfreetype.so.6
12007:	
12007:	
12007:	calling init: /usr/lib/x86_64-linux-gnu/libharfbuzz.so.0
12007:	
12007:	
12007:	calling init: /lib/x86_64-linux-gnu/libresolv.so.2
12007:	
12007:	
12007:	calling init: /usr/lib/x86_64-linux-gnu/libxcb.so.1
12007:	
12007:	
12007:	calling init: /usr/lib/x86_64-linux-gnu/libxcb-render.so.0
12007:	
12007:	
12007:	calling init: /usr/lib/x86_64-linux-gnu/libxcb-shm.so.0
12007:	
12007:	
12007:	calling init: /usr/lib/x86_64-linux-gnu/libpixman-1.so.0
12007:	
12007: ==Sanitizer== CHECK failed: /home/denis/gcc-trunk/gcc/libsanitizer/asan/asan_malloc_linux.cc:40 ((allocated_for_dlsym)) < ((kDlsymAllocPoolSize)) (1036, 1024)

As far as i understood rtld does not provide the order in which shared libs gonna be initializing.
In this case we have faced the situation when asan initializing later than some shared libraries.
As we can see at the debug log above, crash happens when rtld calling static constructor in libpixman
and this constructor has malloc.

call:

static void __attribute__((constructor))
 pixman_constructor (void)
 {
     global_implementation = _pixman_choose_implementation ();
 }

call :_pixman_choose_implementation
call :_pixman_implementation_create_general
call: _pixman_implementation_create

pixman_implementation_t *
 _pixman_implementation_create (pixman_implementation_t *fallback,
                                const pixman_fast_path_t *fast_paths)
 {
    pixman_implementation_t *imp;
         
     assert (fast_paths);
     
     if ((imp = malloc (sizeof (pixman_implementation_t))))
     {
         pixman_implementation_t *d; 
 
         memset (imp, 0, sizeof *imp);
 
         imp->fallback = fallback;
         imp->fast_paths = fast_paths; 
         
         /* Make sure the whole fallback chain has the right toplevel */ 
         for (d = imp; d != NULL; d = d->fallback)
            d->toplevel = imp;
     }
        
    return imp;
 }

In this case we can just increase local pool size.
But should we make more flexible solution with dynamic allocation ?

Thanks.

[m.ostapenko]

In D33784#770431, @eugenis wrote:

Yeah... that's weird.
Can we simply do ENSURE_ASAN_INITED(); thing in the malloc interceptor?

I don't think simple ENSURE_ASAN_INITED(); in malloc will work because during initialization ASan calls malloc again and we'll have nested malloc calls. Perhaps we can add something like:

if (UNLIKELY(!asan_inited) && asan_init_is_running)
  return AllocateFromLocalPool(size);
else if (LIKELY(asan_inited))
  return asan_malloc(size);
else
  AsanInitFromRtl();
return asan_malloc(size);

?

[eugenis]

Yes, something like that should work.

if (UNLIKELY(asan_init_is_running))
  return AllocateFromLocalPool(size);
if (UNLIKELY(!asan_inited))
  AsanInitFromRtl();
return asan_malloc(size);

[denis13]

Updated regarding to Evgenii Stepanov and Maxim Ostapenko comments.

[eugenis]

lib/asan/asan_malloc_linux.cc

88 ↗

(On Diff #101385)

This is fixing another, unrelated bug - right? In the case of realloc() from the dlsym pool to the regular allocator we may read past the end of the pool.

Also, this does not fix the original problem - all reallocs are served from the pool until asan is initialized. Realloc should force asan initialization the same as other interceptors.

[m.ostapenko]

lib/asan/asan_malloc_linux.cc

88 ↗

(On Diff #101385)

This is fixing another, unrelated bug - right? In the case of realloc() from the dlsym pool to the regular allocator we may read past the end of the pool.

Right. Besides that, current code looks scruffy so why not refactor?

Also, this does not fix the original problem - all reallocs are served from the pool until asan is initialized. Realloc should force asan initialization the same as other interceptors.

Yeah, at least realloc(NULL, ...) behaves like malloc and needs to be handled accordingly.

[denis13]

lib/asan/asan_malloc_linux.cc

88 ↗

(On Diff #101385)

Also, this does not fix the original problem - all reallocs are served from the pool until asan is initialized. Realloc should force asan initialization the same as other interceptors.

Thanks, I got this point. In case first allocation was from local pool by malloc => all reallocs for that address will consume from local pool.

[denis13]

Updated realloc interceptor to trigger initialization of asan interceptors if ptr in the local pool.

[m.ostapenko]

Please add a testcase. For me, following commands work:
$ cat foo.c

#include <stdlib.h>

__attribute__((constructor)) void foo() {
  void *p = malloc(1 << 20);
  p = malloc(1 << 20);
}

$ cat bar.c

#include <stdlib.h>

__attribute__((constructor)) void bar() {
  void *p = malloc(1 << 20);
  p = malloc(1 << 20);
}

$ cat main.c

int main() {
  return 0;
}

$ clang -shared -fPIC bar.c -o libbar.so
$ clang -shared -fPIC foo.c -o libfoo.so ./libbar.so
$ clang main.c -o main ./libfoo.so
$ LD_PRELOAD=/usr/local/lib/clang/5.0.0/lib/linux/libclang_rt.asan-x86_64.so ./main

==13629==Sanitizer CHECK failed: /home/max/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:42 ((allocated_for_dlsym)) < ((kDlsymAllocPoolSize)) (131072, 1024)

[eugenis]

A crazy thought - what happens if we add DF_1_INITFIRST to the asan library?
-Wl,-z,initfirst

[m.ostapenko]

In D33784#774238, @eugenis wrote:

A crazy thought - what happens if we add DF_1_INITFIRST to the asan library?
-Wl,-z,initfirst

According to Jakub's comment (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56393#c9) dynamic linker only supports exactly one initfirst shared library, which it assumes is libpthread.so. So I'm afraid we can't use it.

[denis13]

Test was added

[eugenis]

lib/asan/asan_malloc_linux.cc

83 ↗

(On Diff #101776)

Please add a realloc test, too.
I'm concerned that GET_STACK_TRACE_MALLOC reads flags that are only set up in AsanInitFromRtl.

98 ↗

(On Diff #101776)

This is missing the case of realloc(0) while asan_init_is_running. I realize it does not happen in practice, but maybe handle it anyway for completeness.

[m.ostapenko]

lib/asan/asan_malloc_linux.cc

66 ↗

(On Diff #101776)

This code snippet seems to repeat in several places and looks pretty like ENSURE_ASAN_INITED(). Can we use ENSURE_ASAN_INITED() here and below?

[denis13]

Test for realloc was added.
Cover case when asan_init_is_running and realloc get call
with ptr which is not in local pool. (For example realloc (0))

Use ENSURE_ASAN_INITED()
instead
if (UNLIKELY(!asan_inited))

AsanInitFromRtl();

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

INTERCEPTOR(void, cfree, void *ptr) {
   GET_STACK_TRACE_FREE;
   if (UNLIKELY(IsInDlsymAllocPool(ptr)))
     return;
   asan_free(ptr, &stack, FROM_MALLOC);
 }
 #endif // SANITIZER_INTERCEPT_CFREE
 
 INTERCEPTOR(void*, malloc, uptr size) {
-  if (UNLIKELY(!asan_inited))
+  if (UNLIKELY(asan_init_is_running))
     // Hack: dlsym calls malloc before REAL(malloc) is retrieved from dlsym.
     return AllocateFromLocalPool(size);
+  ENSURE_ASAN_INITED();
   GET_STACK_TRACE_MALLOC;
   return asan_malloc(size, &stack);
 }
 
 INTERCEPTOR(void*, calloc, uptr nmemb, uptr size) {
-  if (UNLIKELY(!asan_inited))
+  if (UNLIKELY(asan_init_is_running))
     // Hack: dlsym calls calloc before REAL(calloc) is retrieved from dlsym.
     return AllocateFromLocalPool(nmemb * size);
+  ENSURE_ASAN_INITED();
   GET_STACK_TRACE_MALLOC;
   return asan_calloc(nmemb, size, &stack);
 }
 
 INTERCEPTOR(void*, realloc, void *ptr, uptr size) {
-  GET_STACK_TRACE_MALLOC;
   if (UNLIKELY(IsInDlsymAllocPool(ptr))) {
-    uptr offset = (uptr)ptr - (uptr)alloc_memory_for_dlsym;
-    uptr copy_size = Min(size, kDlsymAllocPoolSize - offset);
+    const uptr offset = (uptr)ptr - (uptr)alloc_memory_for_dlsym;
+    const uptr copy_size = Min(size, kDlsymAllocPoolSize - offset);
     void *new_ptr;
-    if (UNLIKELY(!asan_inited)) {
+    if (UNLIKELY(asan_init_is_running)) {
       new_ptr = AllocateFromLocalPool(size);
     } else {
-      copy_size = size;
-      new_ptr = asan_malloc(copy_size, &stack);
+      ENSURE_ASAN_INITED();
+      GET_STACK_TRACE_MALLOC;
+      new_ptr = asan_malloc(size, &stack);
     }
     internal_memcpy(new_ptr, ptr, copy_size);
     return new_ptr;
   }
+  if (UNLIKELY(asan_init_is_running))
+    return AllocateFromLocalPool(size);
+  ENSURE_ASAN_INITED();
+  GET_STACK_TRACE_MALLOC;
   return asan_realloc(ptr, size, &stack);
 }
 
 #if SANITIZER_INTERCEPT_MEMALIGN
 INTERCEPTOR(void*, memalign, uptr boundary, uptr size) {
   GET_STACK_TRACE_MALLOC;
   return asan_memalign(boundary, size, &stack, FROM_MALLOC);
 }

compiler-rt/trunk/test/asan/TestCases/Linux/asan_preload_test-3.cc

+// Regression test for PR33206
+//
+// RUN: %clang -DDYN=1 -DMALLOC=1 -fPIC -shared %s -o %t-dso1.so
+// RUN: %clang -DDYN=1 -DMALLOC=1 -fPIC -shared %s -o %t-dso2.so %t-dso1.so
+// RUN: %clang %s -o %t-1 %t-dso2.so
+// RUN: env LD_PRELOAD=%shared_libasan %run %t-1 2>&1 | FileCheck %s
+// RUN: %clang -DDYN=1 -DREALLOC=1 -fPIC -shared %s -o %t-dso3.so
+// RUN: %clang -DDYN=1 -DREALLOC=1 -fPIC -shared %s -o %t-dso4.so %t-dso3.so
+// RUN: %clang %s -o %t-2 %t-dso4.so
+// RUN: env LD_PRELOAD=%shared_libasan %run %t-2 2>&1 | FileCheck %s
+// REQUIRES: asan-dynamic-runtime
+
+#include <stdlib.h>
+#include <stdio.h>
+
+#ifdef DYN
+__attribute__((constructor)) void foo() {
+  void *p;
+#ifdef MALLOC
+  p = malloc(1 << 20);
+#endif
+#ifdef REALLOC
+  p = realloc (0, 1 << 20);
+#endif
+  free(p);
+}
+#else
+int main() {
+  // CHECK: Success
+  printf("Success\n");
+  return 0;
+}
+#endif