# HG changeset patch # User Taylor R Campbell # Date 1782415203 0 # Thu Jun 25 19:20:03 2026 +0000 # Branch trunk # Node ID 7022496b52e310c4e250b3864fc738708de5963a # Parent 197592d4e39970f65777022156bf27063f3e1280 # EXP-Topic riastradh-pr59751-dlcloserace ld.elf_so(1): Resolve several races in dlopen/dlclose. 1. If thread A dlcloses foo.so, bringing the reference count to zero, and drops the rtld lock to call destructors, thread B dlopening foo.so can acquire a new reference and return it -- but then thread A will proceed to unmap and free foo.so while thread B thinks it has a good reference. => Resolution: Make dlopen refuse to acquire new references to objects with reference count zero -- instead, drop the rtld lock to wait until the object has been dlclosed, and then start over the lookup. Caveat: This means that the dlopen logic can drop the lock, and the rtld state can change, while dlopen is gathering references to objects, so we can't simply see whether new objects were added to the end of _rtld_objlist to find which ones need to be relocated. So... 2. If thread A dlopens foo.so and sleeps to wait for a previous dlclose to finish, and thread B concurrently dlopens bar.so and finishes relocating it before thread A continues, thread A might try to relocate all of the new objects since it started -- including the ones that thread B just loaded and already relocated. => Resolution: Instead of checking whether _rtld_objtail has changed, store: (a) a global count of the number of objects pending relocation (_rtld_objrelocpending), so we can cheaply test whether there are any before iterating over the list; and (b) a per-object flag of whether it has been relocated yet (obj->relocated), so dlopen can check _rtld_objrelocpending to see whether it needs to do any relocations, and _rtld_relocate_objects can check _all_ objects and only relocate the ones that have yet to be relocated. We could alternatively store a queue of objects to be relocated, so _rtld_relocate_objects need not iterate over all objects when loading one or two objects into a process with zillions of existing ones, but this was quicker to implement for now. 3. If thread A is dlopening an object and loading dependencies, it goes through the list of _all_ objects' dependencies, even those that aren't relevant to the current dlopen. In so doing, it may start loading the dependencies of some object foo.so that thread B wants to dlclose. If foo.so depends on bar.so, and bar.so is concurrently closed by thread C, thread A may wait for that to happen so it can create a new incarnation of bar.so. While thread A waits, thread B might unmap and free foo.so, leading to use-after-free in thread A when it finally wakes up. => Resolution: While a thread is loading an object's dependencies, make dlclose wait for that to finish before unmapping and freeing the object. New reference count obj->neededrefcount for this -- having it separate from obj->refcount obviates the need for logic to GC newly-unreferenced objects in _rtld_load_needed_objects. New state obj->neededwaiter to record a queue of waiters in dlclose. (XXX Maybe that should be a global queue? dlclose can reasonably wake up when any objects are no now-unreferenced and no longer being loaded.) 4. If thread A is dlclosing foo.so which depends on bar.so, and thread B is dlclosing bar.so but has dropped the rtld lock to run bar.so's destructors, thread A might get to the garbage-collection phase at the end of _rtld_unload_object -- and unmap and free bar.so before thread B has finished, because bar.so's reference count is zero and so it looks like garbage to _rtld_unmap_object. => Resolution: Store a flag obj->dlclosing, set when thread A's _rtld_unload_object is dropping the lock to call destructors, so thread B's _rtld_unload_object will not free and unmap obj during garbage collection. This won't leak because thread B will rerun the garbage collection anyway. 5. (a) If thread A is dlclosing foo.so which depends on baz.so, and thread B is dlclosing bar.so which also depends on baz.so, both threads may try to call baz.so's destructors, possibly leading to double-destruction. And either thread might free and unmap baz.so while the other one is still trying to call the destructors. (b) If thread A is dlopening foo.so and has dropped the lock to run constructors, and thread B dlopens foo.so, thread B won't run the constructors again (we already have a mechanism to prevent that) -- but it might return before constructors have finished running in thread A at all, and thus it may return a handle to the caller for a library whose constructors haven't finished initializing the library. => Resolution: New lock obj->initfinilock (implemented as a state variable under the rtld exclusive lock with sleep/wake using _lwp_park/unpark), taken across any calls to constructors or destructors with the exclusive lock dropped, in order to serialize calls to constructors and destructors even while the rtld exclusive lock is dropped. This way: (a) If thread A is running destructors for bar.so as part of dlclose, thread B can skip garbage-collecting bar.so as part of dlclose -- it won't leak anything because thread A will eventually run garbage collection in _rtld_unload_object too. (b) If thread A is running constructors for foo.so, and thread B dlopens foo.so, it can wait until the constructors have finished in thread A before returning. While here, sprinkle various reference count assertions. PR lib/59751: dlclose is not MT-safe depending on the libraries unloaded diff -r 197592d4e399 -r 7022496b52e3 libexec/ld.elf_so/load.c --- a/libexec/ld.elf_so/load.c Thu Jun 25 00:59:42 2026 +0000 +++ b/libexec/ld.elf_so/load.c Thu Jun 25 19:20:03 2026 +0000 @@ -63,7 +63,7 @@ #include "rtld.h" static bool _rtld_load_by_name(const char *, Obj_Entry *, Needed_Entry **, - int); + int, sigset_t *); #ifdef RTLD_LOADER Objlist _rtld_list_main = /* Objects loaded at program startup */ @@ -111,16 +111,27 @@ Objlist_Entry * * on failure. */ Obj_Entry * -_rtld_load_object(const char *filepath, int flags) +_rtld_load_object(const char *filepath, int flags, sigset_t *mask) { Obj_Entry *obj; int fd = -1; struct stat sb; size_t pathlen = strlen(filepath); - for (obj = _rtld_objlist->next; obj != NULL; obj = obj->next) - if (pathlen == obj->pathlen && !strcmp(obj->path, filepath)) +restart: + /* + * Search the list of objects for a matching path. If we find + * a match, but it's concurrently running destructors, wait for + * it with the rtld exclusive lock dropped and start over. + */ + for (obj = _rtld_objlist->next; obj != NULL; obj = obj->next) { + if (pathlen == obj->pathlen && !strcmp(obj->path, filepath)) { + if (__predict_false(_rtld_wait_for_fini(&obj, mask))) + goto restart; + assert(obj->refcount > 0); break; + } + } /* * If we didn't find a match by pathname, open the file and check @@ -143,6 +154,10 @@ Obj_Entry * for (obj = _rtld_objlist->next; obj != NULL; obj = obj->next) { if (obj->ino == sb.st_ino && obj->dev == sb.st_dev) { close(fd); + if (__predict_false(_rtld_wait_for_fini(&obj, + mask))) + goto restart; + assert(obj->refcount > 0); break; } } @@ -179,6 +194,7 @@ Obj_Entry * _rtld_objtail = &obj->next; _rtld_objcount++; _rtld_objloads++; + _rtld_objrelocpending++; #ifdef RTLD_LOADER _rtld_linkmap_add(obj); /* for the debugger */ #endif @@ -186,6 +202,8 @@ Obj_Entry * obj->mapbase + obj->mapsize - 1, obj->path)); if (obj->textrel) dbg((" WARNING: %s has impure text", obj->path)); + } else { + assert(obj->refcount > 0); } ++obj->refcount; @@ -206,7 +224,7 @@ Obj_Entry * static bool _rtld_load_by_name(const char *name, Obj_Entry *obj, Needed_Entry **needed, - int flags) + int flags, sigset_t *mask) { Library_Xform *x = _rtld_xforms; Obj_Entry *o; @@ -219,13 +237,31 @@ static bool char s[16]; } val; + /* + * Caller must hold a reference to prevent concurrent dlclose + * from unloading obj until we're done, even if we + * unlock/wait/relock to wait for a dependency that is being + * concurrently unloaded. + */ + assert(obj->neededrefcount); + dbg(("load by name %s %p", name, x)); - for (o = _rtld_objlist->next; o != NULL; o = o->next) +restart: + /* + * Search the list of objects for a matching path. If we find + * a match, but it's concurrently running destructors, wait for + * it with the rtld exclusive lock dropped and start over. + */ + for (o = _rtld_objlist->next; o != NULL; o = o->next) { if (_rtld_object_match_name(o, name)) { + if (__predict_false(_rtld_wait_for_fini(&o, mask))) + goto restart; + assert(o->refcount > 0); ++o->refcount; (*needed)->obj = o; return true; } + } for (; x; x = x->next) { if (strcmp(x->name, name) != 0) @@ -270,7 +306,7 @@ static bool for (j = 0; j < RTLD_MAX_LIBRARY && x->entry[i].library[j] != NULL; j++) { o = _rtld_load_library(x->entry[i].library[j], obj, - flags); + flags, mask); if (o == NULL) { xwarnx("could not load %s for %s", x->entry[i].library[j], name); @@ -296,7 +332,8 @@ static bool if (got) return true; - return ((*needed)->obj = _rtld_load_library(name, obj, flags)) != NULL; + return ((*needed)->obj = _rtld_load_library(name, obj, flags, mask)) + != NULL; } @@ -306,7 +343,7 @@ static bool * returns -1 on failure. */ int -_rtld_load_needed_objects(Obj_Entry *first, int flags) +_rtld_load_needed_objects(Obj_Entry *first, int flags, sigset_t *mask) { Obj_Entry *obj; int status = 0; @@ -314,18 +351,34 @@ int for (obj = first; obj != NULL; obj = obj->next) { Needed_Entry *needed; + /* + * If obj is already being unloaded, there is no need + * to load its dependencies, so just skip it. + */ + if (__predict_false(obj->refcount == 0)) + continue; + + /* + * Prevent obj from being concurrently unloaded until + * we're done. We hold the rtld exclusive lock right + * now, but _rtld_load_by_name may drop it. + */ + _rtld_load_needed_enter(obj); + for (needed = obj->needed; needed != NULL; needed = needed->next) { const char *name = obj->strtab + needed->name; #ifdef RTLD_LOADER Obj_Entry *nobj; #endif + if (__predict_false(needed->obj != NULL)) + continue; if (!_rtld_load_by_name(name, obj, &needed, - flags & ~_RTLD_NOLOAD)) + flags & ~_RTLD_NOLOAD, mask)) status = -1; /* FIXME - cleanup */ #ifdef RTLD_LOADER if (status == -1) - return status; + break; if (flags & _RTLD_MAIN) continue; @@ -338,6 +391,17 @@ int } #endif } + + /* + * Allow obj to be concurrently unloaded now that we're + * done loading its dependencies. + */ + _rtld_load_needed_exit(obj); + +#ifdef RTLD_LOADER + if (status == -1) + break; +#endif } return status; @@ -345,7 +409,7 @@ int #ifdef RTLD_LOADER int -_rtld_preload(const char *preload_path) +_rtld_preload(const char *preload_path, sigset_t *mask) { const char *path; char *cp, *buf; @@ -354,7 +418,7 @@ int if (preload_path != NULL && *preload_path != '\0') { cp = buf = xstrdup(preload_path); while ((path = strsep(&cp, " :")) != NULL && status == 0) { - if (!_rtld_load_object(path, _RTLD_MAIN)) + if (!_rtld_load_object(path, _RTLD_MAIN, mask)) status = -1; else dbg((" preloaded \"%s\"", path)); diff -r 197592d4e399 -r 7022496b52e3 libexec/ld.elf_so/reloc.c --- a/libexec/ld.elf_so/reloc.c Thu Jun 25 00:59:42 2026 +0000 +++ b/libexec/ld.elf_so/reloc.c Thu Jun 25 19:20:03 2026 +0000 @@ -266,6 +266,10 @@ int int ok = 1; for (obj = first; obj != NULL; obj = obj->next) { + if (obj->relocated) + continue; + obj->relocated = true; + _rtld_objrelocpending--; if ((!obj->sysv_hash && !obj->gnu_hash) || obj->symtab == NULL || obj->strtab == NULL) { _rtld_error("%s: Shared object has no run-time" diff -r 197592d4e399 -r 7022496b52e3 libexec/ld.elf_so/rtld.c --- a/libexec/ld.elf_so/rtld.c Thu Jun 25 00:59:42 2026 +0000 +++ b/libexec/ld.elf_so/rtld.c Thu Jun 25 19:20:03 2026 +0000 @@ -97,6 +97,7 @@ Obj_Entry **_rtld_objtail; /* Link f Obj_Entry *_rtld_objmain; /* The main program shared object */ Obj_Entry _rtld_objself; /* The dynamic linker shared object */ u_int _rtld_objcount; /* Number of objects in _rtld_objlist */ +u_int _rtld_objrelocpending = 1; /* Number of objects pending reloc */ u_int _rtld_objloads; /* Number of objects loaded in _rtld_objlist */ u_int _rtld_objgen; /* Generation count for _rtld_objlist */ const char _rtld_path[] = _PATH_RTLD; @@ -146,6 +147,325 @@ static void _rtld_unref_dag(Obj_Entry *) static Obj_Entry *_rtld_obj_from_addr(const void *); static void _rtld_fill_dl_phdr_info(const Obj_Entry *, struct dl_phdr_info *); +/* + * _rtld_load_needed_enter(obj) + * + * Mark obj as busy loading its dependencies. Multiple threads + * may be working on a single thread's dependencies concurrently; + * dlclose will wait until they are all done. Caller must follow + * this by _rtld_load_needed_enter. + * + * Non-reentrant: a thread must not call this again until it has + * called _rtld_load_needed_exit. + * + * Caller must hold the rtld exclusive lock. obj must have + * positive refcount; if it is already slated for destruction, + * this is not useful. + */ +void +_rtld_load_needed_enter(Obj_Entry *obj) +{ + + assert(obj->refcount > 0); + assert(obj->neededrefcount < INT_MAX); + obj->neededrefcount++; +} + +/* + * _rtld_load_needed_exit(obj) + * + * Mark obj as no longer busy loading its dependencies after + * _rtld_load_needed_enter. + * + * Caller must hold the rtld exclusive lock. Will not release or + * reacquire it. + * + * Caller must have previously called + * _rtld_loadingneeded_enter(obj, ...) in the same thread. + */ +void +_rtld_load_needed_exit(Obj_Entry *obj) +{ + + assert(obj->neededrefcount > 0); + + if (__predict_false(--obj->neededrefcount)) + return; + if (__predict_true(obj->neededwaiter == 0)) + return; + assert(obj->refcount == 0); + _lwp_unpark(obj->neededwaiter, &obj->neededrefcount); + obj->neededwaiter = 0; +} + +/* + * _rtld_wait_for_load_needed(&obj, mask) + * + * If another thread is concurrently loading obj's dependencies, + * release the rtld exclusive lock, wait until it is done, + * reacquire the rtld exclusive lock, and return true. On + * return, obj is nulled out. + * + * Otherwise, if there is no thread concurrently loading obj's + * dependencies, leave it intact and return false without + * releasing and reacquiring the rtld exclusive lock -- obj is + * safe to free now. + * + * Caller must hold the rtld exclusive lock. May release and + * reacquire the rtld exclusive lock. obj must have refcount zero + * already; this is only for when we are preparing to free obj. + */ +static bool +_rtld_wait_for_load_needed(Obj_Entry **objp, sigset_t *mask) +{ + Obj_Entry *obj = *objp; + lwpid_t next; + + /* + * This is only useful when obj is already marked for + * destruction. + */ + assert(obj->refcount == 0); + + /* + * If there are no threads concurrently loading obj's + * dependencies, nothing to do. + */ + if (__predict_true(obj->neededrefcount == 0)) + return false; + + /* + * Queue ourselves up to be notified when all threads are done + * loading obj's dependencies, and remember the next thread to + * be notified. + */ + next = obj->neededwaiter; + obj->neededwaiter = _lwp_self(); + + /* + * Release the rtld exclusive lock to wait and reacquire it + * when done. After we release the lock, we can't dereference + * obj -- it may be concurrently freed by dlclose. + */ + _rtld_exclusive_exit(mask); + *objp = NULL; + _lwp_park(CLOCK_REALTIME, 0, NULL, 0, &obj->neededrefcount, NULL); + _rtld_exclusive_enter(mask); + + /* + * If another thread was waiting too, notify that thread. + */ + if (next) + _lwp_unpark(next, &obj->neededrefcount); + + /* + * Notify the caller that we released/reacquired the rtld + * exclusive lock to wait for a state change so they must start + * over from the top. + */ + return true; +} + +/* + * _rtld_initfini_enter(&obj, mask) + * + * Prepare to call an init/fini routine and return true if the + * caller should do it and then call _rtld_initfini_exit, or false + * if we waited for a state change and the caller must start over + * from the top. + * + * If another thread is concurrently running an init/fini routine + * for the same object, release the rtld exclusive lock, wait + * until it's done (or a spurious wakeup), reacquire the rtld + * exclusive lock, null out obj, and return false. Returning + * false does _not_ imply the init/fini is done -- it only implies + * that it _may_ be done but the caller must reassess the rtld + * state and start over from the top. + * + * Otherwise, mark obj as running an init/fini routine in this + * thread and return true, without releasing and reacquiring the + * rtld exclusive lock. + * + * Caller must hold the rtld exclusive lock. May release and + * reacquire the rtld exclusive lock. + */ +static bool +_rtld_initfini_enter(Obj_Entry **objp, sigset_t *mask) +{ + Obj_Entry *obj = *objp; + lwpid_t next; + + /* + * If no other thread is concurrently running an init/fini + * routine for this object, claim the object for this thread + * and return true without releasing or reacquiring the rtld + * exclusive lock. + */ + if (__predict_true(obj->initfinilock == 0)) { + obj->initfinilock = _lwp_self(); + return true; + } + + /* + * Remember whether anyone else is waiting for the lock, and + * record ourselves as waiting. + */ + next = obj->initfinilockwaiter; + obj->initfinilockwaiter = _lwp_self(); + + /* + * Release the rtld exclusive lock, wait for a state change, + * and reacquire the rtld exclusive lock. Must not touch obj + * after releasing the rtld exclusive lock -- it may be + * concurrently freed by dlclose. + */ + _rtld_exclusive_exit(mask); + *objp = NULL; + _lwp_park(CLOCK_REALTIME, 0, NULL, 0, &obj->initfinilock, NULL); + _rtld_exclusive_enter(mask); + + /* + * If anyone else was waiting for the lock, wake them too. + */ + if (next) + _lwp_unpark(next, &obj->initfinilock); + + /* + * Notify the caller we failed to claim the object and + * released/reacquired the lock to wait for a state change so + * they must start over from the top. + */ + return false; +} + +/* + * _rtld_initfini_exit(obj) + * + * Mark obj as no longer running an init/fini routine in this + * thread, and wake any threads waiting for _rtld_initfini_enter + * on it. + * + * Caller must hold the rtld exclusive lock. Will not release or + * reacquire it. + * + * Caller must have previously called _rtld_initfini_enter(&obj, + * ...) in the same thread, and it must have returned true. + */ +static void +_rtld_initfini_exit(Obj_Entry *obj) +{ + + /* + * We had better have claimed this object. Relinquish our + * claim. + */ + assert(obj->initfinilock == _lwp_self()); + obj->initfinilock = 0; + + /* + * If there's anyone waiting for the lock, wake them. This may + * provoke a thundering herd but it's unlikely that there will + * be much contention on dlopen/dlclose in the real world. + */ + if (__predict_true(obj->initfinilockwaiter == 0)) + return; + _lwp_unpark(obj->initfinilockwaiter, &obj->initfinilock); + obj->initfinilockwaiter = 0; +} + +/* + * _rtld_fini_done(obj) + * + * Mark obj as done running destructors. Wake any waiters in + * _rtld_wait_for_fini(&obj, ...). + * + * Caller must hold the rtld exclusive lock. Will not release or + * reacquire it. + */ +static void +_rtld_fini_done(Obj_Entry *obj) +{ + + assert(obj->refcount == 0); + if (__predict_true(obj->finiwaiter == 0)) + return; + _lwp_unpark(obj->finiwaiter, &obj->refcount); + obj->finiwaiter = 0; +} + +/* + * _rtld_wait_for_fini(&obj, mask) + * + * If another thread is concurrently running destructors for obj, + * release the rtld exclusive lock, wait for that to complete, + * reacquire the rtld exclusive lock, and return true. On return, + * obj is nulled out -- it was in the process of being destroyed + * when we started and it may be completely gone by the time we + * return. + * + * Otherwise, if there is no thread concurrently running + * destructors for obj, leave it intact and return false without + * releasing and reacquiring the rtld exclusive lock -- obj is + * safe to use. + * + * Caller must either hold the rtld exclusive lock, or be + * single-threaded; if single-threaded, this is guaranteed to + * return false, and mask may be null. + */ +bool +_rtld_wait_for_fini(Obj_Entry **objp, sigset_t *mask) +{ + Obj_Entry *obj = *objp; + lwpid_t next; + + /* + * If the object is still referenced, it can't be in the + * process of destruction, so nothing to do -- notify the + * caller we didn't wait. + */ + if (__predict_true(obj->refcount > 0)) + return false; + + /* + * We can only reach this point if there are threads running + * dlopen or dlclose concurrently. This can't happen during + * initial program load -- pthread_create is not available for + * use in a constructor -- so initial program load can skip + * taking the rtld exclusive lock. + */ + assert(mask != NULL); + + /* + * Queue ourselves up to be notified when concurrent fini is + * done, and remember the next thread to be notified. + */ + next = obj->finiwaiter; + obj->finiwaiter = _lwp_self(); + + /* + * Release the rtld exclusive lock to wait and reacquire it + * when done. After we release the lock, we can't dereference + * obj -- it may be concurrently freed by dlclose. + */ + _rtld_exclusive_exit(mask); + *objp = NULL; + _lwp_park(CLOCK_REALTIME, 0, NULL, 0, &obj->refcount, NULL); + _rtld_exclusive_enter(mask); + + /* + * If another thread was waiting too, notify that thread. + */ + if (next) + _lwp_unpark(next, &obj->refcount); + + /* + * Notify the caller that we released/reacquired the rtld + * exclusive lock to wait for a state change so they must start + * over from the top. + */ + return true; +} + static inline void _rtld_call_initfini_function(fptr_t func, sigset_t *mask) { @@ -173,8 +493,11 @@ static void * start to end. We need to make restartable so just advance * the array pointer and decrement the size each time through * the loop. + * + * Paranoia: avoid touching obj if the generation has changed. */ - while (obj->fini_arraysz > 0 && _rtld_objgen == cur_objgen) { + while (__predict_true(_rtld_objgen == cur_objgen) && + obj->fini_arraysz > 0) { fptr_t fini = *obj->fini_array++; obj->fini_arraysz--; dbg (("calling fini array function %s at %p%s", obj->path, @@ -201,33 +524,37 @@ restart: /* First pass: objects _not_ marked with DF_1_INITFIRST. */ SIMPLEQ_FOREACH(elm, &finilist, link) { - Obj_Entry * const obj = elm->obj; + Obj_Entry *obj = elm->obj; if (!obj->z_initfirst) { if (obj->refcount > 0 && !force) { continue; } - /* - * XXX This can race against a concurrent dlclose(). - * XXX In that case, the object could be unmapped before - * XXX the fini() call or the fini_array has completed. - */ + if (!_rtld_initfini_enter(&obj, mask)) { + _rtld_objlist_clear(&finilist); + goto restart; + } _rtld_call_fini_function(obj, mask, cur_objgen); + _rtld_initfini_exit(obj); if (_rtld_objgen != cur_objgen) { dbg(("restarting fini iteration")); _rtld_objlist_clear(&finilist); goto restart; - } + } } } /* Second pass: objects marked with DF_1_INITFIRST. */ SIMPLEQ_FOREACH(elm, &finilist, link) { - Obj_Entry * const obj = elm->obj; + Obj_Entry *obj = elm->obj; if (obj->refcount > 0 && !force) { continue; } - /* XXX See above for the race condition here */ + if (!_rtld_initfini_enter(&obj, mask)) { + _rtld_objlist_clear(&finilist); + goto restart; + } _rtld_call_fini_function(obj, mask, cur_objgen); + _rtld_initfini_exit(obj); if (_rtld_objgen != cur_objgen) { dbg(("restarting fini iteration")); _rtld_objlist_clear(&finilist); @@ -273,6 +600,8 @@ static void static bool _rtld_call_ifunc_functions(sigset_t *mask, Obj_Entry *obj, u_int cur_objgen) { + if (!_rtld_initfini_enter(&obj, mask)) + return true; if (obj->ifunc_remaining #if defined(IFUNC_NONPLT) || obj->ifunc_remaining_nonplt @@ -280,9 +609,11 @@ static bool ) { _rtld_call_ifunc(obj, mask, cur_objgen); if (_rtld_objgen != cur_objgen) { + _rtld_initfini_exit(obj); return true; } } + _rtld_initfini_exit(obj); return false; } @@ -321,7 +652,12 @@ restart: /* First pass: objects with IRELATIVE relocations. */ SIMPLEQ_FOREACH(elm, &initlist, link) { - if (_rtld_call_ifunc_functions(mask, elm->obj, cur_objgen)) { + Obj_Entry *obj = elm->obj; + if (__predict_false(_rtld_wait_for_fini(&obj, mask))) { + _rtld_objlist_clear(&initlist); + goto restart; + } + if (_rtld_call_ifunc_functions(mask, obj, cur_objgen)) { dbg(("restarting init iteration")); _rtld_objlist_clear(&initlist); goto restart; @@ -332,6 +668,7 @@ restart: * from crt0. Don't introduce that mistake for ifunc, so look at * the head of _rtld_objlist that _rtld_initlist_tsort skipped. */ + assert(_rtld_objlist->refcount != 0); if (_rtld_call_ifunc_functions(mask, _rtld_objlist, cur_objgen)) { dbg(("restarting init iteration")); _rtld_objlist_clear(&initlist); @@ -340,9 +677,18 @@ restart: /* Second pass: objects marked with DF_1_INITFIRST. */ SIMPLEQ_FOREACH(elm, &initlist, link) { - Obj_Entry * const obj = elm->obj; + Obj_Entry *obj = elm->obj; + if (__predict_false(_rtld_wait_for_fini(&obj, mask))) { + _rtld_objlist_clear(&initlist); + goto restart; + } if (obj->z_initfirst) { + if (!_rtld_initfini_enter(&obj, mask)) { + _rtld_objlist_clear(&initlist); + goto restart; + } _rtld_call_init_function(obj, mask, cur_objgen); + _rtld_initfini_exit(obj); if (_rtld_objgen != cur_objgen) { dbg(("restarting init iteration")); _rtld_objlist_clear(&initlist); @@ -353,7 +699,17 @@ restart: /* Third pass: all other objects. */ SIMPLEQ_FOREACH(elm, &initlist, link) { - _rtld_call_init_function(elm->obj, mask, cur_objgen); + Obj_Entry *obj = elm->obj; + if (__predict_false(_rtld_wait_for_fini(&obj, mask))) { + _rtld_objlist_clear(&initlist); + goto restart; + } + if (!_rtld_initfini_enter(&obj, mask)) { + _rtld_objlist_clear(&initlist); + goto restart; + } + _rtld_call_init_function(obj, mask, cur_objgen); + _rtld_initfini_exit(obj); if (_rtld_objgen != cur_objgen) { dbg(("restarting init iteration")); _rtld_objlist_clear(&initlist); @@ -748,12 +1104,12 @@ Elf_Addr * but before any shared object dependencies. */ dbg(("preloading objects")); - if (_rtld_preload(ld_preload) == -1) + if (_rtld_preload(ld_preload, NULL) == -1) _rtld_die(); } dbg(("loading needed objects")); - if (_rtld_load_needed_objects(_rtld_objmain, _RTLD_MAIN) == -1) + if (_rtld_load_needed_objects(_rtld_objmain, _RTLD_MAIN, NULL) == -1) _rtld_die(); dbg(("checking for required versions")); @@ -893,6 +1249,21 @@ static void /* dbg(("_rtld_initlist_visit(%s)", obj->path)); */ + /* + * If the object hasn't been relocated yet, we have nothing to + * do -- another thread is running dlopen, and will get to it + * eventually, but it isn't ordered with respect to any + * initializers we have to run. + */ + if (!obj->relocated) + return; + + /* + * If the object has already been visited, we have nothing to + * do, so skip it. Note: This is reset at the beginning of + * _rtld_initlist_topsort; it does not reflect whether the + * initializers have actually run yet. + */ if (obj->init_done) return; obj->init_done = 1; @@ -968,9 +1339,42 @@ static void Obj_Entry **linkp; Objlist_Entry *elm; - /* Finalize objects that are about to be unmapped. */ - if (do_fini_funcs) + assert(root->dl_refcount == 0); + assert(root->dlclosed); + assert(!root->z_nodelete); + assert(!root->ref_nodel); + + /* + * A concurrent dlopen of some other library might have + * picked up this object while loading needed entries. + * Wait for that to complete. The root can't go away + * at this point: we already hold the last actual + * reference to it. + */ + obj = root; + (void)_rtld_wait_for_load_needed(&obj, mask); + assert(root->refcount == 0); + assert(root->dl_refcount == 0); + assert(root->dlclosed); + assert(!root->z_nodelete); + assert(!root->ref_nodel); + + /* + * Finalize objects that are about to be unmapped. Set + * root->dlclosing while we do this so concurrent + * dlclose calls, which can run while the rtld + * exclusive lock is dropped across fini calls, will + * skip this when garbage-collecting the object list. + */ + if (do_fini_funcs) { + root->dlclosing = true; _rtld_call_fini_functions(mask, 0); + assert(root->dlclosing); + assert(root->dlclosed); + assert(root->refcount == 0); + assert(root->dl_refcount == 0); + root->dlclosing = false; + } /* Remove the DAG from all objects' DAG lists. */ SIMPLEQ_FOREACH(elm, &root->dagmembers, link) @@ -982,10 +1386,30 @@ static void _rtld_objlist_remove(&_rtld_list_global, root); } - /* Unmap all objects that are no longer referenced. */ + /* + * Unmap all objects that are no longer referenced. + * + * Objects that are unreferenced but have dlclosing or + * initfinilock set must be in use in a concurrent call + * to _rtld_unload_object, which will go through the + * list of objects to unmap when it is done, so we skip + * them -- this avoids pulling the rug out from under + * the concurrent call, and won't leak. + * + * For objects that are still having their dependencies + * loaded, we have to wait until the loading is done -- + * and while we're waiting, another thread might free + * it, so we have to start over from the top. + */ +restart: linkp = &_rtld_objlist->next; while ((obj = *linkp) != NULL) { - if (obj->refcount == 0) { + if (obj->refcount == 0 && + !obj->dlclosing && + obj->initfinilock == 0) { + if (__predict_false( + _rtld_wait_for_load_needed(&obj, mask))) + goto restart; dbg(("unloading \"%s\"", obj->path)); if (obj->ehdr != MAP_FAILED) munmap(obj->ehdr, _rtld_pagesz); @@ -994,6 +1418,7 @@ static void _rtld_linkmap_delete(obj); *linkp = obj->next; _rtld_objcount--; + _rtld_fini_done(obj); _rtld_obj_free(obj); } else linkp = &obj->next; @@ -1008,6 +1433,8 @@ void const Needed_Entry *needed; assert(root); + assert(root->refcount > 0); + assert(!root->dlclosed); ++root->refcount; @@ -1059,11 +1486,16 @@ dlclose(void *handle) _rtld_exclusive_exit(&mask); return -1; } + assert(root->refcount != 0); + assert(root->dl_refcount != 0); + assert(!root->dlclosed); _rtld_debug.r_state = RT_DELETE; _rtld_debug_state(); - --root->dl_refcount; + if (--root->dl_refcount == 0) + root->dlclosed = true; + assert(root->refcount != 0); _rtld_unload_object(&mask, root, true); _rtld_debug.r_state = RT_CONSISTENT; @@ -1088,7 +1520,6 @@ dlerror(void) void * dlopen(const char *name, int mode) { - Obj_Entry **old_obj_tail; Obj_Entry *obj = NULL; int flags = _RTLD_DLOPEN; bool nodelete; @@ -1100,8 +1531,6 @@ dlopen(const char *name, int mode) _rtld_exclusive_enter(&mask); - old_obj_tail = _rtld_objtail; - flags |= (mode & RTLD_GLOBAL) ? _RTLD_GLOBAL : 0; flags |= (mode & RTLD_NOLOAD) ? _RTLD_NOLOAD : 0; @@ -1113,17 +1542,17 @@ dlopen(const char *name, int mode) if (name == NULL) { obj = _rtld_objmain; + assert(obj->refcount > 0); obj->refcount++; } else - obj = _rtld_load_library(name, _rtld_objmain, flags); - + obj = _rtld_load_library(name, _rtld_objmain, flags, &mask); if (obj != NULL) { + assert(obj->refcount > 0); + assert(!obj->dlclosed); ++obj->dl_refcount; - if (*old_obj_tail != NULL) { /* We loaded something new. */ - assert(*old_obj_tail == obj); - - result = _rtld_load_needed_objects(obj, flags); + if (_rtld_objrelocpending) { /* We loaded something new. */ + result = _rtld_load_needed_objects(obj, flags, &mask); if (result != -1) { Objlist_Entry *entry; _rtld_init_dag(obj); @@ -1133,10 +1562,10 @@ dlopen(const char *name, int mode) break; } } - if (result == -1 || _rtld_relocate_objects(obj, + if (result == -1 || _rtld_relocate_objects(_rtld_objlist, (now || obj->z_now)) == -1) { + obj->dl_refcount--; _rtld_unload_object(&mask, obj, false); - obj->dl_refcount--; obj = NULL; } else { _rtld_call_init_functions(&mask); @@ -1593,8 +2022,10 @@ void else if (delta < 0 && obj->cxa_refcount < -1 + (size_t)-(delta + 1)) _rtld_error("Reference count underflow"); else { - if (obj->cxa_refcount == 0) + if (obj->cxa_refcount == 0) { + assert(obj->refcount > 0); ++obj->refcount; + } obj->cxa_refcount += delta; dbg(("new reference count: %zu", obj->cxa_refcount)); if (obj->cxa_refcount == 0) { diff -r 197592d4e399 -r 7022496b52e3 libexec/ld.elf_so/rtld.h --- a/libexec/ld.elf_so/rtld.h Thu Jun 25 00:59:42 2026 +0000 +++ b/libexec/ld.elf_so/rtld.h Thu Jun 25 19:20:03 2026 +0000 @@ -329,6 +329,14 @@ typedef struct Struct_Obj_Entry { void *exidx_start; size_t exidx_sz; #endif + int finiwaiter; /* dlopen thread waiting for dlclose */ + int initfinilock; /* thread running init/fini */ + int initfinilockwaiter; /* thread waiting to init/fini */ + int neededrefcount; /* #threads loading obj's deps */ + int neededwaiter; /* thread waiting for neededrefcount */ + bool relocated; /* true if already relocated */ + bool dlclosing; /* true if being dlclosed now */ + bool dlclosed; /* true if dlclosed */ } Obj_Entry; typedef struct Struct_DoneList { @@ -344,6 +352,7 @@ extern struct r_debug _rtld_debug; extern Search_Path *_rtld_default_paths; extern Obj_Entry *_rtld_objlist; extern Obj_Entry **_rtld_objtail; +extern u_int _rtld_objrelocpending; extern u_int _rtld_objcount; extern u_int _rtld_objloads; extern const uintptr_t _rtld_compat_obj[]; @@ -408,6 +417,10 @@ void _rtld_objlist_push_tail(Objlist *, Objlist_Entry *_rtld_objlist_find(Objlist *, const Obj_Entry *); void _rtld_ref_dag(Obj_Entry *); +void _rtld_load_needed_enter(Obj_Entry *); +void _rtld_load_needed_exit(Obj_Entry *); +bool _rtld_wait_for_fini(Obj_Entry **, sigset_t *); + void _rtld_shared_enter(void); void _rtld_shared_exit(void); void _rtld_exclusive_enter(sigset_t *); @@ -424,9 +437,9 @@ void _rtld_digest_dynamic(const char *, Obj_Entry *_rtld_digest_phdr(const Elf_Phdr *, int, caddr_t); /* load.c */ -Obj_Entry *_rtld_load_object(const char *, int); -int _rtld_load_needed_objects(Obj_Entry *, int); -int _rtld_preload(const char *); +Obj_Entry *_rtld_load_object(const char *, int, sigset_t *); +int _rtld_load_needed_objects(Obj_Entry *, int, sigset_t *); +int _rtld_preload(const char *, sigset_t *); /* arch//fixup.c */ int _rtld_map_segment_fixup(const char *, int, Elf_Ehdr *, Elf_Phdr *, @@ -452,7 +465,8 @@ Elf_Addr _rtld_resolve_ifunc2(const Obj_ void _rtld_call_ifunc(Obj_Entry *, sigset_t *, u_int); /* search.c */ -Obj_Entry *_rtld_load_library(const char *, const Obj_Entry *, int); +Obj_Entry *_rtld_load_library(const char *, const Obj_Entry *, int, + sigset_t *); /* symbol.c */ const Elf_Sym *_rtld_symlook_obj(const char *, Elf_Hash *, diff -r 197592d4e399 -r 7022496b52e3 libexec/ld.elf_so/search.c --- a/libexec/ld.elf_so/search.c Thu Jun 25 00:59:42 2026 +0000 +++ b/libexec/ld.elf_so/search.c Thu Jun 25 19:20:03 2026 +0000 @@ -63,11 +63,11 @@ static Search_Path *_rtld_invalid_paths; static Obj_Entry *_rtld_search_library_path(const char *, size_t, - const char *, size_t, int); + const char *, size_t, int, sigset_t *); static Obj_Entry * _rtld_search_library_path(const char *name, size_t namelen, - const char *dir, size_t dirlen, int flags) + const char *dir, size_t dirlen, int flags, sigset_t *mask) { char pathname[MAXPATHLEN]; size_t pathnamelen; @@ -93,7 +93,7 @@ static Obj_Entry * pathname[pathnamelen] = '\0'; dbg((" Trying \"%s\"", pathname)); - obj = _rtld_load_object(pathname, flags); + obj = _rtld_load_object(pathname, flags, mask); if (obj == NULL) { Search_Path *path; @@ -115,7 +115,8 @@ static Obj_Entry * * loaded shared object, whose library search path will be searched. */ Obj_Entry * -_rtld_load_library(const char *name, const Obj_Entry *refobj, int flags) +_rtld_load_library(const char *name, const Obj_Entry *refobj, int flags, + sigset_t *mask) { extern char *__progname; char tmperror[512], *tmperrorp; @@ -146,18 +147,19 @@ Obj_Entry * for (sp = _rtld_paths; sp != NULL; sp = sp->sp_next) if ((obj = _rtld_search_library_path(name, namelen, - sp->sp_path, sp->sp_pathlen, flags)) != NULL) + sp->sp_path, sp->sp_pathlen, flags, mask)) != NULL) goto pathfound; if (refobj != NULL) for (sp = refobj->rpaths; sp != NULL; sp = sp->sp_next) if ((obj = _rtld_search_library_path(name, - namelen, sp->sp_path, sp->sp_pathlen, flags)) != NULL) + namelen, sp->sp_path, sp->sp_pathlen, flags, mask)) + != NULL) goto pathfound; for (sp = _rtld_default_paths; sp != NULL; sp = sp->sp_next) if ((obj = _rtld_search_library_path(name, namelen, - sp->sp_path, sp->sp_pathlen, flags)) != NULL) + sp->sp_path, sp->sp_pathlen, flags, mask)) != NULL) goto pathfound; _rtld_error("%s: Shared object \"%s\" not found", @@ -185,7 +187,7 @@ pathfound: return obj; found: - obj = _rtld_load_object(pathname, flags); + obj = _rtld_load_object(pathname, flags, mask); if (obj == OBJ_ERR) return NULL; diff -r 197592d4e399 -r 7022496b52e3 tests/libexec/ld.elf_so/t_dlclose_thread.c --- a/tests/libexec/ld.elf_so/t_dlclose_thread.c Thu Jun 25 00:59:42 2026 +0000 +++ b/tests/libexec/ld.elf_so/t_dlclose_thread.c Thu Jun 25 19:20:03 2026 +0000 @@ -83,8 +83,6 @@ ATF_TC_BODY(dlclose_thread, tc) RZ(pthread_create(&t[i], NULL, &dlclose_thread, (void *)(uintptr_t)i)); } - atf_tc_expect_signal(-1, "PR lib/59751:" - " dlclose is not MT-safe depending on the libraries unloaded"); (void)pthread_barrier_wait(&bar); (void)sleep(5); atomic_store_explicit(&stop, true, memory_order_relaxed); # HG changeset patch # User Taylor R Campbell # Date 1763936859 0 # Sun Nov 23 22:27:39 2025 +0000 # Branch trunk # Node ID 52085066c1f633534df755a3c3955ce1e77c1f8b # Parent 7022496b52e310c4e250b3864fc738708de5963a # EXP-Topic riastradh-pr59751-dlcloserace ld.elf_so(1): Bump _rtld_objgen when changing, not reading, objlist. Prompted by: PR lib/59751: dlclose is not MT-safe depending on the libraries unloaded diff -r 7022496b52e3 -r 52085066c1f6 libexec/ld.elf_so/load.c --- a/libexec/ld.elf_so/load.c Thu Jun 25 19:20:03 2026 +0000 +++ b/libexec/ld.elf_so/load.c Sun Nov 23 22:27:39 2025 +0000 @@ -192,6 +192,7 @@ restart: *_rtld_objtail = obj; _rtld_objtail = &obj->next; + _rtld_objgen++; _rtld_objcount++; _rtld_objloads++; _rtld_objrelocpending++; diff -r 7022496b52e3 -r 52085066c1f6 libexec/ld.elf_so/rtld.c --- a/libexec/ld.elf_so/rtld.c Thu Jun 25 19:20:03 2026 +0000 +++ b/libexec/ld.elf_so/rtld.c Sun Nov 23 22:27:39 2025 +0000 @@ -518,7 +518,7 @@ static void dbg(("_rtld_call_fini_functions(%d)", force)); restart: - cur_objgen = ++_rtld_objgen; + cur_objgen = _rtld_objgen; SIMPLEQ_INIT(&finilist); _rtld_initlist_tsort(&finilist, 1); @@ -646,7 +646,7 @@ static void dbg(("_rtld_call_init_functions()")); restart: - cur_objgen = ++_rtld_objgen; + cur_objgen = _rtld_objgen; SIMPLEQ_INIT(&initlist); _rtld_initlist_tsort(&initlist, 0); @@ -1424,6 +1424,7 @@ restart: linkp = &obj->next; } _rtld_objtail = linkp; + _rtld_objgen++; } }