Skip to content

Commit

Permalink
i#2921: support asynch signal in late exit and when interrupting a cl…
Browse files Browse the repository at this point in the history
…one syscall (#3364)

Fixes an asynch signal arriving late when thread is on its way to exit by blocking all signals during exit. This is ok, because we're at the app's thread_exit system call. When doing a detach, the app's signal mask is restored before going native. For terminate events using kill, we are not blocking the kill signal. The suspend signal is also excluded, because a detaching thread may try to synchronize with an exiting thread and as long as the signal is getting delivered to this thread, we need to reply to it from the signal handler.

Adds support to the signal handler for an asynch signal arriving in the middle of a clone system call or temporarily-native thread, while the spawning thread's tls magic sentinel is invalid. We're now detecting this condition in the signal handler by making assumptions as stated in this patch.

Fixes #2921
  • Loading branch information
Hendrik Greving authored Feb 5, 2019
1 parent 0499b41 commit edb2eed
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 26 deletions.
44 changes: 33 additions & 11 deletions core/unix/os.c
Original file line number Diff line number Diff line change
Expand Up @@ -1301,6 +1301,28 @@ os_slow_exit(void)
IF_NO_MEMQUERY(memcache_exit());
}

/* Helper function that calls cleanup_and_terminate after blocking most signals
*(i#2921).
*/
void
block_cleanup_and_terminate(dcontext_t *dcontext, int sysnum, ptr_uint_t sys_arg1,
ptr_uint_t sys_arg2, bool exitproc,
/* these 2 args are only used for Mac thread exit */
ptr_uint_t sys_arg3, ptr_uint_t sys_arg4)
{
/* This thread is on its way to exit. We are blocking all signals since any
* signal that reaches us now can be delayed until after the exit is complete.
* We may still receive a suspend signal for synchronization that we may need
* to reply to (i#2921).
*/
if (sysnum == SYS_kill)
block_all_signals_except(NULL, 2, dcontext->sys_param0, SUSPEND_SIGNAL);
else
block_all_signals_except(NULL, 1, SUSPEND_SIGNAL);
cleanup_and_terminate(dcontext, sysnum, sys_arg1, sys_arg2, exitproc, sys_arg3,
sys_arg4);
}

/* os-specific atexit cleanup */
void
os_fast_exit(void)
Expand All @@ -1323,8 +1345,8 @@ os_terminate_with_code(dcontext_t *dcontext, terminate_flags_t flags, int exit_c
if (TEST(TERMINATE_CLEANUP, flags)) {
/* we enter from several different places, so rewind until top-level kstat */
KSTOP_REWIND_UNTIL(thread_measured);
cleanup_and_terminate(dcontext, SYSNUM_EXIT_PROCESS, exit_code, 0,
true /*whole process*/, 0, 0);
block_cleanup_and_terminate(dcontext, SYSNUM_EXIT_PROCESS, exit_code, 0,
true /*whole process*/, 0, 0);
} else {
/* clean up may be impossible - just terminate */
config_exit(); /* delete .1config file */
Expand Down Expand Up @@ -3739,8 +3761,8 @@ client_thread_run(void)

LOG(THREAD, LOG_ALL, 1, "\n***** CLIENT THREAD %d EXITING *****\n\n",
get_thread_id());
cleanup_and_terminate(dcontext, SYS_exit, 0, 0, false /*just thread*/,
IF_MACOS_ELSE(dcontext->thread_port, 0), 0);
block_cleanup_and_terminate(dcontext, SYS_exit, 0, 0, false /*just thread*/,
IF_MACOS_ELSE(dcontext->thread_port, 0), 0);
}
# endif

Expand Down Expand Up @@ -5561,9 +5583,9 @@ handle_self_signal(dcontext_t *dcontext, uint sig)
* Should do set_default_signal_action(SIGABRT) (and set a flag so
* no races w/ another thread re-installing?) and then SYS_kill.
*/
cleanup_and_terminate(dcontext, SYSNUM_EXIT_THREAD, -1, 0,
(is_last_app_thread() && !dynamo_exited),
IF_MACOS_ELSE(dcontext->thread_port, 0), 0);
block_cleanup_and_terminate(dcontext, SYSNUM_EXIT_THREAD, -1, 0,
(is_last_app_thread() && !dynamo_exited),
IF_MACOS_ELSE(dcontext->thread_port, 0), 0);
ASSERT_NOT_REACHED();
}
}
Expand Down Expand Up @@ -6375,10 +6397,10 @@ handle_exit(dcontext_t *dcontext)
}
KSTOP(num_exits_dir_syscall);

cleanup_and_terminate(dcontext, MCXT_SYSNUM_REG(mc), sys_param(dcontext, 0),
sys_param(dcontext, 1), exit_process,
/* SYS_bsdthread_terminate has 2 more args */
sys_param(dcontext, 2), sys_param(dcontext, 3));
block_cleanup_and_terminate(dcontext, MCXT_SYSNUM_REG(mc), sys_param(dcontext, 0),
sys_param(dcontext, 1), exit_process,
/* SYS_bsdthread_terminate has 2 more args */
sys_param(dcontext, 2), sys_param(dcontext, 3));
}

