Skip to content

Commit

Permalink
[S390] signal race with restarting system calls
Browse files Browse the repository at this point in the history
For a ERESTARTNOHAND/ERESTARTSYS/ERESTARTNOINTR restarting system call
do_signal will prepare the restart of the system call with a rewind of
the PSW before calling get_signal_to_deliver (where the debugger might
take control). For A ERESTART_RESTARTBLOCK restarting system call
do_signal will set -EINTR as return code.
There are two issues with this approach:
1) strace never sees ERESTARTNOHAND, ERESTARTSYS, ERESTARTNOINTR or
   ERESTART_RESTARTBLOCK as the rewinding already took place or the
   return code has been changed to -EINTR
2) if get_signal_to_deliver does not return with a signal to deliver
   the restart via the repeat of the svc instruction is left in place.
   This opens a race if another signal is made pending before the
   system call instruction can be reexecuted. The original system call
   will be restarted even if the second signal would have ended the
   system call with -EINTR.

These two issues can be solved by dropping the early rewind of the
system call before get_signal_to_deliver has been called and by using
the TIF_RESTART_SVC magic to do the restart if no signal has to be
delivered. The only situation where the system call restart via the
repeat of the svc instruction is appropriate is when a SA_RESTART
signal is delivered to user space.

Unfortunately this breaks inferior calls by the debugger again. The
system call number and the length of the system call instruction is
lost over the inferior call and user space will see ERESTARTNOHAND/
ERESTARTSYS/ERESTARTNOINTR/ERESTART_RESTARTBLOCK. To correct this a
new ptrace interface is added to save/restore the system call number
and system call instruction length.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
  • Loading branch information
Martin Schwidefsky committed Oct 30, 2011
1 parent 3ee49c8 commit 20b40a7
Show file tree
Hide file tree
Showing 10 changed files with 142 additions and 91 deletions.
5 changes: 3 additions & 2 deletions arch/s390/include/asm/ptrace.h
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,7 @@ struct pt_regs
psw_t psw;
unsigned long gprs[NUM_GPRS];
unsigned long orig_gpr2;
unsigned short ilc;
unsigned short svcnr;
unsigned int svc_code;
};

/*
Expand Down Expand Up @@ -487,6 +486,8 @@ typedef struct
#define PTRACE_POKETEXT_AREA 0x5004
#define PTRACE_POKEDATA_AREA 0x5005
#define PTRACE_GET_LAST_BREAK 0x5006
#define PTRACE_PEEK_SYSTEM_CALL 0x5007
#define PTRACE_POKE_SYSTEM_CALL 0x5008

/*
* PT_PROT definition is loosely based on hppa bsd definition in
Expand Down
5 changes: 3 additions & 2 deletions arch/s390/include/asm/syscall.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#define _ASM_SYSCALL_H 1

#include <linux/sched.h>
#include <linux/err.h>
#include <asm/ptrace.h>

/*
Expand All @@ -25,7 +26,7 @@ extern const unsigned int sys_call_table[];
static inline long syscall_get_nr(struct task_struct *task,
struct pt_regs *regs)
{
return regs->svcnr ? regs->svcnr : -1;
return regs->svc_code ? (regs->svc_code & 0xffff) : -1;
}

static inline void syscall_rollback(struct task_struct *task,
Expand All @@ -37,7 +38,7 @@ static inline void syscall_rollback(struct task_struct *task,
static inline long syscall_get_error(struct task_struct *task,
struct pt_regs *regs)
{
return (regs->gprs[2] >= -4096UL) ? -regs->gprs[2] : 0;
return IS_ERR_VALUE(regs->gprs[2]) ? regs->gprs[2] : 0;
}

static inline long syscall_get_return_value(struct task_struct *task,
Expand Down
1 change: 1 addition & 0 deletions arch/s390/include/asm/thread_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ struct thread_info {
unsigned int cpu; /* current CPU */
int preempt_count; /* 0 => preemptable, <0 => BUG */
struct restart_block restart_block;
unsigned int system_call;
__u64 user_timer;
__u64 system_timer;
unsigned long last_break; /* last breaking-event-address. */
Expand Down
3 changes: 1 addition & 2 deletions arch/s390/kernel/asm-offsets.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ int main(void)
DEFINE(__PT_PSW, offsetof(struct pt_regs, psw));
DEFINE(__PT_GPRS, offsetof(struct pt_regs, gprs));
DEFINE(__PT_ORIG_GPR2, offsetof(struct pt_regs, orig_gpr2));
DEFINE(__PT_ILC, offsetof(struct pt_regs, ilc));
DEFINE(__PT_SVCNR, offsetof(struct pt_regs, svcnr));
DEFINE(__PT_SVC_CODE, offsetof(struct pt_regs, svc_code));
DEFINE(__PT_SIZE, sizeof(struct pt_regs));
BLANK();
DEFINE(__SF_BACKCHAIN, offsetof(struct stack_frame, back_chain));
Expand Down
2 changes: 1 addition & 1 deletion arch/s390/kernel/compat_signal.c
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ static int restore_sigregs32(struct pt_regs *regs,_sigregs32 __user *sregs)
return err;

