Bug 15199 - dlopening a load-time library from an earlier library's initializer corrupts TLS state
Summary: dlopening a load-time library from an earlier library's initializer corrupts ...
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: dynamic-link (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: 2.22
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-25 22:13 UTC by Andy Lutomirski
Modified: 2017-03-21 19:23 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Lutomirski 2013-02-25 22:13:57 UTC
I don't have a simple testcase -- I'm triggering this by LD_PRELOADING libtcmalloc, but I don't think this issue has anything to do with libtcmalloc.  It's very sensitive to the exact order of libraries I'm using, though.

My libtcmalloc is linked in at load time, and it ends up with modid 1.  I have only one thread (i.e. the main thread).  In _dl_allocate_tls_init, dtv[1] is set to a freshly allocated block of zeros.  (libtcmalloc's TLS section is all BSS.)  libtcmalloc's l_tls_offset is 32.

Now I do something strange: one of my libraries dlopens a (preloaded) library that has a DT_NEEDED reference to libtcmalloc.  This results in:

#0  _dl_add_to_slotinfo (l=l@entry=0x7ffff7ffd728) at dl-tls.c:885
#1  0x00000034e3c137fc in dl_open_worker (a=a@entry=0x7fffffffdd90) at dl-open.c:504
#2  0x00000034e3c0edc6 in _dl_catch_error (objname=objname@entry=0x7fffffffdd80, errstring=errstring@entry=0x7fffffffdd88, 
    mallocedp=mallocedp@entry=0x7fffffffdd70, operate=operate@entry=0x34e3c12fb0 <dl_open_worker>, 
    args=args@entry=0x7fffffffdd90) at dl-error.c:177
#3  0x00000034e3c12c0c in _dl_open (file=0x7ffff5c06f80 "libamamalloc.so", mode=-2147483643, caller_dlopen=<optimized out>, 
    nsid=-2, argc=5, argv=0x7fffffffe078, env=0x7fffffffe0a8) at dl-open.c:653
#4  0x00000034e4c01026 in dlopen_doit (a=a@entry=0x7fffffffdfa0) at dlopen.c:66
#5  0x00000034e3c0edc6 in _dl_catch_error (objname=0xa3de10, errstring=0xa3de18, mallocedp=0xa3de08, 
    operate=0x34e4c00fc0 <dlopen_doit>, args=0x7fffffffdfa0) at dl-error.c:177
#6  0x00000034e4c0163c in _dlerror_run (operate=operate@entry=0x34e4c00fc0 <dlopen_doit>, args=args@entry=0x7fffffffdfa0)
    at dlerror.c:163
#7  0x00000034e4c010c1 in __dlopen (file=<optimized out>, mode=<optimized out>) at dlopen.c:87

_dl_add_to_slotinfo is called on libtcmalloc and increments dl_tls_dtv_slotinfo_list->slotinfo[1].gen to 2.  I suspect that the fact that _dl_add_to_slotinfo was called at all here is the actual bug.

Later on, after initializing a bunch of things, I try to allocate memory.  The code ends up here:

#0  _dl_update_slotinfo (req_modid=1) at dl-tls.c:672
#1  0x00000034e3c01264 in update_get_addr (ti=0x7ffff7fd4af0) at dl-tls.c:750
#2  0x00007ffff7dc0411 in GetStackTrace (result=0x94c118, max_depth=30, skip_count=3) at src/stacktrace_libunwind-inl.h:85
#3  0x00007ffff7db3869 in RecordGrowth (growth=1048576) at src/page_heap.cc:463
#4  tcmalloc::PageHeap::GrowHeap (this=this@entry=0x98c000, n=n@entry=8) at src/page_heap.cc:489
#5  0x00007ffff7db3b6b in tcmalloc::PageHeap::New (this=0x98c000, n=8) at src/page_heap.cc:120
#6  0x00007ffff7db25c9 in tcmalloc::CentralFreeList::Populate (this=0x7ffff7fee7a0 <tcmalloc::Static::central_cache_+74176>)
    at src/central_freelist.cc:318
