[PATCH x86/mm] x86_64 save_i387_ia32 snafu

[PATCH x86/mm] x86_64 save_i387_ia32 snafu

[Roland McGrath]

Sorry about this one, mea culpa.  This should go right after
the "x86 i387 user_regset" patch, or be rolled into it.

Signed-off-by: Roland McGrath <roland@redhat.com>
---
 arch/x86/ia32/ia32_signal.c |    5 ++---
 arch/x86/kernel/ptrace.c    |    1 -
 include/asm-x86/fpu32.h     |   10 ----------
 include/asm-x86/i387.h      |    6 ++++++
 4 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
index 0e24e3f..0a34c24 100644
--- a/arch/x86/ia32/ia32_signal.c
+++ b/arch/x86/ia32/ia32_signal.c
@@ -29,7 +29,6 @@
 #include <asm/ia32_unistd.h>
 #include <asm/user32.h>
 #include <asm/sigcontext32.h>
-#include <asm/fpu32.h>
 #include <asm/proto.h>
 #include <asm/vdso.h>
 
@@ -258,7 +257,7 @@ static int ia32_restore_sigcontext(struct pt_regs *regs,
 	if (buf) {
 		if (!access_ok(VERIFY_READ, buf, sizeof(*buf)))
 			goto badframe;
-		err |= restore_i387_ia32(current, buf, 0);
+		err |= restore_i387_ia32(buf);
 	} else {
 		struct task_struct *me = current;
 
@@ -377,7 +376,7 @@ static int ia32_setup_sigcontext(struct sigcontext_ia32 __user *sc,
 	err |= __put_user((u32)regs->flags, &sc->flags);
 	err |= __put_user((u32)regs->sp, &sc->sp_at_signal);
 
-	tmp = save_i387_ia32(current, fpstate, regs, 0);
+	tmp = save_i387_ia32(fpstate);
 	if (tmp < 0)
 		err = -EFAULT;
 	else {
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 5d7d82f..96286df 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -981,7 +981,6 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 #include <linux/compat.h>
 #include <linux/syscalls.h>
 #include <asm/ia32.h>
-#include <asm/fpu32.h>
 #include <asm/user32.h>
 
 #define R32(l,q)							\
diff --git a/include/asm-x86/fpu32.h b/include/asm-x86/fpu32.h
deleted file mode 100644
index 4153db5..0000000
--- a/include/asm-x86/fpu32.h
+++ /dev/null
@@ -1,10 +0,0 @@
-#ifndef _FPU32_H
-#define _FPU32_H 1
-
-struct _fpstate_ia32;
-
-int restore_i387_ia32(struct task_struct *tsk, struct _fpstate_ia32 __user *buf, int fsave);
-int save_i387_ia32(struct task_struct *tsk, struct _fpstate_ia32 __user *buf, 
-		   struct pt_regs *regs, int fsave);
-
-#endif
diff --git a/include/asm-x86/i387.h b/include/asm-x86/i387.h
index 3f4c4a5..ba8105c 100644
--- a/include/asm-x86/i387.h
+++ b/include/asm-x86/i387.h
@@ -28,6 +28,12 @@ extern user_regset_active_fn fpregs_active, xfpregs_active;
 extern user_regset_get_fn fpregs_get, xfpregs_get, fpregs_soft_get;
 extern user_regset_set_fn fpregs_set, xfpregs_set, fpregs_soft_set;
 
+#ifdef CONFIG_IA32_EMULATION
+struct _fpstate_ia32;
+extern int save_i387_ia32(struct _fpstate_ia32 __user *buf);
+extern int restore_i387_ia32(struct _fpstate_ia32 __user *buf);
+#endif
+
 #ifdef CONFIG_X86_64
 
 /* Ignore delayed exceptions from user space */

Re: [PATCH x86/mm] x86_64 save_i387_ia32 snafu

[Ingo Molnar]

* Roland McGrath <roland@redhat.com> wrote:

> Sorry about this one, mea culpa.  This should go right after the "x86 
> i387 user_regset" patch, or be rolled into it.

thanks, applied. Does this explain the crash/hang problems with 32-bit 
apps on 64-bit kernels? What was the exact failure mode?

	Ingo

Re: [PATCH x86/mm] x86_64 save_i387_ia32 snafu

[Roland McGrath]

> thanks, applied. Does this explain the crash/hang problems with 32-bit 
> apps on 64-bit kernels? What was the exact failure mode?

It does.  Any 32-bit process trying to run a signal handler when it had
used the FPU, would clobber "current" with FP bits.  The observed failure
mode was shortly after this in the signal handler setup code, when it
crashed due to current->mm being zero (current->pid also being zero, and
the whole first 512 bytes of the task_struct being garbage).


Thanks,
Roland

Re: [PATCH x86/mm] x86_64 save_i387_ia32 snafu

[Valdis.Kletnieks]

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

On Fri, 11 Jan 2008 13:31:47 PST, Roland McGrath said:
> > thanks, applied. Does this explain the crash/hang problems with 32-bit 
> > apps on 64-bit kernels? What was the exact failure mode?
> 
> It does.  Any 32-bit process trying to run a signal handler when it had
> used the FPU, would clobber "current" with FP bits.  The observed failure
> mode was shortly after this in the signal handler setup code, when it
> crashed due to current->mm being zero (current->pid also being zero, and
> the whole first 512 bytes of the task_struct being garbage).

For what it's worth, this patch fixes a problem I had in 24-rc6-mm1 with
kernel panics when certain X programs exited (Eterm being the biggest cause),
that I had bisected to "somewhere in git-x86.patch"....

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [PATCH x86/mm] x86_64 save_i387_ia32 snafu

[Ingo Molnar]

* Valdis.Kletnieks@vt.edu <Valdis.Kletnieks@vt.edu> wrote:

> On Fri, 11 Jan 2008 13:31:47 PST, Roland McGrath said:
> > > thanks, applied. Does this explain the crash/hang problems with 32-bit 
> > > apps on 64-bit kernels? What was the exact failure mode?
> > 
> > It does.  Any 32-bit process trying to run a signal handler when it had
> > used the FPU, would clobber "current" with FP bits.  The observed failure
> > mode was shortly after this in the signal handler setup code, when it
> > crashed due to current->mm being zero (current->pid also being zero, and
> > the whole first 512 bytes of the task_struct being garbage).
> 
> For what it's worth, this patch fixes a problem I had in 24-rc6-mm1 
> with kernel panics when certain X programs exited (Eterm being the 
> biggest cause), that I had bisected to "somewhere in 
> git-x86.patch"....

ok, good. Yesterday's x86.git update has this fix included so the next 
-mm iteration should have the fix as well.

	Ingo