All of lore.kernel.org
 help / color / mirror / Atom feed
From: Suresh Siddha <suresh.b.siddha@intel.com>
To: greg@kroah.com, mingo@elte.hu
Cc: stable@kernel.org, linux-kernel@vger.kernel.org,
	Suresh Siddha <suresh.b.siddha@intel.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: [stable 2/2] x64, fpu: fix possible FPU leakage in error conditions
Date: Mon, 18 Aug 2008 11:35:47 -0700	[thread overview]
Message-ID: <20080818183605.860693000@linux-os.sc.intel.com> (raw)
In-Reply-To: 20080818183605.719267000@linux-os.sc.intel.com

[-- Attachment #1: fix_fpu_leakage.patch --]
[-- Type: text/plain, Size: 3284 bytes --]

[Upstream commit: 6ffac1e90a17ea0aded5c581204397421eec91b6]

On Thu, Jul 24, 2008 at 03:43:44PM -0700, Linus Torvalds wrote:
> So how about this patch as a starting point? This is the RightThing(tm) to
> do regardless, and if it then makes it easier to do some other cleanups,
> we should do it first. What do you think?

restore_fpu_checking() calls init_fpu() in error conditions.

While this is wrong(as our main intention is to clear the fpu state of
the thread), this was benign before commit 92d140e21f1 ("x86: fix taking
DNA during 64bit sigreturn").

Post commit 92d140e21f1, live FPU registers may not belong to this
process at this error scenario.

In the error condition for restore_fpu_checking() (especially during the
64bit signal return), we are doing init_fpu(), which saves the live FPU
register state (possibly belonging to some other process context) into
the thread struct (through unlazy_fpu() in init_fpu()). This is wrong
and can leak the FPU data.

For the signal handler restore error condition in restore_i387(), clear
the fpu state present in the thread struct(before ultimately sending a
SIGSEGV for badframe).

For the paranoid error condition check in math_state_restore(), send a
SIGSEGV, if we fail to restore the state.

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

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-18 11:16:43.000000000 -0700
+++ linux-2.6.26.2/arch/x86/kernel/signal_64.c	2008-08-18 11:16:50.000000000 -0700
@@ -104,7 +104,16 @@
 		clts();
 		task_thread_info(current)->status |= TS_USEDFPU;
 	}
-	return restore_fpu_checking((__force struct i387_fxsave_struct *)buf);
+	err = restore_fpu_checking((__force struct i387_fxsave_struct *)buf);
+	if (unlikely(err)) {
+		/*
+		 * Encountered an error while doing the restore from the
+		 * user buffer, clear the fpu state.
+		 */
+		clear_fpu(tsk);
+		clear_used_math();
+	}
+	return err;
 }
 
 /*
Index: linux-2.6.26.2/arch/x86/kernel/traps_64.c
===================================================================
--- linux-2.6.26.2.orig/arch/x86/kernel/traps_64.c	2008-08-06 09:19:01.000000000 -0700
+++ linux-2.6.26.2/arch/x86/kernel/traps_64.c	2008-08-18 11:17:43.000000000 -0700
@@ -1141,7 +1141,14 @@
 	}
 
 	clts();			/* Allow maths ops (or we recurse) */
-	restore_fpu_checking(&me->thread.xstate->fxsave);
+ 	/*
+ 	 * Paranoid restore. send a SIGSEGV if we fail to restore the state.
+ 	 */
+ 	if (unlikely(restore_fpu_checking(&me->thread.xstate->fxsave))) {
+ 		stts();
+ 		force_sig(SIGSEGV, me);
+ 		return;
+ 	}
 	task_thread_info(me)->status |= TS_USEDFPU;
 	me->fpu_counter++;
 }
Index: linux-2.6.26.2/include/asm-x86/i387.h
===================================================================
--- linux-2.6.26.2.orig/include/asm-x86/i387.h	2008-08-18 11:16:43.000000000 -0700
+++ linux-2.6.26.2/include/asm-x86/i387.h	2008-08-18 11:16:50.000000000 -0700
@@ -62,8 +62,6 @@
 #else
 		     : [fx] "cdaSDb" (fx), "m" (*fx), "0" (0));
 #endif
-	if (unlikely(err))
-		init_fpu(current);
 	return err;
 }
 

-- 


  reply	other threads:[~2008-08-18 18:40 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-18 18:35 [stable 1/2] x86-64: Clean up save/restore_i387() usage Suresh Siddha
2008-08-18 18:35 ` Suresh Siddha [this message]
2008-08-18 18:48 ` Linus Torvalds
2008-08-18 19:32   ` Suresh Siddha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080818183605.860693000@linux-os.sc.intel.com \
    --to=suresh.b.siddha@intel.com \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=stable@kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.