From f09677388a44cd1460f8986ef1b096c73bd5b958 Mon Sep 17 00:00:00 2001 From: Andreas Krebbel Date: Tue, 26 Oct 2010 00:23:14 -0400 Subject: [PATCH] Fix concurrency problem between dl_open and dl_iterate_phdr --- ChangeLog | 16 +++++++++-- elf/dl-load.c | 28 ++++++++----------- elf/dl-object.c | 56 +++++++++++++++++++++----------------- elf/rtld.c | 9 +++++- string/Makefile | 2 +- sysdeps/generic/ldsodefs.h | 7 +++-- 6 files changed, 70 insertions(+), 48 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7b9f810b34..d867f13a86 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,11 +1,23 @@ -2010-10-25 Ulrich Drepper +2010-10-20 Andreas Krebbel + Ulrich Drepper + + * elf/dl-object.c (_dl_new_object): Don't append the new object to + the global list here. Move code to... + (_dl_add_to_namespace_list): ...here. New function. + * elf/rtld.c (dl_main): Invoke _dl_add_to_namespace_list. + * sysdeps/generic/ldsodefs.h (_dl_add_to_namespace_list): Declare. + * elf/dl-load.c (lose): Don't remove the element from the list. + (_dl_map_object_from_fd): Invoke _dl_add_to_namespace_list. + (_dl_map_object): Likewise. + +2010-10-25 Ulrich Drepper [BZ #12159] * sysdeps/x86_64/multiarch/strchr.S: Fix propagation of search byte into all bytes of SSE register. Patch by Richard Li . -2010-10-24 Ulrich Drepper +2010-10-24 Ulrich Drepper [BZ #12140] * malloc/malloc.c (_int_free): Fill correct number of bytes when diff --git a/elf/dl-load.c b/elf/dl-load.c index aa8738f016..4c14f08e55 100644 --- a/elf/dl-load.c +++ b/elf/dl-load.c @@ -798,22 +798,7 @@ lose (int code, int fd, const char *name, char *realname, struct link_map *l, /* The file might already be closed. */ if (fd != -1) (void) __close (fd); - if (l != NULL) - { - /* We modify the list of loaded objects. */ - __rtld_lock_lock_recursive (GL(dl_load_write_lock)); - /* Remove the stillborn object from the list and free it. */ - assert (l->l_next == NULL); - if (l->l_prev == NULL) - /* No other module loaded. This happens only in the static library, - or in rtld under --verify. */ - GL(dl_ns)[l->l_ns]._ns_loaded = NULL; - else - l->l_prev->l_next = NULL; - --GL(dl_ns)[l->l_ns]._ns_nloaded; - free (l); - __rtld_lock_unlock_recursive (GL(dl_load_write_lock)); - } + free (l); free (realname); if (r != NULL) @@ -898,6 +883,9 @@ _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp, never be unloaded. */ __close (fd); + /* Add the map for the mirrored object to the object list. */ + _dl_add_to_namespace_list (l, nsid); + return l; } #endif @@ -1492,6 +1480,9 @@ cannot enable executable stack as shared object requires"); add_name_to_object (l, ((const char *) D_PTR (l, l_info[DT_STRTAB]) + l->l_info[DT_SONAME]->d_un.d_val)); + /* Now that the object is fully initialized add it to the object list. */ + _dl_add_to_namespace_list (l, nsid); + #ifdef SHARED /* Auditing checkpoint: we have a new object. */ if (__builtin_expect (GLRO(dl_naudit) > 0, 0) @@ -2216,7 +2207,7 @@ _dl_map_object (struct link_map *loader, const char *name, have. */ static const Elf_Symndx dummy_bucket = STN_UNDEF; - /* Enter the new object in the list of loaded objects. */ + /* Allocate a new object map. */ if ((name_copy = local_strdup (name)) == NULL || (l = _dl_new_object (name_copy, name, type, loader, mode, nsid)) == NULL) @@ -2234,6 +2225,9 @@ _dl_map_object (struct link_map *loader, const char *name, l->l_nbuckets = 1; l->l_relocated = 1; + /* Enter the object in the object list. */ + _dl_add_to_namespace_list (l, nsid); + return l; } else if (found_other_class) diff --git a/elf/dl-object.c b/elf/dl-object.c index 22a163560b..5d15ce1869 100644 --- a/elf/dl-object.c +++ b/elf/dl-object.c @@ -1,5 +1,5 @@ /* Storage management for the chain of loaded shared objects. - Copyright (C) 1995-2002,2004,2006-2008,2009 Free Software Foundation, Inc. + Copyright (C) 1995-2002,2004,2006-2009,2010 Free Software Foundation, Inc. This file is part of the GNU C Library. The GNU C Library is free software; you can redistribute it and/or @@ -26,16 +26,41 @@ #include +/* Add the new link_map NEW to the end of the namespace list. */ +void +internal_function +_dl_add_to_namespace_list (struct link_map *new, Lmid_t nsid) +{ + /* We modify the list of loaded objects. */ + __rtld_lock_lock_recursive (GL(dl_load_write_lock)); + + if (GL(dl_ns)[nsid]._ns_loaded != NULL) + { + struct link_map *l = GL(dl_ns)[nsid]._ns_loaded; + while (l->l_next != NULL) + l = l->l_next; + new->l_prev = l; + /* new->l_next = NULL; Would be necessary but we use calloc. */ + l->l_next = new; + } + else + GL(dl_ns)[nsid]._ns_loaded = new; + ++GL(dl_ns)[nsid]._ns_nloaded; + new->l_serial = GL(dl_load_adds); + ++GL(dl_load_adds); + + __rtld_lock_unlock_recursive (GL(dl_load_write_lock)); +} + + /* Allocate a `struct link_map' for a new object being loaded, and enter it into the _dl_loaded list. */ - struct link_map * internal_function _dl_new_object (char *realname, const char *libname, int type, struct link_map *loader, int mode, Lmid_t nsid) { struct link_map *l; - int idx; size_t libname_len = strlen (libname) + 1; struct link_map *new; struct libname_list *newname; @@ -93,31 +118,12 @@ _dl_new_object (char *realname, const char *libname, int type, new->l_scope = new->l_scope_mem; new->l_scope_max = sizeof (new->l_scope_mem) / sizeof (new->l_scope_mem[0]); - /* We modify the list of loaded objects. */ - __rtld_lock_lock_recursive (GL(dl_load_write_lock)); - /* Counter for the scopes we have to handle. */ - idx = 0; + int idx = 0; if (GL(dl_ns)[nsid]._ns_loaded != NULL) - { - l = GL(dl_ns)[nsid]._ns_loaded; - while (l->l_next != NULL) - l = l->l_next; - new->l_prev = l; - /* new->l_next = NULL; Would be necessary but we use calloc. */ - l->l_next = new; - - /* Add the global scope. */ - new->l_scope[idx++] = &GL(dl_ns)[nsid]._ns_loaded->l_searchlist; - } - else - GL(dl_ns)[nsid]._ns_loaded = new; - ++GL(dl_ns)[nsid]._ns_nloaded; - new->l_serial = GL(dl_load_adds); - ++GL(dl_load_adds); - - __rtld_lock_unlock_recursive (GL(dl_load_write_lock)); + /* Add the global scope. */ + new->l_scope[idx++] = &GL(dl_ns)[nsid]._ns_loaded->l_searchlist; /* If we have no loader the new object acts as it. */ if (loader == NULL) diff --git a/elf/rtld.c b/elf/rtld.c index 06b534a559..23b3462490 100644 --- a/elf/rtld.c +++ b/elf/rtld.c @@ -1113,6 +1113,10 @@ of this helper program; chances are you did not intend to run this program.\n\ main_map->l_phnum = phnum; main_map->l_entry = *user_entry; + /* Even though the link map is not yet fully initialized we can add + it to the map list since there are no possible users running yet. */ + _dl_add_to_namespace_list (main_map, LM_ID_BASE); + /* At this point we are in a bit of trouble. We would have to fill in the values for l_dev and l_ino. But in general we do not know where the file is. We also do not handle AT_EXECFD @@ -1255,7 +1259,7 @@ of this helper program; chances are you did not intend to run this program.\n\ /* We were invoked directly, so the program might not have a PT_INTERP. */ _dl_rtld_libname.name = GL(dl_rtld_map).l_name; - /* _dl_rtld_libname.next = NULL; Already zero. */ + /* _dl_rtld_libname.next = NULL; Already zero. */ GL(dl_rtld_map).l_libname = &_dl_rtld_libname; } else @@ -1380,6 +1384,9 @@ of this helper program; chances are you did not intend to run this program.\n\ l->l_libname->name = memcpy (copy, dsoname, len); } + /* Add the vDSO to the object list. */ + _dl_add_to_namespace_list (l, LM_ID_BASE); + /* Rearrange the list so this DSO appears after rtld_map. */ assert (l->l_next == NULL); assert (l->l_prev == main_map); diff --git a/string/Makefile b/string/Makefile index 19130dd63a..f836f598bc 100644 --- a/string/Makefile +++ b/string/Makefile @@ -56,7 +56,7 @@ tests := tester inl-tester noinl-tester testcopy test-ffs \ tst-strtok tst-strxfrm bug-strcoll1 tst-strfry \ bug-strtok1 $(addprefix test-,$(strop-tests)) \ bug-envz1 tst-strxfrm2 tst-endian tst-svc2 \ - bug-strstr1 + bug-strstr1 bug-strchr1 distribute := memcopy.h pagecopy.h tst-svc.expect test-string.h \ str-two-way.h diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h index fa4b6b284e..d0405903c1 100644 --- a/sysdeps/generic/ldsodefs.h +++ b/sysdeps/generic/ldsodefs.h @@ -891,8 +891,11 @@ extern lookup_t _dl_lookup_symbol_x (const char *undef, extern ElfW(Addr) _dl_symbol_value (struct link_map *map, const char *name) internal_function; -/* Allocate a `struct link_map' for a new object being loaded, - and enter it into the _dl_main_map list. */ +/* Add the new link_map NEW to the end of the namespace list. */ +extern void _dl_add_to_namespace_list (struct link_map *new, Lmid_t nsid) + internal_function attribute_hidden; + +/* Allocate a `struct link_map' for a new object being loaded. */ extern struct link_map *_dl_new_object (char *realname, const char *libname, int type, struct link_map *loader, int mode, Lmid_t nsid)