Patchwork [Karmic] SRU: arm: fix singal restart when Old EABI enabled

login
register
mail settings
Submitter Eric Miao
Date Dec. 13, 2009, 12:12 p.m.
Message ID <f17812d70912130412g65ac2fa5n5f345677a20a8d8a@mail.gmail.com>
Download mbox | patch
Permalink /patch/41028/
State Accepted
Headers show

Comments

Eric Miao - Dec. 13, 2009, 12:12 p.m.
Stefan,

This patch is verified to fix bug #453682 for Karmic on dove, although not yet
hit -stable.

From 09f30d75269669eca5706fe2105196ee2f2c744c Mon Sep 17 00:00:00 2001
From: Russle King <rmk+kernel@arm.linux.org.uk>
Date: Tue, 27 Oct 2009 11:02:16 +0200
Subject: [PATCH] arm: fix singal restart when Old EABI enabled

Signed-off-by: Saeed Bishara <saeed@marvell.com>
---
 arch/arm/kernel/signal.c |   41 +++++++++++++++++------------------------
 arch/arm/kernel/signal.h |    4 +++-
 arch/arm/kernel/traps.c  |    4 +++-
 3 files changed, 23 insertions(+), 26 deletions(-)
 mode change 100644 => 100755 arch/arm/kernel/signal.c

  * This program is free software; you can redistribute it and/or modify
@@ -742,6 +742,8 @@ void __init early_trap_init(void)
 	 */
 	memcpy((void *)KERN_SIGRETURN_CODE, sigreturn_codes,
 	       sizeof(sigreturn_codes));
+	memcpy((void *)KERN_RESTART_CODE, syscall_restart_code,
+	       sizeof(syscall_restart_code));

 	flush_icache_range(vectors, vectors + PAGE_SIZE);
 	modify_domain(DOMAIN_USER, DOMAIN_CLIENT);
Stefan Bader - Dec. 13, 2009, 12:34 p.m.
Eric Miao wrote:
> Stefan,
> 
> This patch is verified to fix bug #453682 for Karmic on dove, although not yet
> hit -stable.

It also does not have hit upstream as well. While for arm I think we could
go ahead of stable, I would prefer to have it upstream, so I can add that
commit id as reference.

> From 09f30d75269669eca5706fe2105196ee2f2c744c Mon Sep 17 00:00:00 2001
> From: Russle King <rmk+kernel@arm.linux.org.uk>
> Date: Tue, 27 Oct 2009 11:02:16 +0200
> Subject: [PATCH] arm: fix singal restart when Old EABI enabled
> 
<please add some description here>

CC: stable@kernel.org
> Signed-off-by: Saeed Bishara <saeed@marvell.com>
> ---
>  arch/arm/kernel/signal.c |   41 +++++++++++++++++------------------------
>  arch/arm/kernel/signal.h |    4 +++-
>  arch/arm/kernel/traps.c  |    4 +++-
>  3 files changed, 23 insertions(+), 26 deletions(-)
>  mode change 100644 => 100755 arch/arm/kernel/signal.c
> 
> diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
> old mode 100644
> new mode 100755
> index f6bc5d4..b09af17
> --- a/arch/arm/kernel/signal.c
> +++ b/arch/arm/kernel/signal.c
> @@ -1,7 +1,7 @@
>  /*
>   *  linux/arch/arm/kernel/signal.c
>   *
> - *  Copyright (C) 1995-2002 Russell King
> + *  Copyright (C) 1995-2009 Russell King
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 as
> @@ -28,6 +28,7 @@
>   */
>  #define SWI_SYS_SIGRETURN	(0xef000000|(__NR_sigreturn)|(__NR_OABI_SYSCALL_BASE))
>  #define SWI_SYS_RT_SIGRETURN	(0xef000000|(__NR_rt_sigreturn)|(__NR_OABI_SYSCALL_BASE))
> +#define SWI_SYS_RESTART		(0xef000000|__NR_restart_syscall|__NR_OABI_SYSCALL_BASE)

This is just minor nitpick, but is there a specific reason that this definition
does not use the same layout as the definitions above?

