* [PATCH 3/5] mips: sanitize restart logics
@ 2010-09-28 17:50 Al Viro
2010-09-29 18:28 ` David Daney
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Al Viro @ 2010-09-28 17:50 UTC (permalink / raw)
To: ralf; +Cc: linux-kernel, linux-arch
Put the original syscall number into ->regs[0] when we leave syscall
with error. Use it in restart logics. Everything else will have
it 0 since we pass through SAVE_SOME on all the ways in. Note that
in places like bad_stack and inllegal_syscall we leave it 0 - it's
not restartable.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
arch/mips/kernel/branch.c | 1 -
arch/mips/kernel/scall32-o32.S | 9 ++++-----
arch/mips/kernel/scall64-64.S | 7 ++++---
arch/mips/kernel/scall64-n32.S | 6 ++++--
arch/mips/kernel/scall64-o32.S | 7 ++++---
arch/mips/kernel/signal.c | 37 ++++++++++++++++++-------------------
arch/mips/kernel/unaligned.c | 2 --
7 files changed, 34 insertions(+), 35 deletions(-)
diff --git a/arch/mips/kernel/branch.c b/arch/mips/kernel/branch.c
index 0176ed0..32103cc 100644
--- a/arch/mips/kernel/branch.c
+++ b/arch/mips/kernel/branch.c
@@ -40,7 +40,6 @@ int __compute_return_epc(struct pt_regs *regs)
return -EFAULT;
}
- regs->regs[0] = 0;
switch (insn.i_format.opcode) {
/*
* jr and jalr are in r_format format.
diff --git a/arch/mips/kernel/scall32-o32.S b/arch/mips/kernel/scall32-o32.S
index 17202bb..d3edb9f 100644
--- a/arch/mips/kernel/scall32-o32.S
+++ b/arch/mips/kernel/scall32-o32.S
@@ -63,9 +63,9 @@ stack_done:
sw t0, PT_R7(sp) # set error flag
beqz t0, 1f
+ lw t1, PR_R2(sp) # syscall number
negu v0 # error
- sw v0, PT_R0(sp) # set flag for syscall
- # restarting
+ sw t1, PT_R0(sp) # save it for syscall restarting
1: sw v0, PT_R2(sp) # result
o32_syscall_exit:
@@ -104,9 +104,9 @@ syscall_trace_entry:
sw t0, PT_R7(sp) # set error flag
beqz t0, 1f
+ lw t1, PT_R2(sp) # syscall number
negu v0 # error
- sw v0, PT_R0(sp) # set flag for syscall
- # restarting
+ sw t1, PT_R0(sp) # save it for syscall restarting
1: sw v0, PT_R2(sp) # result
j syscall_exit
@@ -170,7 +170,6 @@ stackargs:
*/
bad_stack:
negu v0 # error
- sw v0, PT_R0(sp)
sw v0, PT_R2(sp)
li t0, 1 # set error flag
sw t0, PT_R7(sp)
diff --git a/arch/mips/kernel/scall64-64.S b/arch/mips/kernel/scall64-64.S
index a8a6c59..eb0bb73 100644
--- a/arch/mips/kernel/scall64-64.S
+++ b/arch/mips/kernel/scall64-64.S
@@ -66,9 +66,9 @@ NESTED(handle_sys64, PT_SIZE, sp)
sd t0, PT_R7(sp) # set error flag
beqz t0, 1f
+ ld t1, PT_R2(sp) # syscall number
dnegu v0 # error
- sd v0, PT_R0(sp) # set flag for syscall
- # restarting
+ sd t1, PT_R0(sp) # save it for syscall restarting
1: sd v0, PT_R2(sp) # result
n64_syscall_exit:
@@ -109,8 +109,9 @@ syscall_trace_entry:
sd t0, PT_R7(sp) # set error flag
beqz t0, 1f
+ ld t1, PT_R2(sp) # syscall number
dnegu v0 # error
- sd v0, PT_R0(sp) # set flag for syscall restarting
+ sd t1, PT_R0(sp) # save it for syscall restarting
1: sd v0, PT_R2(sp) # result
j syscall_exit
diff --git a/arch/mips/kernel/scall64-n32.S b/arch/mips/kernel/scall64-n32.S
index a3d6613..4da3faf 100644
--- a/arch/mips/kernel/scall64-n32.S
+++ b/arch/mips/kernel/scall64-n32.S
@@ -65,8 +65,9 @@ NESTED(handle_sysn32, PT_SIZE, sp)
sd t0, PT_R7(sp) # set error flag
beqz t0, 1f
+ ld t1, PT_R2(sp) # syscall number
dnegu v0 # error
- sd v0, PT_R0(sp) # set flag for syscall restarting
+ sd t1, PT_R0(sp) # save it for syscall restarting
1: sd v0, PT_R2(sp) # result
local_irq_disable # make sure need_resched and
@@ -106,8 +107,9 @@ n32_syscall_trace_entry:
sd t0, PT_R7(sp) # set error flag
beqz t0, 1f
+ ld t1, PT_R2(sp) # syscall number
dnegu v0 # error
- sd v0, PT_R0(sp) # set flag for syscall restarting
+ sd t1, PT_R0(sp) # save it for syscall restarting
1: sd v0, PT_R2(sp) # result
j syscall_exit
diff --git a/arch/mips/kernel/scall64-o32.S b/arch/mips/kernel/scall64-o32.S
index 813689e..7ce0a36 100644
--- a/arch/mips/kernel/scall64-o32.S
+++ b/arch/mips/kernel/scall64-o32.S
@@ -93,8 +93,9 @@ NESTED(handle_sys, PT_SIZE, sp)
sd t0, PT_R7(sp) # set error flag
beqz t0, 1f
+ ld t1, PT_R2(sp) # syscall number
dnegu v0 # error
- sd v0, PT_R0(sp) # flag for syscall restarting
+ sd t1, PT_R0(sp) # save it for syscall restarting
1: sd v0, PT_R2(sp) # result
o32_syscall_exit:
@@ -142,8 +143,9 @@ trace_a_syscall:
sd t0, PT_R7(sp) # set error flag
beqz t0, 1f
+ ld t1, PT_R2(sp) # syscall number
dnegu v0 # error
- sd v0, PT_R0(sp) # set flag for syscall restarting
+ sd t1, PT_R0(sp) # save it for syscall restarting
1: sd v0, PT_R2(sp) # result
j syscall_exit
@@ -155,7 +157,6 @@ trace_a_syscall:
*/
bad_stack:
dnegu v0 # error
- sd v0, PT_R0(sp)
sd v0, PT_R2(sp)
li t0, 1 # set error flag
sd t0, PT_R7(sp)
diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
index b3273ae..604f077 100644
--- a/arch/mips/kernel/signal.c
+++ b/arch/mips/kernel/signal.c
@@ -550,23 +550,26 @@ static int handle_signal(unsigned long sig, siginfo_t *info,
struct mips_abi *abi = current->thread.abi;
void *vdso = current->mm->context.vdso;
- switch(regs->regs[0]) {
- case ERESTART_RESTARTBLOCK:
- case ERESTARTNOHAND:
- regs->regs[2] = EINTR;
- break;
- case ERESTARTSYS:
- if (!(ka->sa.sa_flags & SA_RESTART)) {
+ if (regs->regs[0]) {
+ switch(regs->regs[2]) {
+ case ERESTART_RESTARTBLOCK:
+ case ERESTARTNOHAND:
regs->regs[2] = EINTR;
break;
+ case ERESTARTSYS:
+ if (!(ka->sa.sa_flags & SA_RESTART)) {
+ regs->regs[2] = EINTR;
+ break;
+ }
+ /* fallthrough */
+ case ERESTARTNOINTR:
+ regs->regs[7] = regs->regs[26];
+ regs->regs[2] = regs->regs[0];
+ regs->cp0_epc -= 4;
}
- /* fallthrough */
- case ERESTARTNOINTR: /* Userland will reload $v0. */
- regs->regs[7] = regs->regs[26];
- regs->cp0_epc -= 8;
- }
- regs->regs[0] = 0; /* Don't deal with this again. */
+ regs->regs[0] = 0; /* Don't deal with this again. */
+ }
if (sig_uses_siginfo(ka))
ret = abi->setup_rt_frame(vdso + abi->rt_signal_return_offset,
@@ -625,17 +628,13 @@ static void do_signal(struct pt_regs *regs)
return;
}
- /*
- * Who's code doesn't conform to the restartable syscall convention
- * dies here!!! The li instruction, a single machine instruction,
- * must directly be followed by the syscall instruction.
- */
if (regs->regs[0]) {
if (regs->regs[2] == ERESTARTNOHAND ||
regs->regs[2] == ERESTARTSYS ||
regs->regs[2] == ERESTARTNOINTR) {
+ regs->regs[2] = regs->regs[0];
regs->regs[7] = regs->regs[26];
- regs->cp0_epc -= 8;
+ regs->cp0_epc -= 4;
}
if (regs->regs[2] == ERESTART_RESTARTBLOCK) {
regs->regs[2] = current->thread.abi->restart;
diff --git a/arch/mips/kernel/unaligned.c b/arch/mips/kernel/unaligned.c
index 69b039c..33d5a5c 100644
--- a/arch/mips/kernel/unaligned.c
+++ b/arch/mips/kernel/unaligned.c
@@ -109,8 +109,6 @@ static void emulate_load_store_insn(struct pt_regs *regs,
unsigned long value;
unsigned int res;
- regs->regs[0] = 0;
-
/*
* This load never faults.
*/
--
1.5.6.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/5] mips: sanitize restart logics
@ 2010-09-28 17:50 Al Viro
0 siblings, 0 replies; 9+ messages in thread
From: Al Viro @ 2010-09-28 17:50 UTC (permalink / raw)
To: ralf; +Cc: linux-kernel, linux-arch
Put the original syscall number into ->regs[0] when we leave syscall
with error. Use it in restart logics. Everything else will have
it 0 since we pass through SAVE_SOME on all the ways in. Note that
in places like bad_stack and inllegal_syscall we leave it 0 - it's
not restartable.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
arch/mips/kernel/branch.c | 1 -
arch/mips/kernel/scall32-o32.S | 9 ++++-----
arch/mips/kernel/scall64-64.S | 7 ++++---
arch/mips/kernel/scall64-n32.S | 6 ++++--
arch/mips/kernel/scall64-o32.S | 7 ++++---
arch/mips/kernel/signal.c | 37 ++++++++++++++++++-------------------
arch/mips/kernel/unaligned.c | 2 --
7 files changed, 34 insertions(+), 35 deletions(-)
diff --git a/arch/mips/kernel/branch.c b/arch/mips/kernel/branch.c
index 0176ed0..32103cc 100644
--- a/arch/mips/kernel/branch.c
+++ b/arch/mips/kernel/branch.c
@@ -40,7 +40,6 @@ int __compute_return_epc(struct pt_regs *regs)
return -EFAULT;
}
- regs->regs[0] = 0;
switch (insn.i_format.opcode) {
/*
* jr and jalr are in r_format format.
diff --git a/arch/mips/kernel/scall32-o32.S b/arch/mips/kernel/scall32-o32.S
index 17202bb..d3edb9f 100644
--- a/arch/mips/kernel/scall32-o32.S
+++ b/arch/mips/kernel/scall32-o32.S
@@ -63,9 +63,9 @@ stack_done:
sw t0, PT_R7(sp) # set error flag
beqz t0, 1f
+ lw t1, PR_R2(sp) # syscall number
negu v0 # error
- sw v0, PT_R0(sp) # set flag for syscall
- # restarting
+ sw t1, PT_R0(sp) # save it for syscall restarting
1: sw v0, PT_R2(sp) # result
o32_syscall_exit:
@@ -104,9 +104,9 @@ syscall_trace_entry:
sw t0, PT_R7(sp) # set error flag
beqz t0, 1f
+ lw t1, PT_R2(sp) # syscall number
negu v0 # error
- sw v0, PT_R0(sp) # set flag for syscall
- # restarting
+ sw t1, PT_R0(sp) # save it for syscall restarting
1: sw v0, PT_R2(sp) # result
j syscall_exit
@@ -170,7 +170,6 @@ stackargs:
*/
bad_stack:
negu v0 # error
- sw v0, PT_R0(sp)
sw v0, PT_R2(sp)
li t0, 1 # set error flag
sw t0, PT_R7(sp)
diff --git a/arch/mips/kernel/scall64-64.S b/arch/mips/kernel/scall64-64.S
index a8a6c59..eb0bb73 100644
--- a/arch/mips/kernel/scall64-64.S
+++ b/arch/mips/kernel/scall64-64.S
@@ -66,9 +66,9 @@ NESTED(handle_sys64, PT_SIZE, sp)
sd t0, PT_R7(sp) # set error flag
beqz t0, 1f
+ ld t1, PT_R2(sp) # syscall number
dnegu v0 # error
- sd v0, PT_R0(sp) # set flag for syscall
- # restarting
+ sd t1, PT_R0(sp) # save it for syscall restarting
1: sd v0, PT_R2(sp) # result
n64_syscall_exit:
@@ -109,8 +109,9 @@ syscall_trace_entry:
sd t0, PT_R7(sp) # set error flag
beqz t0, 1f
+ ld t1, PT_R2(sp) # syscall number
dnegu v0 # error
- sd v0, PT_R0(sp) # set flag for syscall restarting
+ sd t1, PT_R0(sp) # save it for syscall restarting
1: sd v0, PT_R2(sp) # result
j syscall_exit
diff --git a/arch/mips/kernel/scall64-n32.S b/arch/mips/kernel/scall64-n32.S
index a3d6613..4da3faf 100644
--- a/arch/mips/kernel/scall64-n32.S
+++ b/arch/mips/kernel/scall64-n32.S
@@ -65,8 +65,9 @@ NESTED(handle_sysn32, PT_SIZE, sp)
sd t0, PT_R7(sp) # set error flag
beqz t0, 1f
+ ld t1, PT_R2(sp) # syscall number
dnegu v0 # error
- sd v0, PT_R0(sp) # set flag for syscall restarting
+ sd t1, PT_R0(sp) # save it for syscall restarting
1: sd v0, PT_R2(sp) # result
local_irq_disable # make sure need_resched and
@@ -106,8 +107,9 @@ n32_syscall_trace_entry:
sd t0, PT_R7(sp) # set error flag
beqz t0, 1f
+ ld t1, PT_R2(sp) # syscall number
dnegu v0 # error
- sd v0, PT_R0(sp) # set flag for syscall restarting
+ sd t1, PT_R0(sp) # save it for syscall restarting
1: sd v0, PT_R2(sp) # result
j syscall_exit
diff --git a/arch/mips/kernel/scall64-o32.S b/arch/mips/kernel/scall64-o32.S
index 813689e..7ce0a36 100644
--- a/arch/mips/kernel/scall64-o32.S
+++ b/arch/mips/kernel/scall64-o32.S
@@ -93,8 +93,9 @@ NESTED(handle_sys, PT_SIZE, sp)
sd t0, PT_R7(sp) # set error flag
beqz t0, 1f
+ ld t1, PT_R2(sp) # syscall number
dnegu v0 # error
- sd v0, PT_R0(sp) # flag for syscall restarting
+ sd t1, PT_R0(sp) # save it for syscall restarting
1: sd v0, PT_R2(sp) # result
o32_syscall_exit:
@@ -142,8 +143,9 @@ trace_a_syscall:
sd t0, PT_R7(sp) # set error flag
beqz t0, 1f
+ ld t1, PT_R2(sp) # syscall number
dnegu v0 # error
- sd v0, PT_R0(sp) # set flag for syscall restarting
+ sd t1, PT_R0(sp) # save it for syscall restarting
1: sd v0, PT_R2(sp) # result
j syscall_exit
@@ -155,7 +157,6 @@ trace_a_syscall:
*/
bad_stack:
dnegu v0 # error
- sd v0, PT_R0(sp)
sd v0, PT_R2(sp)
li t0, 1 # set error flag
sd t0, PT_R7(sp)
diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
index b3273ae..604f077 100644
--- a/arch/mips/kernel/signal.c
+++ b/arch/mips/kernel/signal.c
@@ -550,23 +550,26 @@ static int handle_signal(unsigned long sig, siginfo_t *info,
struct mips_abi *abi = current->thread.abi;
void *vdso = current->mm->context.vdso;
- switch(regs->regs[0]) {
- case ERESTART_RESTARTBLOCK:
- case ERESTARTNOHAND:
- regs->regs[2] = EINTR;
- break;
- case ERESTARTSYS:
- if (!(ka->sa.sa_flags & SA_RESTART)) {
+ if (regs->regs[0]) {
+ switch(regs->regs[2]) {
+ case ERESTART_RESTARTBLOCK:
+ case ERESTARTNOHAND:
regs->regs[2] = EINTR;
break;
+ case ERESTARTSYS:
+ if (!(ka->sa.sa_flags & SA_RESTART)) {
+ regs->regs[2] = EINTR;
+ break;
+ }
+ /* fallthrough */
+ case ERESTARTNOINTR:
+ regs->regs[7] = regs->regs[26];
+ regs->regs[2] = regs->regs[0];
+ regs->cp0_epc -= 4;
}
- /* fallthrough */
- case ERESTARTNOINTR: /* Userland will reload $v0. */
- regs->regs[7] = regs->regs[26];
- regs->cp0_epc -= 8;
- }
- regs->regs[0] = 0; /* Don't deal with this again. */
+ regs->regs[0] = 0; /* Don't deal with this again. */
+ }
if (sig_uses_siginfo(ka))
ret = abi->setup_rt_frame(vdso + abi->rt_signal_return_offset,
@@ -625,17 +628,13 @@ static void do_signal(struct pt_regs *regs)
return;
}
- /*
- * Who's code doesn't conform to the restartable syscall convention
- * dies here!!! The li instruction, a single machine instruction,
- * must directly be followed by the syscall instruction.
- */
if (regs->regs[0]) {
if (regs->regs[2] == ERESTARTNOHAND ||
regs->regs[2] == ERESTARTSYS ||
regs->regs[2] == ERESTARTNOINTR) {
+ regs->regs[2] = regs->regs[0];
regs->regs[7] = regs->regs[26];
- regs->cp0_epc -= 8;
+ regs->cp0_epc -= 4;
}
if (regs->regs[2] == ERESTART_RESTARTBLOCK) {
regs->regs[2] = current->thread.abi->restart;
diff --git a/arch/mips/kernel/unaligned.c b/arch/mips/kernel/unaligned.c
index 69b039c..33d5a5c 100644
--- a/arch/mips/kernel/unaligned.c
+++ b/arch/mips/kernel/unaligned.c
@@ -109,8 +109,6 @@ static void emulate_load_store_insn(struct pt_regs *regs,
unsigned long value;
unsigned int res;
- regs->regs[0] = 0;
-
/*
* This load never faults.
*/
--
1.5.6.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/5] mips: sanitize restart logics
2010-09-28 17:50 [PATCH 3/5] mips: sanitize restart logics Al Viro
@ 2010-09-29 18:28 ` David Daney
2010-09-30 1:50 ` Maciej W. Rozycki
2010-10-14 14:44 ` Ralf Baechle
2010-10-16 4:24 ` Shane McDonald
2 siblings, 1 reply; 9+ messages in thread
From: David Daney @ 2010-09-29 18:28 UTC (permalink / raw)
To: Al Viro; +Cc: ralf, linux-kernel, linux-arch, Maciej W. Rozycki
On 09/28/2010 10:50 AM, Al Viro wrote:
>
> Put the original syscall number into ->regs[0] when we leave syscall
> with error. Use it in restart logics. Everything else will have
> it 0 since we pass through SAVE_SOME on all the ways in. Note that
> in places like bad_stack and inllegal_syscall we leave it 0 - it's
> not restartable.
>
IIRC Userspace getcontext/setcontext rely on regs[0] having a value of
zero in the when the signal handlers are invoked.
I haven't fully traced through the code, but I hope that this
'invariant' holds both before and after this patch. I think Maciej did
the work on getcontext/setcontext, he may know for sure what the
requirements are.
David Daney
> Signed-off-by: Al Viro<viro@zeniv.linux.org.uk>
> ---
> arch/mips/kernel/branch.c | 1 -
> arch/mips/kernel/scall32-o32.S | 9 ++++-----
> arch/mips/kernel/scall64-64.S | 7 ++++---
> arch/mips/kernel/scall64-n32.S | 6 ++++--
> arch/mips/kernel/scall64-o32.S | 7 ++++---
> arch/mips/kernel/signal.c | 37 ++++++++++++++++++-------------------
> arch/mips/kernel/unaligned.c | 2 --
> 7 files changed, 34 insertions(+), 35 deletions(-)
>
> diff --git a/arch/mips/kernel/branch.c b/arch/mips/kernel/branch.c
> index 0176ed0..32103cc 100644
> --- a/arch/mips/kernel/branch.c
> +++ b/arch/mips/kernel/branch.c
> @@ -40,7 +40,6 @@ int __compute_return_epc(struct pt_regs *regs)
> return -EFAULT;
> }
>
> - regs->regs[0] = 0;
> switch (insn.i_format.opcode) {
> /*
> * jr and jalr are in r_format format.
> diff --git a/arch/mips/kernel/scall32-o32.S b/arch/mips/kernel/scall32-o32.S
> index 17202bb..d3edb9f 100644
> --- a/arch/mips/kernel/scall32-o32.S
> +++ b/arch/mips/kernel/scall32-o32.S
> @@ -63,9 +63,9 @@ stack_done:
> sw t0, PT_R7(sp) # set error flag
> beqz t0, 1f
>
> + lw t1, PR_R2(sp) # syscall number
> negu v0 # error
> - sw v0, PT_R0(sp) # set flag for syscall
> - # restarting
> + sw t1, PT_R0(sp) # save it for syscall restarting
> 1: sw v0, PT_R2(sp) # result
>
> o32_syscall_exit:
> @@ -104,9 +104,9 @@ syscall_trace_entry:
> sw t0, PT_R7(sp) # set error flag
> beqz t0, 1f
>
> + lw t1, PT_R2(sp) # syscall number
> negu v0 # error
> - sw v0, PT_R0(sp) # set flag for syscall
> - # restarting
> + sw t1, PT_R0(sp) # save it for syscall restarting
> 1: sw v0, PT_R2(sp) # result
>
> j syscall_exit
> @@ -170,7 +170,6 @@ stackargs:
> */
> bad_stack:
> negu v0 # error
> - sw v0, PT_R0(sp)
> sw v0, PT_R2(sp)
> li t0, 1 # set error flag
> sw t0, PT_R7(sp)
> diff --git a/arch/mips/kernel/scall64-64.S b/arch/mips/kernel/scall64-64.S
> index a8a6c59..eb0bb73 100644
> --- a/arch/mips/kernel/scall64-64.S
> +++ b/arch/mips/kernel/scall64-64.S
> @@ -66,9 +66,9 @@ NESTED(handle_sys64, PT_SIZE, sp)
> sd t0, PT_R7(sp) # set error flag
> beqz t0, 1f
>
> + ld t1, PT_R2(sp) # syscall number
> dnegu v0 # error
> - sd v0, PT_R0(sp) # set flag for syscall
> - # restarting
> + sd t1, PT_R0(sp) # save it for syscall restarting
> 1: sd v0, PT_R2(sp) # result
>
> n64_syscall_exit:
> @@ -109,8 +109,9 @@ syscall_trace_entry:
> sd t0, PT_R7(sp) # set error flag
> beqz t0, 1f
>
> + ld t1, PT_R2(sp) # syscall number
> dnegu v0 # error
> - sd v0, PT_R0(sp) # set flag for syscall restarting
> + sd t1, PT_R0(sp) # save it for syscall restarting
> 1: sd v0, PT_R2(sp) # result
>
> j syscall_exit
> diff --git a/arch/mips/kernel/scall64-n32.S b/arch/mips/kernel/scall64-n32.S
> index a3d6613..4da3faf 100644
> --- a/arch/mips/kernel/scall64-n32.S
> +++ b/arch/mips/kernel/scall64-n32.S
> @@ -65,8 +65,9 @@ NESTED(handle_sysn32, PT_SIZE, sp)
> sd t0, PT_R7(sp) # set error flag
> beqz t0, 1f
>
> + ld t1, PT_R2(sp) # syscall number
> dnegu v0 # error
> - sd v0, PT_R0(sp) # set flag for syscall restarting
> + sd t1, PT_R0(sp) # save it for syscall restarting
> 1: sd v0, PT_R2(sp) # result
>
> local_irq_disable # make sure need_resched and
> @@ -106,8 +107,9 @@ n32_syscall_trace_entry:
> sd t0, PT_R7(sp) # set error flag
> beqz t0, 1f
>
> + ld t1, PT_R2(sp) # syscall number
> dnegu v0 # error
> - sd v0, PT_R0(sp) # set flag for syscall restarting
> + sd t1, PT_R0(sp) # save it for syscall restarting
> 1: sd v0, PT_R2(sp) # result
>
> j syscall_exit
> diff --git a/arch/mips/kernel/scall64-o32.S b/arch/mips/kernel/scall64-o32.S
> index 813689e..7ce0a36 100644
> --- a/arch/mips/kernel/scall64-o32.S
> +++ b/arch/mips/kernel/scall64-o32.S
> @@ -93,8 +93,9 @@ NESTED(handle_sys, PT_SIZE, sp)
> sd t0, PT_R7(sp) # set error flag
> beqz t0, 1f
>
> + ld t1, PT_R2(sp) # syscall number
> dnegu v0 # error
> - sd v0, PT_R0(sp) # flag for syscall restarting
> + sd t1, PT_R0(sp) # save it for syscall restarting
> 1: sd v0, PT_R2(sp) # result
>
> o32_syscall_exit:
> @@ -142,8 +143,9 @@ trace_a_syscall:
> sd t0, PT_R7(sp) # set error flag
> beqz t0, 1f
>
> + ld t1, PT_R2(sp) # syscall number
> dnegu v0 # error
> - sd v0, PT_R0(sp) # set flag for syscall restarting
> + sd t1, PT_R0(sp) # save it for syscall restarting
> 1: sd v0, PT_R2(sp) # result
>
> j syscall_exit
> @@ -155,7 +157,6 @@ trace_a_syscall:
> */
> bad_stack:
> dnegu v0 # error
> - sd v0, PT_R0(sp)
> sd v0, PT_R2(sp)
> li t0, 1 # set error flag
> sd t0, PT_R7(sp)
> diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
> index b3273ae..604f077 100644
> --- a/arch/mips/kernel/signal.c
> +++ b/arch/mips/kernel/signal.c
> @@ -550,23 +550,26 @@ static int handle_signal(unsigned long sig, siginfo_t *info,
> struct mips_abi *abi = current->thread.abi;
> void *vdso = current->mm->context.vdso;
>
> - switch(regs->regs[0]) {
> - case ERESTART_RESTARTBLOCK:
> - case ERESTARTNOHAND:
> - regs->regs[2] = EINTR;
> - break;
> - case ERESTARTSYS:
> - if (!(ka->sa.sa_flags& SA_RESTART)) {
> + if (regs->regs[0]) {
> + switch(regs->regs[2]) {
> + case ERESTART_RESTARTBLOCK:
> + case ERESTARTNOHAND:
> regs->regs[2] = EINTR;
> break;
> + case ERESTARTSYS:
> + if (!(ka->sa.sa_flags& SA_RESTART)) {
> + regs->regs[2] = EINTR;
> + break;
> + }
> + /* fallthrough */
> + case ERESTARTNOINTR:
> + regs->regs[7] = regs->regs[26];
> + regs->regs[2] = regs->regs[0];
> + regs->cp0_epc -= 4;
> }
> - /* fallthrough */
> - case ERESTARTNOINTR: /* Userland will reload $v0. */
> - regs->regs[7] = regs->regs[26];
> - regs->cp0_epc -= 8;
> - }
>
> - regs->regs[0] = 0; /* Don't deal with this again. */
> + regs->regs[0] = 0; /* Don't deal with this again. */
> + }
>
> if (sig_uses_siginfo(ka))
> ret = abi->setup_rt_frame(vdso + abi->rt_signal_return_offset,
> @@ -625,17 +628,13 @@ static void do_signal(struct pt_regs *regs)
> return;
> }
>
> - /*
> - * Who's code doesn't conform to the restartable syscall convention
> - * dies here!!! The li instruction, a single machine instruction,
> - * must directly be followed by the syscall instruction.
> - */
> if (regs->regs[0]) {
> if (regs->regs[2] == ERESTARTNOHAND ||
> regs->regs[2] == ERESTARTSYS ||
> regs->regs[2] == ERESTARTNOINTR) {
> + regs->regs[2] = regs->regs[0];
> regs->regs[7] = regs->regs[26];
> - regs->cp0_epc -= 8;
> + regs->cp0_epc -= 4;
> }
> if (regs->regs[2] == ERESTART_RESTARTBLOCK) {
> regs->regs[2] = current->thread.abi->restart;
> diff --git a/arch/mips/kernel/unaligned.c b/arch/mips/kernel/unaligned.c
> index 69b039c..33d5a5c 100644
> --- a/arch/mips/kernel/unaligned.c
> +++ b/arch/mips/kernel/unaligned.c
> @@ -109,8 +109,6 @@ static void emulate_load_store_insn(struct pt_regs *regs,
> unsigned long value;
> unsigned int res;
>
> - regs->regs[0] = 0;
> -
> /*
> * This load never faults.
> */
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/5] mips: sanitize restart logics
2010-09-29 18:28 ` David Daney
@ 2010-09-30 1:50 ` Maciej W. Rozycki
2010-10-08 5:36 ` Al Viro
0 siblings, 1 reply; 9+ messages in thread
From: Maciej W. Rozycki @ 2010-09-30 1:50 UTC (permalink / raw)
To: David Daney; +Cc: Al Viro, ralf, linux-kernel, linux-arch, Maciej W. Rozycki
On Wed, 29 Sep 2010, David Daney wrote:
> > Put the original syscall number into ->regs[0] when we leave syscall
> > with error. Use it in restart logics. Everything else will have
> > it 0 since we pass through SAVE_SOME on all the ways in. Note that
> > in places like bad_stack and inllegal_syscall we leave it 0 - it's
> > not restartable.
> >
>
> IIRC Userspace getcontext/setcontext rely on regs[0] having a value of zero in
> the when the signal handlers are invoked.
Not exactly. These GNU C library functions rely on the magic value of
"1" there to recognise contexts they created themselves and which must
therefore be handled by themselves internally (these contexts are not
complete and only preserve the call-saved registers as specified by the
respective MIPS ABIs, and are therefore unsafe to be passed to the
rt_sigreturn(2) syscall). All the other values, including of course "0",
are not treated specially and the context is passed to rt_sigreturn(2) as
usually. This only matters in cases where e.g. setcontext(3) is used to
exit from or return to a signal handler.
> I haven't fully traced through the code, but I hope that this 'invariant'
> holds both before and after this patch. I think Maciej did the work on
> getcontext/setcontext, he may know for sure what the requirements are.
Thanks for the heads-up, David -- I would have surely missed this e-mail
from the flood I get. I think the change is safe as MIPS/Linux only uses
syscall numbers from 4000 up (not sure why such a non-round number was
chosen; one like 4096 would seem more appropriate to me but there you go),
so there should be no clash unless an invalid syscall number has a chance
to be stored there.
For how exactly the magic value is used please have a look at
sysdeps/unix/sysv/linux/mips/*context.S in glibc-ports and search for
"magic flag" (yes, I know these pieces of assembly butchered and mixed
with CPP mince require some mind-twistedness to parse; sorry about that,
but I tried my best and put some comments throughout so that the magic
flag is easy to find ;) ).
Maciej
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/5] mips: sanitize restart logics
2010-09-30 1:50 ` Maciej W. Rozycki
@ 2010-10-08 5:36 ` Al Viro
2010-10-08 5:36 ` Al Viro
2010-10-16 0:25 ` Maciej W. Rozycki
0 siblings, 2 replies; 9+ messages in thread
From: Al Viro @ 2010-10-08 5:36 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: David Daney, Al Viro, ralf, linux-kernel, linux-arch,
Maciej W. Rozycki
On Thu, Sep 30, 2010 at 02:50:17AM +0100, Maciej W. Rozycki wrote:
> On Wed, 29 Sep 2010, David Daney wrote:
> Not exactly. These GNU C library functions rely on the magic value of
> "1" there to recognise contexts they created themselves and which must
> therefore be handled by themselves internally (these contexts are not
> complete and only preserve the call-saved registers as specified by the
> respective MIPS ABIs, and are therefore unsafe to be passed to the
> rt_sigreturn(2) syscall). All the other values, including of course "0",
> are not treated specially and the context is passed to rt_sigreturn(2) as
> usually. This only matters in cases where e.g. setcontext(3) is used to
> exit from or return to a signal handler.
Nothing has changed in that respect; setup_sigcontext() (and its counterparts)
do _not_ use regs->regs[0]. Note
err |= __put_user(0, &sc->sc_regs[0]);
for (i = 1; i < 32; i++)
err |= __put_user(regs->regs[i], &sc->sc_regs[i]);
in there. The whole point of ->regs[0] uses (both original and modified)
is that $0 is constant 0 and thus the kernel is free to use that member
of pt_regs to indicate that syscall restart might be needed. So's libc,
for that matter (to distinguish between sigreturn and setcontext ones).
When sigframe is created we still discard the value - the fragment above
is not modified at all.
BTW, with original code regs->regs[0] *can* be 1, if you are leaving syscall
with -EINVAL. It won't reach the userland, though.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/5] mips: sanitize restart logics
2010-10-08 5:36 ` Al Viro
@ 2010-10-08 5:36 ` Al Viro
2010-10-16 0:25 ` Maciej W. Rozycki
1 sibling, 0 replies; 9+ messages in thread
From: Al Viro @ 2010-10-08 5:36 UTC (permalink / raw)
To: Maciej W. Rozycki
Cc: David Daney, Al Viro, ralf, linux-kernel, linux-arch,
Maciej W. Rozycki
On Thu, Sep 30, 2010 at 02:50:17AM +0100, Maciej W. Rozycki wrote:
> On Wed, 29 Sep 2010, David Daney wrote:
> Not exactly. These GNU C library functions rely on the magic value of
> "1" there to recognise contexts they created themselves and which must
> therefore be handled by themselves internally (these contexts are not
> complete and only preserve the call-saved registers as specified by the
> respective MIPS ABIs, and are therefore unsafe to be passed to the
> rt_sigreturn(2) syscall). All the other values, including of course "0",
> are not treated specially and the context is passed to rt_sigreturn(2) as
> usually. This only matters in cases where e.g. setcontext(3) is used to
> exit from or return to a signal handler.
Nothing has changed in that respect; setup_sigcontext() (and its counterparts)
do _not_ use regs->regs[0]. Note
err |= __put_user(0, &sc->sc_regs[0]);
for (i = 1; i < 32; i++)
err |= __put_user(regs->regs[i], &sc->sc_regs[i]);
in there. The whole point of ->regs[0] uses (both original and modified)
is that $0 is constant 0 and thus the kernel is free to use that member
of pt_regs to indicate that syscall restart might be needed. So's libc,
for that matter (to distinguish between sigreturn and setcontext ones).
When sigframe is created we still discard the value - the fragment above
is not modified at all.
BTW, with original code regs->regs[0] *can* be 1, if you are leaving syscall
with -EINVAL. It won't reach the userland, though.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/5] mips: sanitize restart logics
2010-09-28 17:50 [PATCH 3/5] mips: sanitize restart logics Al Viro
2010-09-29 18:28 ` David Daney
@ 2010-10-14 14:44 ` Ralf Baechle
2010-10-16 4:24 ` Shane McDonald
2 siblings, 0 replies; 9+ messages in thread
From: Ralf Baechle @ 2010-10-14 14:44 UTC (permalink / raw)
To: Al Viro; +Cc: linux-kernel, linux-arch, linux-mips
Thanks, applied.
Ralf
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/5] mips: sanitize restart logics
2010-10-08 5:36 ` Al Viro
2010-10-08 5:36 ` Al Viro
@ 2010-10-16 0:25 ` Maciej W. Rozycki
1 sibling, 0 replies; 9+ messages in thread
From: Maciej W. Rozycki @ 2010-10-16 0:25 UTC (permalink / raw)
To: Al Viro
Cc: David Daney, Al Viro, ralf, linux-kernel, linux-arch,
Maciej W. Rozycki
On Fri, 8 Oct 2010, Al Viro wrote:
> BTW, with original code regs->regs[0] *can* be 1, if you are leaving syscall
> with -EINVAL. It won't reach the userland, though.
That's all I'm concerned about, thanks.
Maciej
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/5] mips: sanitize restart logics
2010-09-28 17:50 [PATCH 3/5] mips: sanitize restart logics Al Viro
2010-09-29 18:28 ` David Daney
2010-10-14 14:44 ` Ralf Baechle
@ 2010-10-16 4:24 ` Shane McDonald
2 siblings, 0 replies; 9+ messages in thread
From: Shane McDonald @ 2010-10-16 4:24 UTC (permalink / raw)
To: Al Viro; +Cc: ralf, linux-kernel, linux-arch
On Tue, Sep 28, 2010 at 11:50 AM, Al Viro <viro@ftp.linux.org.uk> wrote:
>
> Put the original syscall number into ->regs[0] when we leave syscall
> with error. Use it in restart logics. Everything else will have
> it 0 since we pass through SAVE_SOME on all the ways in. Note that
> in places like bad_stack and inllegal_syscall we leave it 0 - it's
> not restartable.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Building 2.6.36-rc8 on a 32-bit mipsel system gives me
the following compile failure:
LD .tmp_vmlinux1
arch/mips/built-in.o:/home/shane/linux-mips.org/linux/arch/mips/kernel/scall32-o32.S:66:
undefined reference to `PR_R2'
arch/mips/built-in.o:/home/shane/linux-mips.org/linux/arch/mips/kernel/scall32-o32.S:66:
undefined reference to `PR_R2'
make: *** [.tmp_vmlinux1] Error 1
I tracked it down to this patch. In particular, I believe the change
to scall32-o32.S is causing the problem.
> diff --git a/arch/mips/kernel/scall32-o32.S b/arch/mips/kernel/scall32-o32.S
> index 17202bb..d3edb9f 100644
> --- a/arch/mips/kernel/scall32-o32.S
> +++ b/arch/mips/kernel/scall32-o32.S
> @@ -63,9 +63,9 @@ stack_done:
> sw t0, PT_R7(sp) # set error flag
> beqz t0, 1f
>
> + lw t1, PR_R2(sp) # syscall number
Should this be PT_R2(sp), rather than PR_R2(sp)?
> negu v0 # error
> - sw v0, PT_R0(sp) # set flag for syscall
> - # restarting
> + sw t1, PT_R0(sp) # save it for syscall restarting
> 1: sw v0, PT_R2(sp) # result
>
> o32_syscall_exit:
> @@ -104,9 +104,9 @@ syscall_trace_entry:
> sw t0, PT_R7(sp) # set error flag
> beqz t0, 1f
>
> + lw t1, PT_R2(sp) # syscall number
> negu v0 # error
> - sw v0, PT_R0(sp) # set flag for syscall
> - # restarting
> + sw t1, PT_R0(sp) # save it for syscall restarting
> 1: sw v0, PT_R2(sp) # result
>
> j syscall_exit
> @@ -170,7 +170,6 @@ stackargs:
> */
> bad_stack:
> negu v0 # error
> - sw v0, PT_R0(sp)
> sw v0, PT_R2(sp)
> li t0, 1 # set error flag
> sw t0, PT_R7(sp)
Changing PR_R2(sp) to PT_R2(sp) allows me to compile
and successfully boot 2.6.36-rc8. I'll whip up a quick patch
to make this change and submit it.
Shane
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-10-16 4:24 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-28 17:50 [PATCH 3/5] mips: sanitize restart logics Al Viro
2010-09-29 18:28 ` David Daney
2010-09-30 1:50 ` Maciej W. Rozycki
2010-10-08 5:36 ` Al Viro
2010-10-08 5:36 ` Al Viro
2010-10-16 0:25 ` Maciej W. Rozycki
2010-10-14 14:44 ` Ralf Baechle
2010-10-16 4:24 ` Shane McDonald
-- strict thread matches above, loose matches on Subject: below --
2010-09-28 17:50 Al Viro
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).