Messages in this thread | | | Date | Wed, 21 Jan 2009 01:31:18 -0800 | From | Justin Madru <> | Subject | Re: [Bug #12505] 2.6.29-rc1 Firefox crashing on page load |
| |
Ingo Molnar wrote: > * Ingo Molnar <mingo@elte.hu> wrote: > > >> * Ingo Molnar <mingo@elte.hu> wrote: >> >> >>> (added Cc:s) >>> >>> Justin, does it work if you apply the patch below instead of the revert? >>> >> hm, that patch wont build because the protect_asmlinkage macro is rather >> limited - it cannot deal with structure parameters. >> >> We might be better off with a revert here, and a comment added that points >> out the problem. >> > > Below is the revert+document commit that i have queued up. > > Justin, could you try the tip/master tree (which has this fix included): > > http://people.redhat.com/mingo/tip.git/README > > does Firefox still work fine? > > Ingo > > -------------> > >From 779c9b9f8cb87cdfbd299ee7beb62e50ce139a92 Mon Sep 17 00:00:00 2001 > From: Ingo Molnar <mingo@elte.hu> > Date: Tue, 20 Jan 2009 09:31:49 +0100 > Subject: [PATCH] Revert "x86: signal: change type of paramter for sys_rt_sigreturn()" > > This reverts commit 4217458dafaa57d8e26a46f5d05ab8c53cf64191. > > Justin Madru bisected this commit, it was causing weird Firefox > crashes. > > The reason is that GCC mis-optimizes (re-uses) the on-stack parameters of > the calling frame, which corrupts the syscall return pt_regs state and > thus corrupts user-space register state. > > So we go back to the slightly less clean but more optimization-safe > method of getting to pt_regs. Also add a comment to explain this. > > Resolves: http://bugzilla.kernel.org/show_bug.cgi?id=12505 > > Reported-and-bisected-by: Justin Madru <jdm64@gawab.com> > Signed-off-by: Ingo Molnar <mingo@elte.hu> > --- > arch/x86/include/asm/syscalls.h | 2 +- > arch/x86/kernel/signal.c | 11 +++++++++-- > 2 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/include/asm/syscalls.h b/arch/x86/include/asm/syscalls.h > index 9c6797c..c0b0bda 100644 > --- a/arch/x86/include/asm/syscalls.h > +++ b/arch/x86/include/asm/syscalls.h > @@ -40,7 +40,7 @@ asmlinkage int sys_sigaction(int, const struct old_sigaction __user *, > struct old_sigaction __user *); > asmlinkage int sys_sigaltstack(unsigned long); > asmlinkage unsigned long sys_sigreturn(unsigned long); > -asmlinkage int sys_rt_sigreturn(struct pt_regs); > +asmlinkage int sys_rt_sigreturn(unsigned long); > > /* kernel/ioport.c */ > asmlinkage long sys_iopl(unsigned long); > diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c > index 89bb766..df0587f 100644 > --- a/arch/x86/kernel/signal.c > +++ b/arch/x86/kernel/signal.c > @@ -632,9 +632,16 @@ badframe: > } > > #ifdef CONFIG_X86_32 > -asmlinkage int sys_rt_sigreturn(struct pt_regs regs) > +/* > + * Note: do not pass in pt_regs directly as with tail-call optimization > + * GCC will incorrectly stomp on the caller's frame and corrupt user-space > + * register state: > + */ > +asmlinkage int sys_rt_sigreturn(unsigned long __unused) > { > - return do_rt_sigreturn(®s); > + struct pt_regs *regs = (struct pt_regs *)&__unused; > + > + return do_rt_sigreturn(regs); > } > #else /* !CONFIG_X86_32 */ > asmlinkage long sys_rt_sigreturn(struct pt_regs *regs) > > Ok, I officially tested tip/master and firefox is happy it can talk to friends with google talk ;-) Although.... I don't use google talk.... it just seems to be this mandatory tab I can't get rid of on my google page that caused a regression to be noticed so that the kernel could be improved!
Justin Madru
| |