>  /*
>   * With EABI, the syscall number has to be loaded into r7.
> @@ -50,6 +51,18 @@ const unsigned long sigreturn_codes[7] = {
>  static int do_signal(sigset_t *oldset, struct pt_regs * regs, int syscall);
> 
>  /*
> + * Either we support OABI only, or we have EABI with the OABI
> + * compat layer enabled.  In the later case we don't know if
> + * user space is EABI or not, and if not we must not clobber r7.
> + * Always using the OABI syscall solves that issue and works for
> + * all those cases.
> + */
> +const unsigned long syscall_restart_code[2] = {
> +	SWI_SYS_RESTART,	/* swi	__NR_restart_syscall */
> +	0xe49df004,		/* ldr	pc, [sp], #4 */
> +};
> +
> +/*
>   * atomically swap in the new signal mask, and wait for a signal.
>   */
>  asmlinkage int sys_sigsuspend(int restart, unsigned long oldmask,
> old_sigset_t mask, struct pt_regs *regs)
> @@ -663,32 +676,12 @@ static int do_signal(sigset_t *oldset, struct
> pt_regs *regs, int syscall)
>  				regs->ARM_pc -= 4;
>  #else
>  				u32 __user *usp;
> -				u32 swival = __NR_restart_syscall;
> 
> -				regs->ARM_sp -= 12;
> +				regs->ARM_sp -= 4;
>  				usp = (u32 __user *)regs->ARM_sp;
> 
> -				/*
> -				 * Either we supports OABI only, or we have
> -				 * EABI with the OABI compat layer enabled.
> -				 * In the later case we don't know if user
> -				 * space is EABI or not, and if not we must
> -				 * not clobber r7.  Always using the OABI
> -				 * syscall solves that issue and works for
> -				 * all those cases.
> -				 */
> -				swival = swival - __NR_SYSCALL_BASE + __NR_OABI_SYSCALL_BASE;
> -
> -				put_user(regs->ARM_pc, &usp[0]);
> -				/* swi __NR_restart_syscall */
> -				put_user(0xef000000 | swival, &usp[1]);
> -				/* ldr	pc, [sp], #12 */
> -				put_user(0xe49df00c, &usp[2]);
> -
> -				flush_icache_range((unsigned long)usp,
> -						   (unsigned long)(usp + 3));
> -
> -				regs->ARM_pc = regs->ARM_sp + 4;
> +				put_user(regs->ARM_pc, usp);
> +				regs->ARM_pc = KERN_RESTART_CODE;
>  #endif
>  			}
>  		}
> diff --git a/arch/arm/kernel/signal.h b/arch/arm/kernel/signal.h
> index 27beece..6fcfe83 100644
> --- a/arch/arm/kernel/signal.h
> +++ b/arch/arm/kernel/signal.h
> @@ -1,12 +1,14 @@
>  /*
>   *  linux/arch/arm/kernel/signal.h
>   *
> - *  Copyright (C) 2005 Russell King.
> + *  Copyright (C) 2005-2009 Russell King.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 as
>   * published by the Free Software Foundation.
>   */
>  #define KERN_SIGRETURN_CODE	(CONFIG_VECTORS_BASE + 0x00000500)
> +#define KERN_RESTART_CODE	(KERN_SIGRETURN_CODE + sizeof(sigreturn_codes))
> 
>  extern const unsigned long sigreturn_codes[7];
> +extern const unsigned long syscall_restart_code[2];
> diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
> index 57eb0f6..9cc19c9 100644
> --- a/arch/arm/kernel/traps.c
> +++ b/arch/arm/kernel/traps.c
> @@ -1,7 +1,7 @@
>  /*
>   *  linux/arch/arm/kernel/traps.c
>   *
> - *  Copyright (C) 1995-2002 Russell King
> + *  Copyright (C) 1995-2009 Russell King
>   *  Fragments that appear the same as linux/arch/i386/kernel/traps.c
> (C) Linus Torvalds
>   *
>   * This program is free software; you can redistribute it and/or modify
> @@ -742,6 +742,8 @@ void __init early_trap_init(void)
>  	 */
>  	memcpy((void *)KERN_SIGRETURN_CODE, sigreturn_codes,
>  	       sizeof(sigreturn_codes));
> +	memcpy((void *)KERN_RESTART_CODE, syscall_restart_code,
> +	       sizeof(syscall_restart_code));
> 
>  	flush_icache_range(vectors, vectors + PAGE_SIZE);
>  	modify_domain(DOMAIN_USER, DOMAIN_CLIENT);

-Stefan
Eric Miao - Dec. 13, 2009, 12:51 p.m.
On Sun, Dec 13, 2009 at 8:34 PM, Stefan Bader
<stefan.bader@canonical.com> wrote:
> Eric Miao wrote:
>> Stefan,
>>
>> This patch is verified to fix bug #453682 for Karmic on dove, although not yet
>> hit -stable.
>
> It also does not have hit upstream as well. While for arm I think we could
> go ahead of stable, I would prefer to have it upstream, so I can add that
> commit id as reference.
>

Actually it has been upstreamed, but is a slightly different version than
this one, as this is cherry-picked from marvell's dove-kernel tree directly.
We can instead cherry-pick the following one if preferred:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=ab72b00734ae4d0b5ff273a0f6c7abeaa3713c76
Stefan Bader - Dec. 13, 2009, 1:37 p.m.
Eric Miao wrote:
> On Sun, Dec 13, 2009 at 8:34 PM, Stefan Bader
> <stefan.bader@canonical.com> wrote:
>> Eric Miao wrote:
>>> Stefan,
>>>
>>> This patch is verified to fix bug #453682 for Karmic on dove, although not yet
>>> hit -stable.
>> It also does not have hit upstream as well. While for arm I think we could
>> go ahead of stable, I would prefer to have it upstream, so I can add that
>> commit id as reference.
>>
> 
> Actually it has been upstreamed, but is a slightly different version than
> this one, as this is cherry-picked from marvell's dove-kernel tree directly.
> We can instead cherry-pick the following one if preferred:
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=ab72b00734ae4d0b5ff273a0f6c7abeaa3713c76

Yes, if that is working the same, I'd prefer the upstream version. It probably
does not matter that much in the arm branch, but generally I think it is better
to stay close to upstream. I see it will need a bit of modification to apply
cleanly but the resulting patch I might ask for inclusion in 2.6.31.y as long
as that is still open.

-Stefan
Eric Miao - Dec. 13, 2009, 1:48 p.m.
On Sun, Dec 13, 2009 at 9:37 PM, Stefan Bader
<stefan.bader@canonical.com> wrote:
> Eric Miao wrote:
>> On Sun, Dec 13, 2009 at 8:34 PM, Stefan Bader
>> <stefan.bader@canonical.com> wrote:
>>> Eric Miao wrote:
>>>> Stefan,
>>>>
>>>> This patch is verified to fix bug #453682 for Karmic on dove, although not yet
>>>> hit -stable.
>>> It also does not have hit upstream as well. While for arm I think we could
>>> go ahead of stable, I would prefer to have it upstream, so I can add that
>>> commit id as reference.
>>>
>>
>> Actually it has been upstreamed, but is a slightly different version than
>> this one, as this is cherry-picked from marvell's dove-kernel tree directly.
>> We can instead cherry-pick the following one if preferred:
>>
>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=ab72b00734ae4d0b5ff273a0f6c7abeaa3713c76
>
> Yes, if that is working the same, I'd prefer the upstream version. It probably
> does not matter that much in the arm branch, but generally I think it is better
> to stay close to upstream. I see it will need a bit of modification to apply
> cleanly but the resulting patch I might ask for inclusion in 2.6.31.y as long
> as that is still open.
>

OK, will ping Russell on forward that to -stable and this can be automatically
dropped after later sync with -stable.

Stefan,

Is it possible you cherry-pick that commit directly into 'mvl-dove' branch or
I can prepare a git pull request for you tomorrow.
Stefan Bader - Dec. 13, 2009, 8:24 p.m.
Eric Miao wrote:
> On Sun, Dec 13, 2009 at 9:37 PM, Stefan Bader
> <stefan.bader@canonical.com> wrote:
>> Eric Miao wrote:
>>> On Sun, Dec 13, 2009 at 8:34 PM, Stefan Bader
>>> <stefan.bader@canonical.com> wrote:
>>>> Eric Miao wrote:
>>>>> Stefan,
>>>>>
>>>>> This patch is verified to fix bug #453682 for Karmic on dove, although not yet
>>>>> hit -stable.
>>>> It also does not have hit upstream as well. While for arm I think we could
>>>> go ahead of stable, I would prefer to have it upstream, so I can add that
>>>> commit id as reference.
>>>>
>>> Actually it has been upstreamed, but is a slightly different version than
>>> this one, as this is cherry-picked from marvell's dove-kernel tree directly.
>>> We can instead cherry-pick the following one if preferred:
>>>
>>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=ab72b00734ae4d0b5ff273a0f6c7abeaa3713c76
>> Yes, if that is working the same, I'd prefer the upstream version. It probably
>> does not matter that much in the arm branch, but generally I think it is better
>> to stay close to upstream. I see it will need a bit of modification to apply
>> cleanly but the resulting patch I might ask for inclusion in 2.6.31.y as long
>> as that is still open.
>>
> 
> OK, will ping Russell on forward that to -stable and this can be automatically
> dropped after later sync with -stable.
> 
> Stefan,
> 
> Is it possible you cherry-pick that commit directly into 'mvl-dove' branch or
> I can prepare a git pull request for you tomorrow.

Yes, will do so tomorrow.

-Stefan
Stefan Bader - Dec. 14, 2009, 7:29 p.m.
I have applied the patch to mvl-dove on Karmic (creating a new kernel version)
and pushed the result.
Eric Miao - Dec. 15, 2009, 2:43 a.m.
Stefan,

