From owner-svn-src-all@FreeBSD.ORG Thu Mar 3 17:12:24 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A9F531065675; Thu, 3 Mar 2011 17:12:24 +0000 (UTC) (envelope-from jh@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 982398FC1F; Thu, 3 Mar 2011 17:12:24 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id p23HCO6Z054841; Thu, 3 Mar 2011 17:12:24 GMT (envelope-from jh@svn.freebsd.org) Received: (from jh@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id p23HCO8H054839; Thu, 3 Mar 2011 17:12:24 GMT (envelope-from jh@svn.freebsd.org) Message-Id: <201103031712.p23HCO8H054839@svn.freebsd.org> From: Jaakko Heinonen Date: Thu, 3 Mar 2011 17:12:24 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org X-SVN-Group: stable-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r219237 - stable/8/libexec/rtld-elf X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 Mar 2011 17:12:24 -0000 Author: jh Date: Thu Mar 3 17:12:24 2011 New Revision: 219237 URL: http://svn.freebsd.org/changeset/base/219237 Log: MFC r216489: If dlclose() is called recursively from a _fini() function, the inner dlclose() call may unload the object of the outer call prematurely because objects are unreferenced before _fini() calls. Fix this by unreferencing objects after calling objlist_call_fini() in dlclose(). Therefore objlist_call_fini() now calls the fini function if the reference count of an object is 1. In addition we must restart the list_fini traversal after every _fini() call because another dlclose() call might have modified the reference counts. Add an XXX comment to objlist_call_fini() about possible race with dlopen(). PR: 133246, 149464 Modified: stable/8/libexec/rtld-elf/rtld.c Directory Properties: stable/8/libexec/rtld-elf/ (props changed) Modified: stable/8/libexec/rtld-elf/rtld.c ============================================================================== --- stable/8/libexec/rtld-elf/rtld.c Thu Mar 3 17:11:11 2011 (r219236) +++ stable/8/libexec/rtld-elf/rtld.c Thu Mar 3 17:12:24 2011 (r219237) @@ -107,7 +107,7 @@ static int load_needed_objects(Obj_Entry static int load_preload_objects(void); static Obj_Entry *load_object(const char *, const Obj_Entry *, int); static Obj_Entry *obj_from_addr(const void *); -static void objlist_call_fini(Objlist *, bool, int *); +static void objlist_call_fini(Objlist *, Obj_Entry *, int *); static void objlist_call_init(Objlist *, int *); static void objlist_clear(Objlist *); static Objlist_Entry *objlist_find(Objlist *, const Obj_Entry *); @@ -1616,36 +1616,56 @@ obj_from_addr(const void *addr) /* * Call the finalization functions for each of the objects in "list" - * which are unreferenced. All of the objects are expected to have - * non-NULL fini functions. + * belonging to the DAG of "root" and referenced once. If NULL "root" + * is specified, every finalization function will be called regardless + * of the reference count and the list elements won't be freed. All of + * the objects are expected to have non-NULL fini functions. */ static void -objlist_call_fini(Objlist *list, bool force, int *lockstate) +objlist_call_fini(Objlist *list, Obj_Entry *root, int *lockstate) { - Objlist_Entry *elm, *elm_tmp; + Objlist_Entry *elm; char *saved_msg; + assert(root == NULL || root->refcount == 1); + /* * Preserve the current error message since a fini function might * call into the dynamic linker and overwrite it. */ saved_msg = errmsg_save(); - STAILQ_FOREACH_SAFE(elm, list, link, elm_tmp) { - if (elm->obj->refcount == 0 || force) { + do { + STAILQ_FOREACH(elm, list, link) { + if (root != NULL && (elm->obj->refcount != 1 || + objlist_find(&root->dagmembers, elm->obj) == NULL)) + continue; dbg("calling fini function for %s at %p", elm->obj->path, (void *)elm->obj->fini); LD_UTRACE(UTRACE_FINI_CALL, elm->obj, (void *)elm->obj->fini, 0, 0, elm->obj->path); /* Remove object from fini list to prevent recursive invocation. */ STAILQ_REMOVE(list, elm, Struct_Objlist_Entry, link); + /* + * XXX: If a dlopen() call references an object while the + * fini function is in progress, we might end up trying to + * unload the referenced object in dlclose() or the object + * won't be unloaded although its fini function has been + * called. + */ wlock_release(rtld_bind_lock, *lockstate); call_initfini_pointer(elm->obj, elm->obj->fini); *lockstate = wlock_acquire(rtld_bind_lock); /* No need to free anything if process is going down. */ - if (!force) + if (root != NULL) free(elm); + /* + * We must restart the list traversal after every fini call + * because a dlclose() call from the fini function or from + * another thread might have modified the reference counts. + */ + break; } - } + } while (elm != NULL); errmsg_restore(saved_msg); } @@ -1833,7 +1853,7 @@ rtld_exit(void) lockstate = wlock_acquire(rtld_bind_lock); dbg("rtld_exit()"); - objlist_call_fini(&list_fini, true, &lockstate); + objlist_call_fini(&list_fini, NULL, &lockstate); /* No need to remove the items from the list, since we are exiting. */ if (!libmap_disable) lm_fini(); @@ -1946,20 +1966,22 @@ dlclose(void *handle) /* Unreference the object and its dependencies. */ root->dl_refcount--; - unref_dag(root); - - if (root->refcount == 0) { + if (root->refcount == 1) { /* - * The object is no longer referenced, so we must unload it. + * The object will be no longer referenced, so we must unload it. * First, call the fini functions. */ - objlist_call_fini(&list_fini, false, &lockstate); + objlist_call_fini(&list_fini, root, &lockstate); + + unref_dag(root); /* Finish cleaning up the newly-unreferenced objects. */ GDB_STATE(RT_DELETE,&root->linkmap); unload_object(root); GDB_STATE(RT_CONSISTENT,NULL); - } + } else + unref_dag(root); + LD_UTRACE(UTRACE_DLCLOSE_STOP, handle, NULL, 0, 0, NULL); wlock_release(rtld_bind_lock, lockstate); return 0;