lib/glibc
1
0
Fork 0
mirror of https://sourceware.org/git/glibc.git synced 2025-07-28 00:21:52 +03:00
Code Activity

stdlib: Fix data race in __run_exit_handlers [BZ #27749]

Browse Source 
Keep __exit_funcs_lock almost all the time and unlock it only to execute
callbacks. This fixed two issues.

1. f->func.cxa was modified outside the lock with rare data race like:
	thread 0: __run_exit_handlers unlock __exit_funcs_lock
	thread 1: __internal_atexit locks __exit_funcs_lock
	thread 0: f->flavor = ef_free;
	thread 1: sees ef_free and use it as new
	thread 1: new->func.cxa.fn = (void (*) (void *, int)) func;
	thread 1: new->func.cxa.arg = arg;
	thread 1: new->flavor = ef_cxa;
	thread 0: cxafct = f->func.cxa.fn;  // it's wrong fn!
	thread 0: cxafct (f->func.cxa.arg, status);  // it's wrong arg!
	thread 0: goto restart;
	thread 0: call the same exit_function again as it's ef_cxa

2. Don't unlock in main while loop after *listp = cur->next. If *listp
   is NULL and __exit_funcs_done is false another thread may fail in
   __new_exitfn on assert (l != NULL):
	 thread 0: *listp = cur->next;  // It can be the last: *listp = NULL.
	 thread 0: __libc_lock_unlock
	 thread 1: __libc_lock_lock in __on_exit
	 thread 1: __new_exitfn
	 thread 1: if (__exit_funcs_done)  // false: thread 0 isn't there yet.
	 thread 1: l = *listp
	 thread 1: moves one and crashes on assert (l != NULL);

The test needs multiple iterations to consistently fail without the fix.

Fixes https://sourceware.org/bugzilla/show_bug.cgi?id=27749

Checked on x86_64-linux-gnu.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
This commit is contained in:
Vitaly Buka
2021-04-26 12:27:29 -07:00
committed by Adhemerval Zanella
parent 7a7bcddeef
commit 16adc58e73
3 changed files with 133 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
stdlib/exit.c
View File

@ -45,25 +45,21 @@ __run_exit_handlers (int status, struct exit_function_list **listp,
if (run_dtors)
__call_tls_dtors ();
__libc_lock_lock (__exit_funcs_lock);
/* We do it this way to handle recursive calls to exit () made by
the functions registered with `atexit' and `on_exit'. We call
everyone on the list and use the status value in the last
exit (). */
while (true)
{
struct exit_function_list *cur;
__libc_lock_lock (__exit_funcs_lock);
restart:
cur = *listp;
struct exit_function_list *cur = *listp;
if (cur == NULL)
{
/* Exit processing complete. We will not allow any more
atexit/on_exit registrations. */
__exit_funcs_done = true;
__libc_lock_unlock (__exit_funcs_lock);
break;
}
@ -72,49 +68,57 @@ __run_exit_handlers (int status, struct exit_function_list **listp,
struct exit_function *const f = &cur->fns[--cur->idx];
const uint64_t new_exitfn_called = __new_exitfn_called;
/* Unlock the list while we call a foreign function. */
__libc_lock_unlock (__exit_funcs_lock);
switch (f->flavor)
{
void (*atfct) (void);
void (*onfct) (int status, void *arg);
void (*cxafct) (void *arg, int status);
void *arg;
case ef_free:
case ef_us:
break;
case ef_on:
onfct = f->func.on.fn;
arg = f->func.on.arg;
#ifdef PTR_DEMANGLE
PTR_DEMANGLE (onfct);
#endif
onfct (status, f->func.on.arg);
/* Unlock the list while we call a foreign function. */
__libc_lock_unlock (__exit_funcs_lock);
onfct (status, arg);
__libc_lock_lock (__exit_funcs_lock);
break;
case ef_at:
atfct = f->func.at;
#ifdef PTR_DEMANGLE
PTR_DEMANGLE (atfct);
#endif
/* Unlock the list while we call a foreign function. */
__libc_lock_unlock (__exit_funcs_lock);
atfct ();
__libc_lock_lock (__exit_funcs_lock);
break;
case ef_cxa:
/* To avoid dlclose/exit race calling cxafct twice (BZ 22180),
we must mark this function as ef_free. */
f->flavor = ef_free;
cxafct = f->func.cxa.fn;
arg = f->func.cxa.arg;
#ifdef PTR_DEMANGLE
PTR_DEMANGLE (cxafct);
#endif
cxafct (f->func.cxa.arg, status);
/* Unlock the list while we call a foreign function. */
__libc_lock_unlock (__exit_funcs_lock);
cxafct (arg, status);
__libc_lock_lock (__exit_funcs_lock);
break;
}
/* Re-lock again before looking at global state. */
__libc_lock_lock (__exit_funcs_lock);
if (__glibc_unlikely (new_exitfn_called != __new_exitfn_called))
/* The last exit function, or another thread, has registered
more exit functions. Start the loop over. */
goto restart;
continue;
}
*listp = cur->next;
@ -122,10 +126,10 @@ __run_exit_handlers (int status, struct exit_function_list **listp,
/* Don't free the last element in the chain, this is the statically
allocate element. */
free (cur);
__libc_lock_unlock (__exit_funcs_lock);
}
__libc_lock_unlock (__exit_funcs_lock);
if (run_list_atexit)
RUN_HOOK (__libc_atexit, ());