Page MenuHomePhabricator

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

Authored by denis13 on Jun 1 2017, 9:09 AM.

Details

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)

Diff Detail

Repository
rL LLVM

Event Timeline

denis13 created this revision.Jun 1 2017, 9:09 AM
kcc edited edge metadata.Jun 1 2017, 10:44 AM

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 edited edge metadata.Jun 1 2017, 11:11 AM

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 edited edge metadata.Jun 1 2017, 11:18 AM

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?

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

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

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.

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);

?

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 this revision to Diff 101385.Jun 5 2017, 4:09 AM

Updated regarding to Evgenii Stepanov and Maxim Ostapenko comments.

eugenis added inline comments.Jun 5 2017, 1:30 PM
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 added inline comments.Jun 5 2017, 2:05 PM
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 added inline comments.Jun 6 2017, 4:15 AM
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 this revision to Diff 101539.Jun 6 2017, 4:18 AM

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

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)

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

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 updated this revision to Diff 101776.Jun 7 2017, 10:47 AM

Test was added

eugenis added inline comments.Jun 7 2017, 11:23 AM
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 added inline comments.Jun 7 2017, 11:45 AM
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 updated this revision to Diff 101867.Jun 8 2017, 1:11 AM

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();
eugenis accepted this revision.Jun 8 2017, 1:21 PM
This revision is now accepted and ready to land.Jun 8 2017, 1:21 PM
This revision was automatically updated to reflect the committed changes.