Re: [stable 1/2] x86-64: Clean up save/restore_i387() usage

Previous thread: [stable 2/2] x64, fpu: fix possible FPU leakage in error conditions by Suresh Siddha on Monday, August 18, 2008 - 11:35 am. (1 message)

Next thread: include/linux/security.h: In function ‘securi by Vegard Nossum on Monday, August 18, 2008 - 11:43 am. (3 messages)
From: Suresh Siddha
Date: Monday, August 18, 2008 - 11:35 am

[Upstream commit: b30f3ae50cd03ef2ff433a5030fbf88dd8323528]

Suresh Siddha wants to fix a possible FPU leakage in error conditions,
but the fact that save/restore_i387() are inlines in a header file makes
that harder to do than necessary.  So start off with an obvious cleanup.

This just moves the x86-64 version of save/restore_i387() out of the
header file, and moves it to the only file that it is actually used in:
arch/x86/kernel/signal_64.c.  So exposing it in a header file was wrong
to begin with.

[ Side note: I'd like to fix up some of the games we play with the
  32-bit version of these functions too, but that's a separate
  matter.  The 32-bit versions are shared - under different names
  at that! - by both the native x86-32 code and the x86-64 32-bit
  compatibility code ]

Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: <stable@kernel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

Index: linux-2.6.26.2/arch/x86/kernel/signal_64.c
===================================================================
--- linux-2.6.26.2.orig/arch/x86/kernel/signal_64.c	2008-08-06 09:19:01.000000000 -0700
+++ linux-2.6.26.2/arch/x86/kernel/signal_64.c	2008-08-18 11:16:43.000000000 -0700
@@ -53,6 +53,59 @@
 	return do_sigaltstack(uss, uoss, regs->sp);
 }
 
+/*
+ * Signal frame handlers.
+ */
+
+static inline int save_i387(struct _fpstate __user *buf)
+{
+	struct task_struct *tsk = current;
+	int err = 0;
+
+	BUILD_BUG_ON(sizeof(struct user_i387_struct) !=
+			sizeof(tsk->thread.xstate->fxsave));
+
+	if ((unsigned long)buf % 16)
+		printk("save_i387: bad fpstate %p\n", buf);
+
+	if (!used_math())
+		return 0;
+	clear_used_math(); /* trigger finit */
+	if (task_thread_info(tsk)->status & TS_USEDFPU) {
+		err = save_i387_checking((struct i387_fxsave_struct __user *)
+					 buf);
+		if (err)
+			return err;
+		task_thread_info(tsk)->status &= ~TS_USEDFPU;
+		stts();
+	} else {
+		if (__copy_to_user(buf, &tsk->thread.xstate->fxsave,
+				  ...
From: Linus Torvalds
Date: Monday, August 18, 2008 - 11:48 am

And it's authorship got destroyed by you.

Please guys, add teh proper "From:" line at the top. Don't forward patches 
like this unattributed. It doesn't matter if they are trivial or not, it's 
just really bad form. 

			Linus
--

From: Suresh Siddha
Date: Monday, August 18, 2008 - 12:32 pm

Sorry! Noticed it while sending but ignored. Will fix up my scripts
accordingly.

Greg, please ignore the earlier patch titled,
	[stable 1/2] x86-64: Clean up save/restore_i387() usage

And accept the appended one, instead: Thanks.
---
From: Linus Torvalds <torvalds@linux-foundation.org>
Subject: x86-64: Clean up 'save/restore_i387()' usage

    [ Upstream commit b30f3ae50cd03ef2ff433a5030fbf88dd8323528]

    Suresh Siddha wants to fix a possible FPU leakage in error conditions,
    but the fact that save/restore_i387() are inlines in a header file makes
    that harder to do than necessary.  So start off with an obvious cleanup.
    
    This just moves the x86-64 version of save/restore_i387() out of the
    header file, and moves it to the only file that it is actually used in:
    arch/x86/kernel/signal_64.c.  So exposing it in a header file was wrong
    to begin with.
    
    [ Side note: I'd like to fix up some of the games we play with the
      32-bit version of these functions too, but that's a separate
      matter.  The 32-bit versions are shared - under different names
      at that! - by both the native x86-32 code and the x86-64 32-bit
      compatibility code ]
    
    Acked-by: Suresh Siddha <suresh.b.siddha@intel.com>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/arch/x86/kernel/signal_64.c b/arch/x86/kernel/signal_64.c
index 47c3d24..b45ef8d 100644
--- a/arch/x86/kernel/signal_64.c
+++ b/arch/x86/kernel/signal_64.c
@@ -53,6 +53,59 @@ sys_sigaltstack(const stack_t __user *uss, stack_t __user *uoss,
 	return do_sigaltstack(uss, uoss, regs->sp);
 }
 
+/*
+ * Signal frame handlers.
+ */
+
+static inline int save_i387(struct _fpstate __user *buf)
+{
+	struct task_struct *tsk = current;
+	int err = 0;
+
+	BUILD_BUG_ON(sizeof(struct user_i387_struct) !=
+			sizeof(tsk->thread.xstate->fxsave));
+
+	if ((unsigned long)buf % 16)
+		printk("save_i387: bad fpstate %p\n", buf);
+
+	if (!used_math())
+		return ...
Previous thread: [stable 2/2] x64, fpu: fix possible FPU leakage in error conditions by Suresh Siddha on Monday, August 18, 2008 - 11:35 am. (1 message)

Next thread: include/linux/security.h: In function ‘securi by Vegard Nossum on Monday, August 18, 2008 - 11:43 am. (3 messages)