Commit 39efd4ec9a2967e9720be7b66d9a4b31a58dbf61

Authored by Martin Schwidefsky
1 parent c68dba202f

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>

Showing 4 changed files with 21 additions and 14 deletions Side-by-side Diff

arch/s390/kernel/entry.S
... ... @@ -231,12 +231,12 @@
231 231 jo sysc_mcck_pending
232 232 tm __TI_flags+3(%r12),_TIF_NEED_RESCHED
233 233 jo sysc_reschedule
  234 + tm __TI_flags+3(%r12),_TIF_PER_TRAP
  235 + jo sysc_singlestep
234 236 tm __TI_flags+3(%r12),_TIF_SIGPENDING
235 237 jo sysc_sigpending
236 238 tm __TI_flags+3(%r12),_TIF_NOTIFY_RESUME
237 239 jo sysc_notify_resume
238   - tm __TI_flags+3(%r12),_TIF_PER_TRAP
239   - jo sysc_singlestep
240 240 j sysc_return # beware of critical section cleanup
241 241  
242 242 #
... ... @@ -259,7 +259,6 @@
259 259 # _TIF_SIGPENDING is set, call do_signal
260 260 #
261 261 sysc_sigpending:
262   - ni __TI_flags+3(%r12),255-_TIF_PER_TRAP # clear TIF_PER_TRAP
263 262 lr %r2,%r11 # pass pointer to pt_regs
264 263 l %r1,BASED(.Ldo_signal)
265 264 basr %r14,%r1 # call do_signal
... ... @@ -286,7 +285,7 @@
286 285 # _TIF_PER_TRAP is set, call do_per_trap
287 286 #
288 287 sysc_singlestep:
289   - ni __TI_flags+3(%r12),255-(_TIF_SYSCALL | _TIF_PER_TRAP)
  288 + ni __TI_flags+3(%r12),255-_TIF_PER_TRAP
290 289 lr %r2,%r11 # pass pointer to pt_regs
291 290 l %r1,BASED(.Ldo_per_trap)
292 291 la %r14,BASED(sysc_return)
arch/s390/kernel/entry64.S
... ... @@ -262,12 +262,12 @@
262 262 jo sysc_mcck_pending
263 263 tm __TI_flags+7(%r12),_TIF_NEED_RESCHED
264 264 jo sysc_reschedule
  265 + tm __TI_flags+7(%r12),_TIF_PER_TRAP
  266 + jo sysc_singlestep
265 267 tm __TI_flags+7(%r12),_TIF_SIGPENDING
266 268 jo sysc_sigpending
267 269 tm __TI_flags+7(%r12),_TIF_NOTIFY_RESUME
268 270 jo sysc_notify_resume
269   - tm __TI_flags+7(%r12),_TIF_PER_TRAP
270   - jo sysc_singlestep
271 271 j sysc_return # beware of critical section cleanup
272 272  
273 273 #
... ... @@ -288,7 +288,6 @@
288 288 # _TIF_SIGPENDING is set, call do_signal
289 289 #
290 290 sysc_sigpending:
291   - ni __TI_flags+7(%r12),255-_TIF_PER_TRAP # clear TIF_PER_TRAP
292 291 lgr %r2,%r11 # pass pointer to pt_regs
293 292 brasl %r14,do_signal
294 293 tm __TI_flags+7(%r12),_TIF_SYSCALL
... ... @@ -313,7 +312,7 @@
313 312 # _TIF_PER_TRAP is set, call do_per_trap
314 313 #
315 314 sysc_singlestep:
316   - ni __TI_flags+7(%r12),255-(_TIF_SYSCALL | _TIF_PER_TRAP)
  315 + ni __TI_flags+7(%r12),255-_TIF_PER_TRAP
317 316 lgr %r2,%r11 # pass pointer to pt_regs
318 317 larl %r14,sysc_return
319 318 jg do_per_trap
arch/s390/kernel/signal.c
... ... @@ -461,6 +461,8 @@
461 461 /* Restart system call with magic TIF bit. */
462 462 regs->gprs[2] = regs->orig_gpr2;
463 463 set_thread_flag(TIF_SYSCALL);
  464 + if (test_thread_flag(TIF_SINGLE_STEP))
  465 + set_thread_flag(TIF_PER_TRAP);
464 466 break;
465 467 }
466 468 }
arch/s390/mm/fault.c
... ... @@ -277,10 +277,16 @@
277 277 unsigned int flags;
278 278 int fault;
279 279  
  280 + tsk = current;
  281 + /*
  282 + * The instruction that caused the program check has
  283 + * been nullified. Don't signal single step via SIGTRAP.
  284 + */
  285 + clear_tsk_thread_flag(tsk, TIF_PER_TRAP);
  286 +
280 287 if (notify_page_fault(regs))
281 288 return 0;
282 289  
283   - tsk = current;
284 290 mm = tsk->mm;
285 291 trans_exc_code = regs->int_parm_long;
286 292  
... ... @@ -376,11 +382,6 @@
376 382 goto retry;
377 383 }
378 384 }
379   - /*
380   - * The instruction that caused the program check will
381   - * be repeated. Don't signal single step via SIGTRAP.
382   - */
383   - clear_tsk_thread_flag(tsk, TIF_PER_TRAP);
384 385 fault = 0;
385 386 out_up:
386 387 up_read(&mm->mmap_sem);
... ... @@ -426,6 +427,12 @@
426 427 struct mm_struct *mm = current->mm;
427 428 struct vm_area_struct *vma;
428 429 unsigned long trans_exc_code;
  430 +
  431 + /*
  432 + * The instruction that caused the program check has
  433 + * been nullified. Don't signal single step via SIGTRAP.
  434 + */
  435 + clear_tsk_thread_flag(current, TIF_PER_TRAP);
429 436  
430 437 trans_exc_code = regs->int_parm_long;
431 438 if (unlikely(!user_space_fault(trans_exc_code) || in_atomic() || !mm))