aboutsummaryrefslogtreecommitdiffstats
path: root/arch/x86/kernel
diff options
context:
space:
mode:
authorRoland McGrath <roland@redhat.com>2008-07-10 17:50:39 -0400
committerIngo Molnar <mingo@elte.hu>2008-07-12 01:11:10 -0400
commiteca91e7838ec92e8c12122849ec7539c4765b689 (patch)
treebfc7decbcb96189e40307ab80f96aac59526ecc7 /arch/x86/kernel
parentda1f29f5dfcc8641f9ff6f3deaa9f03b57e63229 (diff)
x86_64: fix delayed signals
On three of the several paths in entry_64.S that call do_notify_resume() on the way back to user mode, we fail to properly check again for newly-arrived work that requires another call to do_notify_resume() before going to user mode. These paths set the mask to check only _TIF_NEED_RESCHED, but this is wrong. The other paths that lead to do_notify_resume() do this correctly already, and entry_32.S does it correctly in all cases. All paths back to user mode have to check all the _TIF_WORK_MASK flags at the last possible stage, with interrupts disabled. Otherwise, we miss any flags (TIF_SIGPENDING for example) that were set any time after we entered do_notify_resume(). More work flags can be set (or left set) synchronously inside do_notify_resume(), as TIF_SIGPENDING can be, or asynchronously by interrupts or other CPUs (which then send an asynchronous interrupt). There are many different scenarios that could hit this bug, most of them races. The simplest one to demonstrate does not require any race: when one signal has done handler setup at the check before returning from a syscall, and there is another signal pending that should be handled. The second signal's handler should interrupt the first signal handler before it actually starts (so the interrupted PC is still at the handler's entry point). Instead, it runs away until the next kernel entry (next syscall, tick, etc). This test behaves correctly on 32-bit kernels, and fails on 64-bit (either 32-bit or 64-bit test binary). With this fix, it works. #define _GNU_SOURCE #include <stdio.h> #include <signal.h> #include <string.h> #include <sys/ucontext.h> #ifndef REG_RIP #define REG_RIP REG_EIP #endif static sig_atomic_t hit1, hit2; static void handler (int sig, siginfo_t *info, void *ctx) { ucontext_t *uc = ctx; if ((void *) uc->uc_mcontext.gregs[REG_RIP] == &handler) { if (sig == SIGUSR1) hit1 = 1; else hit2 = 1; } printf ("%s at %#lx\n", strsignal (sig), uc->uc_mcontext.gregs[REG_RIP]); } int main (void) { struct sigaction sa; sigset_t set; sigemptyset (&sa.sa_mask); sa.sa_flags = SA_SIGINFO; sa.sa_sigaction = &handler; if (sigaction (SIGUSR1, &sa, NULL) || sigaction (SIGUSR2, &sa, NULL)) return 2; sigemptyset (&set); sigaddset (&set, SIGUSR1); sigaddset (&set, SIGUSR2); if (sigprocmask (SIG_BLOCK, &set, NULL)) return 3; printf ("main at %p, handler at %p\n", &main, &handler); raise (SIGUSR1); raise (SIGUSR2); if (sigprocmask (SIG_UNBLOCK, &set, NULL)) return 4; if (hit1 + hit2 == 1) { puts ("PASS"); return 0; } puts ("FAIL"); return 1; } Signed-off-by: Roland McGrath <roland@redhat.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Ingo Molnar <mingo@elte.hu>
Diffstat (limited to 'arch/x86/kernel')
-rw-r--r--arch/x86/kernel/entry_64.S7
1 files changed, 3 insertions, 4 deletions
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 466b9284ed2f..bb4e22f4892f 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -306,7 +306,7 @@ sysret_signal:
306 leaq -ARGOFFSET(%rsp),%rdi # &pt_regs -> arg1 306 leaq -ARGOFFSET(%rsp),%rdi # &pt_regs -> arg1
307 xorl %esi,%esi # oldset -> arg2 307 xorl %esi,%esi # oldset -> arg2
308 call ptregscall_common 308 call ptregscall_common
3091: movl $_TIF_NEED_RESCHED,%edi 3091: movl $_TIF_WORK_MASK,%edi
310 /* Use IRET because user could have changed frame. This 310 /* Use IRET because user could have changed frame. This
311 works because ptregscall_common has called FIXUP_TOP_OF_STACK. */ 311 works because ptregscall_common has called FIXUP_TOP_OF_STACK. */
312 DISABLE_INTERRUPTS(CLBR_NONE) 312 DISABLE_INTERRUPTS(CLBR_NONE)
@@ -394,7 +394,7 @@ int_signal:
394 movq %rsp,%rdi # &ptregs -> arg1 394 movq %rsp,%rdi # &ptregs -> arg1
395 xorl %esi,%esi # oldset -> arg2 395 xorl %esi,%esi # oldset -> arg2
396 call do_notify_resume 396 call do_notify_resume
3971: movl $_TIF_NEED_RESCHED,%edi 3971: movl $_TIF_WORK_MASK,%edi
398int_restore_rest: 398int_restore_rest:
399 RESTORE_REST 399 RESTORE_REST
400 DISABLE_INTERRUPTS(CLBR_NONE) 400 DISABLE_INTERRUPTS(CLBR_NONE)
@@ -647,9 +647,8 @@ retint_signal:
647 RESTORE_REST 647 RESTORE_REST
648 DISABLE_INTERRUPTS(CLBR_NONE) 648 DISABLE_INTERRUPTS(CLBR_NONE)
649 TRACE_IRQS_OFF 649 TRACE_IRQS_OFF
650 movl $_TIF_NEED_RESCHED,%edi
651 GET_THREAD_INFO(%rcx) 650 GET_THREAD_INFO(%rcx)
652 jmp retint_check 651 jmp retint_with_reschedule
653 652
654#ifdef CONFIG_PREEMPT 653#ifdef CONFIG_PREEMPT
655 /* Returning to kernel space. Check if we need preemption */ 654 /* Returning to kernel space. Check if we need preemption */