_rtld_bind() read-locks the bind lock, and possible plt resolution

from the dispatcher would also acquire bind lock in read mode, which
is the supported operation. plt is explicitely designed to allow safe
multithreaded updates, so the shared lock do not cause problems.

The error in r228435 is that it allows read lock acquisition after the
write lock for the bind block.  If we dlopened the shared object that
contains IRELATIVE or jump slot which target is STT_GNU_IFUNC, then
possible recursive plt resolve from the dispatcher would cause it.

Postpone the resolution for irelative/ifunc right before initializers
are called, and drop bind lock around calls to dispatcher.  Use
initlist to iterate over the objects instead of the ->next, due to
drop of the bind lock in iteration.

For i386/reloc.c:reloc_iresolve(), fix calculation of the dispatch
function address for dso, by taking into account possible non-zero
relocbase.

MFC after:	3 weeks

libexec/rtld-elf/amd64/reloc.c

@@ -413,6 +413,8 @@ reloc_iresolve(Obj_Entry *obj, RtldLockState *lockstate)
     const Elf_Rela *relalim;
     const Elf_Rela *rela;
 
+    if (!obj->irelative)
+	return (0);
     relalim = (const Elf_Rela *)((char *)obj->pltrela + obj->pltrelasize);
     for (rela = obj->pltrela;  rela < relalim;  rela++) {
 	Elf_Addr *where, target, *ptr;
@@ -424,11 +426,14 @@ reloc_iresolve(Obj_Entry *obj, RtldLockState *lockstate)
 	case R_X86_64_IRELATIVE:
 	  ptr = (Elf_Addr *)(obj->relocbase + rela->r_addend);
 	  where = (Elf_Addr *)(obj->relocbase + rela->r_offset);
+	  lock_release(rtld_bind_lock, lockstate);
 	  target = ((Elf_Addr (*)(void))ptr)();
+	  wlock_acquire(rtld_bind_lock, lockstate);
 	  *where = target;
 	  break;
 	}
     }
+    obj->irelative = false;
     return (0);
 }
 
@@ -455,13 +460,15 @@ reloc_gnu_ifunc(Obj_Entry *obj, RtldLockState *lockstate)
 	      return (-1);
 	  if (ELF_ST_TYPE(def->st_info) != STT_GNU_IFUNC)
 	      continue;
+	  lock_release(rtld_bind_lock, lockstate);
 	  target = (Elf_Addr)rtld_resolve_ifunc(defobj, def);
+	  wlock_acquire(rtld_bind_lock, lockstate);
 	  reloc_jmpslot(where, target, defobj, obj, (const Elf_Rel *)rela);
 	  break;
 	}
     }
     obj->gnu_ifunc = false;
-    return 0;
+    return (0);
 }
 
 void

libexec/rtld-elf/i386/reloc.c

@@ -371,16 +371,21 @@ reloc_iresolve(Obj_Entry *obj, RtldLockState *lockstate)
     const Elf_Rel *rel;
     Elf_Addr *where, target;
 
+    if (!obj->irelative)
+	return (0);
     rellim = (const Elf_Rel *)((char *)obj->pltrel + obj->pltrelsize);
     for (rel = obj->pltrel;  rel < rellim;  rel++) {
 	switch (ELF_R_TYPE(rel->r_info)) {
 	case R_386_IRELATIVE:
 	  where = (Elf_Addr *)(obj->relocbase + rel->r_offset);
-	  target = ((Elf_Addr (*)(void))(*where))();
+	  lock_release(rtld_bind_lock, lockstate);
+	  target = ((Elf_Addr (*)(void))(obj->relocbase + *where))();
+	  wlock_acquire(rtld_bind_lock, lockstate);
 	  *where = target;
 	  break;
 	}
     }
+    obj->irelative = false;
     return (0);
 }
 
@@ -407,7 +412,9 @@ reloc_gnu_ifunc(Obj_Entry *obj, RtldLockState *lockstate)
 	      return (-1);
 	  if (ELF_ST_TYPE(def->st_info) != STT_GNU_IFUNC)
 	      continue;
+	  lock_release(rtld_bind_lock, lockstate);
 	  target = (Elf_Addr)rtld_resolve_ifunc(defobj, def);
+	  wlock_acquire(rtld_bind_lock, lockstate);
 	  reloc_jmpslot(where, target, defobj, obj, rel);
 	  break;
 	}

libexec/rtld-elf/rtld.c

@@ -116,6 +116,8 @@ static void objlist_push_tail(Objlist *, Obj_Entry *);
 static void objlist_remove(Objlist *, Obj_Entry *);
 static void *path_enumerate(const char *, path_enum_proc, void *);
 static int relocate_objects(Obj_Entry *, bool, Obj_Entry *, RtldLockState *);
+static int resolve_objects_ifunc(Obj_Entry *first, bool bind_now,
+    RtldLockState *lockstate);
 static int rtld_dirname(const char *, char *);
 static int rtld_dirname_abs(const char *, char *);
 static void rtld_exit(void);
