kern/133246: commit references a PR

[dfilter service]

The following reply was made to PR kern/133246; it has been noted by GNATS.

From: dfilter at FreeBSD.ORG (dfilter service)
To: bug-followup at FreeBSD.org
Cc:  
Subject: Re: kern/133246: commit references a PR
Date: Thu, 16 Dec 2010 16:56:49 +0000 (UTC)

 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-all at freebsd.org mailing list
 http://lists.freebsd.org/mailman/listinfo/svn-src-all
 To unsubscribe, send any mail to "svn-src-all-unsubscribe at freebsd.org"