public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: x86@kernel.org, andi@firstfloor.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	stable@kernel.org, Ingo Molnar <mingo@elte.hu>
Subject: Re: [patch] x64, fpu: fix possible FPU leakage in error conditions
Date: Thu, 24 Jul 2008 11:31:42 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.1.10.0807241122260.3237@nehalem.linux-foundation.org> (raw)
In-Reply-To: <20080724180429.GI14380@linux-os.sc.intel.com>



On Thu, 24 Jul 2008, Suresh Siddha wrote:
> 
> 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.
> 
> Remove the unlazy_fpu() from the init_fpu(). init_fpu() will now always
> init the FPU data in the thread struct. For the error conditions in
> restore_fpu_checking(), restore the initialized FPU data from the thread
> struct.

Why? The thread struct is guaranteed to contain pointless data.

If we cannot restore the FP state from the signal stack, we should not try 
to restore it from anywhere _else_ either, since nowhere else will have 
any better results.

I suspect we should just reset the x87 state (which was the _intention_ of 
the code), possibly by just doing "stts + used_math = 0". The signal 
handling code already checks for errors, and will force a SIGSEGV if this 
ever happens.

(Yes, there is also a restore_fpu_checking() in math_state_restore(), but 
that one _already_ uses &current->thread.xstate->fxsave as the buffer to 
restore from, so trying to do that _again_ when it fails seems to be 
really really wrong - we already _did_ that, and that was what failed to 
begin with)

				Linus

  reply	other threads:[~2008-07-24 18:35 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-24 18:04 [patch] x64, fpu: fix possible FPU leakage in error conditions Suresh Siddha
2008-07-24 18:31 ` Linus Torvalds [this message]
2008-07-24 18:50   ` Suresh Siddha
2008-07-24 18:59     ` Linus Torvalds
2008-07-24 20:27       ` Suresh Siddha
2008-07-24 20:30         ` Linus Torvalds
2008-07-24 21:23           ` Suresh Siddha
2008-07-24 21:54             ` Linus Torvalds
2008-07-24 22:25               ` Suresh Siddha
2008-07-24 22:43                 ` Linus Torvalds
2008-07-24 23:02                   ` Suresh Siddha
2008-07-24 23:06                   ` Suresh Siddha
2008-07-24 23:16                     ` Linus Torvalds
2008-07-25  1:07                   ` Suresh Siddha
2008-07-26 14:37                     ` Ingo Molnar

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=alpine.LFD.1.10.0807241122260.3237@nehalem.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=stable@kernel.org \
    --cc=suresh.b.siddha@intel.com \
    --cc=x86@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox