Date: Thu, 22 Dec 2016 17:37:40 +0000 (UTC) From: Mark Johnston <markj@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r310420 - head/libexec/rtld-elf Message-ID: <201612221737.uBMHbeb7055342@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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 (%) */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201612221737.uBMHbeb7055342>