On Fri, Mar 31, 2017 at 05:09:59PM +0100, James Cowgill wrote:
> If a restartable syscall is called using the indirect o32 syscall
> handler - eg: syscall(__NR_waitid, ...), then it is possible for the
> incorrect arguments to be passed to the syscall after it has been
> restarted. This is because the syscall handler tries to shift all the
> registers down one place in pt_regs so that when the syscall is restarted,
> the "real" syscall is called instead. Unfortunately it only shifts the
> arguments passed in registers, not the arguments on the user stack. This
> causes the 4th argument to be duplicated when the syscall is restarted.
>
> Fix by removing all the pt_regs shifting so that the indirect syscall
> handler is called again when the syscall is restarted. The comment "some
> syscalls like execve get their arguments from struct pt_regs" is long
> out of date so this should now be safe.
>
> Signed-off-by: James Cowgill <James.Cowgill@xxxxxxxxxx>
Reviewed-by: James Hogan <james.hogan@xxxxxxxxxx>
Tested-by: James Hogan <james.hogan@xxxxxxxxxx>
This is safe to backport as far back as 4.2 too (I just tested), which
is I think as far back as patch 1 (commit f9c4e3a6dae1) can be
backported due to the commit 3033f14ab78c3 ("clone: support passing tls
argument via C rather than pt_regs magic") referenced in patch 1, so I
suggest adding:
Cc: <stable@xxxxxxxxxxxxxxx> # f9c4e3a6dae1: MIPS: Opt into HAVE_COPY_THREAD_TLS
Cc: <stable@xxxxxxxxxxxxxxx> # 4.2+
Thanks
James
> ---
> arch/mips/kernel/scall32-o32.S | 11 -----------
> arch/mips/kernel/scall64-o32.S | 6 ------
> 2 files changed, 17 deletions(-)
>
> diff --git a/arch/mips/kernel/scall32-o32.S b/arch/mips/kernel/scall32-o32.S
> index c29d397eee86..d8d6336c4cc5 100644
> --- a/arch/mips/kernel/scall32-o32.S
> +++ b/arch/mips/kernel/scall32-o32.S
> @@ -190,12 +190,6 @@ illegal_syscall:
> sll t1, t0, 2
> beqz v0, einval
> lw t2, sys_call_table(t1) # syscall routine
> - sw a0, PT_R2(sp) # call routine directly on restart
> -
> - /* Some syscalls like execve get their arguments from struct pt_regs
> - and claim zero arguments in the syscall table. Thus we have to
> - assume the worst case and shuffle around all potential arguments.
> - If you want performance, don't use indirect syscalls. */
>
> move a0, a1 # shift argument registers
> move a1, a2
> @@ -207,11 +201,6 @@ illegal_syscall:
> sw t4, 16(sp)
> sw t5, 20(sp)
> sw t6, 24(sp)
> - sw a0, PT_R4(sp) # .. and push back a0 - a3, some
> - sw a1, PT_R5(sp) # syscalls expect them there
> - sw a2, PT_R6(sp)
> - sw a3, PT_R7(sp)
> - sw a3, PT_R26(sp) # update a3 for syscall restarting
> jr t2
> /* Unreached */
>
> diff --git a/arch/mips/kernel/scall64-o32.S b/arch/mips/kernel/scall64-o32.S
> index 5a47042dd25f..6fd8ecca89e7 100644
> --- a/arch/mips/kernel/scall64-o32.S
> +++ b/arch/mips/kernel/scall64-o32.S
> @@ -198,7 +198,6 @@ LEAF(sys32_syscall)
> dsll t1, t0, 3
> beqz v0, einval
> ld t2, sys32_call_table(t1) # syscall routine
> - sd a0, PT_R2(sp) # call routine directly on restart
>
> move a0, a1 # shift argument registers
> move a1, a2
> @@ -207,11 +206,6 @@ LEAF(sys32_syscall)
> move a4, a5
> move a5, a6
> move a6, a7
> - sd a0, PT_R4(sp) # ... and push back a0 - a3, some
> - sd a1, PT_R5(sp) # syscalls expect them there
> - sd a2, PT_R6(sp)
> - sd a3, PT_R7(sp)
> - sd a3, PT_R26(sp) # update a3 for syscall restarting
> jr t2
> /* Unreached */
>
> --
> 2.11.0
>
>
Attachment:
signature.asc
Description: Digital signature