Thanks. So do I have to update LP bug status or kernel-janitor will?

On Tue, Dec 15, 2009 at 3:29 AM, Stefan Bader
<stefan.bader@canonical.com> wrote:
> I have applied the patch to mvl-dove on Karmic (creating a new kernel version)
> and pushed the result.
>

Patch

diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
old mode 100644
new mode 100755
index f6bc5d4..b09af17
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -1,7 +1,7 @@ 
 /*
  *  linux/arch/arm/kernel/signal.c
  *
- *  Copyright (C) 1995-2002 Russell King
+ *  Copyright (C) 1995-2009 Russell King
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -28,6 +28,7 @@ 
  */
 #define SWI_SYS_SIGRETURN	(0xef000000|(__NR_sigreturn)|(__NR_OABI_SYSCALL_BASE))
 #define SWI_SYS_RT_SIGRETURN	(0xef000000|(__NR_rt_sigreturn)|(__NR_OABI_SYSCALL_BASE))
+#define SWI_SYS_RESTART		(0xef000000|__NR_restart_syscall|__NR_OABI_SYSCALL_BASE)

 /*
  * With EABI, the syscall number has to be loaded into r7.
@@ -50,6 +51,18 @@  const unsigned long sigreturn_codes[7] = {
 static int do_signal(sigset_t *oldset, struct pt_regs * regs, int syscall);

 /*
+ * Either we support OABI only, or we have EABI with the OABI
+ * compat layer enabled.  In the later case we don't know if
+ * user space is EABI or not, and if not we must not clobber r7.
+ * Always using the OABI syscall solves that issue and works for
+ * all those cases.
+ */
+const unsigned long syscall_restart_code[2] = {
+	SWI_SYS_RESTART,	/* swi	__NR_restart_syscall */
+	0xe49df004,		/* ldr	pc, [sp], #4 */
+};
+
+/*
  * atomically swap in the new signal mask, and wait for a signal.
  */
 asmlinkage int sys_sigsuspend(int restart, unsigned long oldmask,
old_sigset_t mask, struct pt_regs *regs)
@@ -663,32 +676,12 @@  static int do_signal(sigset_t *oldset, struct
pt_regs *regs, int syscall)
 				regs->ARM_pc -= 4;
 #else
 				u32 __user *usp;
-				u32 swival = __NR_restart_syscall;

-				regs->ARM_sp -= 12;
+				regs->ARM_sp -= 4;
 				usp = (u32 __user *)regs->ARM_sp;

-				/*
-				 * Either we supports OABI only, or we have
-				 * EABI with the OABI compat layer enabled.
-				 * In the later case we don't know if user
-				 * space is EABI or not, and if not we must
-				 * not clobber r7.  Always using the OABI
-				 * syscall solves that issue and works for
-				 * all those cases.
-				 */
-				swival = swival - __NR_SYSCALL_BASE + __NR_OABI_SYSCALL_BASE;
-
-				put_user(regs->ARM_pc, &usp[0]);
-				/* swi __NR_restart_syscall */
-				put_user(0xef000000 | swival, &usp[1]);
-				/* ldr	pc, [sp], #12 */
-				put_user(0xe49df00c, &usp[2]);
-
-				flush_icache_range((unsigned long)usp,
-						   (unsigned long)(usp + 3));
-
-				regs->ARM_pc = regs->ARM_sp + 4;
+				put_user(regs->ARM_pc, usp);
+				regs->ARM_pc = KERN_RESTART_CODE;
 #endif
 			}
 		}
diff --git a/arch/arm/kernel/signal.h b/arch/arm/kernel/signal.h
index 27beece..6fcfe83 100644
--- a/arch/arm/kernel/signal.h
+++ b/arch/arm/kernel/signal.h
@@ -1,12 +1,14 @@ 
 /*
  *  linux/arch/arm/kernel/signal.h
  *
- *  Copyright (C) 2005 Russell King.
+ *  Copyright (C) 2005-2009 Russell King.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
 #define KERN_SIGRETURN_CODE	(CONFIG_VECTORS_BASE + 0x00000500)
+#define KERN_RESTART_CODE	(KERN_SIGRETURN_CODE + sizeof(sigreturn_codes))

 extern const unsigned long sigreturn_codes[7];
+extern const unsigned long syscall_restart_code[2];
diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index 57eb0f6..9cc19c9 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -1,7 +1,7 @@ 
 /*
  *  linux/arch/arm/kernel/traps.c
  *
- *  Copyright (C) 1995-2002 Russell King
+ *  Copyright (C) 1995-2009 Russell King
  *  Fragments that appear the same as linux/arch/i386/kernel/traps.c
(C) Linus Torvalds
  *