PR/59751 CVS commit: [netbsd-11] src

The following reply was made to PR lib/59751; it has been noted by GNATS.

From: "Martin Husemann" <[email protected]>
To: [email protected]
Cc: 
Subject: PR/59751 CVS commit: [netbsd-11] src
Date: Fri, 3 Jul 2026 18:36:36 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Fri Jul  3 18:36:36 UTC 2026
 
 Modified Files:
 	src/libexec/ld.elf_so [netbsd-11]: load.c reloc.c rtld.c rtld.h
 	    search.c
 	src/tests/libexec/ld.elf_so [netbsd-11]: t_dlclose_thread.c
 	src/usr.bin/ldd [netbsd-11]: ldd.c ldd_elfxx.c
 
 Log Message:
 Pull up following revision(s) (requested by riastradh in ticket #365):
 
 	tests/libexec/ld.elf_so/t_dlclose_thread.c: revision 1.2
 	tests/libexec/ld.elf_so/t_dlclose_thread.c: revision 1.3
 	usr.bin/ldd/ldd.c: revision 1.30
 	libexec/ld.elf_so/rtld.c: revision 1.225
 	libexec/ld.elf_so/rtld.c: revision 1.226
 	libexec/ld.elf_so/load.c: revision 1.50
 	libexec/ld.elf_so/load.c: revision 1.51
 	libexec/ld.elf_so/search.c: revision 1.28
 	libexec/ld.elf_so/rtld.h: revision 1.156
 	usr.bin/ldd/ldd.c: revision 1.29
 	usr.bin/ldd/ldd_elfxx.c: revision 1.9
 	libexec/ld.elf_so/reloc.c: revision 1.121
 
 ld.elf_so(1): Run concurrent dlopen/dlclose test a few more seconds.
 
 More likely to provoke the problem this way.  Still not 100% reliable
 because the problem is a race condition, but better than having the
 test unexpectedly pass half the time.
 
 Also set a timeout of 20sec, since I've seen the test get into an
 infinite loop sometimes and it's now supposed to complete in 5sec +
 epsilon.
 
 PR lib/59751: dlclose is not MT-safe depending on the libraries unloaded
 
 ld.elf_so(1): Resolve several races in dlopen/dlclose.
 
 This is difficult because, although rtld generally has a single
 exclusive lock, i.e., generally runs single-threaded itself, it can't
 hold this lock while calling constructors/destructors (init/fini or
 ifunc) -- if it did, then, for example, lazy symbol binding that
 happens during the constructor/destructor would deadlock against
 itself.
 
 And whenever rtld drops the lock to call constructors/destructors,
 any objects it is working on, during dlopen or dlclose, might have
 been concurrently closed and invalidated by the time it gets the lock
 again.
 
 The key point is that anywhere we pass a sigset_t *mask parameter
 during dlopen or dlclose, we might release the rtld lock to sleep and
 then reacquire the lock.  And anywhere we might release and reacquire
 the lock, any objects we hold may be invalidated -- unless we hold some
 reference to prevent invalidation.  And any object we find in the list
 of objects might be undergoing dlclose, waiting for destructors to
 finish, so we have to be prepared to release and reacquire the lock to
 wait until the destructors are done -- and then start over at the top.
 
 Here is a (probably nonexhaustive) list of races that I encountered:
 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.
       Also, since _rtld_load_needed_objects in one thread may be
       interleaved with _rtld_call_init_functions in another thread,
       make sure to have _rtld_call_init_functions skip the objects
       that have yet to be relocated -- they can't be part of the DAG
       of dependencies of the objecteing dlopened (or else we would
       have passed through _rtld_relocate_objects), and if they
       haven't been relocated then the init/fini function pointers are
       unusable.
       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
 
 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
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.49 -r1.49.10.1 src/libexec/ld.elf_so/load.c
 cvs rdiff -u -r1.120 -r1.120.2.1 src/libexec/ld.elf_so/reloc.c
 cvs rdiff -u -r1.221.2.2 -r1.221.2.3 src/libexec/ld.elf_so/rtld.c
 cvs rdiff -u -r1.150.2.2 -r1.150.2.3 src/libexec/ld.elf_so/rtld.h
 cvs rdiff -u -r1.27 -r1.27.10.1 src/libexec/ld.elf_so/search.c
 cvs rdiff -u -r1.1.2.2 -r1.1.2.3 \
     src/tests/libexec/ld.elf_so/t_dlclose_thread.c
 cvs rdiff -u -r1.28 -r1.28.4.1 src/usr.bin/ldd/ldd.c
 cvs rdiff -u -r1.8 -r1.8.6.1 src/usr.bin/ldd/ldd_elfxx.c
 
 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.