#7  0x00007ffff7db27c8 in tcmalloc::CentralFreeList::FetchFromSpansSafe (
    this=this@entry=0x7ffff7fee7a0 <tcmalloc::Static::central_cache_+74176>) at src/central_freelist.cc:285
#8  0x00007ffff7db2858 in tcmalloc::CentralFreeList::RemoveRange (
    this=0x7ffff7fee7a0 <tcmalloc::Static::central_cache_+74176>, start=0x7ffffffae280, end=0x7ffffffae288, N=1)
    at src/central_freelist.cc:263
#9  0x00007ffff7db58e1 in tcmalloc::ThreadCache::FetchFromCentralCache (this=0xa170c0, cl=<optimized out>, byte_size=65536)
    at src/thread_cache.cc:160
#10 0x00007ffff7dc515b in Allocate (cl=<optimized out>, size=65536, this=0xa170c0) at src/thread_cache.h:364
#11 do_malloc_small (size=65536, heap=0xa170c0) at src/tcmalloc.cc:1088
#12 do_malloc_no_errno (size=65536) at src/tcmalloc.cc:1095
#13 cpp_alloc (nothrow=false, size=65536) at src/tcmalloc.cc:1423
#14 tc_new (size=65536) at src/tcmalloc.cc:1601
#15 0x00007ffff6527ca5 in allocate (this=0xa3e520, __n=<optimized out>)
    at /usr/lib/gcc/x86_64-redhat-linux/4.7.2/../../../../i

This is *not* a recursive allocation or glibc invocation; it's a call to tc_new, which is overriding the standard operator new, from STL.

_dl_update_slotinfo ends up here:

	      /* If there is currently memory allocate for this
		 dtv entry free it.  */
	      /* XXX Ideally we will at some point create a memory
		 pool.  */
	      if (! dtv[modid].pointer.is_static
		  && dtv[modid].pointer.val != TLS_DTV_UNALLOCATED)
		/* Note that free is called for NULL is well.  We
		   deallocate even if it is this dtv entry we are
		   supposed to load.  The reason is that we call
		   memalign and not malloc.  */
		free (dtv[modid].pointer.val);

	      /* This module is loaded dynamically- We defer memory
		 allocation.  */
	      dtv[modid].pointer.is_static = false;
	      dtv[modid].pointer.val = TLS_DTV_UNALLOCATED;

I assume that this code is intended to free memory from a previously dlclose'd module that has its modid reused, but this is *not* the case here.  libtcmalloc is not dynamic -- glibc should not be freeing its TLS space.  (Maybe there should be an assertion here that the slot is dynamic.)

At this point, dtv[0].counter is 1 and dl_tls_dtv_slotinfo_list->slotinfo[1] is 2, which is (I think) how we got here.