# if defined(LINUX) && defined(X86) /* XXX i#58: just until we have Mac support */
Expand Down
7 changes: 7 additions & 0 deletions core/unix/os_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,13 @@ void
signal_thread_init(dcontext_t *dcontext, void *os_data);
void
signal_thread_exit(dcontext_t *dcontext, bool other_thread);
void
block_all_signals_except(kernel_sigset_t *oset, int num_signals, ...);
void
block_cleanup_and_terminate(dcontext_t *dcontext, int sysnum, ptr_uint_t sys_arg1,
ptr_uint_t sys_arg2, bool exitproc,
/* these 2 args are only used for Mac thread exit */
ptr_uint_t sys_arg3, ptr_uint_t sys_arg4);
bool
is_thread_signal_info_initialized(dcontext_t *dcontext);
void
Expand Down
66 changes: 51 additions & 15 deletions core/unix/signal.c
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,21 @@ sigprocmask_syscall(int how, kernel_sigset_t *set, kernel_sigset_t *oset,
set, oset, sigsetsize);
}

void
block_all_signals_except(kernel_sigset_t *oset, int num_signals,
... /* list of signals */)
{
kernel_sigset_t set;
kernel_sigfillset(&set);
va_list ap;
va_start(ap, num_signals);
for (int i = 0; i < num_signals; ++i) {
kernel_sigdelset(&set, va_arg(ap, int));
}
va_end(ap);
sigprocmask_syscall(SIG_SETMASK, &set, oset, sizeof(set));
}

static void
unblock_all_signals(kernel_sigset_t *oset)
{
Expand Down Expand Up @@ -1276,7 +1291,14 @@ signal_thread_exit(dcontext_t *dcontext, bool other_thread)
}
info->num_pending = 0;
}
if (!other_thread)
/* If no detach flag is set, we assume that this thread is on its way to exit.
* In order to prevent receiving signals while a thread is on its way to exit
* without a valid dcontext, signals at this stage are blocked. The exceptions
* are the suspend signal and any signal that a terminating SYS_kill may need.
* (i#2921). In this case, we do not want to restore the signal mask. For detach,
* we do need to restore the app's mask.
*/
if (!other_thread && doing_detach)
signal_swap_mask(dcontext, true /*to_app*/);
#ifdef HAVE_SIGALTSTACK
/* Remove our sigstack and restore the app sigstack if it had one. */
Expand Down Expand Up @@ -4838,12 +4860,27 @@ master_signal_handler_C(byte *xsp)
dcontext = NULL;
}
}
if (dcontext == NULL) {
/* Check for a temporarily-native thread we're synch-ing with. */
tr = thread_lookup(get_sys_thread_id());
if (tr != NULL)
dcontext = tr->dcontext;
}
}
if (dcontext == NULL &&
/* Check for a temporarily-native thread we're synch-ing with. */
(sig == SUSPEND_SIGNAL
#ifdef X86
|| (INTERNAL_OPTION(safe_read_tls_init) &&
/* Check for whether this is a thread with its invalid sentinel magic set.
* In this case, we assume that it is either a thread that is currently
* temporarily-native via API like DR_EMIT_GO_NATIVE, or a thread in the
* clone window. We know by inspection of our own code that it is safe to
* call thread_lookup for either case thread makes a clone or was just
* cloned. i.e. thread_lookup requires a lock that must not be held by the
* calling thread (i#2921).
* XXX: what is ARM doing, any special case w/ dcontext == NULL?
*/
safe_read_tls_magic() == TLS_MAGIC_INVALID)
#endif
)) {
tr = thread_lookup(get_sys_thread_id());
if (tr != NULL)
dcontext = tr->dcontext;
}
if (dcontext == NULL ||
(dcontext != GLOBAL_DCONTEXT &&
Expand Down Expand Up @@ -5574,20 +5611,19 @@ terminate_via_kill(dcontext_t *dcontext)
{
thread_sig_info_t *info = (thread_sig_info_t *)dcontext->signal_field;
ASSERT(dcontext == get_thread_private_dcontext());
unblock_all_signals(NULL);
/* Enure signal_thread_exit() will not re-block */
memset(&info->app_sigblocked, 0, sizeof(info->app_sigblocked));

/* FIXME PR 541760: there can be multiple thread groups and thus
* this may not exit all threads in the address space
*/
cleanup_and_terminate(dcontext, SYS_kill,
/* Pass -pid in case main thread has exited
* in which case will get -ESRCH
*/
IF_VMX86(os_in_vmkernel_userworld() ? -(int)get_process_id() :)
get_process_id(),
dcontext->sys_param0, true, 0, 0);
block_cleanup_and_terminate(
dcontext, SYS_kill,
/* Pass -pid in case main thread has exited
* in which case will get -ESRCH
*/
IF_VMX86(os_in_vmkernel_userworld() ? -(int)get_process_id() :) get_process_id(),
dcontext->sys_param0, true, 0, 0);
ASSERT_NOT_REACHED();
}

Expand Down

0 comments on commit edb2eed

Please sign in to comment.