From 39efd4ec9a2967e9720be7b66d9a4b31a58dbf61 Mon Sep 17 00:00:00 2001 From: Martin Schwidefsky Date: Wed, 21 Nov 2012 16:36:27 +0100 Subject: s390/ptrace: race of single stepping vs signal delivery The current single step code is racy in regard to concurrent delivery of signals. If a signal is delivered after a PER program check occurred but before the TIF_PER_TRAP bit has been checked in entry[64].S the code clears TIF_PER_TRAP and then calls do_signal. This is wrong, if the instruction completed (or has been suppressed) a SIGTRAP should be delivered to the debugger in any case. Only if the instruction has been nullified the SIGTRAP may not be send. The new logic always sets TIF_PER_TRAP if the program check indicates PER tracing but removes it again for all program checks that are nullifying. The effect is that for each change in the PSW address we now get a single SIGTRAP. Reported-by: Andreas Arnez Signed-off-by: Martin Schwidefsky --- arch/s390/kernel/entry.S | 7 +++---- arch/s390/kernel/entry64.S | 7 +++---- arch/s390/kernel/signal.c | 2 ++ arch/s390/mm/fault.c | 19 +++++++++++++------ 4 files changed, 21 insertions(+), 14 deletions(-) (limited to 'arch/s390') diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S index ef46f66bc0d..f954b37740c 100644 --- a/arch/s390/kernel/entry.S +++ b/arch/s390/kernel/entry.S @@ -231,12 +231,12 @@ sysc_work: jo sysc_mcck_pending tm __TI_flags+3(%r12),_TIF_NEED_RESCHED jo sysc_reschedule + tm __TI_flags+3(%r12),_TIF_PER_TRAP + jo sysc_singlestep tm __TI_flags+3(%r12),_TIF_SIGPENDING jo sysc_sigpending tm __TI_flags+3(%r12),_TIF_NOTIFY_RESUME jo sysc_notify_resume - tm __TI_flags+3(%r12),_TIF_PER_TRAP - jo sysc_singlestep j sysc_return # beware of critical section cleanup # @@ -259,7 +259,6 @@ sysc_mcck_pending: # _TIF_SIGPENDING is set, call do_signal # sysc_sigpending: - ni __TI_flags+3(%r12),255-_TIF_PER_TRAP # clear TIF_PER_TRAP lr %r2,%r11 # pass pointer to pt_regs l %r1,BASED(.Ldo_signal) basr %r14,%r1 # call do_signal @@ -286,7 +285,7 @@ sysc_notify_resume: # _TIF_PER_TRAP is set, call do_per_trap # sysc_singlestep: - ni __TI_flags+3(%r12),255-(_TIF_SYSCALL | _TIF_PER_TRAP) + ni __TI_flags+3(%r12),255-_TIF_PER_TRAP lr %r2,%r11 # pass pointer to pt_regs l %r1,BASED(.Ldo_per_trap) la %r14,BASED(sysc_return) diff --git a/arch/s390/kernel/entry64.S b/arch/s390/kernel/entry64.S index e42842a3072..7a2d22dda9e 100644 --- a/arch/s390/kernel/entry64.S +++ b/arch/s390/kernel/entry64.S @@ -262,12 +262,12 @@ sysc_work: jo sysc_mcck_pending tm __TI_flags+7(%r12),_TIF_NEED_RESCHED jo sysc_reschedule + tm __TI_flags+7(%r12),_TIF_PER_TRAP + jo sysc_singlestep tm __TI_flags+7(%r12),_TIF_SIGPENDING jo sysc_sigpending tm __TI_flags+7(%r12),_TIF_NOTIFY_RESUME jo sysc_notify_resume - tm __TI_flags+7(%r12),_TIF_PER_TRAP - jo sysc_singlestep j sysc_return # beware of critical section cleanup # @@ -288,7 +288,6 @@ sysc_mcck_pending: # _TIF_SIGPENDING is set, call do_signal # sysc_sigpending: - ni __TI_flags+7(%r12),255-_TIF_PER_TRAP # clear TIF_PER_TRAP lgr %r2,%r11 # pass pointer to pt_regs brasl %r14,do_signal tm __TI_flags+7(%r12),_TIF_SYSCALL @@ -313,7 +312,7 @@ sysc_notify_resume: # _TIF_PER_TRAP is set, call do_per_trap # sysc_singlestep: - ni __TI_flags+7(%r12),255-(_TIF_SYSCALL | _TIF_PER_TRAP) + ni __TI_flags+7(%r12),255-_TIF_PER_TRAP lgr %r2,%r11 # pass pointer to pt_regs larl %r14,sysc_return jg do_per_trap diff --git a/arch/s390/kernel/signal.c b/arch/s390/kernel/signal.c index d1259d87507..c3ff70a7b24 100644 --- a/arch/s390/kernel/signal.c +++ b/arch/s390/kernel/signal.c @@ -461,6 +461,8 @@ void do_signal(struct pt_regs *regs) /* Restart system call with magic TIF bit. */ regs->gprs[2] = regs->orig_gpr2; set_thread_flag(TIF_SYSCALL); + if (test_thread_flag(TIF_SINGLE_STEP)) + set_thread_flag(TIF_PER_TRAP); break; } } diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c index 870a644895f..42601d6e166 100644 --- a/arch/s390/mm/fault.c +++ b/arch/s390/mm/fault.c @@ -277,10 +277,16 @@ static inline int do_exception(struct pt_regs *regs, int access) unsigned int flags; int fault; + tsk = current; + /* + * The instruction that caused the program check has + * been nullified. Don't signal single step via SIGTRAP. + */ + clear_tsk_thread_flag(tsk, TIF_PER_TRAP); + if (notify_page_fault(regs)) return 0; - tsk = current; mm = tsk->mm; trans_exc_code = regs->int_parm_long; @@ -376,11 +382,6 @@ retry: goto retry; } } - /* - * The instruction that caused the program check will - * be repeated. Don't signal single step via SIGTRAP. - */ - clear_tsk_thread_flag(tsk, TIF_PER_TRAP); fault = 0; out_up: up_read(&mm->mmap_sem); @@ -427,6 +428,12 @@ void __kprobes do_asce_exception(struct pt_regs *regs) struct vm_area_struct *vma; unsigned long trans_exc_code; + /* + * The instruction that caused the program check has + * been nullified. Don't signal single step via SIGTRAP. + */ + clear_tsk_thread_flag(current, TIF_PER_TRAP); + trans_exc_code = regs->int_parm_long; if (unlikely(!user_space_fault(trans_exc_code) || in_atomic() || !mm)) goto no_context; -- cgit v1.2.3