The eventual failure mode is a straightforward infinite loop in tls_get_addr_tail: l_tls_offset is not FORCED_DYNAMIC_TLS_OFFSET and dtv[1].pointer.val == TLS_DTV_UNALLOCATED, so it spins forever waiting for some nonexistent parallel dlopen call to do something.  (I don't understand this code path at all, but the infinite loop is straightforward.)

I understand if calling dlopen from a shared library initializer is verboten, but, if so, it should abort instead of corrupting internal TLS data structures.
Comment 1 Andy Lutomirski 2013-02-26 19:06:57 UTC
Here's a self-contained testcase, tested on Fedora 18.

--- begin a.c ---
#include <dlfcn.h>

void a(void) {}
extern void abort(void);

__attribute__((constructor)) static void init(void)
{
  write(1, "dlopen b\n", 9);
  if (!dlopen("libb.so", RTLD_LAZY | RTLD_NOLOAD)) /* This corrupts TLS state */
    abort();
  write(1, "dlopen done\n", 12);
}
--- end a.c ---

--- begin b.c ---
static __thread int tls;

void b()
{
  write(1, "Begin TLS access\n", 17);
  tls = 1;  /* This will infinite loop because TLS state is corrupt */
  write(1, "Done\n", 5);
}
--- end b.c ---

--- begin main.c ---
extern void a(void), b(void);

int main()
{
  a();  /* Just to DT_NEEDED it. */
  b();  /* This one will hang. */
}
--- end main.c ---

To trigger the bug, do this:

$ gcc -g -fPIC -shared -o liba.so a.c
$ gcc -g -fPIC -shared -o libb.so b.c
$ gcc -g -o main main.c libb.so liba.so -ldl
$ LD_LIBRARY_PATH=. ./main
dlopen b
dlopen done
Begin TLS access
   [this infinite loops]

Reversing the link order of libb.so and liba.so will cause this code to work.
Comment 2 Tulio Magno Quites Machado Filho 2017-03-21 19:23:41 UTC
This issue was fixed in glibc 2.22.
Cherry picking the following commit solves the deadlock from this testcase:

commit f8aeae347377f3dfa8cbadde057adf1827fb1d44
Author: Alexandre Oliva <aoliva@redhat.com>
Date:   Tue Mar 17 01:14:11 2015 -0300

    Fix DTV race, assert, DTV_SURPLUS Static TLS limit, and nptl_db garbage
    
    for  ChangeLog
    
        [BZ #17090]
        [BZ #17620]
        [BZ #17621]
        [BZ #17628]
        * NEWS: Update.
        * elf/dl-tls.c (_dl_update_slotinfo): Clean up outdated DTV
        entries with Static TLS too.  Skip entries past the end of the
        allocated DTV, from Alan Modra.
        (tls_get_addr_tail): Update to glibc_likely/unlikely.  Move
        Static TLS DTV entry set up from...
         (_dl_allocate_tls_init): ... here (fix modid assertion), ...
        * elf/dl-reloc.c (_dl_nothread_init_static_tls): ... here...
        * nptl/allocatestack.c (init_one_static_tls): ... and here...
        * elf/dlopen.c (dl_open_worker): Drop l_tls_modid upper bound
        for Static TLS.
        * elf/tlsdeschtab.h (map_generation): Return size_t.  Check
        that the slot we find is associated with the given map before
        using its generation count.
        * nptl_db/db_info.c: Include ldsodefs.h.
        (rtld_global, dtv_slotinfo_list, dtv_slotinfo): New typedefs.
        * nptl_db/structs.def (DB_RTLD_VARIABLE): New macro.
        (DB_MAIN_VARIABLE, DB_RTLD_GLOBAL_FIELD): Likewise.
        (link_map::l_tls_offset): New struct field.
        (dtv_t::counter): Likewise.
        (rtld_global): New struct.
        (_rtld_global): New rtld variable.
        (dl_tls_dtv_slotinfo_list): New rtld global field.
        (dtv_slotinfo_list): New struct.
        (dtv_slotinfo): Likewise.
        * nptl_db/td_symbol_list.c: Drop gnu/lib-names.h include.
        (td_lookup): Rename to...
        (td_mod_lookup): ... this.  Use new mod parameter instead of
        LIBPTHREAD_SO.
        * nptl_db/td_thr_tlsbase.c: Include link.h.
        (dtv_slotinfo_list, dtv_slotinfo): New functions.
        (td_thr_tlsbase): Check DTV generation.  Compute Static TLS
        addresses even if the DTV is out of date or missing them.
        * nptl_db/fetch-value.c (_td_locate_field): Do not refuse to
        index zero-length arrays.
        * nptl_db/thread_dbP.h: Include gnu/lib-names.h.
        (td_lookup): Make it a macro implemented in terms of...
        (td_mod_lookup): ... this declaration.
        * nptl_db/db-symbols.awk (DB_RTLD_VARIABLE): Override.
        (DB_MAIN_VARIABLE): Likewise.