From a1277fba564ec8c93c14bd87c8f4b371ebfd94a4 Mon Sep 17 00:00:00 2001 From: Derek Bruening Date: Mon, 14 May 2018 16:39:12 -0400 Subject: [PATCH] i#2995: do not blindly "restart" sigreturn 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 --- core/unix/os.c | 8 ++++---- core/unix/os_private.h | 3 +++ core/unix/signal.c | 13 ++++++++++--- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/core/unix/os.c b/core/unix/os.c index 1e2d619c209..4a7f62205a7 100644 --- a/core/unix/os.c +++ b/core/unix/os.c @@ -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; @@ -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 */ diff --git a/core/unix/os_private.h b/core/unix/os_private.h index efc27bf19af..44af75dcd0c 100644 --- a/core/unix/os_private.h +++ b/core/unix/os_private.h @@ -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; diff --git a/core/unix/signal.c b/core/unix/signal.c index c8d892aab7e..f5596f525df 100644 --- a/core/unix/signal.c +++ b/core/unix/signal.c @@ -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); @@ -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