# HG changeset patch # User Taylor R Campbell # Date 1763931895 0 # Sun Nov 23 21:04:55 2025 +0000 # Branch trunk # Node ID 597b4a78df0fcd0c41c0748889efe3b4a3b2a69e # Parent 3898bf9c9139ed5dc3951a075c1796379237c487 # EXP-Topic riastradh-pr59751-dlcloserace ld.elf_so(1): Test concurrent dlopen/dlclose. PR lib/59751: dlclose is not MT-safe depending on the libraries unloaded diff -r 3898bf9c9139 -r 597b4a78df0f distrib/sets/lists/debug/mi --- a/distrib/sets/lists/debug/mi Sat Nov 22 13:03:22 2025 +0000 +++ b/distrib/sets/lists/debug/mi Sun Nov 23 21:04:55 2025 +0000 @@ -2475,6 +2475,7 @@ ./usr/libdata/debug/usr/tests/libexec/ld.elf_so/h_r_rel_pack.debug tests-libexec-debug debug,atf,pic,compattestfile ./usr/libdata/debug/usr/tests/libexec/ld.elf_so/libh_abuse_dynamic.so.1.debug tests-libexec-debug debug,atf,pic,compattestfile ./usr/libdata/debug/usr/tests/libexec/ld.elf_so/libh_abuse_static.so.1.debug tests-libexec-debug debug,atf,pic,compattestfile +./usr/libdata/debug/usr/tests/libexec/ld.elf_so/t_dlclose_thread.debug tests-libexec-debug debug,atf,pic,compattestfile ./usr/libdata/debug/usr/tests/libexec/ld.elf_so/t_dlerror-cleared.debug tests-libexec-debug debug,atf,pic,compattestfile ./usr/libdata/debug/usr/tests/libexec/ld.elf_so/t_dlerror-false.debug tests-libexec-debug debug,atf,pic,compattestfile ./usr/libdata/debug/usr/tests/libexec/ld.elf_so/t_dlinfo.debug tests-libexec-debug debug,atf,pic,compattestfile diff -r 3898bf9c9139 -r 597b4a78df0f distrib/sets/lists/tests/mi --- a/distrib/sets/lists/tests/mi Sat Nov 22 13:03:22 2025 +0000 +++ b/distrib/sets/lists/tests/mi Sun Nov 23 21:04:55 2025 +0000 @@ -4374,6 +4374,7 @@ ./usr/tests/libexec/ld.elf_so/h_thread_local_dtor tests-libexec-tests compattestfile,atf,pic ./usr/tests/libexec/ld.elf_so/t_df_1_noopen tests-libexec-tests compattestfile,atf,pic ./usr/tests/libexec/ld.elf_so/t_dl_symver tests-libexec-tests compattestfile,atf,pic +./usr/tests/libexec/ld.elf_so/t_dlclose_thread tests-libexec-tests compattestfile,atf,pic ./usr/tests/libexec/ld.elf_so/t_dlerror-cleared tests-libexec-tests compattestfile,atf,pic ./usr/tests/libexec/ld.elf_so/t_dlerror-false tests-libexec-tests compattestfile,atf,pic ./usr/tests/libexec/ld.elf_so/t_dlinfo tests-libexec-tests compattestfile,atf,pic diff -r 3898bf9c9139 -r 597b4a78df0f tests/libexec/ld.elf_so/Makefile --- a/tests/libexec/ld.elf_so/Makefile Sat Nov 22 13:03:22 2025 +0000 +++ b/tests/libexec/ld.elf_so/Makefile Sun Nov 23 21:04:55 2025 +0000 @@ -35,6 +35,7 @@ SUBDIR+= helper_use_static TESTSDIR= ${TESTSBASE}/libexec/ld.elf_so +TESTS_C+= t_dlclose_thread TESTS_C+= t_dlerror-cleared TESTS_C+= t_dlerror-false TESTS_C+= t_dlinfo @@ -52,6 +53,10 @@ SRCS.t_ifunc_now= t_ifunc.c SRCS.t_ifunc_norelro= t_ifunc.c SRCS.t_ifunc_norelro_now= t_ifunc.c +LDADD.t_dlclose_thread+= -lpthread +LDADD.t_dlclose_thread+= -Wl,-rpath,${TESTSDIR} +LDADD.t_dlclose_thread+= -Wl,--export-dynamic + LDADD.t_ifunc+= ${${MKRELRO} != "no":?-Wl,-z,relro:} LDADD.t_ifunc_now+= ${${MKRELRO} != "no":?-Wl,-z,relro:} -Wl,-z,now LDADD.t_ifunc_norelro+= ${${MKRELRO} != "no":?-Wl,-z,norelro:} diff -r 3898bf9c9139 -r 597b4a78df0f tests/libexec/ld.elf_so/t_dlclose_thread.c --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/tests/libexec/ld.elf_so/t_dlclose_thread.c Sun Nov 23 21:04:55 2025 +0000 @@ -0,0 +1,98 @@ +/* $NetBSD$ */ + +/*- + * Copyright (c) 2025 The NetBSD Foundation, Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE NETBSD FOUNDATION, INC. AND CONTRIBUTORS + * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE FOUNDATION OR CONTRIBUTORS + * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + +#include +__RCSID("$NetBSD$"); + +#include +#include +#include +#include +#include + +#include "h_macros.h" + +pthread_barrier_t bar; +atomic_bool stop = false; + +int sleep_init; +int sleep_fini; +int dlopen_cookie; +int dlclose_cookie; + +static const char *const libh_helper_dso[] = { + "libh_helper_dso1.so", + "libh_helper_dso2.so", + "libh_helper_dso3.so", +}; + +static void * +dlclose_thread(void *cookie) +{ + const unsigned i = (uintptr_t)cookie % __arraycount(libh_helper_dso); + void *handle; + + (void)pthread_barrier_wait(&bar); + while (!atomic_load_explicit(&stop, memory_order_relaxed)) { + handle = dlopen(libh_helper_dso[i], RTLD_LAZY | RTLD_LOCAL); + ATF_REQUIRE(handle != NULL); + dlclose(handle); + } + return NULL; +} + +ATF_TC(dlclose_thread); +ATF_TC_HEAD(dlclose_thread, tc) +{ + atf_tc_set_md_var(tc, "descr", + "Test concurrent dlopen and dlclose with destructors"); +} +ATF_TC_BODY(dlclose_thread, tc) +{ + pthread_t t[2*__arraycount(libh_helper_dso)]; + unsigned i; + + RZ(pthread_barrier_init(&bar, NULL, 1 + __arraycount(t))); + for (i = 0; i < __arraycount(t); i++) { + 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(1); + atomic_store_explicit(&stop, true, memory_order_relaxed); + for (i = 0; i < __arraycount(t); i++) + RZ(pthread_join(t[i], NULL)); +} + +ATF_TP_ADD_TCS(tp) +{ + ATF_TP_ADD_TC(tp, dlclose_thread); + return atf_no_error(); +} # HG changeset patch # User Taylor R Campbell # Date 1763934437 0 # Sun Nov 23 21:47:17 2025 +0000 # Branch trunk # Node ID e79b77790bf74b6bcd1dc9b9c6dcf916fece7004 # Parent 597b4a78df0fcd0c41c0748889efe3b4a3b2a69e # 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 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. 4. (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 597b4a78df0f -r e79b77790bf7 libexec/ld.elf_so/load.c --- a/libexec/ld.elf_so/load.c Sun Nov 23 21:04:55 2025 +0000 +++ b/libexec/ld.elf_so/load.c Sun Nov 23 21:47:17 2025 +0000 @@ -63,7 +63,7 @@ __RCSID("$NetBSD: load.c,v 1.49 2020/09/ #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 @@ _rtld_objlist_find(Objlist *list, const * 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 @@ _rtld_load_object(const char *filepath, 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 @@ _rtld_load_object(const char *filepath, _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 @@ _rtld_load_object(const char *filepath, 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 @@ _rtld_load_object(const char *filepath, 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; @@ -220,12 +238,22 @@ _rtld_load_by_name(const char *name, Obj } val; 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 +298,7 @@ _rtld_load_by_name(const char *name, Obj 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 +324,8 @@ _rtld_load_by_name(const char *name, Obj 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 +335,7 @@ _rtld_load_by_name(const char *name, Obj * 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; @@ -321,7 +350,7 @@ _rtld_load_needed_objects(Obj_Entry *fir Obj_Entry *nobj; #endif 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) @@ -345,7 +374,7 @@ _rtld_load_needed_objects(Obj_Entry *fir #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 +383,7 @@ _rtld_preload(const char *preload_path) 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 597b4a78df0f -r e79b77790bf7 libexec/ld.elf_so/reloc.c --- a/libexec/ld.elf_so/reloc.c Sun Nov 23 21:04:55 2025 +0000 +++ b/libexec/ld.elf_so/reloc.c Sun Nov 23 21:47:17 2025 +0000 @@ -266,6 +266,10 @@ _rtld_relocate_objects(Obj_Entry *first, 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 597b4a78df0f -r e79b77790bf7 libexec/ld.elf_so/rtld.c --- a/libexec/ld.elf_so/rtld.c Sun Nov 23 21:04:55 2025 +0000 +++ b/libexec/ld.elf_so/rtld.c Sun Nov 23 21:47:17 2025 +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; @@ -145,6 +146,205 @@ 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_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. + */ + _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) { @@ -172,8 +372,11 @@ _rtld_call_fini_function(Obj_Entry *obj, * 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, @@ -200,33 +403,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); @@ -272,6 +479,8 @@ _rtld_call_init_function(Obj_Entry *obj, 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 @@ -279,9 +488,11 @@ _rtld_call_ifunc_functions(sigset_t *mas ) { _rtld_call_ifunc(obj, mask, cur_objgen); if (_rtld_objgen != cur_objgen) { + _rtld_initfini_exit(obj); return true; } } + _rtld_initfini_exit(obj); return false; } @@ -301,7 +512,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; @@ -312,6 +528,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); @@ -320,9 +537,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); @@ -333,7 +559,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); @@ -728,12 +964,12 @@ _rtld(Elf_Addr *sp, Elf_Addr relocbase) * 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")); @@ -945,9 +1181,23 @@ _rtld_unload_object(sigset_t *mask, Obj_ Obj_Entry **linkp; Objlist_Entry *elm; - /* Finalize objects that are about to be unmapped. */ - if (do_fini_funcs) + assert(root->dl_refcount == 0); + + /* + * 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->refcount == 0); + assert(root->dl_refcount == 0); + root->dlclosing = false; + } /* Remove the DAG from all objects' DAG lists. */ SIMPLEQ_FOREACH(elm, &root->dagmembers, link) @@ -959,10 +1209,21 @@ _rtld_unload_object(sigset_t *mask, Obj_ _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. + */ linkp = &_rtld_objlist->next; while ((obj = *linkp) != NULL) { - if (obj->refcount == 0) { + if (obj->refcount == 0 && + !obj->dlclosing && + obj->initfinilock == 0) { dbg(("unloading \"%s\"", obj->path)); if (obj->ehdr != MAP_FAILED) munmap(obj->ehdr, _rtld_pagesz); @@ -971,11 +1232,13 @@ _rtld_unload_object(sigset_t *mask, Obj_ _rtld_linkmap_delete(obj); *linkp = obj->next; _rtld_objcount--; + _rtld_fini_done(obj); _rtld_obj_free(obj); } else linkp = &obj->next; } _rtld_objtail = linkp; + _rtld_objgen++; } } @@ -985,6 +1248,7 @@ _rtld_ref_dag(Obj_Entry *root) const Needed_Entry *needed; assert(root); + assert(root->refcount > 0); ++root->refcount; @@ -1036,11 +1300,14 @@ dlclose(void *handle) _rtld_exclusive_exit(&mask); return -1; } + assert(root->refcount != 0); + assert(root->dl_refcount != 0); _rtld_debug.r_state = RT_DELETE; _rtld_debug_state(); --root->dl_refcount; + assert(root->refcount != 0); _rtld_unload_object(&mask, root, true); _rtld_debug.r_state = RT_CONSISTENT; @@ -1065,7 +1332,6 @@ __strong_alias(__dlopen,dlopen) void * dlopen(const char *name, int mode) { - Obj_Entry **old_obj_tail; Obj_Entry *obj = NULL; int flags = _RTLD_DLOPEN; bool nodelete; @@ -1077,8 +1343,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; @@ -1090,17 +1354,16 @@ 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); ++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); @@ -1110,10 +1373,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); @@ -1570,8 +1833,10 @@ __dl_cxa_refcount(void *addr, ssize_t de 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 597b4a78df0f -r e79b77790bf7 libexec/ld.elf_so/rtld.h --- a/libexec/ld.elf_so/rtld.h Sun Nov 23 21:04:55 2025 +0000 +++ b/libexec/ld.elf_so/rtld.h Sun Nov 23 21:47:17 2025 +0000 @@ -326,6 +326,11 @@ 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 */ + bool relocated; /* true if already relocated */ + bool dlclosing; /* true if being dlclosed now */ } Obj_Entry; typedef struct Struct_DoneList { @@ -341,6 +346,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[]; @@ -405,6 +411,8 @@ void _rtld_objlist_push_tail(Objlist *, Objlist_Entry *_rtld_objlist_find(Objlist *, const Obj_Entry *); void _rtld_ref_dag(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 *); @@ -421,9 +429,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 *); #define OBJ_ERR (Obj_Entry *)(-1) /* path.c */ @@ -445,7 +453,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 597b4a78df0f -r e79b77790bf7 libexec/ld.elf_so/search.c --- a/libexec/ld.elf_so/search.c Sun Nov 23 21:04:55 2025 +0000 +++ b/libexec/ld.elf_so/search.c Sun Nov 23 21:47:17 2025 +0000 @@ -63,11 +63,11 @@ __RCSID("$NetBSD: search.c,v 1.27 2020/0 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 @@ _rtld_search_library_path(const char *na 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 @@ _rtld_search_library_path(const char *na * 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 @@ _rtld_load_library(const char *name, con 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 597b4a78df0f -r e79b77790bf7 tests/libexec/ld.elf_so/t_dlclose_thread.c --- a/tests/libexec/ld.elf_so/t_dlclose_thread.c Sun Nov 23 21:04:55 2025 +0000 +++ b/tests/libexec/ld.elf_so/t_dlclose_thread.c Sun Nov 23 21:47:17 2025 +0000 @@ -82,8 +82,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(1); atomic_store_explicit(&stop, true, memory_order_relaxed);