| Message ID | f17812d70912130412g65ac2fa5n5f345677a20a8d8a@mail.gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
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
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
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
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.
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
I have applied the patch to mvl-dove on Karmic (creating a new kernel version) and pushed the result.
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. >
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 *