Re: [PATCH] newlib: Unlock the mutex while calling atexit()'ed functions

Hi Takashi, Hi Sebastian,

did you discuss this in PM, by any chance?  Is there any difference in
GLibc or Darwin which explains how this potential problem is fixed
there?


Thanks,
Corinna


On Nov 26 05:17, Sebastian Huber wrote:
> Hello Takashi Yano,
> 
> I have some questions to the change.
> 
> ----- Am 18. Nov 2025 um 14:31 schrieb Takashi Yano [email protected]:
> 
> > The atexit()'ed function may deadlock if it waits for another thread
> > which calls atexit() in the current __call_atexit.c code. This patch
> > unlock __atexit_recursive_mutex while calling atexit()'ed functions
> > to avoid the deadlock mentioned above. glibc and Darwin do the same,
> > so it sounds reasonable.
> > 
> > Addresses: https://cygwin.com/pipermail/cygwin/2025-October/258930.html
> > Reported-by: Tomohiro Kashiwada <[email protected]>
> > Reviewed-by:
> > Signed-off-by: Takashi Yano <[email protected]>
> > ---
> > newlib/libc/stdlib/__call_atexit.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> > 
> > diff --git a/newlib/libc/stdlib/__call_atexit.c
> > b/newlib/libc/stdlib/__call_atexit.c
> > index 710440389..44f1f6acc 100644
> > --- a/newlib/libc/stdlib/__call_atexit.c
> > +++ b/newlib/libc/stdlib/__call_atexit.c
> > @@ -114,6 +114,11 @@ __call_exitprocs (int code, void *d)
> > 
> > 	  ind = p->_ind;
> > 
> > +#ifndef __SINGLE_THREAD__
> > +	  /* Unlock __atexit_recursive_mutex; otherwise, the function fn() may
> > +	     deadlock if it waits for another thread which calls atexit(). */
> > +	  __lock_release_recursive(__atexit_recursive_mutex);
> > +#endif
> 
> Here, a potentially shared structure is used through the args pointer. If some other thread calls exit() or atexit() concurrently, then this could result in a use of altered or freed memory. The data of the structure referenced by args should be first copied to local variables. Also I think that the deallocation should be done before we release the lock. We probably also have to restart the process after each handler call unconditionally.
> 
> 
> > 	  /* Call the function.  */
> > 	  if (!args || (args->_fntypes & i) == 0)
> > 	    fn ();
> > @@ -121,6 +126,9 @@ __call_exitprocs (int code, void *d)
> > 	    (*((void (*)(int, void *)) fn))(code, args->_fnargs[n]);
> > 	  else
> > 	    (*((void (*)(void *)) fn))(args->_fnargs[n]);
> > +#ifndef __SINGLE_THREAD__
> > +	  __lock_acquire_recursive(__atexit_recursive_mutex);
> > +#endif
> > 
> > 	  /* The function we called call atexit and registered another
> > 	     function (or functions).  Call these new functions before
> > --
> > 2.51.0
> 
> -- 
> embedded brains GmbH & Co. KG
> Herr Sebastian HUBER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email: [email protected]
> phone: +49-89-18 94 741 - 16
> fax:   +49-89-18 94 741 - 08
> 
> Registergericht: Amtsgericht München
> Registernummer: HRB 157899
> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
> Unsere Datenschutzerklärung finden Sie hier:
> https://embedded-brains.de/datenschutzerklaerung/