@@ -513,6 +515,10 @@ _rtld(Elf_Addr *sp, func_ptr_type *exit_proc, Obj_Entry **objp)
       ld_bind_now != NULL && *ld_bind_now != '\0', &obj_rtld, NULL) == -1)
 	die();
 
+    if (resolve_objects_ifunc(obj_main,
+      ld_bind_now != NULL && *ld_bind_now != '\0', NULL) == -1)
+	die();
+
     dbg("doing copy relocations");
     if (do_copy_relocations(obj_main) == -1)
 	die();
@@ -1978,25 +1984,53 @@ relocate_objects(Obj_Entry *first, bool bind_now, Obj_Entry *rtldobj,
 	obj->version = RTLD_VERSION;
     }
 
-    /*
+    return (0);
-     * The handling of R_MACHINE_IRELATIVE relocations and jumpslots
+}
-     * referencing STT_GNU_IFUNC symbols is postponed till the other
+
-     * relocations are done.  The indirect functions specified as
+/*
-     * ifunc are allowed to call other symbols, so we need to have
+ * The handling of R_MACHINE_IRELATIVE relocations and jumpslots
-     * objects relocated before asking for resolution from indirects.
+ * referencing STT_GNU_IFUNC symbols is postponed till the other
-     *
+ * relocations are done.  The indirect functions specified as
-     * The R_MACHINE_IRELATIVE slots are resolved in greedy fashion,
+ * ifunc are allowed to call other symbols, so we need to have
-     * instead of the usual lazy handling of PLT slots.  It is
+ * objects relocated before asking for resolution from indirects.
-     * consistent with how GNU does it.
+ *
-     */
+ * The R_MACHINE_IRELATIVE slots are resolved in greedy fashion,
-    for (obj = first;  obj != NULL;  obj = obj->next) {
+ * instead of the usual lazy handling of PLT slots.  It is
+ * consistent with how GNU does it.
+ */
+static int
+resolve_object_ifunc(Obj_Entry *obj, bool bind_now, RtldLockState *lockstate)
+{
 	if (obj->irelative && reloc_iresolve(obj, lockstate) == -1)
-	    return (-1);
+		return (-1);
 	if ((obj->bind_now || bind_now) && obj->gnu_ifunc &&
-	  reloc_gnu_ifunc(obj, lockstate) == -1)
+	    reloc_gnu_ifunc(obj, lockstate) == -1)
-	    return (-1);
+		return (-1);
-    }
+	return (0);
-    return 0;
+}
+
+static int
+resolve_objects_ifunc(Obj_Entry *first, bool bind_now, RtldLockState *lockstate)
+{
+	Obj_Entry *obj;
+
+	for (obj = first;  obj != NULL;  obj = obj->next) {
+		if (resolve_object_ifunc(obj, bind_now, lockstate) == -1)
+			return (-1);
+	}
+	return (0);
+}
+
+static int
+initlist_objects_ifunc(Objlist *list, bool bind_now, RtldLockState *lockstate)
+{
+	Objlist_Entry *elm;
+
+	STAILQ_FOREACH(elm, list, link) {
+		if (resolve_object_ifunc(elm->obj, bind_now, lockstate) == -1)
+			return (-1);
+	}
+	return (0);
 }
 
 /*
@@ -2201,6 +2235,16 @@ dlopen(const char *name, int mode)
       mode & (RTLD_MODEMASK | RTLD_GLOBAL)));
 }
 
+static void
+dlopen_cleanup(Obj_Entry *obj)
+{
+
+	obj->dl_refcount--;
+	unref_dag(obj);
+	if (obj->refcount == 0)
+		unload_object(obj);
+}
+
 static Obj_Entry *
 dlopen_object(const char *name, Obj_Entry *refobj, int lo_flags, int mode)
 {
@@ -2239,10 +2283,7 @@ dlopen_object(const char *name, Obj_Entry *refobj, int lo_flags, int mode)
 		goto trace;
 	    if (result == -1 || (relocate_objects(obj, (mode & RTLD_MODEMASK)
 	      == RTLD_NOW, &obj_rtld, &lockstate)) == -1) {
-		obj->dl_refcount--;
+		dlopen_cleanup(obj);
-		unref_dag(obj);
-		if (obj->refcount == 0)
-		    unload_object(obj);
 		obj = NULL;
 	    } else {
 		/* Make list of init functions to call. */
@@ -2276,6 +2317,14 @@ dlopen_object(const char *name, Obj_Entry *refobj, int lo_flags, int mode)
 
     map_stacks_exec(&lockstate);
 
+    if (initlist_objects_ifunc(&initlist, (mode & RTLD_MODEMASK) == RTLD_NOW,
+      &lockstate) == -1) {
+	objlist_clear(&initlist);
+	dlopen_cleanup(obj);
+	lock_release(rtld_bind_lock, &lockstate);
+	return (NULL);
+    }
+
     /* Call the init functions. */
     objlist_call_init(&initlist, &lockstate);
     objlist_clear(&initlist);