Skip to content

Commit

Permalink
i#3178: proper sigaltstack error checking (#3184)
Browse files Browse the repository at this point in the history
Adds proper error checking for the various ways that the kernel fails
the SYS_sigaltstack system call.  Adds tests of each error case.

Fixes #3178
  • Loading branch information
derekbruening authored Sep 22, 2018
1 parent c045aaa commit f2c9756
Show file tree
Hide file tree
Showing 6 changed files with 155 additions and 42 deletions.
11 changes: 9 additions & 2 deletions core/unix/os.c
Original file line number Diff line number Diff line change
Expand Up @@ -7222,9 +7222,16 @@ pre_system_call(dcontext_t *dcontext)
*/
const stack_t *uss = (const stack_t *)sys_param(dcontext, 0);
stack_t *uoss = (stack_t *)sys_param(dcontext, 1);
execute_syscall = handle_sigaltstack(dcontext, uss, uoss);
uint res;
LOG(THREAD, LOG_SYSCALLS, 2, "syscall: sigaltstack " PFX " " PFX "\n", uss, uoss);
execute_syscall =
handle_sigaltstack(dcontext, uss, uoss, get_mcontext(dcontext)->xsp, &res);
if (!execute_syscall) {
set_success_return_val(dcontext, 0);
LOG(THREAD, LOG_SYSCALLS, 2, "sigaltstack emulation => %d\n", -res);
if (res == 0)
set_success_return_val(dcontext, res);
else
set_failure_return_val(dcontext, res);
}
break;
}
Expand Down
3 changes: 2 additions & 1 deletion core/unix/os_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,8 @@ bool
handle_sigreturn(dcontext_t *dcontext, void *ucxt, int style);
#endif
bool
handle_sigaltstack(dcontext_t *dcontext, const stack_t *stack, stack_t *old_stack);
handle_sigaltstack(dcontext_t *dcontext, const stack_t *stack, stack_t *old_stack,
reg_t cur_xsp, OUT uint *result);
bool
handle_sigprocmask(dcontext_t *dcontext, int how, kernel_sigset_t *set,
kernel_sigset_t *oset, size_t sigsetsize);
Expand Down
73 changes: 59 additions & 14 deletions core/unix/signal.c
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,12 @@
#ifndef SA_ONESHOT
# define SA_ONESHOT SA_RESETHAND
#endif
#ifndef SS_AUTODISARM
# define SS_AUTODISARM (1U << 31)
#endif
#ifndef SS_FLAG_BITS
# define SS_FLAG_BITS SS_AUTODISARM
#endif

/**** data structures ***************************************************/

Expand Down Expand Up @@ -1646,6 +1652,7 @@ handle_clone(dcontext_t *dcontext, uint flags)

/* Returns false if should NOT issue syscall.
* In such a case, the result is in "result".
* If *result is non-zero, the syscall should fail.
* We could instead issue the syscall and expect it to fail, which would have a more
* accurate error code, but that risks missing a failure (e.g., RT on Android
* which in some cases returns success on bugus params).
Expand Down Expand Up @@ -1937,20 +1944,57 @@ handle_post_old_sigaction(dcontext_t *dcontext, bool success, int sig,
}
#endif /* LINUX */

/* Returns false if should NOT issue syscall */
/* Returns false and sets *result if should NOT issue syscall.
* If *result is non-zero, the syscall should fail.
*/
bool
handle_sigaltstack(dcontext_t *dcontext, const stack_t *stack, stack_t *old_stack)
handle_sigaltstack(dcontext_t *dcontext, const stack_t *stack, stack_t *old_stack,
reg_t cur_xsp, OUT uint *result)
{
thread_sig_info_t *info = (thread_sig_info_t *)dcontext->signal_field;
stack_t local_stack;
if (old_stack != NULL) {
*old_stack = info->app_sigstack;
if (!safe_write_ex(old_stack, sizeof(*old_stack), &info->app_sigstack, NULL)) {
*result = EFAULT;
return false;
}
}
if (stack != NULL) {
info->app_sigstack = *stack;
LOG(THREAD, LOG_ASYNCH, 2, "app set up signal stack " PFX " - " PFX " %s\n",
stack->ss_sp, stack->ss_sp + stack->ss_size - 1,
(APP_HAS_SIGSTACK(info)) ? "enabled" : "disabled");
/* Fail in the same way the kernel does. */
if (!safe_read(stack, sizeof(local_stack), &local_stack)) {
*result = EFAULT;
return false;
}
if (APP_HAS_SIGSTACK(info)) {
/* The app is not allowed to set a new altstack while on the current one. */
reg_t cur_sigstk = (reg_t)info->app_sigstack.ss_sp;
if (cur_xsp >= cur_sigstk &&
cur_xsp < cur_sigstk + info->app_sigstack.ss_size) {
*result = EPERM;
return false;
}
}
uint key_flag = local_stack.ss_flags & ~SS_FLAG_BITS;
if (key_flag != SS_DISABLE && key_flag != SS_ONSTACK && key_flag != 0) {
*result = EINVAL;
return false;
}
if (key_flag == SS_DISABLE) {
/* Zero the other params and don't even check them. */
local_stack.ss_sp = NULL;
local_stack.ss_size = 0;
} else {
if (local_stack.ss_size < MINSIGSTKSZ) {
*result = ENOMEM;
return false;
}
}
info->app_sigstack = local_stack;
LOG(THREAD, LOG_ASYNCH, 2, "Setting app signal stack to " PFX "-" PFX " %d=%s\n",
local_stack.ss_sp, local_stack.ss_sp + local_stack.ss_size - 1,
local_stack.ss_flags, (APP_HAS_SIGSTACK(info)) ? "enabled" : "disabled");
}
*result = 0;
return false; /* always cancel syscall */
}

