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 <arnez@linux.vnet.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>

diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
index ef46f66bc0d6bdf0caa0dc4ecaae4225f3d3b1ef..f954b37740c0605e6d4a4ad34a9b7d4005b7d5a1 100644 (file)
--- 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 e42842a3072b37b80ca09cb0a96cdd999c016ba3..7a2d22dda9efd16fb73a51350c0bb35bd615e729 100644 (file)
--- 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 d1259d875074179ff11b232258fcc36ef98b90c3..c3ff70a7b2472b5e91626d9aea6f366e217ace46 100644 (file)
--- 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 870a644895f0e791f2a47ae5158e2c7159e85858..42601d6e166fe73db1e6e97382df8158212dd5d7 100644 (file)
--- 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;