New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deadlock when interposing `mmap` using `LD_PRELOAD` #329

Closed
KnairdA opened this Issue Feb 15, 2016 · 2 comments

Comments

Projects
None yet
3 participants
@KnairdA

KnairdA commented Feb 15, 2016

If mmap is interposed in an application using jemalloc 4.0.4 it deadlocks while loading the library.

Minimal example

#ifndef _GNU_SOURCE
#define _GNU_SOURCE
#endif

#include <dlfcn.h>
#include <stdlib.h>

void* mmap(void* addr, size_t length, int prot, int flags, int fd, off_t offset) {
    static void* (*actual_mmap)(void*, size_t, int, int, int, off_t) = NULL;

    if ( !actual_mmap ) {
        actual_mmap = dlsym(RTLD_NEXT, "mmap");
    }

    return actual_mmap(addr, length, prot, flags, fd, offset);
}

Compiled using gcc 5.3.0 running on Linux x86:

gcc -g -W -Wall -Wextra -ldl -fPIC -c example.c -o example.o
gcc -shared -Wl,-soname,libexample.so -o libexample.so example.o

Tested using:

LD_PRELOAD=./libexample.so nvim

This was also tested for various other applications with explicit jemalloc preloading.

Stacktrace

#0  0x00007fe788a62cfc in __lll_lock_wait () from /usr/lib/libpthread.so.0
#1  0x00007fe788a5cc6e in pthread_mutex_lock () from /usr/lib/libpthread.so.0
#2  0x00007fe787aea86c in malloc_init () from /usr/lib/libjemalloc.so.2
#3  0x00007fe787aeb945 in calloc () from /usr/lib/libjemalloc.so.2
#4  0x00007fe78711e697 in ?? () from /usr/lib/libdl.so.2
#5  0x00007fe78711e148 in dlsym () from /usr/lib/libdl.so.2
#6  0x00007fe788e946ed in mmap (addr=0x0, length=2097152, prot=3, flags=34, fd=-1, 
    offset=0) at example.c:12
#7  0x00007fe787b04629 in je_pages_map () from /usr/lib/libjemalloc.so.2
#8  0x00007fe787af7ced in je_chunk_alloc_mmap () from /usr/lib/libjemalloc.so.2
#9  0x00007fe787af7208 in je_chunk_alloc_base () from /usr/lib/libjemalloc.so.2
#10 0x00007fe787af5952 in je_base_alloc () from /usr/lib/libjemalloc.so.2
#11 0x00007fe787af50d6 in je_arena_boot () from /usr/lib/libjemalloc.so.2
#12 0x00007fe787aea54f in malloc_init_hard_a0_locked ()
   from /usr/lib/libjemalloc.so.2
#13 0x00007fe787aea8eb in malloc_init () from /usr/lib/libjemalloc.so.2
#14 0x00007fe7890a427a in call_init.part () from /lib64/ld-linux-x86-64.so.2
#15 0x00007fe7890a438b in _dl_init () from /lib64/ld-linux-x86-64.so.2
#16 0x00007fe789095dba in _dl_start_user () from /lib64/ld-linux-x86-64.so.2
#17 0x0000000000000001 in ?? ()
#18 0x00007fff234e46af in ?? ()
#19 0x0000000000000000 in ?? ()

If further information should be required I would be happy to provide it.

@jasone jasone added the notabug label Feb 16, 2016

@jasone

This comment has been minimized.

Show comment
Hide comment
@jasone

jasone Feb 16, 2016

Member

This is not a bug in jemalloc; the interposed mmap() implementation (because of dlsym()) is trying to recursively allocate.

Member

jasone commented Feb 16, 2016

This is not a bug in jemalloc; the interposed mmap() implementation (because of dlsym()) is trying to recursively allocate.

@jasone jasone closed this Feb 16, 2016

KnairdA added a commit to KnairdA/change that referenced this issue Feb 17, 2016