Expand Down Expand Up @@ -5931,13 +5975,14 @@ handle_sigreturn(dcontext_t *dcontext, void *ucxt_param, int style)
# endif
sc = get_sigcontext_from_app_frame(info, sig, (void *)frame);
ucxt = &frame->uc;
/* Re-set sigstack from the value stored in the frame. */
/* FIXME i#3178: have a routine to set it that fails when the kernel fails */
LOG(THREAD, LOG_ASYNCH, 3, "Restoring app signal stack to " PFX "-" PFX " %d\n",
frame->uc.uc_stack.ss_sp,
frame->uc.uc_stack.ss_sp + frame->uc.uc_stack.ss_size,
frame->uc.uc_stack.ss_flags);
info->app_sigstack = frame->uc.uc_stack;
/* Re-set sigstack from the value stored in the frame. Silently ignore failure,
* just like the kernel does.
*/
uint ignored;
/* The kernel checks for being on the stack *after* swapping stacks, so pass
* sc->SC_XSP as the current stack.
*/
handle_sigaltstack(dcontext, &frame->uc.uc_stack, NULL, sc->SC_XSP, &ignored);
/* Restore DR's so sigreturn syscall won't change it. */
frame->uc.uc_stack = info->sigstack;
#elif defined(MACOS)
Expand Down
40 changes: 37 additions & 3 deletions suite/tests/linux/bad-signal-stack.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include <ucontext.h>
#include <errno.h>
#include <stdlib.h>
#include <assert.h>

#define ALT_STACK_SIZE (SIGSTKSZ * 4)

Expand All @@ -48,13 +49,44 @@ signal_handler(int sig)
int
main(int argc, char *argv[])
{
char *sp;
int rc;
INIT();
stack_t sigstack;
char *alloc = (char *)malloc(ALT_STACK_SIZE);

/* First, test various failures of sigaltstack */
rc = sigaltstack(NULL, (stack_t *)0x4);
assert(rc == -1 && errno == EFAULT);

rc = sigaltstack((stack_t *)0x4, NULL);
assert(rc == -1 && errno == EFAULT);

sigstack.ss_sp = alloc;
sigstack.ss_size = MINSIGSTKSZ - 1;
sigstack.ss_flags = 0;
rc = sigaltstack(&sigstack, NULL);
assert(rc == -1 && errno == ENOMEM);

sigstack.ss_sp = alloc;
sigstack.ss_size = MINSIGSTKSZ - 1;
/* SS_DISABLE causes the kernel to ignore sp and size: it zeroes them out. */
sigstack.ss_flags = SS_DISABLE;
rc = sigaltstack(&sigstack, NULL);
ASSERT_NOERR(rc);
stack_t mystack;
rc = sigaltstack(NULL, &mystack);
ASSERT_NOERR(rc);
assert(mystack.ss_sp == NULL && mystack.ss_size == 0 &&
mystack.ss_flags == SS_DISABLE);

sigstack.ss_sp = alloc;
sigstack.ss_size = ALT_STACK_SIZE;
sigstack.ss_flags = SS_DISABLE | SS_ONSTACK;
rc = sigaltstack(&sigstack, NULL);
assert(rc == -1 && errno == EINVAL);

/* Make an alternate stack that's not writable. */
stack_t sigstack;
sigstack.ss_sp = (char *)malloc(ALT_STACK_SIZE);
sigstack.ss_sp = alloc;
sigstack.ss_size = ALT_STACK_SIZE;
sigstack.ss_flags = SS_ONSTACK;
rc = sigaltstack(&sigstack, NULL);
Expand All @@ -77,6 +109,8 @@ main(int argc, char *argv[])
print("Sending SIGUSR1\n");
kill(getpid(), SIGUSR1);

protect_mem(alloc, ALT_STACK_SIZE, ALLOW_READ | ALLOW_WRITE);
free(alloc);
print("All done\n");
return 0;
}
35 changes: 24 additions & 11 deletions suite/tests/linux/signal-base.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,16 @@ signal_handler(int sig, siginfo_t *siginfo, ucontext_t *ucxt)
print("in signal handler\n");
#endif

