Author: jh
Date: Thu Dec 16 16:56:44 2010
New Revision: 216489
URL: http://svn.freebsd.org/changeset/base/216489

Log:
  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
  Reviewed by:  kan, kib

Modified:
  head/libexec/rtld-elf/rtld.c

Modified: head/libexec/rtld-elf/rtld.c
==============================================================================
--- head/libexec/rtld-elf/rtld.c        Thu Dec 16 16:55:22 2010        
(r216488)
+++ head/libexec/rtld-elf/rtld.c        Thu Dec 16 16:56:44 2010        
(r216489)
@@ -110,7 +110,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 *);
@@ -1609,36 +1609,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);
 }
 
@@ -1826,7 +1846,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();
@@ -1939,20 +1959,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;
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to