linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rik van Riel <riel@redhat.com>
To: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Andy Lutomirski <luto@amacapital.net>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	Fenghua Yu <fenghua.yu@intel.com>,
	the arch/x86 maintainers <x86@kernel.org>,
	Oleg Nesterov <oleg@redhat.com>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: question about save_xstate_sig() - WHY DOES THIS WORK?
Date: Fri, 23 Jan 2015 14:34:29 -0500	[thread overview]
Message-ID: <54C2A245.4010307@redhat.com> (raw)

While working on a patch series to defer FPU state loading until
kernel -> user space transition, and be more lazy with FPU state
while in the kernel, I came across this code in save_xstate_sig().

Not only is this broken with my new code, but it looks like it may
be broken with the current code, too...

Specifically, save_user_xstate() may page fault and sleep. After
returning from the page fault, there is no guarantee that the
FPU state will be restored into the CPU, when the system is not
running with eager fpu mode.

In that case, what prevents us from saving random FPU register state
to the user's stack frame?  Potentially state containing data from
other programs...

        if (user_has_fpu()) {
                /* Save the live register state to the user directly. */
                if (save_user_xstate(buf_fx))
                        return -1;
                /* Update the thread's fxstate to save the fsave header. */
                if (ia32_fxstate)
                        fpu_fxsave(&tsk->thread.fpu);
        } else {
                sanitize_i387_state(tsk);
                if (__copy_to_user(buf_fx, xsave, xstate_size))
                        return -1;
        }

Is this code safe for some reason I have overlooked?

If not, should I post the patch turning the above into "save FPU
state atomically, then copy it to user space" independently from
my optimizations, and submit it for inclusion in -stable as well?

             reply	other threads:[~2015-01-23 19:34 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-23 19:34 Rik van Riel [this message]
2015-01-23 20:51 ` [PATCH, RFC] x86,fpu: make signal handling xstate save & restore preemption safe Rik van Riel
2015-01-23 21:07 ` question about save_xstate_sig() - WHY DOES THIS WORK? H. Peter Anvin
2015-01-24 13:39   ` Rik van Riel
2015-01-24 20:20 ` Oleg Nesterov
2015-01-26 23:27   ` Rik van Riel
2015-01-27 19:40     ` Oleg Nesterov
2015-01-27 20:27       ` Rik van Riel
2015-01-27 20:50         ` Rik van Riel
2015-01-29 21:01           ` Oleg Nesterov
2015-01-29 20:45         ` Oleg Nesterov
2015-01-29 20:52           ` Rik van Riel
2015-01-29 21:00           ` [PATCH RFC] x86,fpu: merge save_init_fpu & unlazy_fpu Rik van Riel
2015-01-29 21:21             ` Oleg Nesterov
2015-01-29 21:07 ` [PATCH 0/3]: x86, fpu: unlazy_fpu fixes/cleanups Oleg Nesterov
2015-01-29 21:07   ` [PATCH 1/3] x86, fpu: unlazy_fpu: don't reset thread.fpu_counter Oleg Nesterov
2015-01-29 21:26     ` Rik van Riel
2015-01-29 21:08   ` [PATCH 2/3] x86, fpu: unlazy_fpu: don't do __thread_fpu_end() if use_eager_fpu() Oleg Nesterov
2015-01-29 21:36     ` Rik van Riel
2015-01-29 21:49       ` Oleg Nesterov
2015-01-29 21:53         ` Rik van Riel
2015-01-29 21:54     ` Rik van Riel
2015-01-29 21:08   ` [PATCH 3/3] x86, fpu: kill save_init_fpu(), change math_error() to use unlazy_fpu() Oleg Nesterov
2015-01-29 21:54     ` Rik van Riel
2015-01-29 21:17   ` [PATCH 0/3]: x86, fpu: unlazy_fpu fixes/cleanups Dave Hansen
2015-01-29 21:33     ` Oleg Nesterov
2015-01-29 21:43       ` Dave Hansen
2015-01-29 21:56         ` Oleg Nesterov
2015-01-29 21:58           ` Rik van Riel
2015-01-29 23:26           ` Dave Hansen
2015-01-30  1:33             ` Rik van Riel
2015-02-02 18:11               ` Dave Hansen
2015-01-30 12:45             ` Oleg Nesterov
2015-01-30 13:30               ` Oleg Nesterov
2015-01-30 13:43                 ` Oleg Nesterov
2015-01-30 17:49   ` [PATCH 0/3] cleanups to the disable lazy fpu restore code riel
2015-01-30 17:49     ` [PATCH 1/3] x86,fpu: move lazy restore functions up a few lines riel
2015-01-30 17:49     ` [PATCH 2/3] x86,fpu: introduce task_disable_lazy_fpu_restore helper riel
2015-01-30 17:49     ` [PATCH 3/3] x86,fpu: use disable_task_lazy_fpu_restore helper riel
2015-01-30 21:46       ` Dave Hansen
2015-01-30 21:48         ` Rik van Riel
2015-02-02 17:56         ` Rik van Riel
2015-02-02 18:00   ` [PATCH 0/6] cleanups to lazy FPU restore code riel
2015-02-02 18:00     ` [PATCH 1/6] x86,fpu: move lazy restore functions up a few lines riel
2015-02-02 18:00     ` [PATCH 2/6] x86,fpu: introduce task_disable_lazy_fpu_restore helper riel
2015-02-02 18:00     ` [PATCH 3/6] x86,fpu: use an explicit if/else in switch_fpu_prepare riel
2015-02-02 18:00     ` [PATCH 4/6] x86,fpu: use disable_task_lazy_fpu_restore helper riel
2015-02-02 19:21       ` Oleg Nesterov
2015-02-02 19:43         ` Rik van Riel
2015-02-03 19:08           ` Oleg Nesterov
2015-02-03 22:01             ` Rik van Riel
2015-02-06 16:42         ` Rik van Riel
2015-02-02 18:00     ` [PATCH 5/6] x86,fpu: also check fpu_lazy_restore when use_eager_fpu riel
2015-02-02 18:55       ` Oleg Nesterov
2015-02-02 19:19         ` Rik van Riel
2015-02-02 18:00     ` [PATCH 6/6] x86,fpu: remove redundant increments of fpu_counter riel
2015-02-02 18:34       ` Oleg Nesterov
2015-02-02 18:40         ` Rik van Riel
2015-02-18 23:40           ` Ingo Molnar
2015-02-18 23:54             ` Borislav Petkov
2015-02-19 20:09             ` Oleg Nesterov

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=54C2A245.4010307@redhat.com \
    --to=riel@redhat.com \
    --cc=fenghua.yu@intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=suresh.b.siddha@intel.com \
    --cc=tglx@linutronix.de \
    --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;
as well as URLs for NNTP newsgroup(s).