nptl: Start new threads with all signals blocked [BZ #25098]

New threads inherit the signal mask from the current thread.  This
means that signal handlers can run on the newly created thread
immediately after the kernel has created the userspace thread, even
before glibc has initialized the TCB.  Consequently, new threads can
observe uninitialized ctype data, among other things.

To address this, block all signals before starting the thread, and
pass the original signal mask to the start routine wrapper.  On the
new thread, first perform all thread initialization, and then unblock
signals.

The cost of doing this is two rt_sigprocmask system calls on the old
thread, and one rt_sigprocmask system call on the new thread.  (If
there was a way to clone a new thread with a signals disabled, this
could be brought down to one system call each.)  The thread descriptor
increases in size, too, and sigset_t is fairly large.  This increase
could be brought down by reusing space the in the descriptor which is
not needed before running user code, or by switching to an internal
sigset_t definition which only covers the signals supported by the
kernel definition.  (Part of the thread descriptor size increase is
already offset by reduced stack usage in the thread start wrapper
routine after this commit.)

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
This commit is contained in:
Florian Weimer 2020-04-27 09:55:10 +02:00
parent 92954ffa5a
commit b3cae39dcb
2 changed files with 32 additions and 24 deletions

View File

@ -332,9 +332,8 @@ struct pthread
/* True if thread must stop at startup time. */ /* True if thread must stop at startup time. */
bool stopped_start; bool stopped_start;
/* The parent's cancel handling at the time of the pthread_create /* Formerly used for dealing with cancellation. */
call. This might be needed to undo the effects of a cancellation. */ int parent_cancelhandling_unsed;
int parent_cancelhandling;
/* Lock to synchronize access to the descriptor. */ /* Lock to synchronize access to the descriptor. */
int lock; int lock;
@ -391,6 +390,11 @@ struct pthread
/* Resolver state. */ /* Resolver state. */
struct __res_state res; struct __res_state res;
/* Signal mask for the new thread. Used during thread startup to
restore the signal mask. (Threads are launched with all signals
masked.) */
sigset_t sigmask;
/* Indicates whether is a C11 thread created by thrd_creat. */ /* Indicates whether is a C11 thread created by thrd_creat. */
bool c11; bool c11;

View File

@ -369,7 +369,6 @@ __free_tcb (struct pthread *pd)
} }
} }
/* Local function to start thread and handle cleanup. /* Local function to start thread and handle cleanup.
createthread.c defines the macro START_THREAD_DEFN to the createthread.c defines the macro START_THREAD_DEFN to the
declaration that its create_thread function will refer to, and declaration that its create_thread function will refer to, and
@ -385,10 +384,6 @@ START_THREAD_DEFN
/* Initialize pointers to locale data. */ /* Initialize pointers to locale data. */
__ctype_init (); __ctype_init ();
/* Allow setxid from now onwards. */
if (__glibc_unlikely (atomic_exchange_acq (&pd->setxid_futex, 0) == -2))
futex_wake (&pd->setxid_futex, 1, FUTEX_PRIVATE);
#ifndef __ASSUME_SET_ROBUST_LIST #ifndef __ASSUME_SET_ROBUST_LIST
if (__set_robust_list_avail >= 0) if (__set_robust_list_avail >= 0)
#endif #endif
@ -399,18 +394,6 @@ START_THREAD_DEFN
sizeof (struct robust_list_head)); sizeof (struct robust_list_head));
} }
/* If the parent was running cancellation handlers while creating
the thread the new thread inherited the signal mask. Reset the
cancellation signal mask. */
if (__glibc_unlikely (pd->parent_cancelhandling & CANCELING_BITMASK))
{
sigset_t mask;
__sigemptyset (&mask);
__sigaddset (&mask, SIGCANCEL);
INTERNAL_SYSCALL_CALL (rt_sigprocmask, SIG_UNBLOCK, &mask,
NULL, _NSIG / 8);
}
/* This is where the try/finally block should be created. For /* This is where the try/finally block should be created. For
compilers without that support we do use setjmp. */ compilers without that support we do use setjmp. */
struct pthread_unwind_buf unwind_buf; struct pthread_unwind_buf unwind_buf;
@ -432,6 +415,12 @@ START_THREAD_DEFN
unwind_buf.priv.data.prev = NULL; unwind_buf.priv.data.prev = NULL;
unwind_buf.priv.data.cleanup = NULL; unwind_buf.priv.data.cleanup = NULL;
__libc_signal_restore_set (&pd->sigmask);
/* Allow setxid from now onwards. */
if (__glibc_unlikely (atomic_exchange_acq (&pd->setxid_futex, 0) == -2))
futex_wake (&pd->setxid_futex, 1, FUTEX_PRIVATE);
if (__glibc_likely (! not_first_call)) if (__glibc_likely (! not_first_call))
{ {
/* Store the new cleanup handler info. */ /* Store the new cleanup handler info. */
@ -722,10 +711,6 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
CHECK_THREAD_SYSINFO (pd); CHECK_THREAD_SYSINFO (pd);
#endif #endif
/* Inform start_thread (above) about cancellation state that might
translate into inherited signal state. */
pd->parent_cancelhandling = THREAD_GETMEM (THREAD_SELF, cancelhandling);
/* Determine scheduling parameters for the thread. */ /* Determine scheduling parameters for the thread. */
if (__builtin_expect ((iattr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0, 0) if (__builtin_expect ((iattr->flags & ATTR_FLAG_NOTINHERITSCHED) != 0, 0)
&& (iattr->flags & (ATTR_FLAG_SCHED_SET | ATTR_FLAG_POLICY_SET)) != 0) && (iattr->flags & (ATTR_FLAG_SCHED_SET | ATTR_FLAG_POLICY_SET)) != 0)
@ -771,6 +756,21 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
ownership of PD (see CONCURRENCY NOTES above). */ ownership of PD (see CONCURRENCY NOTES above). */
bool stopped_start = false; bool thread_ran = false; bool stopped_start = false; bool thread_ran = false;
/* Block all signals, so that the new thread starts out with
signals disabled. This avoids race conditions in the thread
startup. */
sigset_t original_sigmask;
__libc_signal_block_all (&original_sigmask);
/* Conceptually, the new thread needs to inherit the signal mask of
this thread. Therefore, it needs to restore the saved signal
mask of this thread, so save it in the startup information. */
pd->sigmask = original_sigmask;
/* Reset the cancellation signal mask in case this thread is running
cancellation. */
__sigdelset (&pd->sigmask, SIGCANCEL);
/* Start the thread. */ /* Start the thread. */
if (__glibc_unlikely (report_thread_creation (pd))) if (__glibc_unlikely (report_thread_creation (pd)))
{ {
@ -813,6 +813,10 @@ __pthread_create_2_1 (pthread_t *newthread, const pthread_attr_t *attr,
retval = create_thread (pd, iattr, &stopped_start, retval = create_thread (pd, iattr, &stopped_start,
STACK_VARIABLES_ARGS, &thread_ran); STACK_VARIABLES_ARGS, &thread_ran);
/* Return to the previous signal mask, after creating the new
thread. */
__libc_signal_restore_set (&original_sigmask);
if (__glibc_unlikely (retval != 0)) if (__glibc_unlikely (retval != 0))
{ {
if (thread_ran) if (thread_ran)