Author: markj
Date: Fri Feb 3 01:15:31 2017
New Revision: 313124
URL: https://svnweb.freebsd.org/changeset/base/313124
Log:
MFC r310420, r310421, r310422:
Fix races and logic errors around dlclose().
Modified:
stable/11/libexec/rtld-elf/rtld.c
stable/11/libexec/rtld-elf/rtld.h
Directory Properties:
stable/11/ (props changed)
Modified: stable/11/libexec/rtld-elf/rtld.c
==============================================================================
--- stable/11/libexec/rtld-elf/rtld.c Fri Feb 3 01:10:38 2017
(r313123)
+++ stable/11/libexec/rtld-elf/rtld.c Fri Feb 3 01:15:31 2017
(r313124)
@@ -87,7 +87,10 @@ 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_marker(Obj_Entry *);
static void init_pagesizes(Elf_Auxinfo **aux_info);
static void init_rtld(caddr_t, Elf_Auxinfo **);
static void initlist_add_neededs(Needed_Entry *, Objlist *);
@@ -113,6 +116,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,
@@ -1827,6 +1831,14 @@ init_dag(Obj_Entry *root)
root->dag_inited = true;
}
+static void
+init_marker(Obj_Entry *marker)
+{
+
+ bzero(marker, sizeof(*marker));
+ marker->marker = true;
+}
+
Obj_Entry *
globallist_curr(const Obj_Entry *obj)
{
@@ -1853,6 +1865,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)
{
@@ -2210,7 +2239,7 @@ load_object(const char *name, int fd_u,
fd = -1;
if (name != NULL) {
TAILQ_FOREACH(obj, &obj_list, next) {
- if (obj->marker)
+ if (obj->marker || obj->doomed)
continue;
if (object_match_name(obj, name))
return (obj);
@@ -2257,7 +2286,7 @@ load_object(const char *name, int fd_u,
return NULL;
}
TAILQ_FOREACH(obj, &obj_list, next) {
- if (obj->marker)
+ if (obj->marker || obj->doomed)
continue;
if (obj->ino == sb.st_ino && obj->dev == sb.st_dev)
break;
@@ -2399,6 +2428,9 @@ objlist_call_fini(Objlist *list, Obj_Ent
assert(root == NULL || root->refcount == 1);
+ if (root != NULL)
+ root->doomed = true;
+
/*
* Preserve the current error message since a fini function might
* call into the dynamic linker and overwrite it.
@@ -2411,15 +2443,11 @@ objlist_call_fini(Objlist *list, Obj_Ent
continue;
/* 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.
- */
- lock_release(rtld_bind_lock, lockstate);
+ /* Ensure that new references cannot be acquired. */
+ elm->obj->doomed = true;
+ hold_object(elm->obj);
+ lock_release(rtld_bind_lock, lockstate);
/*
* It is legal to have both DT_FINI and DT_FINI_ARRAY defined.
* When this happens, DT_FINI_ARRAY is processed first.
@@ -2445,6 +2473,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);
@@ -2498,6 +2527,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);
/*
@@ -2524,6 +2554,7 @@ objlist_call_init(Objlist *list, RtldLoc
}
}
wlock_acquire(rtld_bind_lock, lockstate);
+ unhold_object(elm->obj);
}
errmsg_restore(saved_msg);
}
@@ -3539,8 +3570,7 @@ dl_iterate_phdr(__dl_iterate_hdr_callbac
RtldLockState bind_lockstate, phdr_lockstate;
int error;
- bzero(&marker, sizeof(marker));
- marker.marker = true;
+ init_marker(&marker);
error = 0;
wlock_acquire(rtld_phdr_lock, &phdr_lockstate);
@@ -3548,11 +3578,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) {
@@ -3807,6 +3839,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.
@@ -4377,7 +4422,7 @@ trace_loaded_objects(Obj_Entry *obj)
static void
unload_object(Obj_Entry *root)
{
- Obj_Entry *obj, *obj1;
+ Obj_Entry marker, *obj, *next;
assert(root->refcount == 0);
@@ -4388,18 +4433,32 @@ unload_object(Obj_Entry *root)
unlink_object(root);
/* Unmap all objects that are no longer referenced. */
- TAILQ_FOREACH_SAFE(obj, &obj_list, next, obj1) {
+ for (obj = TAILQ_FIRST(&obj_list); obj != NULL; obj = next) {
+ next = TAILQ_NEXT(obj, next);
if (obj->marker || obj->refcount != 0)
continue;
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);
+
+ if (obj->filtees_loaded) {
+ if (next != NULL) {
+ init_marker(&marker);
+ TAILQ_INSERT_BEFORE(next, &marker, next);
+ unload_filtees(obj);
+ next = TAILQ_NEXT(&marker, next);
+ TAILQ_REMOVE(&obj_list, &marker, next);
+ } else
+ unload_filtees(obj);
+ }
+ release_object(obj);
}
}
Modified: stable/11/libexec/rtld-elf/rtld.h
==============================================================================
--- stable/11/libexec/rtld-elf/rtld.h Fri Feb 3 01:10:38 2017
(r313123)
+++ stable/11/libexec/rtld-elf/rtld.h Fri Feb 3 01:15:31 2017
(r313124)
@@ -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,8 @@ 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 */
+ bool doomed : 1; /* Object cannot be referenced */
struct link_map linkmap; /* For GDB and dlinfo() */
Objlist dldags; /* Object belongs to these dlopened DAGs (%) */
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
