svn commit: r310420 - head/libexec/rtld-elf

[Mark Johnston]

Author: markj
Date: Thu Dec 22 17:37:39 2016
New Revision: 310420
URL: https://svnweb.freebsd.org/changeset/base/310420

Log:
  rtld: Fix a race between dl_iterate_phdr() and dlclose().
  
  Add a transient reference count to ensure that the phdr argument to the
  callback remains valid while the bind lock is dropped.
  
  Reviewed by:	kib
  MFC after:	2 weeks
  Sponsored by:	Dell EMC Isilon

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

Modified: head/libexec/rtld-elf/rtld.c
==============================================================================
--- head/libexec/rtld-elf/rtld.c	Thu Dec 22 16:19:05 2016	(r310419)
+++ head/libexec/rtld-elf/rtld.c	Thu Dec 22 17:37:39 2016	(r310420)
@@ -87,6 +87,8 @@ static char *errmsg_save(void);
 static void *fill_search_info(const char *, size_t, void *);
 static char *find_library(const char *, const Obj_Entry *, int *);
 static const char *gethints(bool);
+static void hold_object(Obj_Entry *);
+static void unhold_object(Obj_Entry *);
 static void init_dag(Obj_Entry *);
 static void init_pagesizes(Elf_Auxinfo **aux_info);
 static void init_rtld(caddr_t, Elf_Auxinfo **);
@@ -112,6 +114,7 @@ static void objlist_put_after(Objlist *,
 static void objlist_remove(Objlist *, Obj_Entry *);
 static int parse_libdir(const char *);
 static void *path_enumerate(const char *, path_enum_proc, void *);
+static void release_object(Obj_Entry *);
 static int relocate_object_dag(Obj_Entry *root, bool bind_now,
     Obj_Entry *rtldobj, int flags, RtldLockState *lockstate);
 static int relocate_object(Obj_Entry *obj, bool bind_now, Obj_Entry *rtldobj,
@@ -1852,6 +1855,23 @@ globallist_next(const Obj_Entry *obj)
 	}
 }
 
+/* Prevent the object from being unmapped while the bind lock is dropped. */
+static void
+hold_object(Obj_Entry *obj)
+{
+
+	obj->holdcount++;
+}
+
+static void
+unhold_object(Obj_Entry *obj)
+{
+
+	assert(obj->holdcount > 0);
+	if (--obj->holdcount == 0 && obj->unholdfree)
+		release_object(obj);
+}
+
 static void
 process_z(Obj_Entry *root)
 {
@@ -2417,6 +2437,7 @@ objlist_call_fini(Objlist *list, Obj_Ent
 	     * won't be unloaded although its fini function has been
 	     * called.
 	     */
+	    hold_object(elm->obj);
 	    lock_release(rtld_bind_lock, lockstate);
 
 	    /*
@@ -2444,6 +2465,7 @@ objlist_call_fini(Objlist *list, Obj_Ent
 		call_initfini_pointer(elm->obj, elm->obj->fini);
 	    }
 	    wlock_acquire(rtld_bind_lock, lockstate);
+	    unhold_object(elm->obj);
 	    /* No need to free anything if process is going down. */
 	    if (root != NULL)
 	    	free(elm);
@@ -2497,6 +2519,7 @@ objlist_call_init(Objlist *list, RtldLoc
 	 * without better locking.
 	 */
 	elm->obj->init_done = true;
+	hold_object(elm->obj);
 	lock_release(rtld_bind_lock, lockstate);
 
         /*
@@ -2523,6 +2546,7 @@ objlist_call_init(Objlist *list, RtldLoc
 	    }
 	}
 	wlock_acquire(rtld_bind_lock, lockstate);
+	unhold_object(elm->obj);
     }
     errmsg_restore(saved_msg);
 }
@@ -3553,11 +3577,13 @@ dl_iterate_phdr(__dl_iterate_hdr_callbac
 	for (obj = globallist_curr(TAILQ_FIRST(&obj_list)); obj != NULL;) {
 		TAILQ_INSERT_AFTER(&obj_list, obj, &marker, next);
 		rtld_fill_dl_phdr_info(obj, &phdr_info);
+		hold_object(obj);
 		lock_release(rtld_bind_lock, &bind_lockstate);
 
 		error = callback(&phdr_info, sizeof phdr_info, param);
 
 		wlock_acquire(rtld_bind_lock, &bind_lockstate);
+		unhold_object(obj);
 		obj = globallist_next(&marker);
 		TAILQ_REMOVE(&obj_list, &marker, next);
 		if (error != 0) {
@@ -3812,6 +3838,19 @@ _r_debug_postinit(struct link_map *m)
 	__compiler_membar();
 }
 
+static void
+release_object(Obj_Entry *obj)
+{
+
+	if (obj->holdcount > 0) {
+		obj->unholdfree = true;
+		return;
+	}
+	munmap(obj->mapbase, obj->mapsize);
+	linkmap_delete(obj);
+	obj_free(obj);
+}
+
 /*
  * Get address of the pointer variable in the main program.
  * Prefer non-weak symbol over the weak one.
@@ -4399,12 +4438,16 @@ unload_object(Obj_Entry *root)
 		LD_UTRACE(UTRACE_UNLOAD_OBJECT, obj, obj->mapbase,
 		    obj->mapsize, 0, obj->path);
 		dbg("unloading \"%s\"", obj->path);
-		unload_filtees(root);
-		munmap(obj->mapbase, obj->mapsize);
-		linkmap_delete(obj);
+		/*
+		 * Unlink the object now to prevent new references from
+		 * being acquired while the bind lock is dropped in
+		 * recursive dlclose() invocations.
+		 */
 		TAILQ_REMOVE(&obj_list, obj, next);
 		obj_count--;
-		obj_free(obj);
+
+		unload_filtees(root);
+		release_object(obj);
 	}
 }
 

Modified: head/libexec/rtld-elf/rtld.h
==============================================================================
--- head/libexec/rtld-elf/rtld.h	Thu Dec 22 16:19:05 2016	(r310419)
+++ head/libexec/rtld-elf/rtld.h	Thu Dec 22 17:37:39 2016	(r310420)
@@ -142,7 +142,8 @@ typedef struct Struct_Obj_Entry {
     TAILQ_ENTRY(Struct_Obj_Entry) next;
     char *path;			/* Pathname of underlying file (%) */
     char *origin_path;		/* Directory path of origin file */
-    int refcount;
+    int refcount;		/* DAG references */
+    int holdcount;		/* Count of transient references */
     int dl_refcount;		/* Number of times loaded by dlopen */
 
     /* These items are computed by map_object() or by digest_phdr(). */
@@ -265,6 +266,7 @@ typedef struct Struct_Obj_Entry {
     bool valid_hash_gnu : 1;	/* A valid GNU hash tag is available */
     bool dlopened : 1;		/* dlopen()-ed (vs. load statically) */
     bool marker : 1;		/* marker on the global obj list */
+    bool unholdfree : 1;	/* unmap upon last unhold */
 
     struct link_map linkmap;	/* For GDB and dlinfo() */
     Objlist dldags;		/* Object belongs to these dlopened DAGs (%) */