On 17/02/14 14:49, Alex Smith wrote:
> On 32-bit/O32, pt_regs has a padding area at the beginning into which the
> syscall arguments passed via the user stack are copied. 4 arguments
> totalling 16 bytes are copied to offset 16 bytes into this area, however
> the area is only 24 bytes long. This means the last 2 arguments overwrite
> pt_regs->regs[{0,1}].
>
> If a syscall function returns an error, handle_sys stores the original
> syscall number in pt_regs->regs[0] for syscall restart. signal.c checks
> whether regs[0] is non-zero, if it is it will check whether the syscall
> return value is one of the ERESTART* codes to see if it must be
> restarted.
>
> Should a syscall be made that results in a non-zero value being copied
> off the user stack into regs[0], and then returns a positive (non-error)
> value that matches one of the ERESTART* error codes, this can be mistaken
> for requiring a syscall restart.
>
> While the possibility for this to occur has always existed, it is made
> much more likely to occur by commit 46e12c07b3b9 ("MIPS: O32 / 32-bit:
> Always copy 4 stack arguments."), since now every syscall will copy 4
> arguments and overwrite regs[0], rather than just those with 7 or 8
> arguments.
>
> Since that commit, booting Debian under a 32-bit MIPS kernel almost
> always results in a hang early in boot, due to a wait4 syscall returning
> a PID that matches one of the ERESTART* codes, which then causes an
> incorrect restart of the syscall.
>
> The problem is fixed by increasing the size of the padding area so that
> arguments copied off the stack will not overwrite pt_regs->regs[{0,1}].
> Also removed a comment in handle_sys which is no longer relevant after
> the aforementioned commit.
>
> Signed-off-by: Alex Smith <alex.smith@xxxxxxxxxx>
> Reviewed-by: Markos Chandras <markos.chandras@xxxxxxxxxx>
Tested-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@xxxxxxxxxx>
> Cc: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
> Cc: linux-mips@xxxxxxxxxxxxxx
> Cc: stable@xxxxxxxxxxxxxxx [3.13]
>
> ---
> arch/mips/include/asm/ptrace.h | 2 +-
> arch/mips/kernel/scall32-o32.S | 2 --
> arch/mips/kernel/smtc-asm.S | 4 ++--
> 3 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/arch/mips/include/asm/ptrace.h b/arch/mips/include/asm/ptrace.h
> index 7bba9da..6d019ca 100644
> --- a/arch/mips/include/asm/ptrace.h
> +++ b/arch/mips/include/asm/ptrace.h
> @@ -23,7 +23,7 @@
> struct pt_regs {
> #ifdef CONFIG_32BIT
> /* Pad bytes for argument save space on the stack. */
> - unsigned long pad0[6];
> + unsigned long pad0[8];
> #endif
>
> /* Saved main processor registers. */
> diff --git a/arch/mips/kernel/scall32-o32.S b/arch/mips/kernel/scall32-o32.S
> index a5b14f4..4220c2d 100644
> --- a/arch/mips/kernel/scall32-o32.S
> +++ b/arch/mips/kernel/scall32-o32.S
> @@ -66,8 +66,6 @@ NESTED(handle_sys, PT_SIZE, sp)
>
> /*
> * Ok, copy the args from the luser stack to the kernel stack.
> - * t3 is the precomputed number of instruction bytes needed to
> - * load or store arguments 6-8.
> */
>
> .set push
> diff --git a/arch/mips/kernel/smtc-asm.S b/arch/mips/kernel/smtc-asm.S
> index 2866863..c4f0cd9 100644
> --- a/arch/mips/kernel/smtc-asm.S
> +++ b/arch/mips/kernel/smtc-asm.S
> @@ -43,8 +43,8 @@ CAN WE PROVE THAT WE WON'T DO THIS IF INTS DISABLED??
> * to invoke the scheduler and return as appropriate.
> */
>
> -#define PT_PADSLOT4 (PT_R0-8)
> -#define PT_PADSLOT5 (PT_R0-4)
> +#define PT_PADSLOT4 (PT_R0-16)
> +#define PT_PADSLOT5 (PT_R0-12)
>
> .text
> .align 5
>
