Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1502425 > unrolled thread

[PATCH RFC 1/3] fpu/x86: add make_fpregs_active(_newstate) helper functions

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH RFC 1/3] fpu/x86: add make_fpregs_active(_newstate) helper functions riel@redhat.com - 2016-10-17 22:20 +0200
    Re: [PATCH RFC 1/3] fpu/x86: add make_fpregs_active(_newstate) helper functions Andy Lutomirski <luto@amacapital.net> - 2016-10-17 23:00 +0200
      Re: [PATCH RFC 1/3] fpu/x86: add make_fpregs_active(_newstate)  helper functions Yu-cheng Yu <yu-cheng.yu@intel.com> - 2016-10-18 01:10 +0200
        Re: [PATCH RFC 1/3] fpu/x86: add make_fpregs_active(_newstate) helper functions Andy Lutomirski <luto@amacapital.net> - 2016-10-18 01:40 +0200
          Re: [PATCH RFC 1/3] fpu/x86: add make_fpregs_active(_newstate)  helper functions Yu-cheng Yu <yu-cheng.yu@intel.com> - 2016-10-18 01:50 +0200
            Re: [PATCH RFC 1/3] fpu/x86: add make_fpregs_active(_newstate) helper functions Andy Lutomirski <luto@amacapital.net> - 2016-10-18 03:30 +0200

#1502425 — [PATCH RFC 1/3] fpu/x86: add make_fpregs_active(_newstate) helper functions

From: Rik van Riel <riel@redhat.com>

Add helper functions that ensure a task's floating point registers are
set up the way they need to be - either with the task's floating point
state loaded in, or ready to accept a task's new floating point state.

These helpers can be called from code that accesses the floating point
state from a preemptible state, in preparation for the lazier floating
point loading code, using loops like this:

do {
	make_fpregs_active();
	...
} while (unlikely(!fpregs_active()));

This way a task can safely do things like saving the floating point
state of a task to user space memory (the signal handling code does
this), without requiring that the floating point state is restored
at every context switch.

If the floating point registers are still active when leaving the
loop, the floating point state has ended up in its destination
(registers or memory) in one piece, and will be saved at the next
context switch.

When the floating point state is already present, these functions
do nothing.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/x86/include/asm/fpu/internal.h | 53 +++++++++++++++++++++++++++++++++----
 1 file changed, 48 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h
index 621ba3bfa2a7..d40deb337807 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -473,6 +473,14 @@ static inline void copy_kernel_to_fpregs(union fpregs_state *fpstate)
 extern int copy_fpstate_to_sigframe(void __user *buf, void __user *fp, int size);
 
 /*
+ * Is this thread using floating point state.
+ */
+static inline int fpstate_active(void)
+{
+	return current->thread.fpu.fpstate_active;
+}
+
+/*
  * FPU context switch related helper methods:
  */
 
