Date: Fri, 3 Feb 2017 01:15:31 +0000 (UTC) From: Mark Johnston <markj@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r313124 - stable/11/libexec/rtld-elf Message-ID: <201702030115.v131FV0C090027@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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 (%) */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201702030115.v131FV0C090027>