restore_fp_regs(&current->thread.fp_regs);
regs->svcnr = 0; /* disable syscall checks */
regs->svc_code = 0; /* disable syscall checks */
return 0;
}

Expand Down
28 changes: 14 additions & 14 deletions arch/s390/kernel/entry.S
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ SP_R13 = STACK_FRAME_OVERHEAD + __PT_GPRS + 52
SP_R14 = STACK_FRAME_OVERHEAD + __PT_GPRS + 56
SP_R15 = STACK_FRAME_OVERHEAD + __PT_GPRS + 60
SP_ORIG_R2 = STACK_FRAME_OVERHEAD + __PT_ORIG_GPR2
SP_ILC = STACK_FRAME_OVERHEAD + __PT_ILC
SP_SVCNR = STACK_FRAME_OVERHEAD + __PT_SVCNR
SP_SVC_CODE = STACK_FRAME_OVERHEAD + __PT_SVC_CODE
SP_SIZE = STACK_FRAME_OVERHEAD + __PT_SIZE

_TIF_WORK_SVC = (_TIF_SIGPENDING | _TIF_NOTIFY_RESUME | _TIF_NEED_RESCHED | \
Expand Down Expand Up @@ -229,7 +228,7 @@ sysc_saveall:
SAVE_ALL_SVC __LC_SVC_OLD_PSW,__LC_SAVE_AREA
CREATE_STACK_FRAME __LC_SAVE_AREA
mvc SP_PSW(8,%r15),__LC_SVC_OLD_PSW
mvc SP_ILC(4,%r15),__LC_SVC_ILC
mvc SP_SVC_CODE(4,%r15),__LC_SVC_ILC
l %r12,__LC_THREAD_INFO # load pointer to thread_info struct
sysc_vtime:
UPDATE_VTIME __LC_EXIT_TIMER,__LC_SYNC_ENTER_TIMER,__LC_USER_TIMER
Expand All @@ -239,12 +238,12 @@ sysc_update:
mvc __LC_LAST_UPDATE_TIMER(8),__LC_SYNC_ENTER_TIMER
sysc_do_svc:
xr %r7,%r7
icm %r7,3,SP_SVCNR(%r15) # load svc number and test for svc 0
icm %r7,3,SP_SVC_CODE+2(%r15)# load svc number and test for svc 0
bnz BASED(sysc_nr_ok) # svc number > 0
# svc 0: system call number in %r1
cl %r1,BASED(.Lnr_syscalls)
bnl BASED(sysc_nr_ok)
sth %r1,SP_SVCNR(%r15)
sth %r1,SP_SVC_CODE+2(%r15)
lr %r7,%r1 # copy svc number to %r7
sysc_nr_ok:
sll %r7,2 # svc number *4
Expand Down Expand Up @@ -335,18 +334,19 @@ sysc_notify_resume:
#
sysc_restart:
ni __TI_flags+3(%r12),255-_TIF_RESTART_SVC # clear TIF_RESTART_SVC
l %r7,SP_R2(%r15) # load new svc number
mvc SP_R2(4,%r15),SP_ORIG_R2(%r15) # restore first argument
lm %r2,%r6,SP_R2(%r15) # load svc arguments
sth %r7,SP_SVCNR(%r15)
xr %r7,%r7 # svc 0 returns -ENOSYS
clc SP_SVC_CODE+2(%r15),BASED(.Lnr_syscalls+2)
bnl BASED(sysc_nr_ok) # invalid svc number -> do svc 0
icm %r7,3,SP_SVC_CODE+2(%r15)# load new svc number
b BASED(sysc_nr_ok) # restart svc

#
# _TIF_PER_TRAP is set, call do_per_trap
#
sysc_singlestep:
ni __TI_flags+3(%r12),255-_TIF_PER_TRAP # clear TIF_PER_TRAP
xc SP_SVCNR(2,%r15),SP_SVCNR(%r15) # clear svc number
xc SP_SVC_CODE(4,%r15),SP_SVC_CODE(%r15) # clear svc code
la %r2,SP_PTREGS(%r15) # address of register-save area
l %r1,BASED(.Lhandle_per) # load adr. of per handler
la %r14,BASED(sysc_return) # load adr. of system return
Expand All @@ -361,7 +361,7 @@ sysc_tracesys:
la %r2,SP_PTREGS(%r15) # load pt_regs
la %r3,0
xr %r0,%r0
icm %r0,3,SP_SVCNR(%r15)
icm %r0,3,SP_SVC_CODE(%r15)
st %r0,SP_R2(%r15)
basr %r14,%r1
cl %r2,BASED(.Lnr_syscalls)
Expand Down Expand Up @@ -454,7 +454,7 @@ ENTRY(pgm_check_handler)
bnz BASED(pgm_per) # got per exception -> special case
SAVE_ALL_PGM __LC_PGM_OLD_PSW,__LC_SAVE_AREA
CREATE_STACK_FRAME __LC_SAVE_AREA
xc SP_ILC(4,%r15),SP_ILC(%r15)
xc SP_SVC_CODE(4,%r15),SP_SVC_CODE(%r15)
mvc SP_PSW(8,%r15),__LC_PGM_OLD_PSW
l %r12,__LC_THREAD_INFO # load pointer to thread_info struct
tm SP_PSW+1(%r15),0x01 # interrupting from user ?
Expand Down Expand Up @@ -531,7 +531,7 @@ pgm_svcper:
SAVE_ALL_PGM __LC_SVC_OLD_PSW,__LC_SAVE_AREA
CREATE_STACK_FRAME __LC_SAVE_AREA
mvc SP_PSW(8,%r15),__LC_SVC_OLD_PSW
mvc SP_ILC(4,%r15),__LC_SVC_ILC
mvc SP_SVC_CODE(4,%r15),__LC_SVC_ILC
l %r12,__LC_THREAD_INFO # load pointer to thread_info struct
UPDATE_VTIME __LC_EXIT_TIMER,__LC_SYNC_ENTER_TIMER,__LC_USER_TIMER
UPDATE_VTIME __LC_LAST_UPDATE_TIMER,__LC_EXIT_TIMER,__LC_SYSTEM_TIMER
Expand All @@ -550,7 +550,7 @@ pgm_svcper:
#
kernel_per:
REENABLE_IRQS
xc SP_SVCNR(2,%r15),SP_SVCNR(%r15)
xc SP_SVC_CODE(4,%r15),SP_SVC_CODE(%r15)
la %r2,SP_PTREGS(%r15) # address of register-save area
l %r1,BASED(.Lhandle_per) # load adr. of per handler
basr %r14,%r1 # branch to do_single_step
Expand Down Expand Up @@ -966,7 +966,7 @@ cleanup_system_call:
st %r15,12(%r12)
CREATE_STACK_FRAME __LC_SAVE_AREA
mvc SP_PSW(8,%r15),__LC_SVC_OLD_PSW
mvc SP_ILC(4,%r15),__LC_SVC_ILC
mvc SP_SVC_CODE(4,%r15),__LC_SVC_ILC
mvc 0(4,%r12),__LC_THREAD_INFO
cleanup_vtime:
clc __LC_RETURN_PSW+4(4),BASED(cleanup_system_call_insn+12)
Expand Down
30 changes: 15 additions & 15 deletions arch/s390/kernel/entry64.S
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ SP_R13 = STACK_FRAME_OVERHEAD + __PT_GPRS + 104
SP_R14 = STACK_FRAME_OVERHEAD + __PT_GPRS + 112
SP_R15 = STACK_FRAME_OVERHEAD + __PT_GPRS + 120
SP_ORIG_R2 = STACK_FRAME_OVERHEAD + __PT_ORIG_GPR2
SP_ILC = STACK_FRAME_OVERHEAD + __PT_ILC
SP_SVCNR = STACK_FRAME_OVERHEAD + __PT_SVCNR
SP_SVC_CODE = STACK_FRAME_OVERHEAD + __PT_SVC_CODE
SP_SIZE = STACK_FRAME_OVERHEAD + __PT_SIZE

STACK_SHIFT = PAGE_SHIFT + THREAD_ORDER
Expand Down Expand Up @@ -250,7 +249,7 @@ sysc_saveall:
SAVE_ALL_SVC __LC_SVC_OLD_PSW,__LC_SAVE_AREA
CREATE_STACK_FRAME __LC_SAVE_AREA
mvc SP_PSW(16,%r15),__LC_SVC_OLD_PSW
mvc SP_ILC(4,%r15),__LC_SVC_ILC
mvc SP_SVC_CODE(4,%r15),__LC_SVC_ILC
lg %r12,__LC_THREAD_INFO # load pointer to thread_info struct
sysc_vtime:
UPDATE_VTIME __LC_EXIT_TIMER,__LC_SYNC_ENTER_TIMER,__LC_USER_TIMER
Expand All @@ -260,14 +259,14 @@ sysc_update:
mvc __LC_LAST_UPDATE_TIMER(8),__LC_SYNC_ENTER_TIMER
LAST_BREAK
sysc_do_svc:
llgh %r7,SP_SVCNR(%r15)
llgh %r7,SP_SVC_CODE+2(%r15)
slag %r7,%r7,2 # shift and test for svc 0
jnz sysc_nr_ok
# svc 0: system call number in %r1
llgfr %r1,%r1 # clear high word in r1
cghi %r1,NR_syscalls
jnl sysc_nr_ok
sth %r1,SP_SVCNR(%r15)
sth %r1,SP_SVC_CODE+2(%r15)
slag %r7,%r1,2 # shift and test for svc 0
sysc_nr_ok:
larl %r10,sys_call_table
Expand Down Expand Up @@ -358,19 +357,20 @@ sysc_notify_resume:
#
sysc_restart:
ni __TI_flags+7(%r12),255-_TIF_RESTART_SVC # clear TIF_RESTART_SVC
lg %r7,SP_R2(%r15) # load new svc number
mvc SP_R2(8,%r15),SP_ORIG_R2(%r15) # restore first argument
lmg %r2,%r6,SP_R2(%r15) # load svc arguments
sth %r7,SP_SVCNR(%r15)
slag %r7,%r7,2
lghi %r7,0 # svc 0 returns -ENOSYS
lh %r1,SP_SVC_CODE+2(%r15) # load new svc number
cghi %r1,NR_syscalls
jnl sysc_nr_ok # invalid svc number -> do svc 0
slag %r7,%r1,2
j sysc_nr_ok # restart svc

#
# _TIF_PER_TRAP is set, call do_per_trap
#
sysc_singlestep:
ni __TI_flags+7(%r12),255-_TIF_PER_TRAP # clear TIF_PER_TRAP
xc SP_SVCNR(2,%r15),SP_SVCNR(%r15) # clear svc number
xc SP_SVC_CODE(4,%r15),SP_SVC_CODE(%r15) # clear svc code
la %r2,SP_PTREGS(%r15) # address of register-save area
larl %r14,sysc_return # load adr. of system return
jg do_per_trap
Expand All @@ -382,7 +382,7 @@ sysc_singlestep:
sysc_tracesys:
la %r2,SP_PTREGS(%r15) # load pt_regs
la %r3,0
llgh %r0,SP_SVCNR(%r15)
llgh %r0,SP_SVC_CODE+2(%r15)
stg %r0,SP_R2(%r15)
brasl %r14,do_syscall_trace_enter
lghi %r0,NR_syscalls
Expand Down Expand Up @@ -470,7 +470,7 @@ ENTRY(pgm_check_handler)
jnz pgm_per # got per exception -> special case
SAVE_ALL_PGM __LC_PGM_OLD_PSW,__LC_SAVE_AREA
CREATE_STACK_FRAME __LC_SAVE_AREA
xc SP_ILC(4,%r15),SP_ILC(%r15)
xc SP_SVC_CODE(4,%r15),SP_SVC_CODE(%r15)
mvc SP_PSW(16,%r15),__LC_PGM_OLD_PSW
lg %r12,__LC_THREAD_INFO # load pointer to thread_info struct
HANDLE_SIE_INTERCEPT
Expand Down Expand Up @@ -551,7 +551,7 @@ pgm_svcper:
SAVE_ALL_PGM __LC_SVC_OLD_PSW,__LC_SAVE_AREA
CREATE_STACK_FRAME __LC_SAVE_AREA
mvc SP_PSW(16,%r15),__LC_SVC_OLD_PSW
mvc SP_ILC(4,%r15),__LC_SVC_ILC
mvc SP_SVC_CODE(4,%r15),__LC_SVC_ILC
lg %r12,__LC_THREAD_INFO # load pointer to thread_info struct
UPDATE_VTIME __LC_EXIT_TIMER,__LC_SYNC_ENTER_TIMER,__LC_USER_TIMER
UPDATE_VTIME __LC_LAST_UPDATE_TIMER,__LC_EXIT_TIMER,__LC_SYSTEM_TIMER
Expand All @@ -571,7 +571,7 @@ pgm_svcper:
#
kernel_per:
REENABLE_IRQS
xc SP_SVCNR(2,%r15),SP_SVCNR(%r15) # clear svc number
xc SP_SVC_CODE(4,%r15),SP_SVC_CODE(%r15) # clear svc number
la %r2,SP_PTREGS(%r15) # address of register-save area
brasl %r14,do_per_trap
j pgm_exit
Expand Down Expand Up @@ -973,7 +973,7 @@ cleanup_system_call:
stg %r11,0(%r12)
CREATE_STACK_FRAME __LC_SAVE_AREA
mvc SP_PSW(16,%r15),__LC_SVC_OLD_PSW
mvc SP_ILC(4,%r15),__LC_SVC_ILC
mvc SP_SVC_CODE(4,%r15),__LC_SVC_ILC
mvc 8(8,%r12),__LC_THREAD_INFO
cleanup_vtime:
clc __LC_RETURN_PSW+8(8),BASED(cleanup_system_call_insn+24)
Expand Down
51 changes: 50 additions & 1 deletion arch/s390/kernel/ptrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ enum s390_regset {
REGSET_GENERAL,
REGSET_FP,
REGSET_LAST_BREAK,
REGSET_SYSTEM_CALL,
REGSET_GENERAL_EXTENDED,
};

Expand Down Expand Up @@ -303,6 +304,13 @@ static int __poke_user(struct task_struct *child, addr_t addr, addr_t data)
high order bit but older gdb's rely on it */
data |= PSW_ADDR_AMODE;
#endif
if (addr == (addr_t) &dummy->regs.psw.addr)
/*
* The debugger changed the instruction address,
* reset system call restart, see signal.c:do_signal
*/
task_thread_info(child)->system_call = 0;

*(addr_t *)((addr_t) &task_pt_regs(child)->psw + addr) = data;

} else if (addr < (addr_t) (&dummy->regs.orig_gpr2)) {
Expand Down Expand Up @@ -610,6 +618,11 @@ static int __poke_user_compat(struct task_struct *child,
/* Build a 64 bit psw address from 31 bit address. */
task_pt_regs(child)->psw.addr =
(__u64) tmp & PSW32_ADDR_INSN;
/*
* The debugger changed the instruction address,
* reset system call restart, see signal.c:do_signal
*/
task_thread_info(child)->system_call = 0;
} else {
/* gpr 0-15 */
*(__u32*)((addr_t) &task_pt_regs(child)->psw
Expand Down Expand Up @@ -737,7 +750,7 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
* debugger stored an invalid system call number. Skip
* the system call and the system call restart handling.
*/
regs->svcnr = 0;
regs->svc_code = 0;
ret = -1;
}

Expand Down Expand Up @@ -899,6 +912,26 @@ static int s390_last_break_get(struct task_struct *target,

#endif

static int s390_system_call_get(struct task_struct *target,
const struct user_regset *regset,
unsigned int pos, unsigned int count,
void *kbuf, void __user *ubuf)
{
unsigned int *data = &task_thread_info(target)->system_call;
return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
data, 0, sizeof(unsigned int));
}

static int s390_system_call_set(struct task_struct *target,
const struct user_regset *regset,
unsigned int pos, unsigned int count,
const void *kbuf, const void __user *ubuf)
{
unsigned int *data = &task_thread_info(target)->system_call;
return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
data, 0, sizeof(unsigned int));
}

static const struct user_regset s390_regsets[] = {
[REGSET_GENERAL] = {
.core_note_type = NT_PRSTATUS,
Expand All @@ -925,6 +958,14 @@ static const struct user_regset s390_regsets[] = {
.get = s390_last_break_get,
},
#endif
[REGSET_SYSTEM_CALL] = {
.core_note_type = NT_S390_SYSTEM_CALL,
.n = 1,
.size = sizeof(unsigned int),
.align = sizeof(unsigned int),
.get = s390_system_call_get,
.set = s390_system_call_set,
},
};

static const struct user_regset_view user_s390_view = {
Expand Down Expand Up @@ -1104,6 +1145,14 @@ static const struct user_regset s390_compat_regsets[] = {
.align = sizeof(long),
.get = s390_compat_last_break_get,
},
[REGSET_SYSTEM_CALL] = {
.core_note_type = NT_S390_SYSTEM_CALL,
.n = 1,
.size = sizeof(compat_uint_t),
.align = sizeof(compat_uint_t),
.get = s390_system_call_get,
.set = s390_system_call_set,
},
[REGSET_GENERAL_EXTENDED] = {
.core_note_type = NT_S390_HIGH_GPRS,
.n = sizeof(s390_compat_regs_high) / sizeof(compat_long_t),
Expand Down
Loading

0 comments on commit 20b40a7

Please sign in to comment.