@@ -548,6 +556,44 @@ static inline int fpregs_active(void)
 }
 
 /*
+ * Ensure the floating point registers are ready for this task, and
+ * contain the task's floating point state.
+ *
+ * The loadnew variant can be used when loading new floating point
+ * state, and the old floating point register state does not matter.
+ */
+static inline void __make_fpregs_active(struct fpu *fpu, int cpu)
+{
+	if (!fpregs_state_valid(fpu, cpu))
+		copy_kernel_to_fpregs(&fpu->state);
+	fpregs_activate(fpu);
+}
+
+static inline void make_fpregs_active(void)
+{
+	struct fpu *fpu = &current->thread.fpu;
+
+	if (fpregs_active())
+		return;
+
+	preempt_disable();
+	__make_fpregs_active(fpu, raw_smp_processor_id());
+	preempt_enable();
+}
+
+static inline void make_fpregs_active_loadnew(void)
+{
+	struct fpu *fpu = &current->thread.fpu;
+
+	if (fpregs_active())
+		return;
+
+	preempt_disable();
+	fpregs_activate(fpu);
+	preempt_enable();
+}
+
+/*
  * FPU state switching for scheduling.
  *
  * This is a two-stage process:
@@ -587,11 +633,8 @@ static inline void switch_fpu_finish(struct fpu *new_fpu, int cpu)
 	bool preload = static_cpu_has(X86_FEATURE_FPU) &&
 		       new_fpu->fpstate_active;
 
-	if (preload) {
-		if (!fpregs_state_valid(new_fpu, cpu))
-			copy_kernel_to_fpregs(&new_fpu->state);
-		fpregs_activate(new_fpu);
-	}
+	if (preload)
+		__make_fpregs_active(new_fpu, cpu);
 }
 
 /*
-- 
2.7.4

[toc] | [next] | [standalone]

#1502447

On Mon, Oct 17, 2016 at 1:09 PM,  <riel@redhat.com> wrote:
> From: Rik van Riel <riel@redhat.com>
>
> Add helper functions that ensure a task's floating point registers are
> set up the way they need to be - either with the task's floating point
> state loaded in, or ready to accept a task's new floating point state.
>
> These helpers can be called from code that accesses the floating point
> state from a preemptible state, in preparation for the lazier floating
> point loading code, using loops like this:
>
> do {
>         make_fpregs_active();
>         ...
> } while (unlikely(!fpregs_active()));
>
> This way a task can safely do things like saving the floating point
> state of a task to user space memory (the signal handling code does
> this), without requiring that the floating point state is restored
> at every context switch.

Sadly, I think this model is problematic.  An attacker can set up some
memory that causes writes to block in a controlled manner (using
userfaultfd, FUSE, madvise() hammering, etc).  The attacker can
arrange for the uaccess write in the "..." to block and then for some
privileged target task to be scheduled.  The attacker then gets their
task to be scheduled next and the privileged xstate gets written to
the attacker's memory.  Then the attacker either reads it back from a
different thread or arranges for the next iteration of the loop to
fail outright.  Now the attacker has read another task's xstate.

Dave and/or Yu-cheng: didn't one of you have some code to allow a user
xstate buffer to be filled from the copy in kernel memory?  If we did
that, we could avoid this mess entirely.

Alternatively, there could be flag that causes FPU loads to be
temporarily eager.  Maybe the sequence would look like:

pin_fpregs_active();
...
unpin_fpregs_active();

or maybe get_fpregs() / put_fpregs().

--Andy

[toc] | [prev] | [next] | [standalone]

#1502572 — Re: [PATCH RFC 1/3] fpu/x86: add make_fpregs_active(_newstate) helper functions

On Mon, Oct 17, 2016 at 01:57:06PM -0700, Andy Lutomirski wrote:
> Dave and/or Yu-cheng: didn't one of you have some code to allow a user
> xstate buffer to be filled from the copy in kernel memory?  If we did
> that, we could avoid this mess entirely.

In copy_fpstate_to_sigframe() (arch/x86/kernel/fpu/signal.c), the 
assumption was we have lazy fpu:

	if (fpregs_active() || we want an #NM exception)
		copy_fpregs_to_sigframe();
	else
		copy kernel buffer to user buffer;

But this is not the true anymore.  Or do you mean something else?

-- Yu-cheng

[toc] | [prev] | [next] | [standalone]

#1502588

On Mon, Oct 17, 2016 at 4:04 PM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> On Mon, Oct 17, 2016 at 01:57:06PM -0700, Andy Lutomirski wrote:
>> Dave and/or Yu-cheng: didn't one of you have some code to allow a user
>> xstate buffer to be filled from the copy in kernel memory?  If we did
>> that, we could avoid this mess entirely.
>
> In copy_fpstate_to_sigframe() (arch/x86/kernel/fpu/signal.c), the
> assumption was we have lazy fpu:
>
>         if (fpregs_active() || we want an #NM exception)
>                 copy_fpregs_to_sigframe();
>         else
>                 copy kernel buffer to user buffer;
>
> But this is not the true anymore.  Or do you mean something else?

Rik wants to add a different form of FPU laziness, and it would be
simpler if we could just always copy from a kernel buffer.  Does code
to do that exist in the tree?

--Andy

[toc] | [prev] | [next] | [standalone]

#1502591 — Re: [PATCH RFC 1/3] fpu/x86: add make_fpregs_active(_newstate) helper functions

On Mon, Oct 17, 2016 at 04:30:47PM -0700, Andy Lutomirski wrote:
> Rik wants to add a different form of FPU laziness, and it would be
> simpler if we could just always copy from a kernel buffer.  Does code
> to do that exist in the tree?

If we know fpregs are not active at the time of the copy and compacted format
is the concern, can we use copyout_from_xsaves() in xstate.c?

Yu-cheng

[toc] | [prev] | [next] | [standalone]

#1502624

On Mon, Oct 17, 2016 at 4:45 PM, Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> On Mon, Oct 17, 2016 at 04:30:47PM -0700, Andy Lutomirski wrote:
>> Rik wants to add a different form of FPU laziness, and it would be
>> simpler if we could just always copy from a kernel buffer.  Does code
>> to do that exist in the tree?
>
> If we know fpregs are not active at the time of the copy and compacted format
> is the concern, can we use copyout_from_xsaves() in xstate.c?
>

Right, that.  Rik, that could be an alternative solution.

--Andy

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web