Skip to content

Commit

Permalink
i#2995: do not blindly "restart" sigreturn (#3007)
Browse files Browse the repository at this point in the history
With SA_RESTART (#2659), we cannot distinguish
not-yet-executed-syscall from syscall-was-interrupted-in-the-kernel.
This doesn't matter for most syscalls, but it does matter for
sigreturn, whose asynch_target points somewhere other than right after
the syscall, so we exclude it (it can't be interrupted so we know we
haven't executed it yet).

Fixes #2995
  • Loading branch information
derekbruening authored May 14, 2018
1 parent fdcc488 commit 6d02275
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 8 deletions.
8 changes: 4 additions & 4 deletions core/unix/os.c
Original file line number Diff line number Diff line change
Expand Up @@ -5504,8 +5504,8 @@ was_thread_create_syscall(dcontext_t *dcontext)
dcontext->sys_param0);
}

static inline bool
is_sigreturn_syscall_helper(int sysnum)
bool
is_sigreturn_syscall_number(int sysnum)
{
#ifdef MACOS
return sysnum == SYS_sigreturn;
Expand All @@ -5518,13 +5518,13 @@ bool
is_sigreturn_syscall(dcontext_t *dcontext)
{
priv_mcontext_t *mc = get_mcontext(dcontext);
return is_sigreturn_syscall_helper(MCXT_SYSNUM_REG(mc));
return is_sigreturn_syscall_number(MCXT_SYSNUM_REG(mc));
}

bool
was_sigreturn_syscall(dcontext_t *dcontext)
{
return is_sigreturn_syscall_helper(dcontext->sys_num);
return is_sigreturn_syscall_number(dcontext->sys_num);
}

/* process a signal this process/thread is sending to itself */
Expand Down
3 changes: 3 additions & 0 deletions core/unix/os_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@ uint permstr_to_memprot(const char * const perm);
int
os_walk_address_space(memquery_iter_t *iter, bool add_modules);

bool
is_sigreturn_syscall_number(int sysnum);

/* in signal.c */
struct _kernel_sigaction_t;
typedef struct _kernel_sigaction_t kernel_sigaction_t;
Expand Down
3 changes: 2 additions & 1 deletion core/unix/os_public.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* **********************************************************
* Copyright (c) 2011-2016 Google, Inc. All rights reserved.
* Copyright (c) 2011-2018 Google, Inc. All rights reserved.
* Copyright (c) 2000-2010 VMware, Inc. All rights reserved.
* **********************************************************/

Expand Down Expand Up @@ -124,6 +124,7 @@ typedef kernel_sigcontext_t sigcontext_t;
# define SC_LR SC_FIELD(regs[30])
# define SC_XSP SC_FIELD(sp)
# define SC_XFLAGS SC_FIELD(pstate)
# define SC_SYSNUM_REG SC_FIELD(regs[8])
#elif defined(ARM)
# define SC_XIP SC_FIELD(arm_pc)
# define SC_FP SC_FIELD(arm_fp)
Expand Down
13 changes: 10 additions & 3 deletions core/unix/signal.c
Original file line number Diff line number Diff line change
Expand Up @@ -4057,9 +4057,14 @@ record_pending_signal(dcontext_t *dcontext, int sig, kernel_ucontext_t *ucxt,
* we can get rid of this emulation and the auto-restart emulation.
*/
/* The get_at_syscall() check above distinguishes from just having
* arrived at the syscall instr.
* arrived at the syscall instr, but with SA_RESTART we can't distinguish
* not-yet-executed-syscall from syscall-was-interrupted-in-the-kernel.
* This matters for sigreturn (i#2995), whose asynch_target points somewhere
* other than right after the syscall, so we exclude it (it can't be
* interrupted so we know we haven't executed it yet).
*/
if (is_after_syscall_address(dcontext, pc + syslen)) {
if (is_after_syscall_address(dcontext, pc + syslen) &&
!is_sigreturn_syscall_number(sc->SC_SYSNUM_REG)) {
LOG(THREAD, LOG_ASYNCH, 2,
"Adjusting interrupted auto-restart syscall from "PFX" to "PFX"\n",
pc, pc + syslen);
Expand Down Expand Up @@ -4092,7 +4097,9 @@ record_pending_signal(dcontext_t *dcontext, int sig, kernel_ucontext_t *ucxt,
}
}
}
} else if (get_at_syscall(dcontext) && pc == vsyscall_sysenter_return_pc - syslen) {
} else if (get_at_syscall(dcontext) && pc == vsyscall_sysenter_return_pc - syslen &&
/* See i#2995 comment above: rule out sigreturn */
!is_sigreturn_syscall_number(sc->SC_SYSNUM_REG)) {
LOG(THREAD, LOG_ASYNCH, 2,
"record_pending_signal(%d) from restart-vsyscall "PFX"\n", sig, pc);
/* While the kernel points at int 0x80 for a restart, we leverage our
Expand Down

0 comments on commit 6d02275

Please sign in to comment.