2008-01-11 10:44:39

by Roland McGrath

[permalink] [raw]
Subject: [PATCH x86/mm] x86_64 save_i387_ia32 snafu

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 <[email protected]>
---
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 */


2008-01-11 10:49:59

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH x86/mm] x86_64 save_i387_ia32 snafu


* Roland McGrath <[email protected]> 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

2008-01-11 21:31:59

by Roland McGrath

[permalink] [raw]
Subject: Re: [PATCH x86/mm] x86_64 save_i387_ia32 snafu

> 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

2008-01-12 05:52:48

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: [PATCH x86/mm] x86_64 save_i387_ia32 snafu

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"....


Attachments:
(No filename) (226.00 B)

2008-01-12 06:59:50

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH x86/mm] x86_64 save_i387_ia32 snafu


* [email protected] <[email protected]> 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