From owner-freebsd-bugs@FreeBSD.ORG Wed Apr 5 07:50:15 2006 Return-Path: X-Original-To: freebsd-bugs@hub.freebsd.org Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 3C81B16A422 for ; Wed, 5 Apr 2006 07:50:15 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 2C77843D53 for ; Wed, 5 Apr 2006 07:50:14 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.4/8.13.4) with ESMTP id k357oD7A099183 for ; Wed, 5 Apr 2006 07:50:13 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.4/8.13.4/Submit) id k357oDKF099182; Wed, 5 Apr 2006 07:50:13 GMT (envelope-from gnats) Resent-Date: Wed, 5 Apr 2006 07:50:13 GMT Resent-Message-Id: <200604050750.k357oDKF099182@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Kostik Belousov Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id BA11B16A423 for ; Wed, 5 Apr 2006 07:41:33 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (www.freebsd.org [216.136.204.117]) by mx1.FreeBSD.org (Postfix) with ESMTP id CBC4743D48 for ; Wed, 5 Apr 2006 07:41:32 +0000 (GMT) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (localhost [127.0.0.1]) by www.freebsd.org (8.13.1/8.13.1) with ESMTP id k357fW46024606 for ; Wed, 5 Apr 2006 07:41:32 GMT (envelope-from nobody@www.freebsd.org) Received: (from nobody@localhost) by www.freebsd.org (8.13.1/8.13.1/Submit) id k357fW6a024605; Wed, 5 Apr 2006 07:41:32 GMT (envelope-from nobody) Message-Id: <200604050741.k357fW6a024605@www.freebsd.org> Date: Wed, 5 Apr 2006 07:41:32 GMT From: Kostik Belousov To: freebsd-gnats-submit@FreeBSD.org X-Send-Pr-Version: www-2.3 Cc: Subject: bin/95339: [patch] rtld is thread-unsafe. fixes for dlopen mt behavior X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Apr 2006 07:50:15 -0000 >Number: 95339 >Category: bin >Synopsis: [patch] rtld is thread-unsafe. fixes for dlopen mt behavior >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Wed Apr 05 07:50:13 GMT 2006 >Closed-Date: >Last-Modified: >Originator: Kostik Belousov >Release: 7-CURRENT, 6-STABLE >Organization: - >Environment: - >Description: dlopen/dlclose rtld operations are not thread safe. Dynamic loader can be easily segfaulted or trapped into assertion with parallel invocations of rtld operations from multiple threads. Test case and fix for problems exposed by the test are attached. >How-To-Repeat: Use the following test by Kazuaki Oda to see errant behaviour: #include #include #include #include #include #include #define NTHREADS 10 void *func(void *dummy); int main(void) { pthread_t tids[NTHREADS]; int error; int i; for (i = 0; i < NTHREADS; i++) { error = pthread_create(&tids[i], NULL, func, NULL); if (error) errc(1, error, "pthread_create"); } for (;;) sleep(1); /* NOTREACHED */ exit(0); } void *func(void *dummy) { void *h; unsigned long c = 0; for (;;) { if ((h = dlopen("/usr/lib/libm.so", RTLD_NOW)) == NULL) { fprintf(stderr, "dlopen: %s\n", dlerror()); continue; } if (dlclose(h) == -1) fprintf(stderr, "dlclose: %s\n", dlerror()); if (c++ == 10000) { printf(".\n"); c = 0; } } /* NOTREACHED */ return (NULL); } >Fix: Index: libexec/rtld-elf/rtld.c =================================================================== RCS file: /usr/local/arch/ncvs/src/libexec/rtld-elf/rtld.c,v retrieving revision 1.112 diff -u -r1.112 rtld.c --- libexec/rtld-elf/rtld.c 24 Dec 2005 15:37:30 -0000 1.112 +++ libexec/rtld-elf/rtld.c 23 Mar 2006 10:44:27 -0000 @@ -105,7 +105,7 @@ static int load_preload_objects(void); static Obj_Entry *load_object(const char *, const Obj_Entry *); static Obj_Entry *obj_from_addr(const void *); -static void objlist_call_fini(Objlist *); +static void objlist_call_fini(Objlist *, int *lockstate, unsigned long *gen); static void objlist_call_init(Objlist *); static void objlist_clear(Objlist *); static Objlist_Entry *objlist_find(Objlist *, const Obj_Entry *); @@ -165,6 +165,7 @@ STAILQ_HEAD_INITIALIZER(list_main); static Objlist list_fini = /* Objects needing fini() calls */ STAILQ_HEAD_INITIALIZER(list_fini); +static unsigned long list_fini_gen = 0; static Elf_Sym sym_zero; /* For resolving undefined weak refs. */ @@ -1168,8 +1169,10 @@ objlist_push_tail(list, obj); /* Add the object to the global fini list in the reverse order. */ - if (obj->fini != (Elf_Addr)NULL) + if (obj->fini != (Elf_Addr)NULL) { objlist_push_head(&list_fini, obj); + list_fini_gen++; + } } #ifndef FPTR_TARGET @@ -1362,21 +1365,30 @@ * non-NULL fini functions. */ static void -objlist_call_fini(Objlist *list) +objlist_call_fini(Objlist *list, int *lockstate, unsigned long *gen) { - Objlist_Entry *elm; + Objlist_Entry *elm, *elm_tmp; char *saved_msg; + unsigned long saved_gen = *gen; /* * Preserve the current error message since a fini function might * call into the dynamic linker and overwrite it. */ saved_msg = errmsg_save(); - STAILQ_FOREACH(elm, list, link) { + again: + STAILQ_FOREACH_SAFE(elm, list, link, elm_tmp) { if (elm->obj->refcount == 0) { dbg("calling fini function for %s at %p", elm->obj->path, (void *)elm->obj->fini); + elm->obj->fini_now = true; + wlock_release(rtld_bind_lock, *lockstate); call_initfini_pointer(elm->obj, elm->obj->fini); + *lockstate = wlock_acquire(rtld_bind_lock); + if (*gen != saved_gen) { + saved_gen = *gen; + goto again; + } } } errmsg_restore(saved_msg); @@ -1390,7 +1402,7 @@ static void objlist_call_init(Objlist *list) { - Objlist_Entry *elm; + Objlist_Entry *elm, *elm_tmp; char *saved_msg; /* @@ -1398,7 +1410,7 @@ * call into the dynamic linker and overwrite it. */ saved_msg = errmsg_save(); - STAILQ_FOREACH(elm, list, link) { + STAILQ_FOREACH_SAFE(elm, list, link, elm_tmp) { dbg("calling init function for %s at %p", elm->obj->path, (void *)elm->obj->init); call_initfini_pointer(elm->obj, elm->obj->init); @@ -1563,15 +1575,18 @@ rtld_exit(void) { Obj_Entry *obj; + int lockstate; dbg("rtld_exit()"); /* Clear all the reference counts so the fini functions will be called. */ + lockstate = wlock_acquire(rtld_bind_lock); for (obj = obj_list; obj != NULL; obj = obj->next) obj->refcount = 0; - objlist_call_fini(&list_fini); + objlist_call_fini(&list_fini, &lockstate, &list_fini_gen); /* No need to remove the items from the list, since we are exiting. */ if (!libmap_disable) lm_fini(); + wlock_release(rtld_bind_lock, lockstate); } static void * @@ -1685,10 +1700,10 @@ * The object is no longer referenced, so we must unload it. * First, call the fini functions with no locks held. */ - wlock_release(rtld_bind_lock, lockstate); - objlist_call_fini(&list_fini); - lockstate = wlock_acquire(rtld_bind_lock); + objlist_call_fini(&list_fini, &lockstate, &list_fini_gen); + objlist_remove_unref(&list_fini); + list_fini_gen++; /* Finish cleaning up the newly-unreferenced objects. */ GDB_STATE(RT_DELETE,&root->linkmap); @@ -1741,9 +1756,9 @@ if (ld_tracing != NULL) environ = (char **)*get_program_var_addr("environ"); + lockstate = wlock_acquire(rtld_bind_lock); objlist_init(&initlist); - lockstate = wlock_acquire(rtld_bind_lock); GDB_STATE(RT_ADD,NULL); old_obj_tail = obj_tail; @@ -1755,7 +1770,10 @@ obj = load_object(name, obj_main); } - if (obj) { + if (obj && obj->fini_now) { + obj = NULL; + _rtld_error("%s is running finalizers now", name); + } else if (obj) { obj->dl_refcount++; if (mode & RTLD_GLOBAL && objlist_find(&list_global, obj) == NULL) objlist_push_tail(&list_global, obj); Index: libexec/rtld-elf/rtld.h =================================================================== RCS file: /usr/local/arch/ncvs/src/libexec/rtld-elf/rtld.h,v retrieving revision 1.37 diff -u -r1.37 rtld.h --- libexec/rtld-elf/rtld.h 18 Dec 2005 19:43:32 -0000 1.37 +++ libexec/rtld-elf/rtld.h 23 Mar 2006 10:44:27 -0000 @@ -210,6 +210,7 @@ bool jmpslots_done; /* Already have relocated the jump slots */ bool init_done; /* Already have added object to init list */ bool tls_done; /* Already allocated offset for static TLS */ + bool fini_now; /* Finalizer for dso currently runs */ struct link_map linkmap; /* for GDB and dlinfo() */ Objlist dldags; /* Object belongs to these dlopened DAGs (%) */ Please note that patch forces dlopen for dso to fail if another thread run finalizers from the same dso at the time of the call. joerg at britannica dot bec dot de does not like the patch and promised to supply better fix, but still not available. >Release-Note: >Audit-Trail: >Unformatted: