Skip site navigation (1)Skip section navigation (2) 
Date:      Thu, 9 Mar 2017 21:05:47 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r314973 - head/libexec/rtld-elf
Message-ID:  <201703092105.v29L5lPj055690@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: kib
Date: Thu Mar  9 21:05:47 2017
New Revision: 314973
URL: https://svnweb.freebsd.org/changeset/base/314973

Log:
  Avoid bind lock recursion.
  
  When dlclose(3) unloads an object with filtees, it recursively calls
  dlclose(3) on each filtee in free_needed_filtees().  Introduce
  dlclose_locked() helper, called from free_needed_filtees() instead of
  dlclose(), and pass the bind lockstate down to avoid recursing.
  
  Reported and tested by:	jhibbits
  Sponsored by:	The FreeBSD Foundation
  MFC after:	1 week

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

Modified: head/libexec/rtld-elf/rtld.c
==============================================================================
--- head/libexec/rtld-elf/rtld.c	Thu Mar  9 19:02:36 2017	(r314972)
+++ head/libexec/rtld-elf/rtld.c	Thu Mar  9 21:05:47 2017	(r314973)
@@ -77,6 +77,7 @@ static void digest_dynamic2(Obj_Entry *,
 static void digest_dynamic(Obj_Entry *, int);
 static Obj_Entry *digest_phdr(const Elf_Phdr *, int, caddr_t, const char *);
 static Obj_Entry *dlcheck(void *);
+static int dlclose_locked(void *, RtldLockState *);
 static Obj_Entry *dlopen_object(const char *name, int fd, Obj_Entry *refobj,
     int lo_flags, int mode, RtldLockState *lockstate);
 static Obj_Entry *do_load_object(int, const char *, char *, struct stat *, int);
@@ -98,7 +99,7 @@ static void initlist_add_objects(Obj_Ent
 static void linkmap_add(Obj_Entry *);
 static void linkmap_delete(Obj_Entry *);
 static void load_filtees(Obj_Entry *, int flags, RtldLockState *);
-static void unload_filtees(Obj_Entry *);
+static void unload_filtees(Obj_Entry *, RtldLockState *);
 static int load_needed_objects(Obj_Entry *, int);
 static int load_preload_objects(void);
 static Obj_Entry *load_object(const char *, int fd, const Obj_Entry *, int);
@@ -142,7 +143,7 @@ static int symlook_obj1_sysv(SymLook *, 
 static int symlook_obj1_gnu(SymLook *, const Obj_Entry *);
 static void trace_loaded_objects(Obj_Entry *);
 static void unlink_object(Obj_Entry *);
-static void unload_object(Obj_Entry *);
+static void unload_object(Obj_Entry *, RtldLockState *lockstate);
 static void unref_dag(Obj_Entry *);
 static void ref_dag(Obj_Entry *);
 static char *origin_subst_one(Obj_Entry *, char *, const char *,
@@ -2104,13 +2105,13 @@ initlist_add_objects(Obj_Entry *obj, Obj
 #endif
 
 static void
-free_needed_filtees(Needed_Entry *n)
+free_needed_filtees(Needed_Entry *n, RtldLockState *lockstate)
 {
     Needed_Entry *needed, *needed1;
 
     for (needed = n; needed != NULL; needed = needed->next) {
 	if (needed->obj != NULL) {
-	    dlclose(needed->obj);
+	    dlclose_locked(needed->obj, lockstate);
 	    needed->obj = NULL;
 	}
     }
@@ -2121,14 +2122,14 @@ free_needed_filtees(Needed_Entry *n)
 }
 
 static void
-unload_filtees(Obj_Entry *obj)
+unload_filtees(Obj_Entry *obj, RtldLockState *lockstate)
 {
 
-    free_needed_filtees(obj->needed_filtees);
-    obj->needed_filtees = NULL;
-    free_needed_filtees(obj->needed_aux_filtees);
-    obj->needed_aux_filtees = NULL;
-    obj->filtees_loaded = false;
+	free_needed_filtees(obj->needed_filtees, lockstate);
+	obj->needed_filtees = NULL;
+	free_needed_filtees(obj->needed_aux_filtees, lockstate);
+	obj->needed_aux_filtees = NULL;
+	obj->filtees_loaded = false;
 }
 
 static void
@@ -3015,15 +3016,23 @@ search_library_pathfds(const char *name,
 int
 dlclose(void *handle)
 {
+	RtldLockState lockstate;
+	int error;
+
+	wlock_acquire(rtld_bind_lock, &lockstate);
+	error = dlclose_locked(handle, &lockstate);
+	lock_release(rtld_bind_lock, &lockstate);
+	return (error);
+}
+
+static int
+dlclose_locked(void *handle, RtldLockState *lockstate)
+{
     Obj_Entry *root;
-    RtldLockState lockstate;
 
-    wlock_acquire(rtld_bind_lock, &lockstate);
     root = dlcheck(handle);
-    if (root == NULL) {
-	lock_release(rtld_bind_lock, &lockstate);
+    if (root == NULL)
 	return -1;
-    }
     LD_UTRACE(UTRACE_DLCLOSE_START, handle, NULL, 0, root->dl_refcount,
 	root->path);
 
@@ -3035,19 +3044,18 @@ dlclose(void *handle)
 	 * The object will be no longer referenced, so we must unload it.
 	 * First, call the fini functions.
 	 */
-	objlist_call_fini(&list_fini, root, &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);
+	unload_object(root, lockstate);
 	GDB_STATE(RT_CONSISTENT,NULL);
     } else
 	unref_dag(root);
 
     LD_UTRACE(UTRACE_DLCLOSE_STOP, handle, NULL, 0, 0, NULL);
-    lock_release(rtld_bind_lock, &lockstate);
     return 0;
 }
 
@@ -3123,13 +3131,13 @@ rtld_dlopen(const char *name, int fd, in
 }
 
 static void
-dlopen_cleanup(Obj_Entry *obj)
+dlopen_cleanup(Obj_Entry *obj, RtldLockState *lockstate)
 {
 
 	obj->dl_refcount--;
 	unref_dag(obj);
 	if (obj->refcount == 0)
-		unload_object(obj);
+		unload_object(obj, lockstate);
 }
 
 static Obj_Entry *
@@ -3178,7 +3186,7 @@ dlopen_object(const char *name, int fd, 
 	      (mode & RTLD_MODEMASK) == RTLD_NOW, &obj_rtld,
 	      (lo_flags & RTLD_LO_EARLY) ? SYMLOOK_EARLY : 0,
 	      lockstate) == -1) {
-		dlopen_cleanup(obj);
+		dlopen_cleanup(obj, lockstate);
 		obj = NULL;
 	    } else if (lo_flags & RTLD_LO_EARLY) {
 		/*
@@ -3235,7 +3243,7 @@ dlopen_object(const char *name, int fd, 
       (lo_flags & RTLD_LO_EARLY) ? SYMLOOK_EARLY : 0,
       lockstate) == -1) {
 	objlist_clear(&initlist);
-	dlopen_cleanup(obj);
+	dlopen_cleanup(obj, lockstate);
 	if (lockstate == &mlockstate)
 	    lock_release(rtld_bind_lock, lockstate);
 	return (NULL);
@@ -4429,7 +4437,7 @@ trace_loaded_objects(Obj_Entry *obj)
  * reference count of 0.
  */
 static void
-unload_object(Obj_Entry *root)
+unload_object(Obj_Entry *root, RtldLockState *lockstate)
 {
 	Obj_Entry marker, *obj, *next;
 
@@ -4461,11 +4469,11 @@ unload_object(Obj_Entry *root)
 			if (next != NULL) {
 				init_marker(&marker);
 				TAILQ_INSERT_BEFORE(next, &marker, next);
-				unload_filtees(obj);
+				unload_filtees(obj, lockstate);
 				next = TAILQ_NEXT(&marker, next);
 				TAILQ_REMOVE(&obj_list, &marker, next);
 			} else
-				unload_filtees(obj);
+				unload_filtees(obj, lockstate);
 		}
 		release_object(obj);
 	}

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201703092105.v29L5lPj055690>