#if USE_SIGSTACK
/* Ensure setting a new stack while on the current one fails with EPERM. */
stack_t sigstack;
sigstack.ss_sp = siginfo; /* will fail: just need sthg */
sigstack.ss_size = ALT_STACK_SIZE;
sigstack.ss_flags = SS_ONSTACK;
int rc = sigaltstack(&sigstack, NULL);
assert(rc == -1 && errno == EPERM);
#endif

switch (sig) {

case SIGSEGV: {
Expand Down Expand Up @@ -204,16 +214,6 @@ main(int argc, char *argv[])
rc = sigprocmask(SIG_SETMASK, &mask, NULL);
ASSERT_NOERR(rc);

#if USE_TIMER
custom_intercept_signal(SIGVTALRM, (handler_t)signal_handler);
t.it_interval.tv_sec = 0;
t.it_interval.tv_usec = 10000;
t.it_value.tv_sec = 0;
t.it_value.tv_usec = 10000;
rc = setitimer(ITIMER_VIRTUAL, &t, NULL);
ASSERT_NOERR(rc);
#endif

#if USE_SIGSTACK
sigstack.ss_sp = (char *)malloc(ALT_STACK_SIZE);
sigstack.ss_size = ALT_STACK_SIZE;
Expand All @@ -226,6 +226,16 @@ main(int argc, char *argv[])
# endif
#endif

#if USE_TIMER
custom_intercept_signal(SIGVTALRM, (handler_t)signal_handler);
t.it_interval.tv_sec = 0;
t.it_interval.tv_usec = 10000;
t.it_value.tv_sec = 0;
t.it_value.tv_usec = 10000;
rc = setitimer(ITIMER_VIRTUAL, &t, NULL);
ASSERT_NOERR(rc);
#endif

custom_intercept_signal(SIGSEGV, (handler_t)signal_handler);
custom_intercept_signal(SIGUSR1, (handler_t)signal_handler);
custom_intercept_signal(SIGUSR2, (handler_t)SIG_IGN);
Expand Down Expand Up @@ -290,7 +300,10 @@ main(int argc, char *argv[])
print("Got some timer hits!\n");
#endif

#if USE_SIGSTACK
/* We leave the sigstack in place for the timer so any racy alarm arriving
* after we disabled the itimer will be on the alt stack.
*/
#if USE_SIGSTACK && !USE_TIMER
stack_t check_stack;
rc = sigaltstack(NULL, &check_stack);
ASSERT_NOERR(rc);
Expand Down
35 changes: 24 additions & 11 deletions suite/tests/linux/sigplain-base.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,16 @@ static int timer_hits = 0;
static void
signal_handler(int sig)
{
#if USE_SIGSTACK
/* Ensure setting a new stack while on the current one fails with EPERM. */
stack_t sigstack;
sigstack.ss_sp = a; /* will fail: just need sthg */
sigstack.ss_size = ALT_STACK_SIZE;
sigstack.ss_flags = 0;
int rc = sigaltstack(&sigstack, NULL);
assert(rc == -1 && errno == EPERM);
#endif

#if USE_TIMER
if (sig == SIGVTALRM)
timer_hits++;
Expand Down Expand Up @@ -135,16 +145,6 @@ main(int argc, char *argv[])
rc = sigprocmask(SIG_SETMASK, &mask, NULL);
ASSERT_NOERR(rc);

#if USE_TIMER
custom_intercept_signal(SIGVTALRM, signal_handler);
t.it_interval.tv_sec = 0;
t.it_interval.tv_usec = 20000;
t.it_value.tv_sec = 0;
t.it_value.tv_usec = 20000;
rc = setitimer(ITIMER_VIRTUAL, &t, NULL);
ASSERT_NOERR(rc);
#endif

#if USE_SIGSTACK
sigstack.ss_sp = (char *)malloc(ALT_STACK_SIZE);
sigstack.ss_size = ALT_STACK_SIZE;
Expand All @@ -157,6 +157,16 @@ main(int argc, char *argv[])
# endif
#endif

#if USE_TIMER
custom_intercept_signal(SIGVTALRM, signal_handler);
t.it_interval.tv_sec = 0;
t.it_interval.tv_usec = 20000;
t.it_value.tv_sec = 0;
t.it_value.tv_usec = 20000;
rc = setitimer(ITIMER_VIRTUAL, &t, NULL);
ASSERT_NOERR(rc);
#endif

custom_intercept_signal(SIGSEGV, signal_handler);
custom_intercept_signal(SIGUSR1, signal_handler);
custom_intercept_signal(SIGUSR2, SIG_IGN);
Expand Down Expand Up @@ -198,7 +208,10 @@ main(int argc, char *argv[])
print("Got some timer hits!\n");
#endif

#if USE_SIGSTACK
/* We leave the sigstack in place for the timer so any racy alarm arriving
* after we disabled the itimer will be on the alt stack.
*/
#if USE_SIGSTACK && !USE_TIMER
stack_t check_stack;
rc = sigaltstack(NULL, &check_stack);
ASSERT_NOERR(rc);
Expand Down

0 comments on commit f2c9756

Please sign in to comment.