[PATCH] MIPS: Check the range of the syscall number for o32 syscall on 64bit kernel.

[PATCH] MIPS: Check the range of the syscall number for o32 syscall on 64bit kernel.

[David Daney]

From: Vlad Malov <Vlad.Malov@caviumnetworks.com>

On a 64 bit kernel if an o32 syscall was made with a syscall number
less than 4000, we would read the function from outside of the bounds
of the syscall table.  This led to non-deterministic behavior
including system crashes.

While we were at it we reworked the 32 bit version as well to use
fewer instructions.

Signed-off-by: Vlad Malov <Vlad.Malov@caviumnetworks.com>
Signed-off-by: David Daney <ddaney@caviumnetworks.com>
---
 arch/mips/kernel/scall32-o32.S |    9 ++++-----
 arch/mips/kernel/scall64-o32.S |   14 +++++++-------
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/arch/mips/kernel/scall32-o32.S b/arch/mips/kernel/scall32-o32.S
index 759f680..e638363 100644
--- a/arch/mips/kernel/scall32-o32.S
+++ b/arch/mips/kernel/scall32-o32.S
@@ -260,16 +260,15 @@ bad_alignment:
 	END(sys_sysmips)
 
 	LEAF(sys_syscall)
+	.set	noreorder
 	subu	t0, a0, __NR_O32_Linux	# check syscall number
-	sltiu	v0, t0, __NR_O32_Linux_syscalls + 1
+	beqz	t0, einval		# do not recurse
+	sltu	v0, t0, __NR_O32_Linux_syscalls + 1
 	sll	t1, t0, 3
 	beqz	v0, einval
-
+	.set	reorder
 	lw	t2, sys_call_table(t1)		# syscall routine
 
-	li	v1, 4000 - __NR_O32_Linux	# index of sys_syscall
-	beq	t0, v1, einval			# do not recurse
-
 	/* 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.
diff --git a/arch/mips/kernel/scall64-o32.S b/arch/mips/kernel/scall64-o32.S
index 6c7ef83..d8b3cb1 100644
--- a/arch/mips/kernel/scall64-o32.S
+++ b/arch/mips/kernel/scall64-o32.S
@@ -174,14 +174,14 @@ not_o32_scall:
 	END(handle_sys)
 
 LEAF(sys32_syscall)
-	sltu	v0, a0, __NR_O32_Linux + __NR_O32_Linux_syscalls + 1
+	.set	noreorder
+	subu	t0, a0, __NR_O32_Linux	# check syscall number
+	beqz	t0, einval		# do not recurse
+	sltu	v0, t0, __NR_O32_Linux_syscalls + 1
+	dsll	t1, t0, 3
 	beqz	v0, einval
-
-	dsll	v0, a0, 3
-	ld	t2, (sys_call_table - (__NR_O32_Linux * 8))(v0)
-
-	li	v1, 4000		# indirect syscall number
-	beq	a0, v1, einval		# do not recurse
+	.set	reorder
+	lw	t2, sys_call_table(t1)		# syscall routine
 
 	move	a0, a1			# shift argument registers
 	move	a1, a2

Re: [PATCH] MIPS: Check the range of the syscall number for o32 syscall on 64bit kernel.

[David Daney]

David Daney wrote:
[...]
> diff --git a/arch/mips/kernel/scall64-o32.S 
> b/arch/mips/kernel/scall64-o32.S
> index 6c7ef83..d8b3cb1 100644
> --- a/arch/mips/kernel/scall64-o32.S
> +++ b/arch/mips/kernel/scall64-o32.S
> @@ -174,14 +174,14 @@ not_o32_scall:
>     END(handle_sys)
> 
> LEAF(sys32_syscall)
> -    sltu    v0, a0, __NR_O32_Linux + __NR_O32_Linux_syscalls + 1
> +    .set    noreorder
> +    subu    t0, a0, __NR_O32_Linux    # check syscall number
> +    beqz    t0, einval        # do not recurse
> +    sltu    v0, t0, __NR_O32_Linux_syscalls + 1
> +    dsll    t1, t0, 3
>     beqz    v0, einval
> -
> -    dsll    v0, a0, 3
> -    ld    t2, (sys_call_table - (__NR_O32_Linux * 8))(v0)
> -
> -    li    v1, 4000        # indirect syscall number
> -    beq    a0, v1, einval        # do not recurse
> +    .set    reorder
> +    lw    t2, sys_call_table(t1)        # syscall routine
> 

        ^^^ Clearly that should be ld not lw.  Not sure how that slipped 
in, Vlad's original patch had it correct.  Re-testing...


David Daney

Re: [PATCH] MIPS: Check the range of the syscall number for o32 syscall on 64bit kernel.

[Maciej W. Rozycki]

On Thu, 30 Oct 2008, David Daney wrote:

> @@ -260,16 +260,15 @@ bad_alignment:
> 	END(sys_sysmips)
> 
> 	LEAF(sys_syscall)
> +	.set	noreorder

 Please indent branch delay slot instructions by one space if using this
mode.

> 	subu	t0, a0, __NR_O32_Linux	# check syscall number
> -	sltiu	v0, t0, __NR_O32_Linux_syscalls + 1
> +	beqz	t0, einval		# do not recurse
> +	sltu	v0, t0, __NR_O32_Linux_syscalls + 1

 Why not sltiu?  You do want to fit in the delay slot here.  Besides you 
should not need .set noreorder here -- GAS should be smart enough to swap 
sltiu with beqz here (and then you can actually use sltu quite safely).  
The rule of thumb is not to use .set noreorder unless absolutely necessary 
(such as modifying one of the registers used by a branch instruction 
immediately afterwards in its delay slot) as you have to take all the 
pesky details of instruction scheduling into account, including but not 
limited to the MIPS I load delay slots not everybody seems to be aware of.

 Adjust for the other hunk accordingly.

  Maciej