[PATCH v3] dlfcn: Fix dlclose crash in atexit handler after thread_local destructor (BZ 33598)

[Adhemerval Zanella Netto]

On 05/05/26 17:45, Adhemerval Zanella Netto wrote:
> 
> 
> On 05/05/26 16:36, Florian Weimer wrote:
>> * Adhemerval Zanella:
>>
>>> @@ -46,6 +47,9 @@ __run_exit_handlers (int status, struct exit_function_list **listp,
>>>    /* The exit should never return, so there is no need to unlock it.  */
>>>    __libc_lock_lock_recursive (__exit_lock);
>>>  
>>> +  /* Disable unmap objects through dlclose by TLS destructor (BZ 33598).  */
>>> +  GL(dl_at_exit) = true;
>>> +
>>>    /* First, call the TLS destructors.  */
>>>    if (run_dtors)
>>>      call_function_static_weak (__call_tls_dtors);
>>> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
>>> index 15c46598539..236419c593c 100644
>>> --- a/sysdeps/generic/ldsodefs.h
>>> +++ b/sysdeps/generic/ldsodefs.h
>>> @@ -441,6 +441,9 @@ struct rtld_global
>>>    /* Generation counter for the dtv.  */
>>>    EXTERN size_t _dl_tls_generation;
>>>  
>>> +  /* Disable unmap objects during __run_exit_handlers.  */
>>> +  EXTERN bool _dl_at_exit;
>>> +
>>>    /* Scopes to free after next THREAD_GSCOPE_WAIT ().  */
>>>    EXTERN struct dl_scope_free_list
>>>    {
>>
>> Would it be simpler to do what the comment says?  Just guard the
>> munmap call that is part of dlclose?  This could even be considered an
>> optimization.
> 
> It should work, the main difference is with this approach (assume_in_use = false),
> _dl_close_worker during exit marks every object IDX_STILL_USED and returns 
> immediately, so fo finalizers run. The _dl_fini handles every library in its
> own sorted pass later.
> 
> With the DL_UNMAP approach, _dl_close_worker runs normally and unloadable libraries
> get their finalizers called by _dl_call_fini inside _dl_close_worker.
> 
> I think it makes sense to use your suggestion. I will send a new version.

However, analyzing this a bit more I am not sure. With DL_UNMAP the link map is 
still removed from the namespace, but the pages remain mapped and the DSO is gone
from GL(dl_ns[nsid]._ns_loaded), _dl_loaded_lock, and l_initfini chains. This 
creates an inconsistent state where:

* This approach guarantees that _dl_fini runs all fini/fini_array callbacks in a
  single topologically-sorted pass, where DL_UNMAP _dl_close_worker's _dl_call_fini 
  may run destructors for a library during exit, and then _dl_fini may encounter 
  it again (or not, if the link map was removed).

* dl_iterate_phdr and dladdr — used by profilers and alike (ASan/TSan) will miss 
  libraries whose pages are still live.

* LD_AUDIT (la_objclose callbacks) fires for each dlclose during exit. 

This approach makes exit-time dlclose semantically a no-op, although a bit more
complex.

So I leaning a bit more for the proposed approach.