[PATCH] Fixed Microblaze syscall error recovery for invalid syscall IDs.

[PATCH] Fixed Microblaze syscall error recovery for invalid syscall IDs.

[jamie.garside]

From: Jamie Garside <jamie.garside@york.ac.uk>

This patch fixes two bugs in the Microblaze syscall trap handler when an invalid
syscall ID is used.

First, the range check on line 351 only checks for syscall IDs greater than
__NR_syscalls. A negative syscall ID (either passed to `syscall()` or as returned
by `do_syscall_trace_enter()` on error) will still satisfy this test and cause
the Linux kernel to access an invalid memory location and cause a kernel oops.
This has been fixed by also checking for r12 < 0.

Secondly, the current error recovery at line 378 returns using the wrong register
(r15 instead of r14) and does not restore the previous stack state. This has been
fixed by invoking `ret_from_trap` on error, setting r3 to `-ENOSYS`, similar to
what would happen when calling a valid syscall.

Signed-off-by: Jamie Garside <jamie.garside@york.ac.uk>
---
 arch/microblaze/kernel/entry.S | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/microblaze/kernel/entry.S b/arch/microblaze/kernel/entry.S
index 0536bc0..101f13c 100644
--- a/arch/microblaze/kernel/entry.S
+++ b/arch/microblaze/kernel/entry.S
@@ -348,8 +348,9 @@ C_ENTRY(_user_exception):
  * The LP register should point to the location where the called function
  * should return.  [note that MAKE_SYS_CALL uses label 1] */
 	/* See if the system call number is valid */
+        blti    r12, 5f
 	addi	r11, r12, -__NR_syscalls;
-	bgei	r11,5f;
+	bgei	r11, 5f;
 	/* Figure out which function to use for this system call.  */
 	/* Note Microblaze barrel shift is optional, so don't rely on it */
 	add	r12, r12, r12;			/* convert num -> ptr */
@@ -375,7 +376,7 @@ C_ENTRY(_user_exception):
 
 	/* The syscall number is invalid, return an error.  */
 5:
-	rtsd	r15, 8;		/* looks like a normal subroutine return */
+        braid   ret_from_trap
 	addi	r3, r0, -ENOSYS;
 
 /* Entry point used to return from a syscall/trap */
-- 
2.3.0

Re: [PATCH] Fixed Microblaze syscall error recovery for invalid syscall IDs.

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 2638 bytes --]

On 02/23/2015 04:35 PM, jamie.garside@york.ac.uk wrote:
> From: Jamie Garside <jamie.garside@york.ac.uk>
> 
> This patch fixes two bugs in the Microblaze syscall trap handler when an invalid
> syscall ID is used.
> 
> First, the range check on line 351 only checks for syscall IDs greater than
> __NR_syscalls. A negative syscall ID (either passed to `syscall()` or as returned
> by `do_syscall_trace_enter()` on error) will still satisfy this test and cause
> the Linux kernel to access an invalid memory location and cause a kernel oops.
> This has been fixed by also checking for r12 < 0.
> 
> Secondly, the current error recovery at line 378 returns using the wrong register
> (r15 instead of r14) and does not restore the previous stack state. This has been
> fixed by invoking `ret_from_trap` on error, setting r3 to `-ENOSYS`, similar to
> what would happen when calling a valid syscall.
> 
> Signed-off-by: Jamie Garside <jamie.garside@york.ac.uk>
> ---
>  arch/microblaze/kernel/entry.S | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/microblaze/kernel/entry.S b/arch/microblaze/kernel/entry.S
> index 0536bc0..101f13c 100644
> --- a/arch/microblaze/kernel/entry.S
> +++ b/arch/microblaze/kernel/entry.S
> @@ -348,8 +348,9 @@ C_ENTRY(_user_exception):
>   * The LP register should point to the location where the called function
>   * should return.  [note that MAKE_SYS_CALL uses label 1] */
>  	/* See if the system call number is valid */
> +        blti    r12, 5f
>  	addi	r11, r12, -__NR_syscalls;
> -	bgei	r11,5f;
> +	bgei	r11, 5f;
>  	/* Figure out which function to use for this system call.  */
>  	/* Note Microblaze barrel shift is optional, so don't rely on it */
>  	add	r12, r12, r12;			/* convert num -> ptr */
> @@ -375,7 +376,7 @@ C_ENTRY(_user_exception):
>  
>  	/* The syscall number is invalid, return an error.  */
>  5:
> -	rtsd	r15, 8;		/* looks like a normal subroutine return */
> +        braid   ret_from_trap
>  	addi	r3, r0, -ENOSYS;
>  
>  /* Entry point used to return from a syscall/trap */
> 

Looks reasonable and I have also tested it.
I have fixed subject, coding style and I have create one more
patch to fix that coding style issue.
The patches should be in linux-next.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]