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@imgtec.com>

Reviewed-by: James Hogan <james.hogan@imgtec.com>
Tested-by: James Hogan <james.hogan@imgtec.com>

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@vger.kernel.org> # f9c4e3a6dae1: MIPS: Opt into HAVE_COPY_THREAD_TLS
Cc: <stable@vger.kernel.org> # 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
> 
>