Implement static allocator for initialization
The previous interposition logic based on plain usage of `dlsym` analogously to various online examples led to a deadlock during _neovim_ startup. This deadlock was caused by _neovim_'s custom memory allocation library _jemalloc_ because it calls `mmap` during its initialization phase. The problem with calling `mmap` during initialization is that this already leads to executing `libChangeLog`'s `mmap` version whoes static `actual_mmap` function pointer is not initialized at this point in time. This is detected and leads to a call to `dlsym` to remedy this situation. Sadly `dlsym` in turn requires memory allocation using `calloc` which leads us back to initializing _jemalloc_ and as such to a deadlock.

I first saw this as a bug in _jemalloc_ which seemed to be confirmed by a short search in my search engine of choice. This prompted me to create an appropriate [bug report](jemalloc/jemalloc#329) which was dismissed as a problem in the way `mmap` was interposed and not as a bug in the library. Thus it seems to be accepted practice that it is not the responsibility of a custom memory allocator to cater to the initialization needs of other libraries relying on function interposition. This is of course a valid position as the whole issue is a kind of _chicken and egg_ problem where both sides can be argued.

To cut to the chase I was left with the only option of working around this deadlock by adapting `libChangeLog` to call `dlsym` without relying on the wrapped application's memory allocator of choice. The most straight forward way to do this is to provide another custom memory allocator alongside the _payload_ function interpositions of `mmap` and friends.

`init/alloc.cc` implements such a selectively transparent memory allocator that offers a small static buffer for usage in the context of executing `dlsym`.The choice between forwarding memory allocation requests to the wrapped application's allocator and using the static buffer is governed by `init::dlsymContext`. This tiny helper class maintains an `dlsym_level` counter by posing as a scope guard.

The end result of this extension to `libChangeLog` is that it now also works with applications using _jemalloc_ such as _neovim_ and should overall be much more robust during its initialization phase.
@leelamv

This comment has been minimized.

Show comment
Hide comment
@leelamv

leelamv Aug 30, 2016

I could workaround this problem by calling _dl_sym (libc private function) instead of dlsym().

So, why can't we use "dlsym" ?
libdl's dlsym() allocate memory for its internal purposes(actually, for dlerror support). Allocation using malloc/calloc interfaces can put us into deadlock when trying to resolve the libc functions.

Use of "_dl_sym" and other details:
a) "_dl_sym" is unpublished libc function which is functionally equivalent to libdl's dlsym.
b) Internally, dlsym calls libc's "_dl_sym" but with below benefits implemented in the libdl layer.
- Protection against concurrent loads and unloads.
- dlerror support
c) It has a third argument which is return address of the caller.
d) libraries using GLIBC_PRIVATE will run into rpm packaging issues if you don't relax those checks.

When is it okay to relax on the benefits and use _dl_sym.
a) Your library should be loaded as a LD_PRELOAD or linked as one of the first libraries.
- That is, your library should be loaded only when program is in single-threaded env.
b) These are known/published functions which are expected to be found.
- Remember, we don't have dlerror support

leelamv commented Aug 30, 2016

I could workaround this problem by calling _dl_sym (libc private function) instead of dlsym().

So, why can't we use "dlsym" ?
libdl's dlsym() allocate memory for its internal purposes(actually, for dlerror support). Allocation using malloc/calloc interfaces can put us into deadlock when trying to resolve the libc functions.

Use of "_dl_sym" and other details:
a) "_dl_sym" is unpublished libc function which is functionally equivalent to libdl's dlsym.
b) Internally, dlsym calls libc's "_dl_sym" but with below benefits implemented in the libdl layer.
- Protection against concurrent loads and unloads.
- dlerror support
c) It has a third argument which is return address of the caller.
d) libraries using GLIBC_PRIVATE will run into rpm packaging issues if you don't relax those checks.

When is it okay to relax on the benefits and use _dl_sym.
a) Your library should be loaded as a LD_PRELOAD or linked as one of the first libraries.
- That is, your library should be loaded only when program is in single-threaded env.
b) These are known/published functions which are expected to be found.
- Remember, we don't have dlerror support

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment