linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header"
@ 2017-09-24 10:59 Ingo Molnar
  2017-09-24 10:59 ` [PATCH 01/10] x86/fpu: Introduce validate_xstate_header() Ingo Molnar
                   ` (10 more replies)
  0 siblings, 11 replies; 33+ messages in thread
From: Ingo Molnar @ 2017-09-24 10:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Morton, Eric Biggers, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu

As mentioned before, the patch was too big and too complex, and I've split it
up into 10 smaller, bisectable patches:

Eric Biggers (10):
  x86/fpu: Introduce validate_xstate_header()
  x86/fpu: Use validate_xstate_header() to validate the xstate_header in xstateregs_set()
  x86/fpu: Use validate_xstate_header() to validate the xstate_header in sanitize_restored_xstate()
  x86/fpu: Copy the full state_header in copy_kernel_to_xstate()
  x86/fpu: Eliminate the 'xfeatures' local variable in copy_kernel_to_xstate()
  x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_kernel_to_xstate()
  x86/fpu: Copy the full header in copy_user_to_xstate()
  x86/fpu: Eliminate the 'xfeatures' local variable in copy_user_to_xstate()
  x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_user_to_xstate()
  x86/fpu: Use using_compacted_format() instead of open coded X86_FEATURE_XSAVES

 arch/x86/include/asm/fpu/xstate.h |  4 ++++
 arch/x86/kernel/fpu/regset.c      | 21 ++++++-----------
 arch/x86/kernel/fpu/signal.c      | 17 ++++++-------
 arch/x86/kernel/fpu/xstate.c      | 76 +++++++++++++++++++++++++++++++++--------------------------
 4 files changed, 63 insertions(+), 55 deletions(-)

I kept the attribution in place, because the end result is almost the same.

Below is the interdiff from the original version, I uninlined validate_xstate_header(),
because it's way too large to be inlined everywhere, plus I removed a couple of
spurious "!= 0" patterns.

The latest x86/fpu bits can be found in -tip:

    git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git master
    git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git WIP.x86/fpu

Thanks,

    Ingo

===
 include/asm/fpu/xstate.h |   23 +----------------------
 kernel/fpu/xstate.c      |   28 ++++++++++++++++++++++++++--
 2 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 3d79d0ee4d30..83fee2469eb7 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -54,27 +54,6 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf);
 int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf);
 
 /* Validate an xstate header supplied by userspace (ptrace or sigreturn) */
-static inline int validate_xstate_header(const struct xstate_header *hdr)
-{
-	/* No unknown or supervisor features may be set */
-	if (hdr->xfeatures & (~xfeatures_mask | XFEATURE_MASK_SUPERVISOR))
-		return -EINVAL;
-
-	/* Userspace must use the uncompacted format */
-	if (hdr->xcomp_bv)
-		return -EINVAL;
-
-	/*
-	 * If 'reserved' is shrunken to add a new field, make sure to validate
-	 * that new field here!
-	 */
-	BUILD_BUG_ON(sizeof(hdr->reserved) != 48);
-
-	/* No reserved bits may be set */
-	if (memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
-		return -EINVAL;
-
-	return 0;
-}
+extern int validate_xstate_header(const struct xstate_header *hdr);
 
 #endif
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 7ebd1a0811b6..f1d5476c9022 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -483,6 +483,30 @@ int using_compacted_format(void)
 	return boot_cpu_has(X86_FEATURE_XSAVES);
 }
 
+/* Validate an xstate header supplied by userspace (ptrace or sigreturn) */
+int validate_xstate_header(const struct xstate_header *hdr)
+{
+	/* No unknown or supervisor features may be set */
+	if (hdr->xfeatures & (~xfeatures_mask | XFEATURE_MASK_SUPERVISOR))
+		return -EINVAL;
+
+	/* Userspace must use the uncompacted format */
+	if (hdr->xcomp_bv)
+		return -EINVAL;
+
+	/*
+	 * If 'reserved' is shrunken to add a new field, make sure to validate
+	 * that new field here!
+	 */
+	BUILD_BUG_ON(sizeof(hdr->reserved) != 48);
+
+	/* No reserved bits may be set */
+	if (memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
+		return -EINVAL;
+
+	return 0;
+}
+
 static void __xstate_dump_leaves(void)
 {
 	int i;
@@ -1127,7 +1151,7 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 
 	memcpy(&hdr, kbuf + offset, size);
 
-	if (validate_xstate_header(&hdr) != 0)
+	if (validate_xstate_header(&hdr))
 		return -EINVAL;
 
 	for (i = 0; i < XFEATURE_MAX; i++) {
@@ -1181,7 +1205,7 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 	if (__copy_from_user(&hdr, ubuf + offset, size))
 		return -EFAULT;
 
-	if (validate_xstate_header(&hdr) != 0)
+	if (validate_xstate_header(&hdr))
 		return -EINVAL;
 
 	for (i = 0; i < XFEATURE_MAX; i++) {

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 01/10] x86/fpu: Introduce validate_xstate_header()
  2017-09-24 10:59 [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header" Ingo Molnar
@ 2017-09-24 10:59 ` Ingo Molnar
  2017-09-26  8:34   ` [tip:x86/fpu] " tip-bot for Eric Biggers
  2017-09-24 10:59 ` [PATCH 02/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in xstateregs_set() Ingo Molnar
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 33+ messages in thread
From: Ingo Molnar @ 2017-09-24 10:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Morton, Eric Biggers, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu

From: Eric Biggers <ebiggers@google.com>

Move validation of user-supplied xstate_header into a helper function,
in preparation of calling it from both the ptrace and sigreturn syscall
paths.

The new function also considers it to be an error if *any* reserved bits
are set, whereas before we were just clearing most of them silently.

This should reduce the chance of bugs that fail to correctly validate
user-supplied XSAVE areas.  It also will expose any broken userspace
programs that set the other reserved bits; this is desirable because
such programs will lose compatibility with future CPUs and kernels if
those bits are ever used for anything.  (There shouldn't be any such
programs, and in fact in the case where the compacted format is in use
we were already validating xfeatures.  But you never know...)

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/fpu/xstate.h |  4 ++++
 arch/x86/kernel/fpu/xstate.c      | 24 ++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 579ac2358e63..83fee2469eb7 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -52,4 +52,8 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int of
 int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned int offset, unsigned int size);
 int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf);
 int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf);
+
+/* Validate an xstate header supplied by userspace (ptrace or sigreturn) */
+extern int validate_xstate_header(const struct xstate_header *hdr);
+
 #endif
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 703e76d027ee..2427aeea33b5 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -483,6 +483,30 @@ int using_compacted_format(void)
 	return boot_cpu_has(X86_FEATURE_XSAVES);
 }
 
+/* Validate an xstate header supplied by userspace (ptrace or sigreturn) */
+int validate_xstate_header(const struct xstate_header *hdr)
+{
+	/* No unknown or supervisor features may be set */
+	if (hdr->xfeatures & (~xfeatures_mask | XFEATURE_MASK_SUPERVISOR))
+		return -EINVAL;
+
+	/* Userspace must use the uncompacted format */
+	if (hdr->xcomp_bv)
+		return -EINVAL;
+
+	/*
+	 * If 'reserved' is shrunken to add a new field, make sure to validate
+	 * that new field here!
+	 */
+	BUILD_BUG_ON(sizeof(hdr->reserved) != 48);
+
+	/* No reserved bits may be set */
+	if (memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
+		return -EINVAL;
+
+	return 0;
+}
+
 static void __xstate_dump_leaves(void)
 {
 	int i;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 02/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in xstateregs_set()
  2017-09-24 10:59 [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header" Ingo Molnar
  2017-09-24 10:59 ` [PATCH 01/10] x86/fpu: Introduce validate_xstate_header() Ingo Molnar
@ 2017-09-24 10:59 ` Ingo Molnar
  2017-09-26  8:34   ` [tip:x86/fpu] " tip-bot for Eric Biggers
  2017-09-24 10:59 ` [PATCH 03/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in sanitize_restored_xstate() Ingo Molnar
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 33+ messages in thread
From: Ingo Molnar @ 2017-09-24 10:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Morton, Eric Biggers, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu

From: Eric Biggers <ebiggers@google.com>

Tighten the checks in xstateregs_set().

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/regset.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index ee8d2f049818..b831d5b9de99 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -141,27 +141,20 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
 			ret = copy_user_to_xstate(xsave, ubuf);
 	} else {
 		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0, -1);
-
-		/* xcomp_bv must be 0 when using uncompacted format */
-		if (!ret && xsave->header.xcomp_bv)
-			ret = -EINVAL;
+		if (!ret)
+			ret = validate_xstate_header(&xsave->header);
 	}
 
 	/*
-	 * In case of failure, mark all states as init:
-	 */
-	if (ret)
-		fpstate_init(&fpu->state);
-
-	/*
 	 * mxcsr reserved bits must be masked to zero for security reasons.
 	 */
 	xsave->i387.mxcsr &= mxcsr_feature_mask;
-	xsave->header.xfeatures &= xfeatures_mask;
+
 	/*
-	 * These bits must be zero.
+	 * In case of failure, mark all states as init:
 	 */
-	memset(&xsave->header.reserved, 0, 48);
+	if (ret)
+		fpstate_init(&fpu->state);
 
 	return ret;
 }
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 03/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in sanitize_restored_xstate()
  2017-09-24 10:59 [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header" Ingo Molnar
  2017-09-24 10:59 ` [PATCH 01/10] x86/fpu: Introduce validate_xstate_header() Ingo Molnar
  2017-09-24 10:59 ` [PATCH 02/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in xstateregs_set() Ingo Molnar
@ 2017-09-24 10:59 ` Ingo Molnar
  2017-09-24 18:51   ` Eric Biggers
  2017-09-26  8:35   ` [tip:x86/fpu] x86/fpu: Use validate_xstate_header() to validate the xstate_header in __fpu__restore_sig() tip-bot for Eric Biggers
  2017-09-24 10:59 ` [PATCH 04/10] x86/fpu: Copy the full state_header in copy_kernel_to_xstate() Ingo Molnar
                   ` (7 subsequent siblings)
  10 siblings, 2 replies; 33+ messages in thread
From: Ingo Molnar @ 2017-09-24 10:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Morton, Eric Biggers, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu

From: Eric Biggers <ebiggers@google.com>

Tighten the checks in sanitize_restored_xstate().

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/signal.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 1ef1b228b9fd..afe54247cf27 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -214,8 +214,11 @@ sanitize_restored_xstate(struct task_struct *tsk,
 	struct xstate_header *header = &xsave->header;
 
 	if (use_xsave()) {
-		/* These bits must be zero. */
-		memset(header->reserved, 0, 48);
+		/*
+		 * Note: we don't need to zero the reserved bits in the
+		 * xstate_header here because we either didn't copy them at all,
+		 * or we checked earlier that they aren't set.
+		 */
 
 		/*
 		 * Init the state that is not present in the memory
@@ -224,7 +227,7 @@ sanitize_restored_xstate(struct task_struct *tsk,
 		if (fx_only)
 			header->xfeatures = XFEATURE_MASK_FPSSE;
 		else
-			header->xfeatures &= (xfeatures_mask & xfeatures);
+			header->xfeatures &= xfeatures;
 	}
 
 	if (use_fxsr()) {
@@ -308,7 +311,7 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 		/*
 		 * For 32-bit frames with fxstate, copy the user state to the
 		 * thread's fpu state, reconstruct fxstate from the fsave
-		 * header. Sanitize the copied state etc.
+		 * header. Validate and sanitize the copied state.
 		 */
 		struct fpu *fpu = &tsk->thread.fpu;
 		struct user_i387_ia32_struct env;
@@ -328,10 +331,8 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 			err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
 		} else {
 			err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
-
-			/* xcomp_bv must be 0 when using uncompacted format */
-			if (!err && fpu->state.xsave.header.xcomp_bv)
-				err = -EINVAL;
+			if (!err)
+				err = validate_xstate_header(&fpu->state.xsave.header);
 		}
 
 		if (err || __copy_from_user(&env, buf, sizeof(env))) {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 04/10] x86/fpu: Copy the full state_header in copy_kernel_to_xstate()
  2017-09-24 10:59 [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header" Ingo Molnar
                   ` (2 preceding siblings ...)
  2017-09-24 10:59 ` [PATCH 03/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in sanitize_restored_xstate() Ingo Molnar
@ 2017-09-24 10:59 ` Ingo Molnar
  2017-09-26  8:35   ` [tip:x86/fpu] " tip-bot for Eric Biggers
  2017-09-24 10:59 ` [PATCH 05/10] x86/fpu: Eliminate the 'xfeatures' local variable " Ingo Molnar
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 33+ messages in thread
From: Ingo Molnar @ 2017-09-24 10:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Morton, Eric Biggers, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu

From: Eric Biggers <ebiggers@google.com>

This is in preparation to verify the full xstate header as supplied by user-space.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/xstate.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 2427aeea33b5..02591b96bb25 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1148,11 +1148,13 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 	int i;
 	u64 xfeatures;
 	u64 allowed_features;
+	struct xstate_header hdr;
 
 	offset = offsetof(struct xregs_state, header);
-	size = sizeof(xfeatures);
+	size = sizeof(hdr);
 
-	memcpy(&xfeatures, kbuf + offset, size);
+	memcpy(&hdr, kbuf + offset, size);
+	xfeatures = hdr.xfeatures;
 
 	/*
 	 * Reject if the user sets any disabled or supervisor features:
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 05/10] x86/fpu: Eliminate the 'xfeatures' local variable in copy_kernel_to_xstate()
  2017-09-24 10:59 [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header" Ingo Molnar
                   ` (3 preceding siblings ...)
  2017-09-24 10:59 ` [PATCH 04/10] x86/fpu: Copy the full state_header in copy_kernel_to_xstate() Ingo Molnar
@ 2017-09-24 10:59 ` Ingo Molnar
  2017-09-26  8:35   ` [tip:x86/fpu] " tip-bot for Eric Biggers
  2017-09-24 10:59 ` [PATCH 06/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header " Ingo Molnar
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 33+ messages in thread
From: Ingo Molnar @ 2017-09-24 10:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Morton, Eric Biggers, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu

From: Eric Biggers <ebiggers@google.com>

We have this information in the xstate_header.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/xstate.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 02591b96bb25..c97c4a9db52a 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1146,7 +1146,6 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 {
 	unsigned int offset, size;
 	int i;
-	u64 xfeatures;
 	u64 allowed_features;
 	struct xstate_header hdr;
 
@@ -1154,20 +1153,19 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 	size = sizeof(hdr);
 
 	memcpy(&hdr, kbuf + offset, size);
-	xfeatures = hdr.xfeatures;
 
 	/*
 	 * Reject if the user sets any disabled or supervisor features:
 	 */
 	allowed_features = xfeatures_mask & ~XFEATURE_MASK_SUPERVISOR;
 
-	if (xfeatures & ~allowed_features)
+	if (hdr.xfeatures & ~allowed_features)
 		return -EINVAL;
 
 	for (i = 0; i < XFEATURE_MAX; i++) {
 		u64 mask = ((u64)1 << i);
 
-		if (xfeatures & mask) {
+		if (hdr.xfeatures & mask) {
 			void *dst = __raw_xsave_addr(xsave, 1 << i);
 
 			offset = xstate_offsets[i];
@@ -1177,7 +1175,7 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 		}
 	}
 
-	if (xfeatures_mxcsr_quirk(xfeatures)) {
+	if (xfeatures_mxcsr_quirk(hdr.xfeatures)) {
 		offset = offsetof(struct fxregs_state, mxcsr);
 		size = MXCSR_AND_FLAGS_SIZE;
 		memcpy(&xsave->i387.mxcsr, kbuf + offset, size);
@@ -1192,7 +1190,7 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 	/*
 	 * Add back in the features that came in from userspace:
 	 */
-	xsave->header.xfeatures |= xfeatures;
+	xsave->header.xfeatures |= hdr.xfeatures;
 
 	return 0;
 }
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 06/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_kernel_to_xstate()
  2017-09-24 10:59 [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header" Ingo Molnar
                   ` (4 preceding siblings ...)
  2017-09-24 10:59 ` [PATCH 05/10] x86/fpu: Eliminate the 'xfeatures' local variable " Ingo Molnar
@ 2017-09-24 10:59 ` Ingo Molnar
  2017-09-26  8:36   ` [tip:x86/fpu] " tip-bot for Eric Biggers
  2017-09-24 10:59 ` [PATCH 07/10] x86/fpu: Copy the full header in copy_user_to_xstate() Ingo Molnar
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 33+ messages in thread
From: Ingo Molnar @ 2017-09-24 10:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Morton, Eric Biggers, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu

From: Eric Biggers <ebiggers@google.com>

Tighten the checks in copy_kernel_to_xstate().

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/xstate.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index c97c4a9db52a..325db7850335 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1138,15 +1138,12 @@ int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned i
 
 /*
  * Convert from a ptrace standard-format kernel buffer to kernel XSAVES format
- * and copy to the target thread. This is called from xstateregs_set() and
- * there we check the CPU has XSAVES and a whole standard-sized buffer
- * exists.
+ * and copy to the target thread. This is called from xstateregs_set().
  */
 int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 {
 	unsigned int offset, size;
 	int i;
-	u64 allowed_features;
 	struct xstate_header hdr;
 
 	offset = offsetof(struct xregs_state, header);
@@ -1154,12 +1151,7 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 
 	memcpy(&hdr, kbuf + offset, size);
 
-	/*
-	 * Reject if the user sets any disabled or supervisor features:
-	 */
-	allowed_features = xfeatures_mask & ~XFEATURE_MASK_SUPERVISOR;
-
-	if (hdr.xfeatures & ~allowed_features)
+	if (validate_xstate_header(&hdr))
 		return -EINVAL;
 
 	for (i = 0; i < XFEATURE_MAX; i++) {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 07/10] x86/fpu: Copy the full header in copy_user_to_xstate()
  2017-09-24 10:59 [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header" Ingo Molnar
                   ` (5 preceding siblings ...)
  2017-09-24 10:59 ` [PATCH 06/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header " Ingo Molnar
@ 2017-09-24 10:59 ` Ingo Molnar
  2017-09-26  8:36   ` [tip:x86/fpu] " tip-bot for Eric Biggers
  2017-09-24 10:59 ` [PATCH 08/10] x86/fpu: Eliminate the 'xfeatures' local variable " Ingo Molnar
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 33+ messages in thread
From: Ingo Molnar @ 2017-09-24 10:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Morton, Eric Biggers, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu

From: Eric Biggers <ebiggers@google.com>

This is in preparation to verify the full xstate header as supplied by user-space.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/xstate.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 325db7850335..0cd7b73c25e8 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1199,13 +1199,16 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 	int i;
 	u64 xfeatures;
 	u64 allowed_features;
+	struct xstate_header hdr;
 
 	offset = offsetof(struct xregs_state, header);
-	size = sizeof(xfeatures);
+	size = sizeof(hdr);
 
-	if (__copy_from_user(&xfeatures, ubuf + offset, size))
+	if (__copy_from_user(&hdr, ubuf + offset, size))
 		return -EFAULT;
 
+	xfeatures = hdr.xfeatures;
+
 	/*
 	 * Reject if the user sets any disabled or supervisor features:
 	 */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 08/10] x86/fpu: Eliminate the 'xfeatures' local variable in copy_user_to_xstate()
  2017-09-24 10:59 [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header" Ingo Molnar
                   ` (6 preceding siblings ...)
  2017-09-24 10:59 ` [PATCH 07/10] x86/fpu: Copy the full header in copy_user_to_xstate() Ingo Molnar
@ 2017-09-24 10:59 ` Ingo Molnar
  2017-09-26  8:37   ` [tip:x86/fpu] " tip-bot for Eric Biggers
  2017-09-24 10:59 ` [PATCH 09/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header " Ingo Molnar
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 33+ messages in thread
From: Ingo Molnar @ 2017-09-24 10:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Morton, Eric Biggers, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu

From: Eric Biggers <ebiggers@google.com>

We now have this field in hdr.xfeatures.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/xstate.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 0cd7b73c25e8..b6d78b78b5c2 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1197,7 +1197,6 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 {
 	unsigned int offset, size;
 	int i;
-	u64 xfeatures;
 	u64 allowed_features;
 	struct xstate_header hdr;
 
@@ -1207,20 +1206,18 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 	if (__copy_from_user(&hdr, ubuf + offset, size))
 		return -EFAULT;
 
-	xfeatures = hdr.xfeatures;
-
 	/*
 	 * Reject if the user sets any disabled or supervisor features:
 	 */
 	allowed_features = xfeatures_mask & ~XFEATURE_MASK_SUPERVISOR;
 
-	if (xfeatures & ~allowed_features)
+	if (hdr.xfeatures & ~allowed_features)
 		return -EINVAL;
 
 	for (i = 0; i < XFEATURE_MAX; i++) {
 		u64 mask = ((u64)1 << i);
 
-		if (xfeatures & mask) {
+		if (hdr.xfeatures & mask) {
 			void *dst = __raw_xsave_addr(xsave, 1 << i);
 
 			offset = xstate_offsets[i];
@@ -1231,7 +1228,7 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 		}
 	}
 
-	if (xfeatures_mxcsr_quirk(xfeatures)) {
+	if (xfeatures_mxcsr_quirk(hdr.xfeatures)) {
 		offset = offsetof(struct fxregs_state, mxcsr);
 		size = MXCSR_AND_FLAGS_SIZE;
 		if (__copy_from_user(&xsave->i387.mxcsr, ubuf + offset, size))
@@ -1247,7 +1244,7 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 	/*
 	 * Add back in the features that came in from userspace:
 	 */
-	xsave->header.xfeatures |= xfeatures;
+	xsave->header.xfeatures |= hdr.xfeatures;
 
 	return 0;
 }
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 09/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_user_to_xstate()
  2017-09-24 10:59 [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header" Ingo Molnar
                   ` (7 preceding siblings ...)
  2017-09-24 10:59 ` [PATCH 08/10] x86/fpu: Eliminate the 'xfeatures' local variable " Ingo Molnar
@ 2017-09-24 10:59 ` Ingo Molnar
  2017-09-26  8:37   ` [tip:x86/fpu] " tip-bot for Eric Biggers
  2017-09-24 10:59 ` [PATCH 10/10] x86/fpu: Use using_compacted_format() instead of open coded X86_FEATURE_XSAVES Ingo Molnar
  2017-09-24 18:04 ` [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header" Linus Torvalds
  10 siblings, 1 reply; 33+ messages in thread
From: Ingo Molnar @ 2017-09-24 10:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Morton, Eric Biggers, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu

From: Eric Biggers <ebiggers@google.com>

Tighten the checks in copy_user_to_xstate().

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/xstate.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index b6d78b78b5c2..f1d5476c9022 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1188,16 +1188,15 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 }
 
 /*
- * Convert from a ptrace standard-format user-space buffer to kernel XSAVES format
- * and copy to the target thread. This is called from xstateregs_set() and
- * there we check the CPU has XSAVES and a whole standard-sized buffer
- * exists.
+ * Convert from a ptrace or sigreturn standard-format user-space buffer to
+ * kernel XSAVES format and copy to the target thread. This is called from
+ * xstateregs_set(), as well as potentially from the sigreturn() and
+ * rt_sigreturn() system calls.
  */
 int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 {
 	unsigned int offset, size;
 	int i;
-	u64 allowed_features;
 	struct xstate_header hdr;
 
 	offset = offsetof(struct xregs_state, header);
@@ -1206,12 +1205,7 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 	if (__copy_from_user(&hdr, ubuf + offset, size))
 		return -EFAULT;
 
-	/*
-	 * Reject if the user sets any disabled or supervisor features:
-	 */
-	allowed_features = xfeatures_mask & ~XFEATURE_MASK_SUPERVISOR;
-
-	if (hdr.xfeatures & ~allowed_features)
+	if (validate_xstate_header(&hdr))
 		return -EINVAL;
 
 	for (i = 0; i < XFEATURE_MAX; i++) {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [PATCH 10/10] x86/fpu: Use using_compacted_format() instead of open coded X86_FEATURE_XSAVES
  2017-09-24 10:59 [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header" Ingo Molnar
                   ` (8 preceding siblings ...)
  2017-09-24 10:59 ` [PATCH 09/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header " Ingo Molnar
@ 2017-09-24 10:59 ` Ingo Molnar
  2017-09-26  8:37   ` [tip:x86/fpu] " tip-bot for Eric Biggers
  2017-09-24 18:04 ` [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header" Linus Torvalds
  10 siblings, 1 reply; 33+ messages in thread
From: Ingo Molnar @ 2017-09-24 10:59 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrew Morton, Eric Biggers, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu

From: Eric Biggers <ebiggers@google.com>

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/regset.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index b831d5b9de99..3ea151372389 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -134,7 +134,7 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
 
 	fpu__prepare_write(fpu);
 
-	if (boot_cpu_has(X86_FEATURE_XSAVES)) {
+	if (using_compacted_format()) {
 		if (kbuf)
 			ret = copy_kernel_to_xstate(xsave, kbuf);
 		else
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header"
  2017-09-24 10:59 [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header" Ingo Molnar
                   ` (9 preceding siblings ...)
  2017-09-24 10:59 ` [PATCH 10/10] x86/fpu: Use using_compacted_format() instead of open coded X86_FEATURE_XSAVES Ingo Molnar
@ 2017-09-24 18:04 ` Linus Torvalds
  2017-09-24 19:01   ` Ingo Molnar
  10 siblings, 1 reply; 33+ messages in thread
From: Linus Torvalds @ 2017-09-24 18:04 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Linux Kernel Mailing List, Andrew Morton, Eric Biggers,
	Andy Lutomirski, Borislav Petkov, Dave Hansen, Fenghua Yu,
	H . Peter Anvin, Oleg Nesterov, Peter Zijlstra, Rik van Riel,
	Thomas Gleixner, Yu-cheng Yu

On Sun, Sep 24, 2017 at 3:59 AM, Ingo Molnar <mingo@kernel.org> wrote:
> As mentioned before, the patch was too big and too complex, and I've split it
> up into 10 smaller, bisectable patches:

Is this the (updated) stuff you want to send in for 4.14? Because I
like this smaller set more tor that than the big series I saw earlier.

I'm sure the bigger series is a better cleanup, but maybe we can wait
with that bigger change for 4.15?

             Linus

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 03/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in sanitize_restored_xstate()
  2017-09-24 10:59 ` [PATCH 03/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in sanitize_restored_xstate() Ingo Molnar
@ 2017-09-24 18:51   ` Eric Biggers
  2017-09-24 19:02     ` Ingo Molnar
  2017-09-26  8:35   ` [tip:x86/fpu] x86/fpu: Use validate_xstate_header() to validate the xstate_header in __fpu__restore_sig() tip-bot for Eric Biggers
  1 sibling, 1 reply; 33+ messages in thread
From: Eric Biggers @ 2017-09-24 18:51 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Andrew Morton, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu

On Sun, Sep 24, 2017 at 12:59:06PM +0200, Ingo Molnar wrote:
> @@ -328,10 +331,8 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
>  			err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
>  		} else {
>  			err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
> -
> -			/* xcomp_bv must be 0 when using uncompacted format */
> -			if (!err && fpu->state.xsave.header.xcomp_bv)
> -				err = -EINVAL;
> +			if (!err)
> +				err = validate_xstate_header(&fpu->state.xsave.header);
>  		}
>  

Sorry, this is the buggy part.  The problem is that this code runs even if XSAVE
isn't being used --- and in that case the state size is 512 bytes or less, so
the state doesn't actually include the xstate_header.  So
validate_xstate_header() was reading out of bounds and seeing invalid values.

So I think we need to check use_xsave() here, but it really needs to be in the
earlier patch which added the check for just ->xcomp_bv ("x86/fpu: Don't let
userspace set bogus xcomp_bv"), not in this one.

As far the split of patch 2/3 into these 10 patches, it looks fine (though it
suddenly became a *lot* of patches!).  One nit: the subject of this one really
should say "__fpu__restore_sig()", not "sanitize_restored_xstate()".

I can send a fixed series when I have a chance.

Eric

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header"
  2017-09-24 18:04 ` [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header" Linus Torvalds
@ 2017-09-24 19:01   ` Ingo Molnar
  2017-09-26 16:28     ` [RFC GIT PULL] x86 FPU fixes and cleanups Ingo Molnar
  0 siblings, 1 reply; 33+ messages in thread
From: Ingo Molnar @ 2017-09-24 19:01 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Linux Kernel Mailing List, Andrew Morton, Eric Biggers,
	Andy Lutomirski, Borislav Petkov, Dave Hansen, Fenghua Yu,
	H . Peter Anvin, Oleg Nesterov, Peter Zijlstra, Rik van Riel,
	Thomas Gleixner, Yu-cheng Yu


* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Sun, Sep 24, 2017 at 3:59 AM, Ingo Molnar <mingo@kernel.org> wrote:
> > As mentioned before, the patch was too big and too complex, and I've split it
> > up into 10 smaller, bisectable patches:
> 
> Is this the (updated) stuff you want to send in for 4.14? Because I
> like this smaller set more tor that than the big series I saw earlier.

It's a 10 patch split-up of one of the 34 patches from the bigger series :-/

> I'm sure the bigger series is a better cleanup, but maybe we can wait with that 
> bigger change for 4.15?

So all of this is pretty unfortunate timing, caused in part because I delayed the 
FPU changes in this merge window due to having so many x86 changes already.

I'd really love to have the bigger series, firstly because beyond the 
simplifications and the fixes in the 10-patch series it also fixes some other 
problems such as this SkyLake bug:

  0852b374173b: x86/fpu: Add FPU state copying quirk to handle XRSTOR failure on Intel Skylake CPUs

Secondly, with v4.14 being an LTS, doing the cleanups/simplifications afterwards 
adds a big backporting barrier.

But I can extract all the fixes and re-structure and re-test it all if you prefer 
it that way.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 03/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in sanitize_restored_xstate()
  2017-09-24 18:51   ` Eric Biggers
@ 2017-09-24 19:02     ` Ingo Molnar
  2017-09-24 20:08       ` Eric Biggers
  0 siblings, 1 reply; 33+ messages in thread
From: Ingo Molnar @ 2017-09-24 19:02 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-kernel, Andrew Morton, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu


* Eric Biggers <ebiggers3@gmail.com> wrote:

> On Sun, Sep 24, 2017 at 12:59:06PM +0200, Ingo Molnar wrote:
> > @@ -328,10 +331,8 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
> >  			err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
> >  		} else {
> >  			err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
> > -
> > -			/* xcomp_bv must be 0 when using uncompacted format */
> > -			if (!err && fpu->state.xsave.header.xcomp_bv)
> > -				err = -EINVAL;
> > +			if (!err)
> > +				err = validate_xstate_header(&fpu->state.xsave.header);
> >  		}
> >  
> 
> Sorry, this is the buggy part.  The problem is that this code runs even if XSAVE
> isn't being used --- and in that case the state size is 512 bytes or less, so
> the state doesn't actually include the xstate_header.  So
> validate_xstate_header() was reading out of bounds and seeing invalid values.
> 
> So I think we need to check use_xsave() here, but it really needs to be in the
> earlier patch which added the check for just ->xcomp_bv ("x86/fpu: Don't let
> userspace set bogus xcomp_bv"), not in this one.
> 
> As far the split of patch 2/3 into these 10 patches, it looks fine (though it
> suddenly became a *lot* of patches!).  One nit: the subject of this one really
> should say "__fpu__restore_sig()", not "sanitize_restored_xstate()".
> 
> I can send a fixed series when I have a chance.

Could you please just send the delta patch against the whole tree to fix the bug? 
I'll worry about the patch dependencies and back-merge it to the proper place.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 03/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in sanitize_restored_xstate()
  2017-09-24 19:02     ` Ingo Molnar
@ 2017-09-24 20:08       ` Eric Biggers
  2017-09-25  6:07         ` Ingo Molnar
  2017-09-25  6:14         ` Ingo Molnar
  0 siblings, 2 replies; 33+ messages in thread
From: Eric Biggers @ 2017-09-24 20:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Andrew Morton, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu

On Sun, Sep 24, 2017 at 09:02:42PM +0200, Ingo Molnar wrote:
> 
> * Eric Biggers <ebiggers3@gmail.com> wrote:
> 
> > On Sun, Sep 24, 2017 at 12:59:06PM +0200, Ingo Molnar wrote:
> > > @@ -328,10 +331,8 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
> > >  			err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
> > >  		} else {
> > >  			err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
> > > -
> > > -			/* xcomp_bv must be 0 when using uncompacted format */
> > > -			if (!err && fpu->state.xsave.header.xcomp_bv)
> > > -				err = -EINVAL;
> > > +			if (!err)
> > > +				err = validate_xstate_header(&fpu->state.xsave.header);
> > >  		}
> > >  
> > 
> > Sorry, this is the buggy part.  The problem is that this code runs even if XSAVE
> > isn't being used --- and in that case the state size is 512 bytes or less, so
> > the state doesn't actually include the xstate_header.  So
> > validate_xstate_header() was reading out of bounds and seeing invalid values.
> > 
> > So I think we need to check use_xsave() here, but it really needs to be in the
> > earlier patch which added the check for just ->xcomp_bv ("x86/fpu: Don't let
> > userspace set bogus xcomp_bv"), not in this one.
> > 
> > As far the split of patch 2/3 into these 10 patches, it looks fine (though it
> > suddenly became a *lot* of patches!).  One nit: the subject of this one really
> > should say "__fpu__restore_sig()", not "sanitize_restored_xstate()".
> > 
> > I can send a fixed series when I have a chance.
> 
> Could you please just send the delta patch against the whole tree to fix the bug? 
> I'll worry about the patch dependencies and back-merge it to the proper place.
> 

The following diff against tip/master fixes the bug.  Note: we *could* check
'use_xsave()' instead of 'state_size > offsetof(struct xregs_state, header)',
but that might be confusing in the case where we couldn't find the xstate
information in the memory layout and only copy the fxregs_state, since then we'd
actually be validating the xsave_header which was already there, which shouldn't
ever fail.

diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index afe54247cf27..fb639e70048f 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -331,7 +331,8 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 			err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
 		} else {
 			err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
-			if (!err)
+
+			if (!err && state_size > offsetof(struct xregs_state, header))
 				err = validate_xstate_header(&fpu->state.xsave.header);
 		}

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [PATCH 03/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in sanitize_restored_xstate()
  2017-09-24 20:08       ` Eric Biggers
@ 2017-09-25  6:07         ` Ingo Molnar
  2017-09-25  6:14         ` Ingo Molnar
  1 sibling, 0 replies; 33+ messages in thread
From: Ingo Molnar @ 2017-09-25  6:07 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-kernel, Andrew Morton, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu


* Eric Biggers <ebiggers3@gmail.com> wrote:

> The following diff against tip/master fixes the bug.  Note: we *could* check
> 'use_xsave()' instead of 'state_size > offsetof(struct xregs_state, header)',
> but that might be confusing in the case where we couldn't find the xstate
> information in the memory layout and only copy the fxregs_state, since then we'd
> actually be validating the xsave_header which was already there, which shouldn't
> ever fail.
> 
> diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
> index afe54247cf27..fb639e70048f 100644
> --- a/arch/x86/kernel/fpu/signal.c
> +++ b/arch/x86/kernel/fpu/signal.c
> @@ -331,7 +331,8 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
>  			err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
>  		} else {
>  			err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
> -			if (!err)
> +
> +			if (!err && state_size > offsetof(struct xregs_state, header))
>  				err = validate_xstate_header(&fpu->state.xsave.header);
>  		}

Yeah, I agree that checking 'state_size' is cleaner although note that technically 
this check isn't enough, because if 'state_size' is pointing somewhere inside the 
header (i.e. does not fully include it), the code still attempts a bad memcpy().

But that cannot happen, due to how state_size is set up:

        int state_size = fpu_kernel_xstate_size;

	...

                        state_size = sizeof(struct fxregs_state);
		...
                } else {
		...
                        state_size = fx_sw_user.xstate_size;
		...

and because fx_sw_user.xstate_size has to be at least:

        int min_xstate_size = sizeof(struct fxregs_state) +
                              sizeof(struct xstate_header);

i.e. the 'state_size' variable has a discrete set of possible values, none of 
which values point inside the header. Something to keep in mind ...


Note that there's some room for improvement within both the signal and the regset 
copying of FPU state. We have this pattern:

                if (using_compacted_format()) {
                        err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
                } else {
                        err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
                        if (!err)
                                err = validate_xstate_header(&fpu->state.xsave.header);
                }

... and copy_user_to_xstate() does:

        if (__copy_from_user(&hdr, ubuf + offset, size))
                return -EFAULT;

        if (validate_xstate_header(&hdr))
                return -EINVAL;

I.e. what we probably want is a helper function that just copies the darn thing 
and validates everything.

Note how regset.c duplicates a similar pattern:

        if (using_compacted_format()) {
                if (kbuf)
                        ret = copy_kernel_to_xstate(xsave, kbuf);
                else
                        ret = copy_user_to_xstate(xsave, ubuf);
        } else {
                ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0, -1);
                if (!ret)
                        ret = validate_xstate_header(&xsave->header);
        }


I.e. what we should probably do is to push the using_compacted_format() check into 
copy_user_to_xstate(). That makes copy_user_to_xstate() a high level method that 
can deal with all formats and which does all verification.

But that's a separate cleanup.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 03/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in sanitize_restored_xstate()
  2017-09-24 20:08       ` Eric Biggers
  2017-09-25  6:07         ` Ingo Molnar
@ 2017-09-25  6:14         ` Ingo Molnar
  2017-09-25  7:20           ` Eric Biggers
  1 sibling, 1 reply; 33+ messages in thread
From: Ingo Molnar @ 2017-09-25  6:14 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-kernel, Andrew Morton, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu


* Eric Biggers <ebiggers3@gmail.com> wrote:

> On Sun, Sep 24, 2017 at 09:02:42PM +0200, Ingo Molnar wrote:
> > 
> > * Eric Biggers <ebiggers3@gmail.com> wrote:
> > 
> > > On Sun, Sep 24, 2017 at 12:59:06PM +0200, Ingo Molnar wrote:
> > > > @@ -328,10 +331,8 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
> > > >  			err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
> > > >  		} else {
> > > >  			err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
> > > > -
> > > > -			/* xcomp_bv must be 0 when using uncompacted format */
> > > > -			if (!err && fpu->state.xsave.header.xcomp_bv)
> > > > -				err = -EINVAL;
> > > > +			if (!err)
> > > > +				err = validate_xstate_header(&fpu->state.xsave.header);
> > > >  		}
> > > >  
> > > 
> > > Sorry, this is the buggy part.  The problem is that this code runs even if XSAVE
> > > isn't being used --- and in that case the state size is 512 bytes or less, so
> > > the state doesn't actually include the xstate_header.  So
> > > validate_xstate_header() was reading out of bounds and seeing invalid values.
> > > 
> > > So I think we need to check use_xsave() here, but it really needs to be in the
> > > earlier patch which added the check for just ->xcomp_bv ("x86/fpu: Don't let
> > > userspace set bogus xcomp_bv"), not in this one.
> > > 
> > > As far the split of patch 2/3 into these 10 patches, it looks fine (though it
> > > suddenly became a *lot* of patches!).  One nit: the subject of this one really
> > > should say "__fpu__restore_sig()", not "sanitize_restored_xstate()".
> > > 
> > > I can send a fixed series when I have a chance.
> > 
> > Could you please just send the delta patch against the whole tree to fix the bug? 
> > I'll worry about the patch dependencies and back-merge it to the proper place.
> > 
> 
> The following diff against tip/master fixes the bug.  Note: we *could* check
> 'use_xsave()' instead of 'state_size > offsetof(struct xregs_state, header)',
> but that might be confusing in the case where we couldn't find the xstate
> information in the memory layout and only copy the fxregs_state, since then we'd
> actually be validating the xsave_header which was already there, which shouldn't
> ever fail.
> 
> diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
> index afe54247cf27..fb639e70048f 100644
> --- a/arch/x86/kernel/fpu/signal.c
> +++ b/arch/x86/kernel/fpu/signal.c
> @@ -331,7 +331,8 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
>  			err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
>  		} else {
>  			err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
> -			if (!err)
> +
> +			if (!err && state_size > offsetof(struct xregs_state, header))
>  				err = validate_xstate_header(&fpu->state.xsave.header);
>  		}

I.e. a better check would be to check that the whole header can be accessed:

	state_size >= offsetof(struct xregs_state, header) + sizeof(struct xstate_header)

Not that there should ever be a 'state_size' that points inside the header - so in 
the end I back-merged your original (and tested ...) version.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 03/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in sanitize_restored_xstate()
  2017-09-25  6:14         ` Ingo Molnar
@ 2017-09-25  7:20           ` Eric Biggers
  2017-09-25  7:30             ` Ingo Molnar
  0 siblings, 1 reply; 33+ messages in thread
From: Eric Biggers @ 2017-09-25  7:20 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Andrew Morton, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu

On Mon, Sep 25, 2017 at 08:14:45AM +0200, Ingo Molnar wrote:
> > > 
> > > Could you please just send the delta patch against the whole tree to fix the bug? 
> > > I'll worry about the patch dependencies and back-merge it to the proper place.
> > > 
> > 
> > The following diff against tip/master fixes the bug.  Note: we *could* check
> > 'use_xsave()' instead of 'state_size > offsetof(struct xregs_state, header)',
> > but that might be confusing in the case where we couldn't find the xstate
> > information in the memory layout and only copy the fxregs_state, since then we'd
> > actually be validating the xsave_header which was already there, which shouldn't
> > ever fail.
> > 
> > diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
> > index afe54247cf27..fb639e70048f 100644
> > --- a/arch/x86/kernel/fpu/signal.c
> > +++ b/arch/x86/kernel/fpu/signal.c
> > @@ -331,7 +331,8 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
> >  			err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
> >  		} else {
> >  			err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
> > -			if (!err)
> > +
> > +			if (!err && state_size > offsetof(struct xregs_state, header))
> >  				err = validate_xstate_header(&fpu->state.xsave.header);
> >  		}
> 
> I.e. a better check would be to check that the whole header can be accessed:
> 
> 	state_size >= offsetof(struct xregs_state, header) + sizeof(struct xstate_header)
> 
> Not that there should ever be a 'state_size' that points inside the header - so in 
> the end I back-merged your original (and tested ...) version.
> 

Well, actually we'd need to validate the header if userspace overwrote any part
of it.

But more importantly, I think the state_size check needs to go into the first
patch (the one that's Cc'ed to stable as it fixes the real bug), since
->xcomp_bv is part of the xstate_header.  So *before* we switch to
validate_xstate_header() in this patch, the code should already be:

		if (using_compacted_format()) {
			err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
		} else {
			err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);

			/* xcomp_bv must be 0 when using uncompacted format */
			if (!err &&
			    state_size > offsetof(struct xregs_state, header) &&
			    fpu->state.xsave.header.xcomp_bv)
				err = -EINVAL;
		}

Also can you please fix the commit title and message of this patch?  It should
say "__fpu__restore_sig()", not "sanitize_restored_xstate()".

Thanks,

Eric

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 03/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in sanitize_restored_xstate()
  2017-09-25  7:20           ` Eric Biggers
@ 2017-09-25  7:30             ` Ingo Molnar
  0 siblings, 0 replies; 33+ messages in thread
From: Ingo Molnar @ 2017-09-25  7:30 UTC (permalink / raw)
  To: Eric Biggers
  Cc: linux-kernel, Andrew Morton, Andy Lutomirski, Borislav Petkov,
	Dave Hansen, Fenghua Yu, H . Peter Anvin, Linus Torvalds,
	Oleg Nesterov, Peter Zijlstra, Rik van Riel, Thomas Gleixner,
	Yu-cheng Yu


* Eric Biggers <ebiggers3@gmail.com> wrote:

> On Mon, Sep 25, 2017 at 08:14:45AM +0200, Ingo Molnar wrote:
> > > > 
> > > > Could you please just send the delta patch against the whole tree to fix the bug? 
> > > > I'll worry about the patch dependencies and back-merge it to the proper place.
> > > > 
> > > 
> > > The following diff against tip/master fixes the bug.  Note: we *could* check
> > > 'use_xsave()' instead of 'state_size > offsetof(struct xregs_state, header)',
> > > but that might be confusing in the case where we couldn't find the xstate
> > > information in the memory layout and only copy the fxregs_state, since then we'd
> > > actually be validating the xsave_header which was already there, which shouldn't
> > > ever fail.
> > > 
> > > diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
> > > index afe54247cf27..fb639e70048f 100644
> > > --- a/arch/x86/kernel/fpu/signal.c
> > > +++ b/arch/x86/kernel/fpu/signal.c
> > > @@ -331,7 +331,8 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
> > >  			err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
> > >  		} else {
> > >  			err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
> > > -			if (!err)
> > > +
> > > +			if (!err && state_size > offsetof(struct xregs_state, header))
> > >  				err = validate_xstate_header(&fpu->state.xsave.header);
> > >  		}
> > 
> > I.e. a better check would be to check that the whole header can be accessed:
> > 
> > 	state_size >= offsetof(struct xregs_state, header) + sizeof(struct xstate_header)
> > 
> > Not that there should ever be a 'state_size' that points inside the header - so in 
> > the end I back-merged your original (and tested ...) version.
> > 
> 
> Well, actually we'd need to validate the header if userspace overwrote any part
> of it.
> 
> But more importantly, I think the state_size check needs to go into the first
> patch (the one that's Cc'ed to stable as it fixes the real bug), since
> ->xcomp_bv is part of the xstate_header.  So *before* we switch to
> validate_xstate_header() in this patch, the code should already be:
> 
> 		if (using_compacted_format()) {
> 			err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
> 		} else {
> 			err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
> 
> 			/* xcomp_bv must be 0 when using uncompacted format */
> 			if (!err &&
> 			    state_size > offsetof(struct xregs_state, header) &&
> 			    fpu->state.xsave.header.xcomp_bv)
> 				err = -EINVAL;
> 		}

Note that I think the whole series is more robust if it goes to -stable as-is, as 
the ABI aspect should not be underestimated either. Nevertheless I've backmerged 
the fix further to the original commit, to maintain bisectability.

> Also can you please fix the commit title and message of this patch?  It should
> say "__fpu__restore_sig()", not "sanitize_restored_xstate()".

Indeed - and I fixed that too. I have pushed out the latest tip:WIP.x86/fpu - no 
change in the end result tree, but different inner structure.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [tip:x86/fpu] x86/fpu: Introduce validate_xstate_header()
  2017-09-24 10:59 ` [PATCH 01/10] x86/fpu: Introduce validate_xstate_header() Ingo Molnar
@ 2017-09-26  8:34   ` tip-bot for Eric Biggers
  0 siblings, 0 replies; 33+ messages in thread
From: tip-bot for Eric Biggers @ 2017-09-26  8:34 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mhalcrow, fenghua.yu, riel, dave.hansen, peterz, ebiggers,
	wanpeng.li, dvyukov, akpm, mingo, hpa, oleg, torvalds, luto,
	linux-kernel, keescook, luto, bp, yu-cheng.yu, haokexin, tglx,
	ebiggers3

Commit-ID:  e63e5d5c15c6b1dba26f7cbd1b1089a1d6155db5
Gitweb:     http://git.kernel.org/tip/e63e5d5c15c6b1dba26f7cbd1b1089a1d6155db5
Author:     Eric Biggers <ebiggers@google.com>
AuthorDate: Sun, 24 Sep 2017 12:59:04 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 26 Sep 2017 09:43:45 +0200

x86/fpu: Introduce validate_xstate_header()

Move validation of user-supplied xstate_header into a helper function,
in preparation of calling it from both the ptrace and sigreturn syscall
paths.

The new function also considers it to be an error if *any* reserved bits
are set, whereas before we were just clearing most of them silently.

This should reduce the chance of bugs that fail to correctly validate
user-supplied XSAVE areas.  It also will expose any broken userspace
programs that set the other reserved bits; this is desirable because
such programs will lose compatibility with future CPUs and kernels if
those bits are ever used for anything.  (There shouldn't be any such
programs, and in fact in the case where the compacted format is in use
we were already validating xfeatures.  But you never know...)

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Link: http://lkml.kernel.org/r/20170924105913.9157-2-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/fpu/xstate.h |  4 ++++
 arch/x86/kernel/fpu/xstate.c      | 24 ++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 579ac23..83fee24 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -52,4 +52,8 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int of
 int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned int offset, unsigned int size);
 int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf);
 int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf);
+
+/* Validate an xstate header supplied by userspace (ptrace or sigreturn) */
+extern int validate_xstate_header(const struct xstate_header *hdr);
+
 #endif
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 703e76d..2427aee 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -483,6 +483,30 @@ int using_compacted_format(void)
 	return boot_cpu_has(X86_FEATURE_XSAVES);
 }
 
+/* Validate an xstate header supplied by userspace (ptrace or sigreturn) */
+int validate_xstate_header(const struct xstate_header *hdr)
+{
+	/* No unknown or supervisor features may be set */
+	if (hdr->xfeatures & (~xfeatures_mask | XFEATURE_MASK_SUPERVISOR))
+		return -EINVAL;
+
+	/* Userspace must use the uncompacted format */
+	if (hdr->xcomp_bv)
+		return -EINVAL;
+
+	/*
+	 * If 'reserved' is shrunken to add a new field, make sure to validate
+	 * that new field here!
+	 */
+	BUILD_BUG_ON(sizeof(hdr->reserved) != 48);
+
+	/* No reserved bits may be set */
+	if (memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
+		return -EINVAL;
+
+	return 0;
+}
+
 static void __xstate_dump_leaves(void)
 {
 	int i;

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [tip:x86/fpu] x86/fpu: Use validate_xstate_header() to validate the xstate_header in xstateregs_set()
  2017-09-24 10:59 ` [PATCH 02/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in xstateregs_set() Ingo Molnar
@ 2017-09-26  8:34   ` tip-bot for Eric Biggers
  0 siblings, 0 replies; 33+ messages in thread
From: tip-bot for Eric Biggers @ 2017-09-26  8:34 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: bp, ebiggers3, akpm, dave.hansen, dvyukov, tglx, ebiggers,
	fenghua.yu, mhalcrow, haokexin, wanpeng.li, linux-kernel,
	torvalds, luto, oleg, hpa, keescook, mingo, yu-cheng.yu, riel,
	peterz, luto

Commit-ID:  cf9df81b139b6ebaec188d73758f02ca3b2110e4
Gitweb:     http://git.kernel.org/tip/cf9df81b139b6ebaec188d73758f02ca3b2110e4
Author:     Eric Biggers <ebiggers@google.com>
AuthorDate: Sun, 24 Sep 2017 12:59:05 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 26 Sep 2017 09:43:45 +0200

x86/fpu: Use validate_xstate_header() to validate the xstate_header in xstateregs_set()

Tighten the checks in xstateregs_set().

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Link: http://lkml.kernel.org/r/20170924105913.9157-3-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/regset.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index ee8d2f0..b831d5b 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -141,27 +141,20 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
 			ret = copy_user_to_xstate(xsave, ubuf);
 	} else {
 		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0, -1);
-
-		/* xcomp_bv must be 0 when using uncompacted format */
-		if (!ret && xsave->header.xcomp_bv)
-			ret = -EINVAL;
+		if (!ret)
+			ret = validate_xstate_header(&xsave->header);
 	}
 
 	/*
-	 * In case of failure, mark all states as init:
-	 */
-	if (ret)
-		fpstate_init(&fpu->state);
-
-	/*
 	 * mxcsr reserved bits must be masked to zero for security reasons.
 	 */
 	xsave->i387.mxcsr &= mxcsr_feature_mask;
-	xsave->header.xfeatures &= xfeatures_mask;
+
 	/*
-	 * These bits must be zero.
+	 * In case of failure, mark all states as init:
 	 */
-	memset(&xsave->header.reserved, 0, 48);
+	if (ret)
+		fpstate_init(&fpu->state);
 
 	return ret;
 }

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [tip:x86/fpu] x86/fpu: Use validate_xstate_header() to validate the xstate_header in __fpu__restore_sig()
  2017-09-24 10:59 ` [PATCH 03/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in sanitize_restored_xstate() Ingo Molnar
  2017-09-24 18:51   ` Eric Biggers
@ 2017-09-26  8:35   ` tip-bot for Eric Biggers
  1 sibling, 0 replies; 33+ messages in thread
From: tip-bot for Eric Biggers @ 2017-09-26  8:35 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, dvyukov, mingo, oleg, riel, yu-cheng.yu, torvalds, hpa, bp,
	peterz, keescook, linux-kernel, haokexin, dave.hansen,
	wanpeng.li, fenghua.yu, mhalcrow, akpm, luto, ebiggers, luto,
	ebiggers3

Commit-ID:  b11e2e18a7fc8eaa3d592c260d50c7129e094ded
Gitweb:     http://git.kernel.org/tip/b11e2e18a7fc8eaa3d592c260d50c7129e094ded
Author:     Eric Biggers <ebiggers@google.com>
AuthorDate: Sun, 24 Sep 2017 12:59:06 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 26 Sep 2017 09:43:46 +0200

x86/fpu: Use validate_xstate_header() to validate the xstate_header in __fpu__restore_sig()

Tighten the checks in __fpu__restore_sig() and update comments.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Link: http://lkml.kernel.org/r/20170924105913.9157-4-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/signal.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 7fa3bdb..fb639e7 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -214,8 +214,11 @@ sanitize_restored_xstate(struct task_struct *tsk,
 	struct xstate_header *header = &xsave->header;
 
 	if (use_xsave()) {
-		/* These bits must be zero. */
-		memset(header->reserved, 0, 48);
+		/*
+		 * Note: we don't need to zero the reserved bits in the
+		 * xstate_header here because we either didn't copy them at all,
+		 * or we checked earlier that they aren't set.
+		 */
 
 		/*
 		 * Init the state that is not present in the memory
@@ -224,7 +227,7 @@ sanitize_restored_xstate(struct task_struct *tsk,
 		if (fx_only)
 			header->xfeatures = XFEATURE_MASK_FPSSE;
 		else
-			header->xfeatures &= (xfeatures_mask & xfeatures);
+			header->xfeatures &= xfeatures;
 	}
 
 	if (use_fxsr()) {
@@ -308,7 +311,7 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 		/*
 		 * For 32-bit frames with fxstate, copy the user state to the
 		 * thread's fpu state, reconstruct fxstate from the fsave
-		 * header. Sanitize the copied state etc.
+		 * header. Validate and sanitize the copied state.
 		 */
 		struct fpu *fpu = &tsk->thread.fpu;
 		struct user_i387_ia32_struct env;
@@ -329,9 +332,8 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 		} else {
 			err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
 
-			/* xcomp_bv must be 0 when using uncompacted format */
-			if (!err && state_size > offsetof(struct xregs_state, header) && fpu->state.xsave.header.xcomp_bv)
-				err = -EINVAL;
+			if (!err && state_size > offsetof(struct xregs_state, header))
+				err = validate_xstate_header(&fpu->state.xsave.header);
 		}
 
 		if (err || __copy_from_user(&env, buf, sizeof(env))) {

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [tip:x86/fpu] x86/fpu: Copy the full state_header in copy_kernel_to_xstate()
  2017-09-24 10:59 ` [PATCH 04/10] x86/fpu: Copy the full state_header in copy_kernel_to_xstate() Ingo Molnar
@ 2017-09-26  8:35   ` tip-bot for Eric Biggers
  0 siblings, 0 replies; 33+ messages in thread
From: tip-bot for Eric Biggers @ 2017-09-26  8:35 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: luto, dvyukov, tglx, torvalds, hpa, bp, akpm, oleg, linux-kernel,
	dave.hansen, haokexin, keescook, mingo, yu-cheng.yu, ebiggers,
	mhalcrow, peterz, fenghua.yu, luto, ebiggers3, wanpeng.li, riel

Commit-ID:  80d8ae86b36791a545ca28ddc95133ea59bba6e0
Gitweb:     http://git.kernel.org/tip/80d8ae86b36791a545ca28ddc95133ea59bba6e0
Author:     Eric Biggers <ebiggers@google.com>
AuthorDate: Sun, 24 Sep 2017 12:59:07 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 26 Sep 2017 09:43:46 +0200

x86/fpu: Copy the full state_header in copy_kernel_to_xstate()

This is in preparation to verify the full xstate header as supplied by user-space.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Link: http://lkml.kernel.org/r/20170924105913.9157-5-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/xstate.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 2427aee..02591b96 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1148,11 +1148,13 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 	int i;
 	u64 xfeatures;
 	u64 allowed_features;
+	struct xstate_header hdr;
 
 	offset = offsetof(struct xregs_state, header);
-	size = sizeof(xfeatures);
+	size = sizeof(hdr);
 
-	memcpy(&xfeatures, kbuf + offset, size);
+	memcpy(&hdr, kbuf + offset, size);
+	xfeatures = hdr.xfeatures;
 
 	/*
 	 * Reject if the user sets any disabled or supervisor features:

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [tip:x86/fpu] x86/fpu: Eliminate the 'xfeatures' local variable in copy_kernel_to_xstate()
  2017-09-24 10:59 ` [PATCH 05/10] x86/fpu: Eliminate the 'xfeatures' local variable " Ingo Molnar
@ 2017-09-26  8:35   ` tip-bot for Eric Biggers
  0 siblings, 0 replies; 33+ messages in thread
From: tip-bot for Eric Biggers @ 2017-09-26  8:35 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, luto, dvyukov, luto, mingo, linux-kernel, yu-cheng.yu,
	keescook, ebiggers3, fenghua.yu, riel, dave.hansen, oleg,
	mhalcrow, haokexin, akpm, tglx, bp, wanpeng.li, peterz, ebiggers,
	torvalds

Commit-ID:  b89eda482d7849a1c146b6d0a42f4e76369bb08e
Gitweb:     http://git.kernel.org/tip/b89eda482d7849a1c146b6d0a42f4e76369bb08e
Author:     Eric Biggers <ebiggers@google.com>
AuthorDate: Sun, 24 Sep 2017 12:59:08 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 26 Sep 2017 09:43:46 +0200

x86/fpu: Eliminate the 'xfeatures' local variable in copy_kernel_to_xstate()

We have this information in the xstate_header.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Link: http://lkml.kernel.org/r/20170924105913.9157-6-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/xstate.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 02591b96..c97c4a9 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1146,7 +1146,6 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 {
 	unsigned int offset, size;
 	int i;
-	u64 xfeatures;
 	u64 allowed_features;
 	struct xstate_header hdr;
 
@@ -1154,20 +1153,19 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 	size = sizeof(hdr);
 
 	memcpy(&hdr, kbuf + offset, size);
-	xfeatures = hdr.xfeatures;
 
 	/*
 	 * Reject if the user sets any disabled or supervisor features:
 	 */
 	allowed_features = xfeatures_mask & ~XFEATURE_MASK_SUPERVISOR;
 
-	if (xfeatures & ~allowed_features)
+	if (hdr.xfeatures & ~allowed_features)
 		return -EINVAL;
 
 	for (i = 0; i < XFEATURE_MAX; i++) {
 		u64 mask = ((u64)1 << i);
 
-		if (xfeatures & mask) {
+		if (hdr.xfeatures & mask) {
 			void *dst = __raw_xsave_addr(xsave, 1 << i);
 
 			offset = xstate_offsets[i];
@@ -1177,7 +1175,7 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 		}
 	}
 
-	if (xfeatures_mxcsr_quirk(xfeatures)) {
+	if (xfeatures_mxcsr_quirk(hdr.xfeatures)) {
 		offset = offsetof(struct fxregs_state, mxcsr);
 		size = MXCSR_AND_FLAGS_SIZE;
 		memcpy(&xsave->i387.mxcsr, kbuf + offset, size);
@@ -1192,7 +1190,7 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 	/*
 	 * Add back in the features that came in from userspace:
 	 */
-	xsave->header.xfeatures |= xfeatures;
+	xsave->header.xfeatures |= hdr.xfeatures;
 
 	return 0;
 }

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [tip:x86/fpu] x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_kernel_to_xstate()
  2017-09-24 10:59 ` [PATCH 06/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header " Ingo Molnar
@ 2017-09-26  8:36   ` tip-bot for Eric Biggers
  0 siblings, 0 replies; 33+ messages in thread
From: tip-bot for Eric Biggers @ 2017-09-26  8:36 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, oleg, ebiggers3, keescook, luto, yu-cheng.yu, hpa,
	dave.hansen, haokexin, peterz, akpm, mhalcrow, ebiggers,
	fenghua.yu, torvalds, wanpeng.li, luto, dvyukov, mingo,
	linux-kernel, bp, riel

Commit-ID:  af95774b3ca080b0e1e651c0fc7680f3444ddda7
Gitweb:     http://git.kernel.org/tip/af95774b3ca080b0e1e651c0fc7680f3444ddda7
Author:     Eric Biggers <ebiggers@google.com>
AuthorDate: Sun, 24 Sep 2017 12:59:09 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 26 Sep 2017 09:43:47 +0200

x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_kernel_to_xstate()

Tighten the checks in copy_kernel_to_xstate().

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Link: http://lkml.kernel.org/r/20170924105913.9157-7-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/xstate.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index c97c4a9..325db78 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1138,15 +1138,12 @@ int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned i
 
 /*
  * Convert from a ptrace standard-format kernel buffer to kernel XSAVES format
- * and copy to the target thread. This is called from xstateregs_set() and
- * there we check the CPU has XSAVES and a whole standard-sized buffer
- * exists.
+ * and copy to the target thread. This is called from xstateregs_set().
  */
 int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 {
 	unsigned int offset, size;
 	int i;
-	u64 allowed_features;
 	struct xstate_header hdr;
 
 	offset = offsetof(struct xregs_state, header);
@@ -1154,12 +1151,7 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 
 	memcpy(&hdr, kbuf + offset, size);
 
-	/*
-	 * Reject if the user sets any disabled or supervisor features:
-	 */
-	allowed_features = xfeatures_mask & ~XFEATURE_MASK_SUPERVISOR;
-
-	if (hdr.xfeatures & ~allowed_features)
+	if (validate_xstate_header(&hdr))
 		return -EINVAL;
 
 	for (i = 0; i < XFEATURE_MAX; i++) {

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [tip:x86/fpu] x86/fpu: Copy the full header in copy_user_to_xstate()
  2017-09-24 10:59 ` [PATCH 07/10] x86/fpu: Copy the full header in copy_user_to_xstate() Ingo Molnar
@ 2017-09-26  8:36   ` tip-bot for Eric Biggers
  0 siblings, 0 replies; 33+ messages in thread
From: tip-bot for Eric Biggers @ 2017-09-26  8:36 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, luto, hpa, mingo, oleg, peterz, wanpeng.li, keescook, bp,
	haokexin, ebiggers, torvalds, luto, riel, dave.hansen,
	fenghua.yu, akpm, ebiggers3, yu-cheng.yu, linux-kernel, dvyukov,
	mhalcrow

Commit-ID:  af2c4322d986a08a6e793b74b83a62b325019c20
Gitweb:     http://git.kernel.org/tip/af2c4322d986a08a6e793b74b83a62b325019c20
Author:     Eric Biggers <ebiggers@google.com>
AuthorDate: Sun, 24 Sep 2017 12:59:10 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 26 Sep 2017 09:43:47 +0200

x86/fpu: Copy the full header in copy_user_to_xstate()

This is in preparation to verify the full xstate header as supplied by user-space.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Link: http://lkml.kernel.org/r/20170924105913.9157-8-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/xstate.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 325db78..0cd7b73 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1199,13 +1199,16 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 	int i;
 	u64 xfeatures;
 	u64 allowed_features;
+	struct xstate_header hdr;
 
 	offset = offsetof(struct xregs_state, header);
-	size = sizeof(xfeatures);
+	size = sizeof(hdr);
 
-	if (__copy_from_user(&xfeatures, ubuf + offset, size))
+	if (__copy_from_user(&hdr, ubuf + offset, size))
 		return -EFAULT;
 
+	xfeatures = hdr.xfeatures;
+
 	/*
 	 * Reject if the user sets any disabled or supervisor features:
 	 */

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [tip:x86/fpu] x86/fpu: Eliminate the 'xfeatures' local variable in copy_user_to_xstate()
  2017-09-24 10:59 ` [PATCH 08/10] x86/fpu: Eliminate the 'xfeatures' local variable " Ingo Molnar
@ 2017-09-26  8:37   ` tip-bot for Eric Biggers
  0 siblings, 0 replies; 33+ messages in thread
From: tip-bot for Eric Biggers @ 2017-09-26  8:37 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: luto, torvalds, yu-cheng.yu, linux-kernel, haokexin, dvyukov,
	luto, wanpeng.li, dave.hansen, ebiggers, akpm, bp, fenghua.yu,
	keescook, peterz, ebiggers3, tglx, riel, mhalcrow, oleg, hpa,
	mingo

Commit-ID:  3d703477bcfe8bb57079d97198cf1e342fe1fef9
Gitweb:     http://git.kernel.org/tip/3d703477bcfe8bb57079d97198cf1e342fe1fef9
Author:     Eric Biggers <ebiggers@google.com>
AuthorDate: Sun, 24 Sep 2017 12:59:11 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 26 Sep 2017 09:43:48 +0200

x86/fpu: Eliminate the 'xfeatures' local variable in copy_user_to_xstate()

We now have this field in hdr.xfeatures.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Link: http://lkml.kernel.org/r/20170924105913.9157-9-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/xstate.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 0cd7b73..b6d78b7 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1197,7 +1197,6 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 {
 	unsigned int offset, size;
 	int i;
-	u64 xfeatures;
 	u64 allowed_features;
 	struct xstate_header hdr;
 
@@ -1207,20 +1206,18 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 	if (__copy_from_user(&hdr, ubuf + offset, size))
 		return -EFAULT;
 
-	xfeatures = hdr.xfeatures;
-
 	/*
 	 * Reject if the user sets any disabled or supervisor features:
 	 */
 	allowed_features = xfeatures_mask & ~XFEATURE_MASK_SUPERVISOR;
 
-	if (xfeatures & ~allowed_features)
+	if (hdr.xfeatures & ~allowed_features)
 		return -EINVAL;
 
 	for (i = 0; i < XFEATURE_MAX; i++) {
 		u64 mask = ((u64)1 << i);
 
-		if (xfeatures & mask) {
+		if (hdr.xfeatures & mask) {
 			void *dst = __raw_xsave_addr(xsave, 1 << i);
 
 			offset = xstate_offsets[i];
@@ -1231,7 +1228,7 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 		}
 	}
 
-	if (xfeatures_mxcsr_quirk(xfeatures)) {
+	if (xfeatures_mxcsr_quirk(hdr.xfeatures)) {
 		offset = offsetof(struct fxregs_state, mxcsr);
 		size = MXCSR_AND_FLAGS_SIZE;
 		if (__copy_from_user(&xsave->i387.mxcsr, ubuf + offset, size))
@@ -1247,7 +1244,7 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 	/*
 	 * Add back in the features that came in from userspace:
 	 */
-	xsave->header.xfeatures |= xfeatures;
+	xsave->header.xfeatures |= hdr.xfeatures;
 
 	return 0;
 }

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [tip:x86/fpu] x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_user_to_xstate()
  2017-09-24 10:59 ` [PATCH 09/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header " Ingo Molnar
@ 2017-09-26  8:37   ` tip-bot for Eric Biggers
  0 siblings, 0 replies; 33+ messages in thread
From: tip-bot for Eric Biggers @ 2017-09-26  8:37 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, haokexin, peterz, linux-kernel, riel, keescook, ebiggers3,
	mhalcrow, fenghua.yu, luto, yu-cheng.yu, luto, ebiggers, tglx,
	oleg, dvyukov, hpa, dave.hansen, wanpeng.li, bp, torvalds, akpm

Commit-ID:  98c0fad9d60e8b2cd47e15b7bee7df343648f5bb
Gitweb:     http://git.kernel.org/tip/98c0fad9d60e8b2cd47e15b7bee7df343648f5bb
Author:     Eric Biggers <ebiggers@google.com>
AuthorDate: Sun, 24 Sep 2017 12:59:12 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 26 Sep 2017 09:43:48 +0200

x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_user_to_xstate()

Tighten the checks in copy_user_to_xstate().

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Link: http://lkml.kernel.org/r/20170924105913.9157-10-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/xstate.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index b6d78b7..f1d5476 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1188,16 +1188,15 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 }
 
 /*
- * Convert from a ptrace standard-format user-space buffer to kernel XSAVES format
- * and copy to the target thread. This is called from xstateregs_set() and
- * there we check the CPU has XSAVES and a whole standard-sized buffer
- * exists.
+ * Convert from a ptrace or sigreturn standard-format user-space buffer to
+ * kernel XSAVES format and copy to the target thread. This is called from
+ * xstateregs_set(), as well as potentially from the sigreturn() and
+ * rt_sigreturn() system calls.
  */
 int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 {
 	unsigned int offset, size;
 	int i;
-	u64 allowed_features;
 	struct xstate_header hdr;
 
 	offset = offsetof(struct xregs_state, header);
@@ -1206,12 +1205,7 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 	if (__copy_from_user(&hdr, ubuf + offset, size))
 		return -EFAULT;
 
-	/*
-	 * Reject if the user sets any disabled or supervisor features:
-	 */
-	allowed_features = xfeatures_mask & ~XFEATURE_MASK_SUPERVISOR;
-
-	if (hdr.xfeatures & ~allowed_features)
+	if (validate_xstate_header(&hdr))
 		return -EINVAL;
 
 	for (i = 0; i < XFEATURE_MAX; i++) {

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [tip:x86/fpu] x86/fpu: Use using_compacted_format() instead of open coded X86_FEATURE_XSAVES
  2017-09-24 10:59 ` [PATCH 10/10] x86/fpu: Use using_compacted_format() instead of open coded X86_FEATURE_XSAVES Ingo Molnar
@ 2017-09-26  8:37   ` tip-bot for Eric Biggers
  0 siblings, 0 replies; 33+ messages in thread
From: tip-bot for Eric Biggers @ 2017-09-26  8:37 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: dvyukov, keescook, haokexin, akpm, fenghua.yu, luto, bp, peterz,
	ebiggers3, dave.hansen, mhalcrow, torvalds, riel, ebiggers,
	linux-kernel, mingo, wanpeng.li, hpa, tglx, yu-cheng.yu, luto,
	oleg

Commit-ID:  738f48cb5fdd5878d11934f1898aa2bcf1578289
Gitweb:     http://git.kernel.org/tip/738f48cb5fdd5878d11934f1898aa2bcf1578289
Author:     Eric Biggers <ebiggers@google.com>
AuthorDate: Sun, 24 Sep 2017 12:59:13 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 26 Sep 2017 09:43:48 +0200

x86/fpu: Use using_compacted_format() instead of open coded X86_FEATURE_XSAVES

This is the canonical method to use.

Signed-off-by: Eric Biggers <ebiggers@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Eric Biggers <ebiggers3@gmail.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kevin Hao <haokexin@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michael Halcrow <mhalcrow@google.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpeng.li@hotmail.com>
Cc: Yu-cheng Yu <yu-cheng.yu@intel.com>
Cc: kernel-hardening@lists.openwall.com
Link: http://lkml.kernel.org/r/20170924105913.9157-11-mingo@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/fpu/regset.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index b831d5b..3ea1513 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -134,7 +134,7 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
 
 	fpu__prepare_write(fpu);
 
-	if (boot_cpu_has(X86_FEATURE_XSAVES)) {
+	if (using_compacted_format()) {
 		if (kbuf)
 			ret = copy_kernel_to_xstate(xsave, kbuf);
 		else

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [RFC GIT PULL] x86 FPU fixes and cleanups
  2017-09-24 19:01   ` Ingo Molnar
@ 2017-09-26 16:28     ` Ingo Molnar
  2017-09-26 18:17       ` Linus Torvalds
  0 siblings, 1 reply; 33+ messages in thread
From: Ingo Molnar @ 2017-09-26 16:28 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Linux Kernel Mailing List, Andrew Morton, Eric Biggers,
	Andy Lutomirski, Borislav Petkov, Dave Hansen, Fenghua Yu,
	H . Peter Anvin, Oleg Nesterov, Peter Zijlstra, Rik van Riel,
	Thomas Gleixner, Yu-cheng Yu


* Ingo Molnar <mingo@kernel.org> wrote:

> > I'm sure the bigger series is a better cleanup, but maybe we can wait with that 
> > bigger change for 4.15?
> 
> So all of this is pretty unfortunate timing, caused in part because I delayed the 
> FPU changes in this merge window due to having so many x86 changes already.
> 
> I'd really love to have the bigger series, firstly because beyond the 
> simplifications and the fixes in the 10-patch series it also fixes some other 
> problems such as this SkyLake bug:
> 
>   0852b374173b: x86/fpu: Add FPU state copying quirk to handle XRSTOR failure on Intel Skylake CPUs
> 
> Secondly, with v4.14 being an LTS, doing the cleanups/simplifications afterwards 
> adds a big backporting barrier.
> 
> But I can extract all the fixes and re-structure and re-test it all if you prefer 
> it that way.

So this an RFC pull request, you can pull the latest x86-fpu-for-linus git tree 
from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86-fpu-for-linus

   # HEAD: 8474c532b558fb6754c0ddff42a39ed6636671c9 Merge branch 'WIP.x86/fpu' into x86/fpu, because it's ready

This includes the recent XSAVE handling fixes from Eric Biggers (with all known 
bugs in it fixed), and the SkyLake XRSTOR handling fix from Rik van Riel, plus a 
number of cleanups/simplifications that addressed a number of more structural 
problems that enabled those bugs.

I have tested it on a range of x86 systems, new and old alike, and tested a number 
of corner cases as well - but as you can see it from the Git tree it got rebased 
recent to stay bisectable.

Due to v4.14 being an LTS release the restructuring would be a pretty big 
backporting barrier, but the tree is large and this was intended for v4.15 until 
the latest rounds of fixes showed up - so the timing is unfortunate and this pull 
request is late.

If you cannot take this then I'll extract the fixes and redo the tree and send you 
another pull request in a couple of days.

 Thanks,

	Ingo

------------------>
Andi Kleen (1):
      x86/fpu: Turn WARN_ON() in context switch into WARN_ON_FPU()

Eric Biggers (12):
      x86/fpu: Don't let userspace set bogus xcomp_bv
      x86/fpu: Reinitialize FPU registers if restoring FPU state fails
      x86/fpu: Introduce validate_xstate_header()
      x86/fpu: Use validate_xstate_header() to validate the xstate_header in xstateregs_set()
      x86/fpu: Use validate_xstate_header() to validate the xstate_header in __fpu__restore_sig()
      x86/fpu: Copy the full state_header in copy_kernel_to_xstate()
      x86/fpu: Eliminate the 'xfeatures' local variable in copy_kernel_to_xstate()
      x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_kernel_to_xstate()
      x86/fpu: Copy the full header in copy_user_to_xstate()
      x86/fpu: Eliminate the 'xfeatures' local variable in copy_user_to_xstate()
      x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_user_to_xstate()
      x86/fpu: Use using_compacted_format() instead of open coded X86_FEATURE_XSAVES

Ingo Molnar (27):
      x86/fpu: Rename copyin_to_xsaves()/copyout_from_xsaves() to copy_user_to_xstate()/copy_xstate_to_user()
      x86/fpu: Split copy_xstate_to_user() into copy_xstate_to_kernel() & copy_xstate_to_user()
      x86/fpu: Remove 'ubuf' parameter from the copy_xstate_to_kernel() APIs
      x86/fpu: Remove 'kbuf' parameter from the copy_xstate_to_user() APIs
      x86/fpu: Clean up parameter order in the copy_xstate_to_*() APIs
      x86/fpu: Clean up the parameter definitions of copy_xstate_to_*()
      x86/fpu: Remove the 'start_pos' parameter from the __copy_xstate_to_*() functions
      x86/fpu: Clarify parameter names in the copy_xstate_to_*() methods
      x86/fpu: Change 'size_total' parameter to unsigned and standardize the size checks in copy_xstate_to_*()
      x86/fpu: Simplify __copy_xstate_to_kernel() return values
      x86/fpu: Split copy_user_to_xstate() into copy_kernel_to_xstate() & copy_user_to_xstate()
      x86/fpu: Remove 'ubuf' parameter from the copy_kernel_to_xstate() API
      x86/fpu: Remove 'kbuf' parameter from the copy_user_to_xstate() API
      x86/fpu: Flip the parameter order in copy_*_to_xstate()
      x86/fpu: Simplify fpu->fpregs_active use
      x86/fpu: Make the fpu state change in fpu__clear() scheduler-atomic
      x86/fpu: Split the state handling in fpu__drop()
      x86/fpu: Change fpu->fpregs_active users to fpu->fpstate_active
      x86/fpu: Decouple fpregs_activate()/fpregs_deactivate() from fpu->fpregs_active
      x86/fpu: Remove struct fpu::fpregs_active
      x86/fpu: Fix fpu__activate_fpstate_read() and update comments
      x86/fpu: Remove fpu__current_fpstate_write_begin/end()
      x86/fpu: Rename fpu::fpstate_active to fpu::initialized
      x86/fpu: Fix stale comments about lazy FPU logic
      x86/fpu: Simplify and speed up fpu__copy()
      x86/fpu: Rename fpu__activate_curr() to fpu__initialize()
      x86/fpu: Rename fpu__activate_fpstate_read/write() to fpu__prepare_[read|write]()

Rik van Riel (1):
      x86/fpu: Add FPU state copying quirk to handle XRSTOR failure on Intel Skylake CPUs

kbuild test robot (1):
      x86/fpu: Fix boolreturn.cocci warnings


 arch/x86/ia32/ia32_signal.c         |   2 +-
 arch/x86/include/asm/fpu/internal.h |  90 +++---------
 arch/x86/include/asm/fpu/types.h    |  32 +----
 arch/x86/include/asm/fpu/xstate.h   |  12 +-
 arch/x86/include/asm/trace/fpu.h    |  11 +-
 arch/x86/kernel/fpu/core.c          | 155 ++++++---------------
 arch/x86/kernel/fpu/init.c          |   2 +-
 arch/x86/kernel/fpu/regset.c        |  48 ++++---
 arch/x86/kernel/fpu/signal.c        |  37 ++---
 arch/x86/kernel/fpu/xstate.c        | 264 +++++++++++++++++++++++++++++-------
 arch/x86/kernel/signal.c            |   6 +-
 arch/x86/kvm/x86.c                  |   2 +-
 arch/x86/math-emu/fpu_entry.c       |   2 +-
 arch/x86/mm/extable.c               |  24 ++++
 arch/x86/mm/pkeys.c                 |   3 +-
 15 files changed, 375 insertions(+), 315 deletions(-)

diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
index e0bb46c02857..0e2a5edbce00 100644
--- a/arch/x86/ia32/ia32_signal.c
+++ b/arch/x86/ia32/ia32_signal.c
@@ -231,7 +231,7 @@ static void __user *get_sigframe(struct ksignal *ksig, struct pt_regs *regs,
 		 ksig->ka.sa.sa_restorer)
 		sp = (unsigned long) ksig->ka.sa.sa_restorer;
 
-	if (fpu->fpstate_active) {
+	if (fpu->initialized) {
 		unsigned long fx_aligned, math_size;
 
 		sp = fpu__alloc_mathframe(sp, 1, &fx_aligned, &math_size);
diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h
index 554cdb205d17..e3221ffa304e 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -23,11 +23,9 @@
 /*
  * High level FPU state handling functions:
  */
-extern void fpu__activate_curr(struct fpu *fpu);
-extern void fpu__activate_fpstate_read(struct fpu *fpu);
-extern void fpu__activate_fpstate_write(struct fpu *fpu);
-extern void fpu__current_fpstate_write_begin(void);
-extern void fpu__current_fpstate_write_end(void);
+extern void fpu__initialize(struct fpu *fpu);
+extern void fpu__prepare_read(struct fpu *fpu);
+extern void fpu__prepare_write(struct fpu *fpu);
 extern void fpu__save(struct fpu *fpu);
 extern void fpu__restore(struct fpu *fpu);
 extern int  fpu__restore_sig(void __user *buf, int ia32_frame);
@@ -120,20 +118,11 @@ extern void fpstate_sanitize_xstate(struct fpu *fpu);
 	err;								\
 })
 
-#define check_insn(insn, output, input...)				\
-({									\
-	int err;							\
+#define kernel_insn(insn, output, input...)				\
 	asm volatile("1:" #insn "\n\t"					\
 		     "2:\n"						\
-		     ".section .fixup,\"ax\"\n"				\
-		     "3:  movl $-1,%[err]\n"				\
-		     "    jmp  2b\n"					\
-		     ".previous\n"					\
-		     _ASM_EXTABLE(1b, 3b)				\
-		     : [err] "=r" (err), output				\
-		     : "0"(0), input);					\
-	err;								\
-})
+		     _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_fprestore)	\
+		     : output : input)
 
 static inline int copy_fregs_to_user(struct fregs_state __user *fx)
 {
@@ -153,20 +142,16 @@ static inline int copy_fxregs_to_user(struct fxregs_state __user *fx)
 
 static inline void copy_kernel_to_fxregs(struct fxregs_state *fx)
 {
-	int err;
-
 	if (IS_ENABLED(CONFIG_X86_32)) {
-		err = check_insn(fxrstor %[fx], "=m" (*fx), [fx] "m" (*fx));
+		kernel_insn(fxrstor %[fx], "=m" (*fx), [fx] "m" (*fx));
 	} else {
 		if (IS_ENABLED(CONFIG_AS_FXSAVEQ)) {
-			err = check_insn(fxrstorq %[fx], "=m" (*fx), [fx] "m" (*fx));
+			kernel_insn(fxrstorq %[fx], "=m" (*fx), [fx] "m" (*fx));
 		} else {
 			/* See comment in copy_fxregs_to_kernel() below. */
-			err = check_insn(rex64/fxrstor (%[fx]), "=m" (*fx), [fx] "R" (fx), "m" (*fx));
+			kernel_insn(rex64/fxrstor (%[fx]), "=m" (*fx), [fx] "R" (fx), "m" (*fx));
 		}
 	}
-	/* Copying from a kernel buffer to FPU registers should never fail: */
-	WARN_ON_FPU(err);
 }
 
 static inline int copy_user_to_fxregs(struct fxregs_state __user *fx)
@@ -183,9 +168,7 @@ static inline int copy_user_to_fxregs(struct fxregs_state __user *fx)
 
 static inline void copy_kernel_to_fregs(struct fregs_state *fx)
 {
-	int err = check_insn(frstor %[fx], "=m" (*fx), [fx] "m" (*fx));
-
-	WARN_ON_FPU(err);
+	kernel_insn(frstor %[fx], "=m" (*fx), [fx] "m" (*fx));
 }
 
 static inline int copy_user_to_fregs(struct fregs_state __user *fx)
@@ -281,18 +264,13 @@ static inline void copy_fxregs_to_kernel(struct fpu *fpu)
  * Use XRSTORS to restore context if it is enabled. XRSTORS supports compact
  * XSAVE area format.
  */
-#define XSTATE_XRESTORE(st, lmask, hmask, err)				\
+#define XSTATE_XRESTORE(st, lmask, hmask)				\
 	asm volatile(ALTERNATIVE(XRSTOR,				\
 				 XRSTORS, X86_FEATURE_XSAVES)		\
 		     "\n"						\
-		     "xor %[err], %[err]\n"				\
 		     "3:\n"						\
-		     ".pushsection .fixup,\"ax\"\n"			\
-		     "4: movl $-2, %[err]\n"				\
-		     "jmp 3b\n"						\
-		     ".popsection\n"					\
-		     _ASM_EXTABLE(661b, 4b)				\
-		     : [err] "=r" (err)					\
+		     _ASM_EXTABLE_HANDLE(661b, 3b, ex_handler_fprestore)\
+		     :							\
 		     : "D" (st), "m" (*st), "a" (lmask), "d" (hmask)	\
 		     : "memory")
 
@@ -336,7 +314,10 @@ static inline void copy_kernel_to_xregs_booting(struct xregs_state *xstate)
 	else
 		XSTATE_OP(XRSTOR, xstate, lmask, hmask, err);
 
-	/* We should never fault when copying from a kernel buffer: */
+	/*
+	 * We should never fault when copying from a kernel buffer, and the FPU
+	 * state we set at boot time should be valid.
+	 */
 	WARN_ON_FPU(err);
 }
 
@@ -350,7 +331,7 @@ static inline void copy_xregs_to_kernel(struct xregs_state *xstate)
 	u32 hmask = mask >> 32;
 	int err;
 
-	WARN_ON(!alternatives_patched);
+	WARN_ON_FPU(!alternatives_patched);
 
 	XSTATE_XSAVE(xstate, lmask, hmask, err);
 
@@ -365,12 +346,8 @@ static inline void copy_kernel_to_xregs(struct xregs_state *xstate, u64 mask)
 {
 	u32 lmask = mask;
 	u32 hmask = mask >> 32;
-	int err;
-
-	XSTATE_XRESTORE(xstate, lmask, hmask, err);
 
-	/* We should never fault when copying from a kernel buffer: */
-	WARN_ON_FPU(err);
+	XSTATE_XRESTORE(xstate, lmask, hmask);
 }
 
 /*
@@ -526,38 +503,17 @@ static inline int fpregs_state_valid(struct fpu *fpu, unsigned int cpu)
  */
 static inline void fpregs_deactivate(struct fpu *fpu)
 {
-	WARN_ON_FPU(!fpu->fpregs_active);
-
-	fpu->fpregs_active = 0;
 	this_cpu_write(fpu_fpregs_owner_ctx, NULL);
 	trace_x86_fpu_regs_deactivated(fpu);
 }
 
 static inline void fpregs_activate(struct fpu *fpu)
 {
-	WARN_ON_FPU(fpu->fpregs_active);
-
-	fpu->fpregs_active = 1;
 	this_cpu_write(fpu_fpregs_owner_ctx, fpu);
 	trace_x86_fpu_regs_activated(fpu);
 }
 
 /*
- * The question "does this thread have fpu access?"
- * is slightly racy, since preemption could come in
- * and revoke it immediately after the test.
- *
- * However, even in that very unlikely scenario,
- * we can just assume we have FPU access - typically
- * to save the FP state - we'll just take a #NM
- * fault and get the FPU access back.
- */
-static inline int fpregs_active(void)
-{
-	return current->thread.fpu.fpregs_active;
-}
-
-/*
  * FPU state switching for scheduling.
  *
  * This is a two-stage process:
@@ -571,14 +527,13 @@ static inline int fpregs_active(void)
 static inline void
 switch_fpu_prepare(struct fpu *old_fpu, int cpu)
 {
-	if (old_fpu->fpregs_active) {
+	if (old_fpu->initialized) {
 		if (!copy_fpregs_to_fpstate(old_fpu))
 			old_fpu->last_cpu = -1;
 		else
 			old_fpu->last_cpu = cpu;
 
 		/* But leave fpu_fpregs_owner_ctx! */
-		old_fpu->fpregs_active = 0;
 		trace_x86_fpu_regs_deactivated(old_fpu);
 	} else
 		old_fpu->last_cpu = -1;
@@ -595,7 +550,7 @@ switch_fpu_prepare(struct fpu *old_fpu, int cpu)
 static inline void switch_fpu_finish(struct fpu *new_fpu, int cpu)
 {
 	bool preload = static_cpu_has(X86_FEATURE_FPU) &&
-		       new_fpu->fpstate_active;
+		       new_fpu->initialized;
 
 	if (preload) {
 		if (!fpregs_state_valid(new_fpu, cpu))
@@ -617,8 +572,7 @@ static inline void user_fpu_begin(void)
 	struct fpu *fpu = &current->thread.fpu;
 
 	preempt_disable();
-	if (!fpregs_active())
-		fpregs_activate(fpu);
+	fpregs_activate(fpu);
 	preempt_enable();
 }
 
diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index 3c80f5b9c09d..a1520575d86b 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -68,6 +68,9 @@ struct fxregs_state {
 /* Default value for fxregs_state.mxcsr: */
 #define MXCSR_DEFAULT		0x1f80
 
+/* Copy both mxcsr & mxcsr_flags with a single u64 memcpy: */
+#define MXCSR_AND_FLAGS_SIZE sizeof(u64)
+
 /*
  * Software based FPU emulation state. This is arbitrary really,
  * it matches the x87 format to make it easier to understand:
@@ -290,36 +293,13 @@ struct fpu {
 	unsigned int			last_cpu;
 
 	/*
-	 * @fpstate_active:
+	 * @initialized:
 	 *
-	 * This flag indicates whether this context is active: if the task
+	 * This flag indicates whether this context is initialized: if the task
 	 * is not running then we can restore from this context, if the task
 	 * is running then we should save into this context.
 	 */
-	unsigned char			fpstate_active;
-
-	/*
-	 * @fpregs_active:
-	 *
-	 * This flag determines whether a given context is actively
-	 * loaded into the FPU's registers and that those registers
-	 * represent the task's current FPU state.
-	 *
-	 * Note the interaction with fpstate_active:
-	 *
-	 *   # task does not use the FPU:
-	 *   fpstate_active == 0
-	 *
-	 *   # task uses the FPU and regs are active:
-	 *   fpstate_active == 1 && fpregs_active == 1
-	 *
-	 *   # the regs are inactive but still match fpstate:
-	 *   fpstate_active == 1 && fpregs_active == 0 && fpregs_owner == fpu
-	 *
-	 * The third state is what we use for the lazy restore optimization
-	 * on lazy-switching CPUs.
-	 */
-	unsigned char			fpregs_active;
+	unsigned char			initialized;
 
 	/*
 	 * @state:
diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 1b2799e0699a..83fee2469eb7 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -48,8 +48,12 @@ void fpu__xstate_clear_all_cpu_caps(void);
 void *get_xsave_addr(struct xregs_state *xsave, int xstate);
 const void *get_xsave_field_ptr(int xstate_field);
 int using_compacted_format(void);
-int copyout_from_xsaves(unsigned int pos, unsigned int count, void *kbuf,
-			void __user *ubuf, struct xregs_state *xsave);
-int copyin_to_xsaves(const void *kbuf, const void __user *ubuf,
-		     struct xregs_state *xsave);
+int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int offset, unsigned int size);
+int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned int offset, unsigned int size);
+int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf);
+int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf);
+
+/* Validate an xstate header supplied by userspace (ptrace or sigreturn) */
+extern int validate_xstate_header(const struct xstate_header *hdr);
+
 #endif
diff --git a/arch/x86/include/asm/trace/fpu.h b/arch/x86/include/asm/trace/fpu.h
index 342e59789fcd..39f7a27bef13 100644
--- a/arch/x86/include/asm/trace/fpu.h
+++ b/arch/x86/include/asm/trace/fpu.h
@@ -12,25 +12,22 @@ DECLARE_EVENT_CLASS(x86_fpu,
 
 	TP_STRUCT__entry(
 		__field(struct fpu *, fpu)
-		__field(bool, fpregs_active)
-		__field(bool, fpstate_active)
+		__field(bool, initialized)
 		__field(u64, xfeatures)
 		__field(u64, xcomp_bv)
 		),
 
 	TP_fast_assign(
 		__entry->fpu		= fpu;
-		__entry->fpregs_active	= fpu->fpregs_active;
-		__entry->fpstate_active	= fpu->fpstate_active;
+		__entry->initialized	= fpu->initialized;
 		if (boot_cpu_has(X86_FEATURE_OSXSAVE)) {
 			__entry->xfeatures = fpu->state.xsave.header.xfeatures;
 			__entry->xcomp_bv  = fpu->state.xsave.header.xcomp_bv;
 		}
 	),
-	TP_printk("x86/fpu: %p fpregs_active: %d fpstate_active: %d xfeatures: %llx xcomp_bv: %llx",
+	TP_printk("x86/fpu: %p initialized: %d xfeatures: %llx xcomp_bv: %llx",
 			__entry->fpu,
-			__entry->fpregs_active,
-			__entry->fpstate_active,
+			__entry->initialized,
 			__entry->xfeatures,
 			__entry->xcomp_bv
 	)
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index e1114f070c2d..f92a6593de1e 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -100,7 +100,7 @@ void __kernel_fpu_begin(void)
 
 	kernel_fpu_disable();
 
-	if (fpu->fpregs_active) {
+	if (fpu->initialized) {
 		/*
 		 * Ignore return value -- we don't care if reg state
 		 * is clobbered.
@@ -116,7 +116,7 @@ void __kernel_fpu_end(void)
 {
 	struct fpu *fpu = &current->thread.fpu;
 
-	if (fpu->fpregs_active)
+	if (fpu->initialized)
 		copy_kernel_to_fpregs(&fpu->state);
 
 	kernel_fpu_enable();
@@ -148,7 +148,7 @@ void fpu__save(struct fpu *fpu)
 
 	preempt_disable();
 	trace_x86_fpu_before_save(fpu);
-	if (fpu->fpregs_active) {
+	if (fpu->initialized) {
 		if (!copy_fpregs_to_fpstate(fpu)) {
 			copy_kernel_to_fpregs(&fpu->state);
 		}
@@ -189,10 +189,9 @@ EXPORT_SYMBOL_GPL(fpstate_init);
 
 int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu)
 {
-	dst_fpu->fpregs_active = 0;
 	dst_fpu->last_cpu = -1;
 
-	if (!src_fpu->fpstate_active || !static_cpu_has(X86_FEATURE_FPU))
+	if (!src_fpu->initialized || !static_cpu_has(X86_FEATURE_FPU))
 		return 0;
 
 	WARN_ON_FPU(src_fpu != &current->thread.fpu);
@@ -206,26 +205,14 @@ int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu)
 	/*
 	 * Save current FPU registers directly into the child
 	 * FPU context, without any memory-to-memory copying.
-	 * In lazy mode, if the FPU context isn't loaded into
-	 * fpregs, CR0.TS will be set and do_device_not_available
-	 * will load the FPU context.
 	 *
-	 * We have to do all this with preemption disabled,
-	 * mostly because of the FNSAVE case, because in that
-	 * case we must not allow preemption in the window
-	 * between the FNSAVE and us marking the context lazy.
-	 *
-	 * It shouldn't be an issue as even FNSAVE is plenty
-	 * fast in terms of critical section length.
+	 * ( The function 'fails' in the FNSAVE case, which destroys
+	 *   register contents so we have to copy them back. )
 	 */
-	preempt_disable();
 	if (!copy_fpregs_to_fpstate(dst_fpu)) {
-		memcpy(&src_fpu->state, &dst_fpu->state,
-		       fpu_kernel_xstate_size);
-
+		memcpy(&src_fpu->state, &dst_fpu->state, fpu_kernel_xstate_size);
 		copy_kernel_to_fpregs(&src_fpu->state);
 	}
-	preempt_enable();
 
 	trace_x86_fpu_copy_src(src_fpu);
 	trace_x86_fpu_copy_dst(dst_fpu);
@@ -237,45 +224,48 @@ int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu)
  * Activate the current task's in-memory FPU context,
  * if it has not been used before:
  */
-void fpu__activate_curr(struct fpu *fpu)
+void fpu__initialize(struct fpu *fpu)
 {
 	WARN_ON_FPU(fpu != &current->thread.fpu);
 
-	if (!fpu->fpstate_active) {
+	if (!fpu->initialized) {
 		fpstate_init(&fpu->state);
 		trace_x86_fpu_init_state(fpu);
 
 		trace_x86_fpu_activate_state(fpu);
 		/* Safe to do for the current task: */
-		fpu->fpstate_active = 1;
+		fpu->initialized = 1;
 	}
 }
-EXPORT_SYMBOL_GPL(fpu__activate_curr);
+EXPORT_SYMBOL_GPL(fpu__initialize);
 
 /*
  * This function must be called before we read a task's fpstate.
  *
- * If the task has not used the FPU before then initialize its
- * fpstate.
+ * There's two cases where this gets called:
+ *
+ * - for the current task (when coredumping), in which case we have
+ *   to save the latest FPU registers into the fpstate,
+ *
+ * - or it's called for stopped tasks (ptrace), in which case the
+ *   registers were already saved by the context-switch code when
+ *   the task scheduled out - we only have to initialize the registers
+ *   if they've never been initialized.
  *
  * If the task has used the FPU before then save it.
  */
-void fpu__activate_fpstate_read(struct fpu *fpu)
+void fpu__prepare_read(struct fpu *fpu)
 {
-	/*
-	 * If fpregs are active (in the current CPU), then
-	 * copy them to the fpstate:
-	 */
-	if (fpu->fpregs_active) {
+	if (fpu == &current->thread.fpu) {
 		fpu__save(fpu);
 	} else {
-		if (!fpu->fpstate_active) {
+		if (!fpu->initialized) {
 			fpstate_init(&fpu->state);
 			trace_x86_fpu_init_state(fpu);
 
 			trace_x86_fpu_activate_state(fpu);
 			/* Safe to do for current and for stopped child tasks: */
-			fpu->fpstate_active = 1;
+			fpu->initialized = 1;
 		}
 	}
 }
@@ -283,17 +273,17 @@ void fpu__activate_fpstate_read(struct fpu *fpu)
 /*
  * This function must be called before we write a task's fpstate.
  *
- * If the task has used the FPU before then unlazy it.
+ * If the task has used the FPU before then invalidate any cached FPU registers.
  * If the task has not used the FPU before then initialize its fpstate.
  *
  * After this function call, after registers in the fpstate are
  * modified and the child task has woken up, the child task will
  * restore the modified FPU state from the modified context. If we
- * didn't clear its lazy status here then the lazy in-registers
+ * didn't clear its cached status here then the cached in-registers
  * state pending on its former CPU could be restored, corrupting
  * the modifications.
  */
-void fpu__activate_fpstate_write(struct fpu *fpu)
+void fpu__prepare_write(struct fpu *fpu)
 {
 	/*
 	 * Only stopped child tasks can be used to modify the FPU
@@ -301,8 +291,8 @@ void fpu__activate_fpstate_write(struct fpu *fpu)
 	 */
 	WARN_ON_FPU(fpu == &current->thread.fpu);
 
-	if (fpu->fpstate_active) {
-		/* Invalidate any lazy state: */
+	if (fpu->initialized) {
+		/* Invalidate any cached state: */
 		__fpu_invalidate_fpregs_state(fpu);
 	} else {
 		fpstate_init(&fpu->state);
@@ -310,74 +300,11 @@ void fpu__activate_fpstate_write(struct fpu *fpu)
 
 		trace_x86_fpu_activate_state(fpu);
 		/* Safe to do for stopped child tasks: */
-		fpu->fpstate_active = 1;
+		fpu->initialized = 1;
 	}
 }
 
 /*
- * This function must be called before we write the current
- * task's fpstate.
- *
- * This call gets the current FPU register state and moves
- * it in to the 'fpstate'.  Preemption is disabled so that
- * no writes to the 'fpstate' can occur from context
- * swiches.
- *
- * Must be followed by a fpu__current_fpstate_write_end().
- */
-void fpu__current_fpstate_write_begin(void)
-{
-	struct fpu *fpu = &current->thread.fpu;
-
-	/*
-	 * Ensure that the context-switching code does not write
-	 * over the fpstate while we are doing our update.
-	 */
-	preempt_disable();
-
-	/*
-	 * Move the fpregs in to the fpu's 'fpstate'.
-	 */
-	fpu__activate_fpstate_read(fpu);
-
-	/*
-	 * The caller is about to write to 'fpu'.  Ensure that no
-	 * CPU thinks that its fpregs match the fpstate.  This
-	 * ensures we will not be lazy and skip a XRSTOR in the
-	 * future.
-	 */
-	__fpu_invalidate_fpregs_state(fpu);
-}
-
-/*
- * This function must be paired with fpu__current_fpstate_write_begin()
- *
- * This will ensure that the modified fpstate gets placed back in
- * the fpregs if necessary.
- *
- * Note: This function may be called whether or not an _actual_
- * write to the fpstate occurred.
- */
-void fpu__current_fpstate_write_end(void)
-{
-	struct fpu *fpu = &current->thread.fpu;
-
-	/*
-	 * 'fpu' now has an updated copy of the state, but the
-	 * registers may still be out of date.  Update them with
-	 * an XRSTOR if they are active.
-	 */
-	if (fpregs_active())
-		copy_kernel_to_fpregs(&fpu->state);
-
-	/*
-	 * Our update is done and the fpregs/fpstate are in sync
-	 * if necessary.  Context switches can happen again.
-	 */
-	preempt_enable();
-}
-
-/*
  * 'fpu__restore()' is called to copy FPU registers from
  * the FPU fpstate to the live hw registers and to activate
  * access to the hardware registers, so that FPU instructions
@@ -389,7 +316,7 @@ void fpu__current_fpstate_write_end(void)
  */
 void fpu__restore(struct fpu *fpu)
 {
-	fpu__activate_curr(fpu);
+	fpu__initialize(fpu);
 
 	/* Avoid __kernel_fpu_begin() right after fpregs_activate() */
 	kernel_fpu_disable();
@@ -414,15 +341,17 @@ void fpu__drop(struct fpu *fpu)
 {
 	preempt_disable();
 
-	if (fpu->fpregs_active) {
-		/* Ignore delayed exceptions from user space */
-		asm volatile("1: fwait\n"
-			     "2:\n"
-			     _ASM_EXTABLE(1b, 2b));
-		fpregs_deactivate(fpu);
+	if (fpu == &current->thread.fpu) {
+		if (fpu->initialized) {
+			/* Ignore delayed exceptions from user space */
+			asm volatile("1: fwait\n"
+				     "2:\n"
+				     _ASM_EXTABLE(1b, 2b));
+			fpregs_deactivate(fpu);
+		}
 	}
 
-	fpu->fpstate_active = 0;
+	fpu->initialized = 0;
 
 	trace_x86_fpu_dropped(fpu);
 
@@ -462,9 +391,11 @@ void fpu__clear(struct fpu *fpu)
 	 * Make sure fpstate is cleared and initialized.
 	 */
 	if (static_cpu_has(X86_FEATURE_FPU)) {
-		fpu__activate_curr(fpu);
+		preempt_disable();
+		fpu__initialize(fpu);
 		user_fpu_begin();
 		copy_init_fpstate_to_fpregs();
+		preempt_enable();
 	}
 }
 
diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index d5d44c452624..7affb7e3d9a5 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -240,7 +240,7 @@ static void __init fpu__init_system_ctx_switch(void)
 	WARN_ON_FPU(!on_boot_cpu);
 	on_boot_cpu = 0;
 
-	WARN_ON_FPU(current->thread.fpu.fpstate_active);
+	WARN_ON_FPU(current->thread.fpu.initialized);
 }
 
 /*
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index b188b16841e3..3ea151372389 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -16,14 +16,14 @@ int regset_fpregs_active(struct task_struct *target, const struct user_regset *r
 {
 	struct fpu *target_fpu = &target->thread.fpu;
 
-	return target_fpu->fpstate_active ? regset->n : 0;
+	return target_fpu->initialized ? regset->n : 0;
 }
 
 int regset_xregset_fpregs_active(struct task_struct *target, const struct user_regset *regset)
 {
 	struct fpu *target_fpu = &target->thread.fpu;
 
-	if (boot_cpu_has(X86_FEATURE_FXSR) && target_fpu->fpstate_active)
+	if (boot_cpu_has(X86_FEATURE_FXSR) && target_fpu->initialized)
 		return regset->n;
 	else
 		return 0;
@@ -38,7 +38,7 @@ int xfpregs_get(struct task_struct *target, const struct user_regset *regset,
 	if (!boot_cpu_has(X86_FEATURE_FXSR))
 		return -ENODEV;
 
-	fpu__activate_fpstate_read(fpu);
+	fpu__prepare_read(fpu);
 	fpstate_sanitize_xstate(fpu);
 
 	return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
@@ -55,7 +55,7 @@ int xfpregs_set(struct task_struct *target, const struct user_regset *regset,
 	if (!boot_cpu_has(X86_FEATURE_FXSR))
 		return -ENODEV;
 
-	fpu__activate_fpstate_write(fpu);
+	fpu__prepare_write(fpu);
 	fpstate_sanitize_xstate(fpu);
 
 	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
@@ -89,10 +89,13 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
 
 	xsave = &fpu->state.xsave;
 
-	fpu__activate_fpstate_read(fpu);
+	fpu__prepare_read(fpu);
 
 	if (using_compacted_format()) {
-		ret = copyout_from_xsaves(pos, count, kbuf, ubuf, xsave);
+		if (kbuf)
+			ret = copy_xstate_to_kernel(kbuf, xsave, pos, count);
+		else
+			ret = copy_xstate_to_user(ubuf, xsave, pos, count);
 	} else {
 		fpstate_sanitize_xstate(fpu);
 		/*
@@ -129,28 +132,29 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
 
 	xsave = &fpu->state.xsave;
 
-	fpu__activate_fpstate_write(fpu);
+	fpu__prepare_write(fpu);
 
-	if (boot_cpu_has(X86_FEATURE_XSAVES))
-		ret = copyin_to_xsaves(kbuf, ubuf, xsave);
-	else
+	if (using_compacted_format()) {
+		if (kbuf)
+			ret = copy_kernel_to_xstate(xsave, kbuf);
+		else
+			ret = copy_user_to_xstate(xsave, ubuf);
+	} else {
 		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0, -1);
-
-	/*
-	 * In case of failure, mark all states as init:
-	 */
-	if (ret)
-		fpstate_init(&fpu->state);
+		if (!ret)
+			ret = validate_xstate_header(&xsave->header);
+	}
 
 	/*
 	 * mxcsr reserved bits must be masked to zero for security reasons.
 	 */
 	xsave->i387.mxcsr &= mxcsr_feature_mask;
-	xsave->header.xfeatures &= xfeatures_mask;
+
 	/*
-	 * These bits must be zero.
+	 * In case of failure, mark all states as init:
 	 */
-	memset(&xsave->header.reserved, 0, 48);
+	if (ret)
+		fpstate_init(&fpu->state);
 
 	return ret;
 }
@@ -299,7 +303,7 @@ int fpregs_get(struct task_struct *target, const struct user_regset *regset,
 	struct fpu *fpu = &target->thread.fpu;
 	struct user_i387_ia32_struct env;
 
-	fpu__activate_fpstate_read(fpu);
+	fpu__prepare_read(fpu);
 
 	if (!boot_cpu_has(X86_FEATURE_FPU))
 		return fpregs_soft_get(target, regset, pos, count, kbuf, ubuf);
@@ -329,7 +333,7 @@ int fpregs_set(struct task_struct *target, const struct user_regset *regset,
 	struct user_i387_ia32_struct env;
 	int ret;
 
-	fpu__activate_fpstate_write(fpu);
+	fpu__prepare_write(fpu);
 	fpstate_sanitize_xstate(fpu);
 
 	if (!boot_cpu_has(X86_FEATURE_FPU))
@@ -369,7 +373,7 @@ int dump_fpu(struct pt_regs *regs, struct user_i387_struct *ufpu)
 	struct fpu *fpu = &tsk->thread.fpu;
 	int fpvalid;
 
-	fpvalid = fpu->fpstate_active;
+	fpvalid = fpu->initialized;
 	if (fpvalid)
 		fpvalid = !fpregs_get(tsk, NULL,
 				      0, sizeof(struct user_i387_ia32_struct),
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 83c23c230b4c..fb639e70048f 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -155,7 +155,8 @@ static inline int copy_fpregs_to_sigframe(struct xregs_state __user *buf)
  */
 int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
 {
-	struct xregs_state *xsave = &current->thread.fpu.state.xsave;
+	struct fpu *fpu = &current->thread.fpu;
+	struct xregs_state *xsave = &fpu->state.xsave;
 	struct task_struct *tsk = current;
 	int ia32_fxstate = (buf != buf_fx);
 
@@ -170,13 +171,13 @@ int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
 			sizeof(struct user_i387_ia32_struct), NULL,
 			(struct _fpstate_32 __user *) buf) ? -1 : 1;
 
-	if (fpregs_active() || using_compacted_format()) {
+	if (fpu->initialized || using_compacted_format()) {
 		/* Save the live register state to the user directly. */
 		if (copy_fpregs_to_sigframe(buf_fx))
 			return -1;
 		/* Update the thread's fxstate to save the fsave header. */
 		if (ia32_fxstate)
-			copy_fxregs_to_kernel(&tsk->thread.fpu);
+			copy_fxregs_to_kernel(fpu);
 	} else {
 		/*
 		 * It is a *bug* if kernel uses compacted-format for xsave
@@ -189,7 +190,7 @@ int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
 			return -1;
 		}
 
-		fpstate_sanitize_xstate(&tsk->thread.fpu);
+		fpstate_sanitize_xstate(fpu);
 		if (__copy_to_user(buf_fx, xsave, fpu_user_xstate_size))
 			return -1;
 	}
@@ -213,8 +214,11 @@ sanitize_restored_xstate(struct task_struct *tsk,
 	struct xstate_header *header = &xsave->header;
 
 	if (use_xsave()) {
-		/* These bits must be zero. */
-		memset(header->reserved, 0, 48);
+		/*
+		 * Note: we don't need to zero the reserved bits in the
+		 * xstate_header here because we either didn't copy them at all,
+		 * or we checked earlier that they aren't set.
+		 */
 
 		/*
 		 * Init the state that is not present in the memory
@@ -223,7 +227,7 @@ sanitize_restored_xstate(struct task_struct *tsk,
 		if (fx_only)
 			header->xfeatures = XFEATURE_MASK_FPSSE;
 		else
-			header->xfeatures &= (xfeatures_mask & xfeatures);
+			header->xfeatures &= xfeatures;
 	}
 
 	if (use_fxsr()) {
@@ -279,7 +283,7 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 	if (!access_ok(VERIFY_READ, buf, size))
 		return -EACCES;
 
-	fpu__activate_curr(fpu);
+	fpu__initialize(fpu);
 
 	if (!static_cpu_has(X86_FEATURE_FPU))
 		return fpregs_soft_set(current, NULL,
@@ -307,28 +311,29 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 		/*
 		 * For 32-bit frames with fxstate, copy the user state to the
 		 * thread's fpu state, reconstruct fxstate from the fsave
-		 * header. Sanitize the copied state etc.
+		 * header. Validate and sanitize the copied state.
 		 */
 		struct fpu *fpu = &tsk->thread.fpu;
 		struct user_i387_ia32_struct env;
 		int err = 0;
 
 		/*
-		 * Drop the current fpu which clears fpu->fpstate_active. This ensures
+		 * Drop the current fpu which clears fpu->initialized. This ensures
 		 * that any context-switch during the copy of the new state,
 		 * avoids the intermediate state from getting restored/saved.
 		 * Thus avoiding the new restored state from getting corrupted.
 		 * We will be ready to restore/save the state only after
-		 * fpu->fpstate_active is again set.
+		 * fpu->initialized is again set.
 		 */
 		fpu__drop(fpu);
 
 		if (using_compacted_format()) {
-			err = copyin_to_xsaves(NULL, buf_fx,
-					       &fpu->state.xsave);
+			err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
 		} else {
-			err = __copy_from_user(&fpu->state.xsave,
-					       buf_fx, state_size);
+			err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
+
+			if (!err && state_size > offsetof(struct xregs_state, header))
+				err = validate_xstate_header(&fpu->state.xsave.header);
 		}
 
 		if (err || __copy_from_user(&env, buf, sizeof(env))) {
@@ -339,7 +344,7 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 			sanitize_restored_xstate(tsk, &env, xfeatures, fx_only);
 		}
 
-		fpu->fpstate_active = 1;
+		fpu->initialized = 1;
 		preempt_disable();
 		fpu__restore(fpu);
 		preempt_enable();
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index c24ac1efb12d..f1d5476c9022 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -483,6 +483,30 @@ int using_compacted_format(void)
 	return boot_cpu_has(X86_FEATURE_XSAVES);
 }
 
+/* Validate an xstate header supplied by userspace (ptrace or sigreturn) */
+int validate_xstate_header(const struct xstate_header *hdr)
+{
+	/* No unknown or supervisor features may be set */
+	if (hdr->xfeatures & (~xfeatures_mask | XFEATURE_MASK_SUPERVISOR))
+		return -EINVAL;
+
+	/* Userspace must use the uncompacted format */
+	if (hdr->xcomp_bv)
+		return -EINVAL;
+
+	/*
+	 * If 'reserved' is shrunken to add a new field, make sure to validate
+	 * that new field here!
+	 */
+	BUILD_BUG_ON(sizeof(hdr->reserved) != 48);
+
+	/* No reserved bits may be set */
+	if (memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
+		return -EINVAL;
+
+	return 0;
+}
+
 static void __xstate_dump_leaves(void)
 {
 	int i;
@@ -867,7 +891,7 @@ const void *get_xsave_field_ptr(int xsave_state)
 {
 	struct fpu *fpu = &current->thread.fpu;
 
-	if (!fpu->fpstate_active)
+	if (!fpu->initialized)
 		return NULL;
 	/*
 	 * fpu__save() takes the CPU's xstate registers
@@ -921,38 +945,129 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 #endif /* ! CONFIG_ARCH_HAS_PKEYS */
 
 /*
+ * Weird legacy quirk: SSE and YMM states store information in the
+ * MXCSR and MXCSR_FLAGS fields of the FP area. That means if the FP
+ * area is marked as unused in the xfeatures header, we need to copy
+ * MXCSR and MXCSR_FLAGS if either SSE or YMM are in use.
+ */
+static inline bool xfeatures_mxcsr_quirk(u64 xfeatures)
+{
+	if (!(xfeatures & (XFEATURE_MASK_SSE|XFEATURE_MASK_YMM)))
+		return false;
+
+	if (xfeatures & XFEATURE_MASK_FP)
+		return false;
+
+	return true;
+}
+
+/*
  * This is similar to user_regset_copyout(), but will not add offset to
  * the source data pointer or increment pos, count, kbuf, and ubuf.
  */
-static inline int xstate_copyout(unsigned int pos, unsigned int count,
-				 void *kbuf, void __user *ubuf,
-				 const void *data, const int start_pos,
-				 const int end_pos)
+static inline void
+__copy_xstate_to_kernel(void *kbuf, const void *data,
+			unsigned int offset, unsigned int size, unsigned int size_total)
 {
-	if ((count == 0) || (pos < start_pos))
-		return 0;
+	if (offset < size_total) {
+		unsigned int copy = min(size, size_total - offset);
 
-	if (end_pos < 0 || pos < end_pos) {
-		unsigned int copy = (end_pos < 0 ? count : min(count, end_pos - pos));
+		memcpy(kbuf + offset, data, copy);
+	}
+}
 
-		if (kbuf) {
-			memcpy(kbuf + pos, data, copy);
-		} else {
-			if (__copy_to_user(ubuf + pos, data, copy))
-				return -EFAULT;
+/*
+ * Convert from kernel XSAVES compacted format to standard format and copy
+ * to a kernel-space ptrace buffer.
+ *
+ * It supports partial copy but pos always starts from zero. This is called
+ * from xstateregs_get() and there we check the CPU has XSAVES.
+ */
+int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int offset_start, unsigned int size_total)
+{
+	unsigned int offset, size;
+	struct xstate_header header;
+	int i;
+
+	/*
+	 * Currently copy_regset_to_user() starts from pos 0:
+	 */
+	if (unlikely(offset_start != 0))
+		return -EFAULT;
+
+	/*
+	 * The destination is a ptrace buffer; we put in only user xstates:
+	 */
+	memset(&header, 0, sizeof(header));
+	header.xfeatures = xsave->header.xfeatures;
+	header.xfeatures &= ~XFEATURE_MASK_SUPERVISOR;
+
+	/*
+	 * Copy xregs_state->header:
+	 */
+	offset = offsetof(struct xregs_state, header);
+	size = sizeof(header);
+
+	__copy_xstate_to_kernel(kbuf, &header, offset, size, size_total);
+
+	for (i = 0; i < XFEATURE_MAX; i++) {
+		/*
+		 * Copy only in-use xstates:
+		 */
+		if ((header.xfeatures >> i) & 1) {
+			void *src = __raw_xsave_addr(xsave, 1 << i);
+
+			offset = xstate_offsets[i];
+			size = xstate_sizes[i];
+
+			/* The next component has to fit fully into the output buffer: */
+			if (offset + size > size_total)
+				break;
+
+			__copy_xstate_to_kernel(kbuf, src, offset, size, size_total);
 		}
+
+	}
+
+	if (xfeatures_mxcsr_quirk(header.xfeatures)) {
+		offset = offsetof(struct fxregs_state, mxcsr);
+		size = MXCSR_AND_FLAGS_SIZE;
+		__copy_xstate_to_kernel(kbuf, &xsave->i387.mxcsr, offset, size, size_total);
+	}
+
+	/*
+	 * Fill xsave->i387.sw_reserved value for ptrace frame:
+	 */
+	offset = offsetof(struct fxregs_state, sw_reserved);
+	size = sizeof(xstate_fx_sw_bytes);
+
+	__copy_xstate_to_kernel(kbuf, xstate_fx_sw_bytes, offset, size, size_total);
+
+	return 0;
+}
+
+static inline int
+__copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int offset, unsigned int size, unsigned int size_total)
+{
+	if (!size)
+		return 0;
+
+	if (offset < size_total) {
+		unsigned int copy = min(size, size_total - offset);
+
+		if (__copy_to_user(ubuf + offset, data, copy))
+			return -EFAULT;
 	}
 	return 0;
 }
 
 /*
  * Convert from kernel XSAVES compacted format to standard format and copy
- * to a ptrace buffer. It supports partial copy but pos always starts from
+ * to a user-space buffer. It supports partial copy but pos always starts from
  * zero. This is called from xstateregs_get() and there we check the CPU
  * has XSAVES.
  */
-int copyout_from_xsaves(unsigned int pos, unsigned int count, void *kbuf,
-			void __user *ubuf, struct xregs_state *xsave)
+int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned int offset_start, unsigned int size_total)
 {
 	unsigned int offset, size;
 	int ret, i;
@@ -961,7 +1076,7 @@ int copyout_from_xsaves(unsigned int pos, unsigned int count, void *kbuf,
 	/*
 	 * Currently copy_regset_to_user() starts from pos 0:
 	 */
-	if (unlikely(pos != 0))
+	if (unlikely(offset_start != 0))
 		return -EFAULT;
 
 	/*
@@ -977,8 +1092,7 @@ int copyout_from_xsaves(unsigned int pos, unsigned int count, void *kbuf,
 	offset = offsetof(struct xregs_state, header);
 	size = sizeof(header);
 
-	ret = xstate_copyout(offset, size, kbuf, ubuf, &header, 0, count);
-
+	ret = __copy_xstate_to_user(ubuf, &header, offset, size, size_total);
 	if (ret)
 		return ret;
 
@@ -992,25 +1106,30 @@ int copyout_from_xsaves(unsigned int pos, unsigned int count, void *kbuf,
 			offset = xstate_offsets[i];
 			size = xstate_sizes[i];
 
-			ret = xstate_copyout(offset, size, kbuf, ubuf, src, 0, count);
+			/* The next component has to fit fully into the output buffer: */
+			if (offset + size > size_total)
+				break;
 
+			ret = __copy_xstate_to_user(ubuf, src, offset, size, size_total);
 			if (ret)
 				return ret;
-
-			if (offset + size >= count)
-				break;
 		}
 
 	}
 
+	if (xfeatures_mxcsr_quirk(header.xfeatures)) {
+		offset = offsetof(struct fxregs_state, mxcsr);
+		size = MXCSR_AND_FLAGS_SIZE;
+		__copy_xstate_to_user(ubuf, &xsave->i387.mxcsr, offset, size, size_total);
+	}
+
 	/*
 	 * Fill xsave->i387.sw_reserved value for ptrace frame:
 	 */
 	offset = offsetof(struct fxregs_state, sw_reserved);
 	size = sizeof(xstate_fx_sw_bytes);
 
-	ret = xstate_copyout(offset, size, kbuf, ubuf, xstate_fx_sw_bytes, 0, count);
-
+	ret = __copy_xstate_to_user(ubuf, xstate_fx_sw_bytes, offset, size, size_total);
 	if (ret)
 		return ret;
 
@@ -1018,55 +1137,98 @@ int copyout_from_xsaves(unsigned int pos, unsigned int count, void *kbuf,
 }
 
 /*
- * Convert from a ptrace standard-format buffer to kernel XSAVES format
- * and copy to the target thread. This is called from xstateregs_set() and
- * there we check the CPU has XSAVES and a whole standard-sized buffer
- * exists.
+ * Convert from a ptrace standard-format kernel buffer to kernel XSAVES format
+ * and copy to the target thread. This is called from xstateregs_set().
  */
-int copyin_to_xsaves(const void *kbuf, const void __user *ubuf,
-		     struct xregs_state *xsave)
+int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 {
 	unsigned int offset, size;
 	int i;
-	u64 xfeatures;
-	u64 allowed_features;
+	struct xstate_header hdr;
 
 	offset = offsetof(struct xregs_state, header);
-	size = sizeof(xfeatures);
+	size = sizeof(hdr);
 
-	if (kbuf) {
-		memcpy(&xfeatures, kbuf + offset, size);
-	} else {
-		if (__copy_from_user(&xfeatures, ubuf + offset, size))
-			return -EFAULT;
+	memcpy(&hdr, kbuf + offset, size);
+
+	if (validate_xstate_header(&hdr))
+		return -EINVAL;
+
+	for (i = 0; i < XFEATURE_MAX; i++) {
+		u64 mask = ((u64)1 << i);
+
+		if (hdr.xfeatures & mask) {
+			void *dst = __raw_xsave_addr(xsave, 1 << i);
+
+			offset = xstate_offsets[i];
+			size = xstate_sizes[i];
+
+			memcpy(dst, kbuf + offset, size);
+		}
+	}
+
+	if (xfeatures_mxcsr_quirk(hdr.xfeatures)) {
+		offset = offsetof(struct fxregs_state, mxcsr);
+		size = MXCSR_AND_FLAGS_SIZE;
+		memcpy(&xsave->i387.mxcsr, kbuf + offset, size);
 	}
 
 	/*
-	 * Reject if the user sets any disabled or supervisor features:
+	 * The state that came in from userspace was user-state only.
+	 * Mask all the user states out of 'xfeatures':
+	 */
+	xsave->header.xfeatures &= XFEATURE_MASK_SUPERVISOR;
+
+	/*
+	 * Add back in the features that came in from userspace:
 	 */
-	allowed_features = xfeatures_mask & ~XFEATURE_MASK_SUPERVISOR;
+	xsave->header.xfeatures |= hdr.xfeatures;
 
-	if (xfeatures & ~allowed_features)
+	return 0;
+}
+
+/*
+ * Convert from a ptrace or sigreturn standard-format user-space buffer to
+ * kernel XSAVES format and copy to the target thread. This is called from
+ * xstateregs_set(), as well as potentially from the sigreturn() and
+ * rt_sigreturn() system calls.
+ */
+int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
+{
+	unsigned int offset, size;
+	int i;
+	struct xstate_header hdr;
+
+	offset = offsetof(struct xregs_state, header);
+	size = sizeof(hdr);
+
+	if (__copy_from_user(&hdr, ubuf + offset, size))
+		return -EFAULT;
+
+	if (validate_xstate_header(&hdr))
 		return -EINVAL;
 
 	for (i = 0; i < XFEATURE_MAX; i++) {
 		u64 mask = ((u64)1 << i);
 
-		if (xfeatures & mask) {
+		if (hdr.xfeatures & mask) {
 			void *dst = __raw_xsave_addr(xsave, 1 << i);
 
 			offset = xstate_offsets[i];
 			size = xstate_sizes[i];
 
-			if (kbuf) {
-				memcpy(dst, kbuf + offset, size);
-			} else {
-				if (__copy_from_user(dst, ubuf + offset, size))
-					return -EFAULT;
-			}
+			if (__copy_from_user(dst, ubuf + offset, size))
+				return -EFAULT;
 		}
 	}
 
+	if (xfeatures_mxcsr_quirk(hdr.xfeatures)) {
+		offset = offsetof(struct fxregs_state, mxcsr);
+		size = MXCSR_AND_FLAGS_SIZE;
+		if (__copy_from_user(&xsave->i387.mxcsr, ubuf + offset, size))
+			return -EFAULT;
+	}
+
 	/*
 	 * The state that came in from userspace was user-state only.
 	 * Mask all the user states out of 'xfeatures':
@@ -1076,7 +1238,7 @@ int copyin_to_xsaves(const void *kbuf, const void __user *ubuf,
 	/*
 	 * Add back in the features that came in from userspace:
 	 */
-	xsave->header.xfeatures |= xfeatures;
+	xsave->header.xfeatures |= hdr.xfeatures;
 
 	return 0;
 }
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index e04442345fc0..4e188fda5961 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -263,7 +263,7 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs *regs, size_t frame_size,
 		sp = (unsigned long) ka->sa.sa_restorer;
 	}
 
-	if (fpu->fpstate_active) {
+	if (fpu->initialized) {
 		sp = fpu__alloc_mathframe(sp, IS_ENABLED(CONFIG_X86_32),
 					  &buf_fx, &math_size);
 		*fpstate = (void __user *)sp;
@@ -279,7 +279,7 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs *regs, size_t frame_size,
 		return (void __user *)-1L;
 
 	/* save i387 and extended state */
-	if (fpu->fpstate_active &&
+	if (fpu->initialized &&
 	    copy_fpstate_to_sigframe(*fpstate, (void __user *)buf_fx, math_size) < 0)
 		return (void __user *)-1L;
 
@@ -755,7 +755,7 @@ handle_signal(struct ksignal *ksig, struct pt_regs *regs)
 		/*
 		 * Ensure the signal handler starts with the new fpu state.
 		 */
-		if (fpu->fpstate_active)
+		if (fpu->initialized)
 			fpu__clear(fpu);
 	}
 	signal_setup_done(failed, ksig, stepping);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cd17b7d9a107..03869eb7fcd6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7225,7 +7225,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	int r;
 	sigset_t sigsaved;
 
-	fpu__activate_curr(fpu);
+	fpu__initialize(fpu);
 
 	if (vcpu->sigset_active)
 		sigprocmask(SIG_SETMASK, &vcpu->sigset, &sigsaved);
diff --git a/arch/x86/math-emu/fpu_entry.c b/arch/x86/math-emu/fpu_entry.c
index d4a7df2205b8..220638a4cb94 100644
--- a/arch/x86/math-emu/fpu_entry.c
+++ b/arch/x86/math-emu/fpu_entry.c
@@ -114,7 +114,7 @@ void math_emulate(struct math_emu_info *info)
 	struct desc_struct code_descriptor;
 	struct fpu *fpu = &current->thread.fpu;
 
-	fpu__activate_curr(fpu);
+	fpu__initialize(fpu);
 
 #ifdef RE_ENTRANT_CHECKING
 	if (emulating) {
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index c076f710de4c..c3521e2be396 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -2,6 +2,7 @@
 #include <linux/uaccess.h>
 #include <linux/sched/debug.h>
 
+#include <asm/fpu/internal.h>
 #include <asm/traps.h>
 #include <asm/kdebug.h>
 
@@ -78,6 +79,29 @@ bool ex_handler_refcount(const struct exception_table_entry *fixup,
 }
 EXPORT_SYMBOL_GPL(ex_handler_refcount);
 
+/*
+ * Handler for when we fail to restore a task's FPU state.  We should never get
+ * here because the FPU state of a task using the FPU (task->thread.fpu.state)
+ * should always be valid.  However, past bugs have allowed userspace to set
+ * reserved bits in the XSAVE area using PTRACE_SETREGSET or sys_rt_sigreturn().
+ * These caused XRSTOR to fail when switching to the task, leaking the FPU
+ * registers of the task previously executing on the CPU.  Mitigate this class
+ * of vulnerability by restoring from the initial state (essentially, zeroing
+ * out all the FPU registers) if we can't restore from the task's FPU state.
+ */
+bool ex_handler_fprestore(const struct exception_table_entry *fixup,
+			  struct pt_regs *regs, int trapnr)
+{
+	regs->ip = ex_fixup_addr(fixup);
+
+	WARN_ONCE(1, "Bad FPU state detected at %pB, reinitializing FPU registers.",
+		  (void *)instruction_pointer(regs));
+
+	__copy_kernel_to_fpregs(&init_fpstate, -1);
+	return true;
+}
+EXPORT_SYMBOL_GPL(ex_handler_fprestore);
+
 bool ex_handler_ext(const struct exception_table_entry *fixup,
 		   struct pt_regs *regs, int trapnr)
 {
diff --git a/arch/x86/mm/pkeys.c b/arch/x86/mm/pkeys.c
index 2dab69a706ec..d7bc0eea20a5 100644
--- a/arch/x86/mm/pkeys.c
+++ b/arch/x86/mm/pkeys.c
@@ -18,7 +18,6 @@
 
 #include <asm/cpufeature.h>             /* boot_cpu_has, ...            */
 #include <asm/mmu_context.h>            /* vma_pkey()                   */
-#include <asm/fpu/internal.h>           /* fpregs_active()              */
 
 int __execute_only_pkey(struct mm_struct *mm)
 {
@@ -45,7 +44,7 @@ int __execute_only_pkey(struct mm_struct *mm)
 	 */
 	preempt_disable();
 	if (!need_to_set_mm_pkey &&
-	    fpregs_active() &&
+	    current->thread.fpu.initialized &&
 	    !__pkru_allows_read(read_pkru(), execute_only_pkey)) {
 		preempt_enable();
 		return execute_only_pkey;

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [RFC GIT PULL] x86 FPU fixes and cleanups
  2017-09-26 16:28     ` [RFC GIT PULL] x86 FPU fixes and cleanups Ingo Molnar
@ 2017-09-26 18:17       ` Linus Torvalds
  2017-09-27  7:40         ` [RFC GIT PULL, v2] " Ingo Molnar
  0 siblings, 1 reply; 33+ messages in thread
From: Linus Torvalds @ 2017-09-26 18:17 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Linux Kernel Mailing List, Andrew Morton, Eric Biggers,
	Andy Lutomirski, Borislav Petkov, Dave Hansen, Fenghua Yu,
	H . Peter Anvin, Oleg Nesterov, Peter Zijlstra, Rik van Riel,
	Thomas Gleixner, Yu-cheng Yu

On Tue, Sep 26, 2017 at 9:28 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> So this an RFC pull request, you can pull the latest x86-fpu-for-linus git tree
> from:

Ugh.

Ok, I think I'll do this, but I'd ask that instead of that final merge
commit, you'll just send me the series as normal, and with a
_extensive_ explanation that I can then put in my merge message about
why this is all a good idea at this time, and what is going on.

How does that sound?

                 Linus

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [RFC GIT PULL, v2] x86 FPU fixes and cleanups
  2017-09-26 18:17       ` Linus Torvalds
@ 2017-09-27  7:40         ` Ingo Molnar
  0 siblings, 0 replies; 33+ messages in thread
From: Ingo Molnar @ 2017-09-27  7:40 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Linux Kernel Mailing List, Andrew Morton, Eric Biggers,
	Andy Lutomirski, Borislav Petkov, Dave Hansen, Fenghua Yu,
	H . Peter Anvin, Oleg Nesterov, Peter Zijlstra, Rik van Riel,
	Thomas Gleixner, Yu-cheng Yu


* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Tue, Sep 26, 2017 at 9:28 AM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > So this an RFC pull request, you can pull the latest x86-fpu-for-linus git tree
> > from:
> 
> Ugh.
> 
> Ok, I think I'll do this, but I'd ask that instead of that final merge
> commit, you'll just send me the series as normal, and with a
> _extensive_ explanation that I can then put in my merge message about
> why this is all a good idea at this time, and what is going on.
> 
> How does that sound?

Ok!

Here's the updated pull request that is pointing to the linear series of commits 
with no merge commits, plus (much...) more information about what it all does:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86-fpu-for-linus

   # HEAD: 8474c532b558fb6754c0ddff42a39ed6636671c9 Merge branch 'WIP.x86/fpu' into x86/fpu, because it's ready

Here's a list of all the commits, in mostly chronological/topical order (i.e. it's 
mostly reverse git log order, with a bit of topical re-grouping which are marked 
with '...'):

These 14 commits:

656f083116a4: x86/fpu: Rename copyin_to_xsaves()/copyout_from_xsaves() to copy_user_to_xstate()/copy_xstate_to_user()
f0d4f30a7fd2: x86/fpu: Split copy_xstate_to_user() into copy_xstate_to_kernel() & copy_xstate_to_user()
4d981cf2d96f: x86/fpu: Remove 'ubuf' parameter from the copy_xstate_to_kernel() APIs
a69c158fb3e7: x86/fpu: Remove 'kbuf' parameter from the copy_xstate_to_user() APIs
d7eda6c99cc7: x86/fpu: Clean up parameter order in the copy_xstate_to_*() APIs
becb2bb72ff9: x86/fpu: Clean up the parameter definitions of copy_xstate_to_*()
8a5b731889cb: x86/fpu: Remove the 'start_pos' parameter from the __copy_xstate_to_*() functions
56583c9a1400: x86/fpu: Clarify parameter names in the copy_xstate_to_*() methods
6ff15f8db7ea: x86/fpu: Change 'size_total' parameter to unsigned and standardize the size checks in copy_xstate_to_*()
8c0817f4a318: x86/fpu: Simplify __copy_xstate_to_kernel() return values
79fecc2b7506: x86/fpu: Split copy_user_to_xstate() into copy_kernel_to_xstate() & copy_user_to_xstate()
59dffa4edba1: x86/fpu: Remove 'ubuf' parameter from the copy_kernel_to_xstate() API
7b9094c688f8: x86/fpu: Remove 'kbuf' parameter from the copy_user_to_xstate() API
6d7f7da5533a: x86/fpu: Flip the parameter order in copy_*_to_xstate()

... clean up and simplify the xstate user copy handling functions, in reaction to 
the collective head-scratching about the xstate user-copy handling code when this 
SkyLake xstate handling bug was fixed:

0852b374173b: x86/fpu: Add FPU state copying quirk to handle XRSTOR failure on Intel Skylake CPUs

The cleanups don't change any functionality, they just (hopefully) make it all 
clearer, more consistent, more debuggable and more robust. Note that most of the 
linecount increase comes from these commits, where we better split the user/kernel 
copy logic by having more variants, instead repeated fragile patterns of:

+               if (kbuf) {
+                       memcpy(kbuf + pos, data, copy);
+               } else {
+                       if (__copy_to_user(ubuf + pos, data, copy))
+                               return -EFAULT;
+               }

These 13 commits:

b3a163081c28: x86/fpu: Simplify fpu->fpregs_active use
a10b6a16cdad: x86/fpu: Make the fpu state change in fpu__clear() scheduler-atomic
b6aa85558d7e: x86/fpu: Split the state handling in fpu__drop()
f1c8cd017607: x86/fpu: Change fpu->fpregs_active users to fpu->fpstate_active
6cf4edbe0526: x86/fpu: Decouple fpregs_activate()/fpregs_deactivate() from fpu->fpregs_active
99dc26bda233: x86/fpu: Remove struct fpu::fpregs_active
...
4618e90965f2: x86/fpu: Fix fpu__activate_fpstate_read() and update comments
685c930d6e58: x86/fpu: Remove fpu__current_fpstate_write_begin/end()
e4a81bfcaae1: x86/fpu: Rename fpu::fpstate_active to fpu::initialized
7f1487c59b7c: x86/fpu: Fix stale comments about lazy FPU logic
e10078eba698: x86/fpu: Simplify and speed up fpu__copy()
2ce03d850b9a: x86/fpu: Rename fpu__activate_curr() to fpu__initialize()
369a036de206: x86/fpu: Rename fpu__activate_fpstate_read/write() to fpu__prepare_[read|write]()

... simplify the FPU state-machine to get rid of old lazy-FPU idiosyncrasies - a 
defensive simplification to make all the code easier to review and fix. No change 
in functionality.

These two commits:

4f8cef59bad2: x86/fpu: Fix boolreturn.cocci warnings
03eaec81ac09: x86/fpu: Turn WARN_ON() in context switch into WARN_ON_FPU()

... are debugging tweaks: static checker warning fix and move an FPU related 
warning to under WARN_ON_FPU().


These 12 commits:

814fb7bb7db5: x86/fpu: Don't let userspace set bogus xcomp_bv
...
d5c8028b4788: x86/fpu: Reinitialize FPU registers if restoring FPU state fails
e63e5d5c15c6: x86/fpu: Introduce validate_xstate_header()
cf9df81b139b: x86/fpu: Use validate_xstate_header() to validate the xstate_header in xstateregs_set()
b11e2e18a7fc: x86/fpu: Use validate_xstate_header() to validate the xstate_header in __fpu__restore_sig()
80d8ae86b367: x86/fpu: Copy the full state_header in copy_kernel_to_xstate()
b89eda482d78: x86/fpu: Eliminate the 'xfeatures' local variable in copy_kernel_to_xstate()
af95774b3ca0: x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_kernel_to_xstate()
af2c4322d986: x86/fpu: Copy the full header in copy_user_to_xstate()
3d703477bcfe: x86/fpu: Eliminate the 'xfeatures' local variable in copy_user_to_xstate()
98c0fad9d60e: x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_user_to_xstate()
738f48cb5fdd: x86/fpu: Use using_compacted_format() instead of open coded X86_FEATURE_XSAVES

... represent a finegrained split-up of the fixes from Eric Biggers to handle 
weird xstate bits properly. I did this finegrained split-up because some of these 
fixes also impact the ABI for weird xstate handling, for which we'd like to have 
good bisection results, should they cause any problems. (We also had one 
regression with the more monolithic fixes, so splitting it all up sounded prudent 
for robustness reasons as well.)

About the whole series: the commits up to 03eaec81ac09 have been in -next for 
months - but I've recently rebased them to remove a state machine clean-up commit 
that was objected to, and to make it more bisectable - so technically it's a new, 
rebased tree.

Robustness history: this series had some regressions along the way, and all 
reported regressions have been fixed. All but one of the regressions manifested 
itself as easy to report warnings. The previous version of this latest series was 
also in linux-next, with one (warning-only) regression reported which is fixed in 
the latest version.

Barring last minute brown paper bag bugs (and the commits are now older by a day 
which I'd hope helps paperbag reduction), I'm reasonably confident about its 
general robustness. Famous last words ...

 Thanks,

	Ingo

------------------>
Andi Kleen (1):
      x86/fpu: Turn WARN_ON() in context switch into WARN_ON_FPU()

Eric Biggers (12):
      x86/fpu: Don't let userspace set bogus xcomp_bv
      x86/fpu: Reinitialize FPU registers if restoring FPU state fails
      x86/fpu: Introduce validate_xstate_header()
      x86/fpu: Use validate_xstate_header() to validate the xstate_header in xstateregs_set()
      x86/fpu: Use validate_xstate_header() to validate the xstate_header in __fpu__restore_sig()
      x86/fpu: Copy the full state_header in copy_kernel_to_xstate()
      x86/fpu: Eliminate the 'xfeatures' local variable in copy_kernel_to_xstate()
      x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_kernel_to_xstate()
      x86/fpu: Copy the full header in copy_user_to_xstate()
      x86/fpu: Eliminate the 'xfeatures' local variable in copy_user_to_xstate()
      x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_user_to_xstate()
      x86/fpu: Use using_compacted_format() instead of open coded X86_FEATURE_XSAVES

Ingo Molnar (27):
      x86/fpu: Rename copyin_to_xsaves()/copyout_from_xsaves() to copy_user_to_xstate()/copy_xstate_to_user()
      x86/fpu: Split copy_xstate_to_user() into copy_xstate_to_kernel() & copy_xstate_to_user()
      x86/fpu: Remove 'ubuf' parameter from the copy_xstate_to_kernel() APIs
      x86/fpu: Remove 'kbuf' parameter from the copy_xstate_to_user() APIs
      x86/fpu: Clean up parameter order in the copy_xstate_to_*() APIs
      x86/fpu: Clean up the parameter definitions of copy_xstate_to_*()
      x86/fpu: Remove the 'start_pos' parameter from the __copy_xstate_to_*() functions
      x86/fpu: Clarify parameter names in the copy_xstate_to_*() methods
      x86/fpu: Change 'size_total' parameter to unsigned and standardize the size checks in copy_xstate_to_*()
      x86/fpu: Simplify __copy_xstate_to_kernel() return values
      x86/fpu: Split copy_user_to_xstate() into copy_kernel_to_xstate() & copy_user_to_xstate()
      x86/fpu: Remove 'ubuf' parameter from the copy_kernel_to_xstate() API
      x86/fpu: Remove 'kbuf' parameter from the copy_user_to_xstate() API
      x86/fpu: Flip the parameter order in copy_*_to_xstate()
      x86/fpu: Simplify fpu->fpregs_active use
      x86/fpu: Make the fpu state change in fpu__clear() scheduler-atomic
      x86/fpu: Split the state handling in fpu__drop()
      x86/fpu: Change fpu->fpregs_active users to fpu->fpstate_active
      x86/fpu: Decouple fpregs_activate()/fpregs_deactivate() from fpu->fpregs_active
      x86/fpu: Remove struct fpu::fpregs_active
      x86/fpu: Fix fpu__activate_fpstate_read() and update comments
      x86/fpu: Remove fpu__current_fpstate_write_begin/end()
      x86/fpu: Rename fpu::fpstate_active to fpu::initialized
      x86/fpu: Fix stale comments about lazy FPU logic
      x86/fpu: Simplify and speed up fpu__copy()
      x86/fpu: Rename fpu__activate_curr() to fpu__initialize()
      x86/fpu: Rename fpu__activate_fpstate_read/write() to fpu__prepare_[read|write]()

Rik van Riel (1):
      x86/fpu: Add FPU state copying quirk to handle XRSTOR failure on Intel Skylake CPUs

kbuild test robot (1):
      x86/fpu: Fix boolreturn.cocci warnings


 arch/x86/ia32/ia32_signal.c         |   2 +-
 arch/x86/include/asm/fpu/internal.h |  90 +++---------
 arch/x86/include/asm/fpu/types.h    |  32 +----
 arch/x86/include/asm/fpu/xstate.h   |  12 +-
 arch/x86/include/asm/trace/fpu.h    |  11 +-
 arch/x86/kernel/fpu/core.c          | 155 ++++++---------------
 arch/x86/kernel/fpu/init.c          |   2 +-
 arch/x86/kernel/fpu/regset.c        |  48 ++++---
 arch/x86/kernel/fpu/signal.c        |  37 ++---
 arch/x86/kernel/fpu/xstate.c        | 264 +++++++++++++++++++++++++++++-------
 arch/x86/kernel/signal.c            |   6 +-
 arch/x86/kvm/x86.c                  |   2 +-
 arch/x86/math-emu/fpu_entry.c       |   2 +-
 arch/x86/mm/extable.c               |  24 ++++
 arch/x86/mm/pkeys.c                 |   3 +-
 15 files changed, 375 insertions(+), 315 deletions(-)

diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
index e0bb46c02857..0e2a5edbce00 100644
--- a/arch/x86/ia32/ia32_signal.c
+++ b/arch/x86/ia32/ia32_signal.c
@@ -231,7 +231,7 @@ static void __user *get_sigframe(struct ksignal *ksig, struct pt_regs *regs,
 		 ksig->ka.sa.sa_restorer)
 		sp = (unsigned long) ksig->ka.sa.sa_restorer;
 
-	if (fpu->fpstate_active) {
+	if (fpu->initialized) {
 		unsigned long fx_aligned, math_size;
 
 		sp = fpu__alloc_mathframe(sp, 1, &fx_aligned, &math_size);
diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h
index 554cdb205d17..e3221ffa304e 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -23,11 +23,9 @@
 /*
  * High level FPU state handling functions:
  */
-extern void fpu__activate_curr(struct fpu *fpu);
-extern void fpu__activate_fpstate_read(struct fpu *fpu);
-extern void fpu__activate_fpstate_write(struct fpu *fpu);
-extern void fpu__current_fpstate_write_begin(void);
-extern void fpu__current_fpstate_write_end(void);
+extern void fpu__initialize(struct fpu *fpu);
+extern void fpu__prepare_read(struct fpu *fpu);
+extern void fpu__prepare_write(struct fpu *fpu);
 extern void fpu__save(struct fpu *fpu);
 extern void fpu__restore(struct fpu *fpu);
 extern int  fpu__restore_sig(void __user *buf, int ia32_frame);
@@ -120,20 +118,11 @@ extern void fpstate_sanitize_xstate(struct fpu *fpu);
 	err;								\
 })
 
-#define check_insn(insn, output, input...)				\
-({									\
-	int err;							\
+#define kernel_insn(insn, output, input...)				\
 	asm volatile("1:" #insn "\n\t"					\
 		     "2:\n"						\
-		     ".section .fixup,\"ax\"\n"				\
-		     "3:  movl $-1,%[err]\n"				\
-		     "    jmp  2b\n"					\
-		     ".previous\n"					\
-		     _ASM_EXTABLE(1b, 3b)				\
-		     : [err] "=r" (err), output				\
-		     : "0"(0), input);					\
-	err;								\
-})
+		     _ASM_EXTABLE_HANDLE(1b, 2b, ex_handler_fprestore)	\
+		     : output : input)
 
 static inline int copy_fregs_to_user(struct fregs_state __user *fx)
 {
@@ -153,20 +142,16 @@ static inline int copy_fxregs_to_user(struct fxregs_state __user *fx)
 
 static inline void copy_kernel_to_fxregs(struct fxregs_state *fx)
 {
-	int err;
-
 	if (IS_ENABLED(CONFIG_X86_32)) {
-		err = check_insn(fxrstor %[fx], "=m" (*fx), [fx] "m" (*fx));
+		kernel_insn(fxrstor %[fx], "=m" (*fx), [fx] "m" (*fx));
 	} else {
 		if (IS_ENABLED(CONFIG_AS_FXSAVEQ)) {
-			err = check_insn(fxrstorq %[fx], "=m" (*fx), [fx] "m" (*fx));
+			kernel_insn(fxrstorq %[fx], "=m" (*fx), [fx] "m" (*fx));
 		} else {
 			/* See comment in copy_fxregs_to_kernel() below. */
-			err = check_insn(rex64/fxrstor (%[fx]), "=m" (*fx), [fx] "R" (fx), "m" (*fx));
+			kernel_insn(rex64/fxrstor (%[fx]), "=m" (*fx), [fx] "R" (fx), "m" (*fx));
 		}
 	}
-	/* Copying from a kernel buffer to FPU registers should never fail: */
-	WARN_ON_FPU(err);
 }
 
 static inline int copy_user_to_fxregs(struct fxregs_state __user *fx)
@@ -183,9 +168,7 @@ static inline int copy_user_to_fxregs(struct fxregs_state __user *fx)
 
 static inline void copy_kernel_to_fregs(struct fregs_state *fx)
 {
-	int err = check_insn(frstor %[fx], "=m" (*fx), [fx] "m" (*fx));
-
-	WARN_ON_FPU(err);
+	kernel_insn(frstor %[fx], "=m" (*fx), [fx] "m" (*fx));
 }
 
 static inline int copy_user_to_fregs(struct fregs_state __user *fx)
@@ -281,18 +264,13 @@ static inline void copy_fxregs_to_kernel(struct fpu *fpu)
  * Use XRSTORS to restore context if it is enabled. XRSTORS supports compact
  * XSAVE area format.
  */
-#define XSTATE_XRESTORE(st, lmask, hmask, err)				\
+#define XSTATE_XRESTORE(st, lmask, hmask)				\
 	asm volatile(ALTERNATIVE(XRSTOR,				\
 				 XRSTORS, X86_FEATURE_XSAVES)		\
 		     "\n"						\
-		     "xor %[err], %[err]\n"				\
 		     "3:\n"						\
-		     ".pushsection .fixup,\"ax\"\n"			\
-		     "4: movl $-2, %[err]\n"				\
-		     "jmp 3b\n"						\
-		     ".popsection\n"					\
-		     _ASM_EXTABLE(661b, 4b)				\
-		     : [err] "=r" (err)					\
+		     _ASM_EXTABLE_HANDLE(661b, 3b, ex_handler_fprestore)\
+		     :							\
 		     : "D" (st), "m" (*st), "a" (lmask), "d" (hmask)	\
 		     : "memory")
 
@@ -336,7 +314,10 @@ static inline void copy_kernel_to_xregs_booting(struct xregs_state *xstate)
 	else
 		XSTATE_OP(XRSTOR, xstate, lmask, hmask, err);
 
-	/* We should never fault when copying from a kernel buffer: */
+	/*
+	 * We should never fault when copying from a kernel buffer, and the FPU
+	 * state we set at boot time should be valid.
+	 */
 	WARN_ON_FPU(err);
 }
 
@@ -350,7 +331,7 @@ static inline void copy_xregs_to_kernel(struct xregs_state *xstate)
 	u32 hmask = mask >> 32;
 	int err;
 
-	WARN_ON(!alternatives_patched);
+	WARN_ON_FPU(!alternatives_patched);
 
 	XSTATE_XSAVE(xstate, lmask, hmask, err);
 
@@ -365,12 +346,8 @@ static inline void copy_kernel_to_xregs(struct xregs_state *xstate, u64 mask)
 {
 	u32 lmask = mask;
 	u32 hmask = mask >> 32;
-	int err;
-
-	XSTATE_XRESTORE(xstate, lmask, hmask, err);
 
-	/* We should never fault when copying from a kernel buffer: */
-	WARN_ON_FPU(err);
+	XSTATE_XRESTORE(xstate, lmask, hmask);
 }
 
 /*
@@ -526,38 +503,17 @@ static inline int fpregs_state_valid(struct fpu *fpu, unsigned int cpu)
  */
 static inline void fpregs_deactivate(struct fpu *fpu)
 {
-	WARN_ON_FPU(!fpu->fpregs_active);
-
-	fpu->fpregs_active = 0;
 	this_cpu_write(fpu_fpregs_owner_ctx, NULL);
 	trace_x86_fpu_regs_deactivated(fpu);
 }
 
 static inline void fpregs_activate(struct fpu *fpu)
 {
-	WARN_ON_FPU(fpu->fpregs_active);
-
-	fpu->fpregs_active = 1;
 	this_cpu_write(fpu_fpregs_owner_ctx, fpu);
 	trace_x86_fpu_regs_activated(fpu);
 }
 
 /*
- * The question "does this thread have fpu access?"
- * is slightly racy, since preemption could come in
- * and revoke it immediately after the test.
- *
- * However, even in that very unlikely scenario,
- * we can just assume we have FPU access - typically
- * to save the FP state - we'll just take a #NM
- * fault and get the FPU access back.
- */
-static inline int fpregs_active(void)
-{
-	return current->thread.fpu.fpregs_active;
-}
-
-/*
  * FPU state switching for scheduling.
  *
  * This is a two-stage process:
@@ -571,14 +527,13 @@ static inline int fpregs_active(void)
 static inline void
 switch_fpu_prepare(struct fpu *old_fpu, int cpu)
 {
-	if (old_fpu->fpregs_active) {
+	if (old_fpu->initialized) {
 		if (!copy_fpregs_to_fpstate(old_fpu))
 			old_fpu->last_cpu = -1;
 		else
 			old_fpu->last_cpu = cpu;
 
 		/* But leave fpu_fpregs_owner_ctx! */
-		old_fpu->fpregs_active = 0;
 		trace_x86_fpu_regs_deactivated(old_fpu);
 	} else
 		old_fpu->last_cpu = -1;
@@ -595,7 +550,7 @@ switch_fpu_prepare(struct fpu *old_fpu, int cpu)
 static inline void switch_fpu_finish(struct fpu *new_fpu, int cpu)
 {
 	bool preload = static_cpu_has(X86_FEATURE_FPU) &&
-		       new_fpu->fpstate_active;
+		       new_fpu->initialized;
 
 	if (preload) {
 		if (!fpregs_state_valid(new_fpu, cpu))
@@ -617,8 +572,7 @@ static inline void user_fpu_begin(void)
 	struct fpu *fpu = &current->thread.fpu;
 
 	preempt_disable();
-	if (!fpregs_active())
-		fpregs_activate(fpu);
+	fpregs_activate(fpu);
 	preempt_enable();
 }
 
diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index 3c80f5b9c09d..a1520575d86b 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -68,6 +68,9 @@ struct fxregs_state {
 /* Default value for fxregs_state.mxcsr: */
 #define MXCSR_DEFAULT		0x1f80
 
+/* Copy both mxcsr & mxcsr_flags with a single u64 memcpy: */
+#define MXCSR_AND_FLAGS_SIZE sizeof(u64)
+
 /*
  * Software based FPU emulation state. This is arbitrary really,
  * it matches the x87 format to make it easier to understand:
@@ -290,36 +293,13 @@ struct fpu {
 	unsigned int			last_cpu;
 
 	/*
-	 * @fpstate_active:
+	 * @initialized:
 	 *
-	 * This flag indicates whether this context is active: if the task
+	 * This flag indicates whether this context is initialized: if the task
 	 * is not running then we can restore from this context, if the task
 	 * is running then we should save into this context.
 	 */
-	unsigned char			fpstate_active;
-
-	/*
-	 * @fpregs_active:
-	 *
-	 * This flag determines whether a given context is actively
-	 * loaded into the FPU's registers and that those registers
-	 * represent the task's current FPU state.
-	 *
-	 * Note the interaction with fpstate_active:
-	 *
-	 *   # task does not use the FPU:
-	 *   fpstate_active == 0
-	 *
-	 *   # task uses the FPU and regs are active:
-	 *   fpstate_active == 1 && fpregs_active == 1
-	 *
-	 *   # the regs are inactive but still match fpstate:
-	 *   fpstate_active == 1 && fpregs_active == 0 && fpregs_owner == fpu
-	 *
-	 * The third state is what we use for the lazy restore optimization
-	 * on lazy-switching CPUs.
-	 */
-	unsigned char			fpregs_active;
+	unsigned char			initialized;
 
 	/*
 	 * @state:
diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 1b2799e0699a..83fee2469eb7 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -48,8 +48,12 @@ void fpu__xstate_clear_all_cpu_caps(void);
 void *get_xsave_addr(struct xregs_state *xsave, int xstate);
 const void *get_xsave_field_ptr(int xstate_field);
 int using_compacted_format(void);
-int copyout_from_xsaves(unsigned int pos, unsigned int count, void *kbuf,
-			void __user *ubuf, struct xregs_state *xsave);
-int copyin_to_xsaves(const void *kbuf, const void __user *ubuf,
-		     struct xregs_state *xsave);
+int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int offset, unsigned int size);
+int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned int offset, unsigned int size);
+int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf);
+int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf);
+
+/* Validate an xstate header supplied by userspace (ptrace or sigreturn) */
+extern int validate_xstate_header(const struct xstate_header *hdr);
+
 #endif
diff --git a/arch/x86/include/asm/trace/fpu.h b/arch/x86/include/asm/trace/fpu.h
index 342e59789fcd..39f7a27bef13 100644
--- a/arch/x86/include/asm/trace/fpu.h
+++ b/arch/x86/include/asm/trace/fpu.h
@@ -12,25 +12,22 @@ DECLARE_EVENT_CLASS(x86_fpu,
 
 	TP_STRUCT__entry(
 		__field(struct fpu *, fpu)
-		__field(bool, fpregs_active)
-		__field(bool, fpstate_active)
+		__field(bool, initialized)
 		__field(u64, xfeatures)
 		__field(u64, xcomp_bv)
 		),
 
 	TP_fast_assign(
 		__entry->fpu		= fpu;
-		__entry->fpregs_active	= fpu->fpregs_active;
-		__entry->fpstate_active	= fpu->fpstate_active;
+		__entry->initialized	= fpu->initialized;
 		if (boot_cpu_has(X86_FEATURE_OSXSAVE)) {
 			__entry->xfeatures = fpu->state.xsave.header.xfeatures;
 			__entry->xcomp_bv  = fpu->state.xsave.header.xcomp_bv;
 		}
 	),
-	TP_printk("x86/fpu: %p fpregs_active: %d fpstate_active: %d xfeatures: %llx xcomp_bv: %llx",
+	TP_printk("x86/fpu: %p initialized: %d xfeatures: %llx xcomp_bv: %llx",
 			__entry->fpu,
-			__entry->fpregs_active,
-			__entry->fpstate_active,
+			__entry->initialized,
 			__entry->xfeatures,
 			__entry->xcomp_bv
 	)
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index e1114f070c2d..f92a6593de1e 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -100,7 +100,7 @@ void __kernel_fpu_begin(void)
 
 	kernel_fpu_disable();
 
-	if (fpu->fpregs_active) {
+	if (fpu->initialized) {
 		/*
 		 * Ignore return value -- we don't care if reg state
 		 * is clobbered.
@@ -116,7 +116,7 @@ void __kernel_fpu_end(void)
 {
 	struct fpu *fpu = &current->thread.fpu;
 
-	if (fpu->fpregs_active)
+	if (fpu->initialized)
 		copy_kernel_to_fpregs(&fpu->state);
 
 	kernel_fpu_enable();
@@ -148,7 +148,7 @@ void fpu__save(struct fpu *fpu)
 
 	preempt_disable();
 	trace_x86_fpu_before_save(fpu);
-	if (fpu->fpregs_active) {
+	if (fpu->initialized) {
 		if (!copy_fpregs_to_fpstate(fpu)) {
 			copy_kernel_to_fpregs(&fpu->state);
 		}
@@ -189,10 +189,9 @@ EXPORT_SYMBOL_GPL(fpstate_init);
 
 int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu)
 {
-	dst_fpu->fpregs_active = 0;
 	dst_fpu->last_cpu = -1;
 
-	if (!src_fpu->fpstate_active || !static_cpu_has(X86_FEATURE_FPU))
+	if (!src_fpu->initialized || !static_cpu_has(X86_FEATURE_FPU))
 		return 0;
 
 	WARN_ON_FPU(src_fpu != &current->thread.fpu);
@@ -206,26 +205,14 @@ int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu)
 	/*
 	 * Save current FPU registers directly into the child
 	 * FPU context, without any memory-to-memory copying.
-	 * In lazy mode, if the FPU context isn't loaded into
-	 * fpregs, CR0.TS will be set and do_device_not_available
-	 * will load the FPU context.
 	 *
-	 * We have to do all this with preemption disabled,
-	 * mostly because of the FNSAVE case, because in that
-	 * case we must not allow preemption in the window
-	 * between the FNSAVE and us marking the context lazy.
-	 *
-	 * It shouldn't be an issue as even FNSAVE is plenty
-	 * fast in terms of critical section length.
+	 * ( The function 'fails' in the FNSAVE case, which destroys
+	 *   register contents so we have to copy them back. )
 	 */
-	preempt_disable();
 	if (!copy_fpregs_to_fpstate(dst_fpu)) {
-		memcpy(&src_fpu->state, &dst_fpu->state,
-		       fpu_kernel_xstate_size);
-
+		memcpy(&src_fpu->state, &dst_fpu->state, fpu_kernel_xstate_size);
 		copy_kernel_to_fpregs(&src_fpu->state);
 	}
-	preempt_enable();
 
 	trace_x86_fpu_copy_src(src_fpu);
 	trace_x86_fpu_copy_dst(dst_fpu);
@@ -237,45 +224,48 @@ int fpu__copy(struct fpu *dst_fpu, struct fpu *src_fpu)
  * Activate the current task's in-memory FPU context,
  * if it has not been used before:
  */
-void fpu__activate_curr(struct fpu *fpu)
+void fpu__initialize(struct fpu *fpu)
 {
 	WARN_ON_FPU(fpu != &current->thread.fpu);
 
-	if (!fpu->fpstate_active) {
+	if (!fpu->initialized) {
 		fpstate_init(&fpu->state);
 		trace_x86_fpu_init_state(fpu);
 
 		trace_x86_fpu_activate_state(fpu);
 		/* Safe to do for the current task: */
-		fpu->fpstate_active = 1;
+		fpu->initialized = 1;
 	}
 }
-EXPORT_SYMBOL_GPL(fpu__activate_curr);
+EXPORT_SYMBOL_GPL(fpu__initialize);
 
 /*
  * This function must be called before we read a task's fpstate.
  *
- * If the task has not used the FPU before then initialize its
- * fpstate.
+ * There's two cases where this gets called:
+ *
+ * - for the current task (when coredumping), in which case we have
+ *   to save the latest FPU registers into the fpstate,
+ *
+ * - or it's called for stopped tasks (ptrace), in which case the
+ *   registers were already saved by the context-switch code when
+ *   the task scheduled out - we only have to initialize the registers
+ *   if they've never been initialized.
  *
  * If the task has used the FPU before then save it.
  */
-void fpu__activate_fpstate_read(struct fpu *fpu)
+void fpu__prepare_read(struct fpu *fpu)
 {
-	/*
-	 * If fpregs are active (in the current CPU), then
-	 * copy them to the fpstate:
-	 */
-	if (fpu->fpregs_active) {
+	if (fpu == &current->thread.fpu) {
 		fpu__save(fpu);
 	} else {
-		if (!fpu->fpstate_active) {
+		if (!fpu->initialized) {
 			fpstate_init(&fpu->state);
 			trace_x86_fpu_init_state(fpu);
 
 			trace_x86_fpu_activate_state(fpu);
 			/* Safe to do for current and for stopped child tasks: */
-			fpu->fpstate_active = 1;
+			fpu->initialized = 1;
 		}
 	}
 }
@@ -283,17 +273,17 @@ void fpu__activate_fpstate_read(struct fpu *fpu)
 /*
  * This function must be called before we write a task's fpstate.
  *
- * If the task has used the FPU before then unlazy it.
+ * If the task has used the FPU before then invalidate any cached FPU registers.
  * If the task has not used the FPU before then initialize its fpstate.
  *
  * After this function call, after registers in the fpstate are
  * modified and the child task has woken up, the child task will
  * restore the modified FPU state from the modified context. If we
- * didn't clear its lazy status here then the lazy in-registers
+ * didn't clear its cached status here then the cached in-registers
  * state pending on its former CPU could be restored, corrupting
  * the modifications.
  */
-void fpu__activate_fpstate_write(struct fpu *fpu)
+void fpu__prepare_write(struct fpu *fpu)
 {
 	/*
 	 * Only stopped child tasks can be used to modify the FPU
@@ -301,8 +291,8 @@ void fpu__activate_fpstate_write(struct fpu *fpu)
 	 */
 	WARN_ON_FPU(fpu == &current->thread.fpu);
 
-	if (fpu->fpstate_active) {
-		/* Invalidate any lazy state: */
+	if (fpu->initialized) {
+		/* Invalidate any cached state: */
 		__fpu_invalidate_fpregs_state(fpu);
 	} else {
 		fpstate_init(&fpu->state);
@@ -310,74 +300,11 @@ void fpu__activate_fpstate_write(struct fpu *fpu)
 
 		trace_x86_fpu_activate_state(fpu);
 		/* Safe to do for stopped child tasks: */
-		fpu->fpstate_active = 1;
+		fpu->initialized = 1;
 	}
 }
 
 /*
- * This function must be called before we write the current
- * task's fpstate.
- *
- * This call gets the current FPU register state and moves
- * it in to the 'fpstate'.  Preemption is disabled so that
- * no writes to the 'fpstate' can occur from context
- * swiches.
- *
- * Must be followed by a fpu__current_fpstate_write_end().
- */
-void fpu__current_fpstate_write_begin(void)
-{
-	struct fpu *fpu = &current->thread.fpu;
-
-	/*
-	 * Ensure that the context-switching code does not write
-	 * over the fpstate while we are doing our update.
-	 */
-	preempt_disable();
-
-	/*
-	 * Move the fpregs in to the fpu's 'fpstate'.
-	 */
-	fpu__activate_fpstate_read(fpu);
-
-	/*
-	 * The caller is about to write to 'fpu'.  Ensure that no
-	 * CPU thinks that its fpregs match the fpstate.  This
-	 * ensures we will not be lazy and skip a XRSTOR in the
-	 * future.
-	 */
-	__fpu_invalidate_fpregs_state(fpu);
-}
-
-/*
- * This function must be paired with fpu__current_fpstate_write_begin()
- *
- * This will ensure that the modified fpstate gets placed back in
- * the fpregs if necessary.
- *
- * Note: This function may be called whether or not an _actual_
- * write to the fpstate occurred.
- */
-void fpu__current_fpstate_write_end(void)
-{
-	struct fpu *fpu = &current->thread.fpu;
-
-	/*
-	 * 'fpu' now has an updated copy of the state, but the
-	 * registers may still be out of date.  Update them with
-	 * an XRSTOR if they are active.
-	 */
-	if (fpregs_active())
-		copy_kernel_to_fpregs(&fpu->state);
-
-	/*
-	 * Our update is done and the fpregs/fpstate are in sync
-	 * if necessary.  Context switches can happen again.
-	 */
-	preempt_enable();
-}
-
-/*
  * 'fpu__restore()' is called to copy FPU registers from
  * the FPU fpstate to the live hw registers and to activate
  * access to the hardware registers, so that FPU instructions
@@ -389,7 +316,7 @@ void fpu__current_fpstate_write_end(void)
  */
 void fpu__restore(struct fpu *fpu)
 {
-	fpu__activate_curr(fpu);
+	fpu__initialize(fpu);
 
 	/* Avoid __kernel_fpu_begin() right after fpregs_activate() */
 	kernel_fpu_disable();
@@ -414,15 +341,17 @@ void fpu__drop(struct fpu *fpu)
 {
 	preempt_disable();
 
-	if (fpu->fpregs_active) {
-		/* Ignore delayed exceptions from user space */
-		asm volatile("1: fwait\n"
-			     "2:\n"
-			     _ASM_EXTABLE(1b, 2b));
-		fpregs_deactivate(fpu);
+	if (fpu == &current->thread.fpu) {
+		if (fpu->initialized) {
+			/* Ignore delayed exceptions from user space */
+			asm volatile("1: fwait\n"
+				     "2:\n"
+				     _ASM_EXTABLE(1b, 2b));
+			fpregs_deactivate(fpu);
+		}
 	}
 
-	fpu->fpstate_active = 0;
+	fpu->initialized = 0;
 
 	trace_x86_fpu_dropped(fpu);
 
@@ -462,9 +391,11 @@ void fpu__clear(struct fpu *fpu)
 	 * Make sure fpstate is cleared and initialized.
 	 */
 	if (static_cpu_has(X86_FEATURE_FPU)) {
-		fpu__activate_curr(fpu);
+		preempt_disable();
+		fpu__initialize(fpu);
 		user_fpu_begin();
 		copy_init_fpstate_to_fpregs();
+		preempt_enable();
 	}
 }
 
diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
index d5d44c452624..7affb7e3d9a5 100644
--- a/arch/x86/kernel/fpu/init.c
+++ b/arch/x86/kernel/fpu/init.c
@@ -240,7 +240,7 @@ static void __init fpu__init_system_ctx_switch(void)
 	WARN_ON_FPU(!on_boot_cpu);
 	on_boot_cpu = 0;
 
-	WARN_ON_FPU(current->thread.fpu.fpstate_active);
+	WARN_ON_FPU(current->thread.fpu.initialized);
 }
 
 /*
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index b188b16841e3..3ea151372389 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -16,14 +16,14 @@ int regset_fpregs_active(struct task_struct *target, const struct user_regset *r
 {
 	struct fpu *target_fpu = &target->thread.fpu;
 
-	return target_fpu->fpstate_active ? regset->n : 0;
+	return target_fpu->initialized ? regset->n : 0;
 }
 
 int regset_xregset_fpregs_active(struct task_struct *target, const struct user_regset *regset)
 {
 	struct fpu *target_fpu = &target->thread.fpu;
 
-	if (boot_cpu_has(X86_FEATURE_FXSR) && target_fpu->fpstate_active)
+	if (boot_cpu_has(X86_FEATURE_FXSR) && target_fpu->initialized)
 		return regset->n;
 	else
 		return 0;
@@ -38,7 +38,7 @@ int xfpregs_get(struct task_struct *target, const struct user_regset *regset,
 	if (!boot_cpu_has(X86_FEATURE_FXSR))
 		return -ENODEV;
 
-	fpu__activate_fpstate_read(fpu);
+	fpu__prepare_read(fpu);
 	fpstate_sanitize_xstate(fpu);
 
 	return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
@@ -55,7 +55,7 @@ int xfpregs_set(struct task_struct *target, const struct user_regset *regset,
 	if (!boot_cpu_has(X86_FEATURE_FXSR))
 		return -ENODEV;
 
-	fpu__activate_fpstate_write(fpu);
+	fpu__prepare_write(fpu);
 	fpstate_sanitize_xstate(fpu);
 
 	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
@@ -89,10 +89,13 @@ int xstateregs_get(struct task_struct *target, const struct user_regset *regset,
 
 	xsave = &fpu->state.xsave;
 
-	fpu__activate_fpstate_read(fpu);
+	fpu__prepare_read(fpu);
 
 	if (using_compacted_format()) {
-		ret = copyout_from_xsaves(pos, count, kbuf, ubuf, xsave);
+		if (kbuf)
+			ret = copy_xstate_to_kernel(kbuf, xsave, pos, count);
+		else
+			ret = copy_xstate_to_user(ubuf, xsave, pos, count);
 	} else {
 		fpstate_sanitize_xstate(fpu);
 		/*
@@ -129,28 +132,29 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
 
 	xsave = &fpu->state.xsave;
 
-	fpu__activate_fpstate_write(fpu);
+	fpu__prepare_write(fpu);
 
-	if (boot_cpu_has(X86_FEATURE_XSAVES))
-		ret = copyin_to_xsaves(kbuf, ubuf, xsave);
-	else
+	if (using_compacted_format()) {
+		if (kbuf)
+			ret = copy_kernel_to_xstate(xsave, kbuf);
+		else
+			ret = copy_user_to_xstate(xsave, ubuf);
+	} else {
 		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0, -1);
-
-	/*
-	 * In case of failure, mark all states as init:
-	 */
-	if (ret)
-		fpstate_init(&fpu->state);
+		if (!ret)
+			ret = validate_xstate_header(&xsave->header);
+	}
 
 	/*
 	 * mxcsr reserved bits must be masked to zero for security reasons.
 	 */
 	xsave->i387.mxcsr &= mxcsr_feature_mask;
-	xsave->header.xfeatures &= xfeatures_mask;
+
 	/*
-	 * These bits must be zero.
+	 * In case of failure, mark all states as init:
 	 */
-	memset(&xsave->header.reserved, 0, 48);
+	if (ret)
+		fpstate_init(&fpu->state);
 
 	return ret;
 }
@@ -299,7 +303,7 @@ int fpregs_get(struct task_struct *target, const struct user_regset *regset,
 	struct fpu *fpu = &target->thread.fpu;
 	struct user_i387_ia32_struct env;
 
-	fpu__activate_fpstate_read(fpu);
+	fpu__prepare_read(fpu);
 
 	if (!boot_cpu_has(X86_FEATURE_FPU))
 		return fpregs_soft_get(target, regset, pos, count, kbuf, ubuf);
@@ -329,7 +333,7 @@ int fpregs_set(struct task_struct *target, const struct user_regset *regset,
 	struct user_i387_ia32_struct env;
 	int ret;
 
-	fpu__activate_fpstate_write(fpu);
+	fpu__prepare_write(fpu);
 	fpstate_sanitize_xstate(fpu);
 
 	if (!boot_cpu_has(X86_FEATURE_FPU))
@@ -369,7 +373,7 @@ int dump_fpu(struct pt_regs *regs, struct user_i387_struct *ufpu)
 	struct fpu *fpu = &tsk->thread.fpu;
 	int fpvalid;
 
-	fpvalid = fpu->fpstate_active;
+	fpvalid = fpu->initialized;
 	if (fpvalid)
 		fpvalid = !fpregs_get(tsk, NULL,
 				      0, sizeof(struct user_i387_ia32_struct),
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 83c23c230b4c..fb639e70048f 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -155,7 +155,8 @@ static inline int copy_fpregs_to_sigframe(struct xregs_state __user *buf)
  */
 int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
 {
-	struct xregs_state *xsave = &current->thread.fpu.state.xsave;
+	struct fpu *fpu = &current->thread.fpu;
+	struct xregs_state *xsave = &fpu->state.xsave;
 	struct task_struct *tsk = current;
 	int ia32_fxstate = (buf != buf_fx);
 
@@ -170,13 +171,13 @@ int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
 			sizeof(struct user_i387_ia32_struct), NULL,
 			(struct _fpstate_32 __user *) buf) ? -1 : 1;
 
-	if (fpregs_active() || using_compacted_format()) {
+	if (fpu->initialized || using_compacted_format()) {
 		/* Save the live register state to the user directly. */
 		if (copy_fpregs_to_sigframe(buf_fx))
 			return -1;
 		/* Update the thread's fxstate to save the fsave header. */
 		if (ia32_fxstate)
-			copy_fxregs_to_kernel(&tsk->thread.fpu);
+			copy_fxregs_to_kernel(fpu);
 	} else {
 		/*
 		 * It is a *bug* if kernel uses compacted-format for xsave
@@ -189,7 +190,7 @@ int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
 			return -1;
 		}
 
-		fpstate_sanitize_xstate(&tsk->thread.fpu);
+		fpstate_sanitize_xstate(fpu);
 		if (__copy_to_user(buf_fx, xsave, fpu_user_xstate_size))
 			return -1;
 	}
@@ -213,8 +214,11 @@ sanitize_restored_xstate(struct task_struct *tsk,
 	struct xstate_header *header = &xsave->header;
 
 	if (use_xsave()) {
-		/* These bits must be zero. */
-		memset(header->reserved, 0, 48);
+		/*
+		 * Note: we don't need to zero the reserved bits in the
+		 * xstate_header here because we either didn't copy them at all,
+		 * or we checked earlier that they aren't set.
+		 */
 
 		/*
 		 * Init the state that is not present in the memory
@@ -223,7 +227,7 @@ sanitize_restored_xstate(struct task_struct *tsk,
 		if (fx_only)
 			header->xfeatures = XFEATURE_MASK_FPSSE;
 		else
-			header->xfeatures &= (xfeatures_mask & xfeatures);
+			header->xfeatures &= xfeatures;
 	}
 
 	if (use_fxsr()) {
@@ -279,7 +283,7 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 	if (!access_ok(VERIFY_READ, buf, size))
 		return -EACCES;
 
-	fpu__activate_curr(fpu);
+	fpu__initialize(fpu);
 
 	if (!static_cpu_has(X86_FEATURE_FPU))
 		return fpregs_soft_set(current, NULL,
@@ -307,28 +311,29 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 		/*
 		 * For 32-bit frames with fxstate, copy the user state to the
 		 * thread's fpu state, reconstruct fxstate from the fsave
-		 * header. Sanitize the copied state etc.
+		 * header. Validate and sanitize the copied state.
 		 */
 		struct fpu *fpu = &tsk->thread.fpu;
 		struct user_i387_ia32_struct env;
 		int err = 0;
 
 		/*
-		 * Drop the current fpu which clears fpu->fpstate_active. This ensures
+		 * Drop the current fpu which clears fpu->initialized. This ensures
 		 * that any context-switch during the copy of the new state,
 		 * avoids the intermediate state from getting restored/saved.
 		 * Thus avoiding the new restored state from getting corrupted.
 		 * We will be ready to restore/save the state only after
-		 * fpu->fpstate_active is again set.
+		 * fpu->initialized is again set.
 		 */
 		fpu__drop(fpu);
 
 		if (using_compacted_format()) {
-			err = copyin_to_xsaves(NULL, buf_fx,
-					       &fpu->state.xsave);
+			err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
 		} else {
-			err = __copy_from_user(&fpu->state.xsave,
-					       buf_fx, state_size);
+			err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
+
+			if (!err && state_size > offsetof(struct xregs_state, header))
+				err = validate_xstate_header(&fpu->state.xsave.header);
 		}
 
 		if (err || __copy_from_user(&env, buf, sizeof(env))) {
@@ -339,7 +344,7 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 			sanitize_restored_xstate(tsk, &env, xfeatures, fx_only);
 		}
 
-		fpu->fpstate_active = 1;
+		fpu->initialized = 1;
 		preempt_disable();
 		fpu__restore(fpu);
 		preempt_enable();
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index c24ac1efb12d..f1d5476c9022 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -483,6 +483,30 @@ int using_compacted_format(void)
 	return boot_cpu_has(X86_FEATURE_XSAVES);
 }
 
+/* Validate an xstate header supplied by userspace (ptrace or sigreturn) */
+int validate_xstate_header(const struct xstate_header *hdr)
+{
+	/* No unknown or supervisor features may be set */
+	if (hdr->xfeatures & (~xfeatures_mask | XFEATURE_MASK_SUPERVISOR))
+		return -EINVAL;
+
+	/* Userspace must use the uncompacted format */
+	if (hdr->xcomp_bv)
+		return -EINVAL;
+
+	/*
+	 * If 'reserved' is shrunken to add a new field, make sure to validate
+	 * that new field here!
+	 */
+	BUILD_BUG_ON(sizeof(hdr->reserved) != 48);
+
+	/* No reserved bits may be set */
+	if (memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
+		return -EINVAL;
+
+	return 0;
+}
+
 static void __xstate_dump_leaves(void)
 {
 	int i;
@@ -867,7 +891,7 @@ const void *get_xsave_field_ptr(int xsave_state)
 {
 	struct fpu *fpu = &current->thread.fpu;
 
-	if (!fpu->fpstate_active)
+	if (!fpu->initialized)
 		return NULL;
 	/*
 	 * fpu__save() takes the CPU's xstate registers
@@ -921,38 +945,129 @@ int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
 #endif /* ! CONFIG_ARCH_HAS_PKEYS */
 
 /*
+ * Weird legacy quirk: SSE and YMM states store information in the
+ * MXCSR and MXCSR_FLAGS fields of the FP area. That means if the FP
+ * area is marked as unused in the xfeatures header, we need to copy
+ * MXCSR and MXCSR_FLAGS if either SSE or YMM are in use.
+ */
+static inline bool xfeatures_mxcsr_quirk(u64 xfeatures)
+{
+	if (!(xfeatures & (XFEATURE_MASK_SSE|XFEATURE_MASK_YMM)))
+		return false;
+
+	if (xfeatures & XFEATURE_MASK_FP)
+		return false;
+
+	return true;
+}
+
+/*
  * This is similar to user_regset_copyout(), but will not add offset to
  * the source data pointer or increment pos, count, kbuf, and ubuf.
  */
-static inline int xstate_copyout(unsigned int pos, unsigned int count,
-				 void *kbuf, void __user *ubuf,
-				 const void *data, const int start_pos,
-				 const int end_pos)
+static inline void
+__copy_xstate_to_kernel(void *kbuf, const void *data,
+			unsigned int offset, unsigned int size, unsigned int size_total)
 {
-	if ((count == 0) || (pos < start_pos))
-		return 0;
+	if (offset < size_total) {
+		unsigned int copy = min(size, size_total - offset);
 
-	if (end_pos < 0 || pos < end_pos) {
-		unsigned int copy = (end_pos < 0 ? count : min(count, end_pos - pos));
+		memcpy(kbuf + offset, data, copy);
+	}
+}
 
-		if (kbuf) {
-			memcpy(kbuf + pos, data, copy);
-		} else {
-			if (__copy_to_user(ubuf + pos, data, copy))
-				return -EFAULT;
+/*
+ * Convert from kernel XSAVES compacted format to standard format and copy
+ * to a kernel-space ptrace buffer.
+ *
+ * It supports partial copy but pos always starts from zero. This is called
+ * from xstateregs_get() and there we check the CPU has XSAVES.
+ */
+int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int offset_start, unsigned int size_total)
+{
+	unsigned int offset, size;
+	struct xstate_header header;
+	int i;
+
+	/*
+	 * Currently copy_regset_to_user() starts from pos 0:
+	 */
+	if (unlikely(offset_start != 0))
+		return -EFAULT;
+
+	/*
+	 * The destination is a ptrace buffer; we put in only user xstates:
+	 */
+	memset(&header, 0, sizeof(header));
+	header.xfeatures = xsave->header.xfeatures;
+	header.xfeatures &= ~XFEATURE_MASK_SUPERVISOR;
+
+	/*
+	 * Copy xregs_state->header:
+	 */
+	offset = offsetof(struct xregs_state, header);
+	size = sizeof(header);
+
+	__copy_xstate_to_kernel(kbuf, &header, offset, size, size_total);
+
+	for (i = 0; i < XFEATURE_MAX; i++) {
+		/*
+		 * Copy only in-use xstates:
+		 */
+		if ((header.xfeatures >> i) & 1) {
+			void *src = __raw_xsave_addr(xsave, 1 << i);
+
+			offset = xstate_offsets[i];
+			size = xstate_sizes[i];
+
+			/* The next component has to fit fully into the output buffer: */
+			if (offset + size > size_total)
+				break;
+
+			__copy_xstate_to_kernel(kbuf, src, offset, size, size_total);
 		}
+
+	}
+
+	if (xfeatures_mxcsr_quirk(header.xfeatures)) {
+		offset = offsetof(struct fxregs_state, mxcsr);
+		size = MXCSR_AND_FLAGS_SIZE;
+		__copy_xstate_to_kernel(kbuf, &xsave->i387.mxcsr, offset, size, size_total);
+	}
+
+	/*
+	 * Fill xsave->i387.sw_reserved value for ptrace frame:
+	 */
+	offset = offsetof(struct fxregs_state, sw_reserved);
+	size = sizeof(xstate_fx_sw_bytes);
+
+	__copy_xstate_to_kernel(kbuf, xstate_fx_sw_bytes, offset, size, size_total);
+
+	return 0;
+}
+
+static inline int
+__copy_xstate_to_user(void __user *ubuf, const void *data, unsigned int offset, unsigned int size, unsigned int size_total)
+{
+	if (!size)
+		return 0;
+
+	if (offset < size_total) {
+		unsigned int copy = min(size, size_total - offset);
+
+		if (__copy_to_user(ubuf + offset, data, copy))
+			return -EFAULT;
 	}
 	return 0;
 }
 
 /*
  * Convert from kernel XSAVES compacted format to standard format and copy
- * to a ptrace buffer. It supports partial copy but pos always starts from
+ * to a user-space buffer. It supports partial copy but pos always starts from
  * zero. This is called from xstateregs_get() and there we check the CPU
  * has XSAVES.
  */
-int copyout_from_xsaves(unsigned int pos, unsigned int count, void *kbuf,
-			void __user *ubuf, struct xregs_state *xsave)
+int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned int offset_start, unsigned int size_total)
 {
 	unsigned int offset, size;
 	int ret, i;
@@ -961,7 +1076,7 @@ int copyout_from_xsaves(unsigned int pos, unsigned int count, void *kbuf,
 	/*
 	 * Currently copy_regset_to_user() starts from pos 0:
 	 */
-	if (unlikely(pos != 0))
+	if (unlikely(offset_start != 0))
 		return -EFAULT;
 
 	/*
@@ -977,8 +1092,7 @@ int copyout_from_xsaves(unsigned int pos, unsigned int count, void *kbuf,
 	offset = offsetof(struct xregs_state, header);
 	size = sizeof(header);
 
-	ret = xstate_copyout(offset, size, kbuf, ubuf, &header, 0, count);
-
+	ret = __copy_xstate_to_user(ubuf, &header, offset, size, size_total);
 	if (ret)
 		return ret;
 
@@ -992,25 +1106,30 @@ int copyout_from_xsaves(unsigned int pos, unsigned int count, void *kbuf,
 			offset = xstate_offsets[i];
 			size = xstate_sizes[i];
 
-			ret = xstate_copyout(offset, size, kbuf, ubuf, src, 0, count);
+			/* The next component has to fit fully into the output buffer: */
+			if (offset + size > size_total)
+				break;
 
+			ret = __copy_xstate_to_user(ubuf, src, offset, size, size_total);
 			if (ret)
 				return ret;
-
-			if (offset + size >= count)
-				break;
 		}
 
 	}
 
+	if (xfeatures_mxcsr_quirk(header.xfeatures)) {
+		offset = offsetof(struct fxregs_state, mxcsr);
+		size = MXCSR_AND_FLAGS_SIZE;
+		__copy_xstate_to_user(ubuf, &xsave->i387.mxcsr, offset, size, size_total);
+	}
+
 	/*
 	 * Fill xsave->i387.sw_reserved value for ptrace frame:
 	 */
 	offset = offsetof(struct fxregs_state, sw_reserved);
 	size = sizeof(xstate_fx_sw_bytes);
 
-	ret = xstate_copyout(offset, size, kbuf, ubuf, xstate_fx_sw_bytes, 0, count);
-
+	ret = __copy_xstate_to_user(ubuf, xstate_fx_sw_bytes, offset, size, size_total);
 	if (ret)
 		return ret;
 
@@ -1018,55 +1137,98 @@ int copyout_from_xsaves(unsigned int pos, unsigned int count, void *kbuf,
 }
 
 /*
- * Convert from a ptrace standard-format buffer to kernel XSAVES format
- * and copy to the target thread. This is called from xstateregs_set() and
- * there we check the CPU has XSAVES and a whole standard-sized buffer
- * exists.
+ * Convert from a ptrace standard-format kernel buffer to kernel XSAVES format
+ * and copy to the target thread. This is called from xstateregs_set().
  */
-int copyin_to_xsaves(const void *kbuf, const void __user *ubuf,
-		     struct xregs_state *xsave)
+int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 {
 	unsigned int offset, size;
 	int i;
-	u64 xfeatures;
-	u64 allowed_features;
+	struct xstate_header hdr;
 
 	offset = offsetof(struct xregs_state, header);
-	size = sizeof(xfeatures);
+	size = sizeof(hdr);
 
-	if (kbuf) {
-		memcpy(&xfeatures, kbuf + offset, size);
-	} else {
-		if (__copy_from_user(&xfeatures, ubuf + offset, size))
-			return -EFAULT;
+	memcpy(&hdr, kbuf + offset, size);
+
+	if (validate_xstate_header(&hdr))
+		return -EINVAL;
+
+	for (i = 0; i < XFEATURE_MAX; i++) {
+		u64 mask = ((u64)1 << i);
+
+		if (hdr.xfeatures & mask) {
+			void *dst = __raw_xsave_addr(xsave, 1 << i);
+
+			offset = xstate_offsets[i];
+			size = xstate_sizes[i];
+
+			memcpy(dst, kbuf + offset, size);
+		}
+	}
+
+	if (xfeatures_mxcsr_quirk(hdr.xfeatures)) {
+		offset = offsetof(struct fxregs_state, mxcsr);
+		size = MXCSR_AND_FLAGS_SIZE;
+		memcpy(&xsave->i387.mxcsr, kbuf + offset, size);
 	}
 
 	/*
-	 * Reject if the user sets any disabled or supervisor features:
+	 * The state that came in from userspace was user-state only.
+	 * Mask all the user states out of 'xfeatures':
+	 */
+	xsave->header.xfeatures &= XFEATURE_MASK_SUPERVISOR;
+
+	/*
+	 * Add back in the features that came in from userspace:
 	 */
-	allowed_features = xfeatures_mask & ~XFEATURE_MASK_SUPERVISOR;
+	xsave->header.xfeatures |= hdr.xfeatures;
 
-	if (xfeatures & ~allowed_features)
+	return 0;
+}
+
+/*
+ * Convert from a ptrace or sigreturn standard-format user-space buffer to
+ * kernel XSAVES format and copy to the target thread. This is called from
+ * xstateregs_set(), as well as potentially from the sigreturn() and
+ * rt_sigreturn() system calls.
+ */
+int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
+{
+	unsigned int offset, size;
+	int i;
+	struct xstate_header hdr;
+
+	offset = offsetof(struct xregs_state, header);
+	size = sizeof(hdr);
+
+	if (__copy_from_user(&hdr, ubuf + offset, size))
+		return -EFAULT;
+
+	if (validate_xstate_header(&hdr))
 		return -EINVAL;
 
 	for (i = 0; i < XFEATURE_MAX; i++) {
 		u64 mask = ((u64)1 << i);
 
-		if (xfeatures & mask) {
+		if (hdr.xfeatures & mask) {
 			void *dst = __raw_xsave_addr(xsave, 1 << i);
 
 			offset = xstate_offsets[i];
 			size = xstate_sizes[i];
 
-			if (kbuf) {
-				memcpy(dst, kbuf + offset, size);
-			} else {
-				if (__copy_from_user(dst, ubuf + offset, size))
-					return -EFAULT;
-			}
+			if (__copy_from_user(dst, ubuf + offset, size))
+				return -EFAULT;
 		}
 	}
 
+	if (xfeatures_mxcsr_quirk(hdr.xfeatures)) {
+		offset = offsetof(struct fxregs_state, mxcsr);
+		size = MXCSR_AND_FLAGS_SIZE;
+		if (__copy_from_user(&xsave->i387.mxcsr, ubuf + offset, size))
+			return -EFAULT;
+	}
+
 	/*
 	 * The state that came in from userspace was user-state only.
 	 * Mask all the user states out of 'xfeatures':
@@ -1076,7 +1238,7 @@ int copyin_to_xsaves(const void *kbuf, const void __user *ubuf,
 	/*
 	 * Add back in the features that came in from userspace:
 	 */
-	xsave->header.xfeatures |= xfeatures;
+	xsave->header.xfeatures |= hdr.xfeatures;
 
 	return 0;
 }
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index e04442345fc0..4e188fda5961 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -263,7 +263,7 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs *regs, size_t frame_size,
 		sp = (unsigned long) ka->sa.sa_restorer;
 	}
 
-	if (fpu->fpstate_active) {
+	if (fpu->initialized) {
 		sp = fpu__alloc_mathframe(sp, IS_ENABLED(CONFIG_X86_32),
 					  &buf_fx, &math_size);
 		*fpstate = (void __user *)sp;
@@ -279,7 +279,7 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs *regs, size_t frame_size,
 		return (void __user *)-1L;
 
 	/* save i387 and extended state */
-	if (fpu->fpstate_active &&
+	if (fpu->initialized &&
 	    copy_fpstate_to_sigframe(*fpstate, (void __user *)buf_fx, math_size) < 0)
 		return (void __user *)-1L;
 
@@ -755,7 +755,7 @@ handle_signal(struct ksignal *ksig, struct pt_regs *regs)
 		/*
 		 * Ensure the signal handler starts with the new fpu state.
 		 */
-		if (fpu->fpstate_active)
+		if (fpu->initialized)
 			fpu__clear(fpu);
 	}
 	signal_setup_done(failed, ksig, stepping);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cd17b7d9a107..03869eb7fcd6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7225,7 +7225,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 	int r;
 	sigset_t sigsaved;
 
-	fpu__activate_curr(fpu);
+	fpu__initialize(fpu);
 
 	if (vcpu->sigset_active)
 		sigprocmask(SIG_SETMASK, &vcpu->sigset, &sigsaved);
diff --git a/arch/x86/math-emu/fpu_entry.c b/arch/x86/math-emu/fpu_entry.c
index d4a7df2205b8..220638a4cb94 100644
--- a/arch/x86/math-emu/fpu_entry.c
+++ b/arch/x86/math-emu/fpu_entry.c
@@ -114,7 +114,7 @@ void math_emulate(struct math_emu_info *info)
 	struct desc_struct code_descriptor;
 	struct fpu *fpu = &current->thread.fpu;
 
-	fpu__activate_curr(fpu);
+	fpu__initialize(fpu);
 
 #ifdef RE_ENTRANT_CHECKING
 	if (emulating) {
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index c076f710de4c..c3521e2be396 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -2,6 +2,7 @@
 #include <linux/uaccess.h>
 #include <linux/sched/debug.h>
 
+#include <asm/fpu/internal.h>
 #include <asm/traps.h>
 #include <asm/kdebug.h>
 
@@ -78,6 +79,29 @@ bool ex_handler_refcount(const struct exception_table_entry *fixup,
 }
 EXPORT_SYMBOL_GPL(ex_handler_refcount);
 
+/*
+ * Handler for when we fail to restore a task's FPU state.  We should never get
+ * here because the FPU state of a task using the FPU (task->thread.fpu.state)
+ * should always be valid.  However, past bugs have allowed userspace to set
+ * reserved bits in the XSAVE area using PTRACE_SETREGSET or sys_rt_sigreturn().
+ * These caused XRSTOR to fail when switching to the task, leaking the FPU
+ * registers of the task previously executing on the CPU.  Mitigate this class
+ * of vulnerability by restoring from the initial state (essentially, zeroing
+ * out all the FPU registers) if we can't restore from the task's FPU state.
+ */
+bool ex_handler_fprestore(const struct exception_table_entry *fixup,
+			  struct pt_regs *regs, int trapnr)
+{
+	regs->ip = ex_fixup_addr(fixup);
+
+	WARN_ONCE(1, "Bad FPU state detected at %pB, reinitializing FPU registers.",
+		  (void *)instruction_pointer(regs));
+
+	__copy_kernel_to_fpregs(&init_fpstate, -1);
+	return true;
+}
+EXPORT_SYMBOL_GPL(ex_handler_fprestore);
+
 bool ex_handler_ext(const struct exception_table_entry *fixup,
 		   struct pt_regs *regs, int trapnr)
 {
diff --git a/arch/x86/mm/pkeys.c b/arch/x86/mm/pkeys.c
index 2dab69a706ec..d7bc0eea20a5 100644
--- a/arch/x86/mm/pkeys.c
+++ b/arch/x86/mm/pkeys.c
@@ -18,7 +18,6 @@
 
 #include <asm/cpufeature.h>             /* boot_cpu_has, ...            */
 #include <asm/mmu_context.h>            /* vma_pkey()                   */
-#include <asm/fpu/internal.h>           /* fpregs_active()              */
 
 int __execute_only_pkey(struct mm_struct *mm)
 {
@@ -45,7 +44,7 @@ int __execute_only_pkey(struct mm_struct *mm)
 	 */
 	preempt_disable();
 	if (!need_to_set_mm_pkey &&
-	    fpregs_active() &&
+	    current->thread.fpu.initialized &&
 	    !__pkru_allows_read(read_pkru(), execute_only_pkey)) {
 		preempt_enable();
 		return execute_only_pkey;

^ permalink raw reply related	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2017-09-27  7:40 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-24 10:59 [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header" Ingo Molnar
2017-09-24 10:59 ` [PATCH 01/10] x86/fpu: Introduce validate_xstate_header() Ingo Molnar
2017-09-26  8:34   ` [tip:x86/fpu] " tip-bot for Eric Biggers
2017-09-24 10:59 ` [PATCH 02/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in xstateregs_set() Ingo Molnar
2017-09-26  8:34   ` [tip:x86/fpu] " tip-bot for Eric Biggers
2017-09-24 10:59 ` [PATCH 03/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header in sanitize_restored_xstate() Ingo Molnar
2017-09-24 18:51   ` Eric Biggers
2017-09-24 19:02     ` Ingo Molnar
2017-09-24 20:08       ` Eric Biggers
2017-09-25  6:07         ` Ingo Molnar
2017-09-25  6:14         ` Ingo Molnar
2017-09-25  7:20           ` Eric Biggers
2017-09-25  7:30             ` Ingo Molnar
2017-09-26  8:35   ` [tip:x86/fpu] x86/fpu: Use validate_xstate_header() to validate the xstate_header in __fpu__restore_sig() tip-bot for Eric Biggers
2017-09-24 10:59 ` [PATCH 04/10] x86/fpu: Copy the full state_header in copy_kernel_to_xstate() Ingo Molnar
2017-09-26  8:35   ` [tip:x86/fpu] " tip-bot for Eric Biggers
2017-09-24 10:59 ` [PATCH 05/10] x86/fpu: Eliminate the 'xfeatures' local variable " Ingo Molnar
2017-09-26  8:35   ` [tip:x86/fpu] " tip-bot for Eric Biggers
2017-09-24 10:59 ` [PATCH 06/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header " Ingo Molnar
2017-09-26  8:36   ` [tip:x86/fpu] " tip-bot for Eric Biggers
2017-09-24 10:59 ` [PATCH 07/10] x86/fpu: Copy the full header in copy_user_to_xstate() Ingo Molnar
2017-09-26  8:36   ` [tip:x86/fpu] " tip-bot for Eric Biggers
2017-09-24 10:59 ` [PATCH 08/10] x86/fpu: Eliminate the 'xfeatures' local variable " Ingo Molnar
2017-09-26  8:37   ` [tip:x86/fpu] " tip-bot for Eric Biggers
2017-09-24 10:59 ` [PATCH 09/10] x86/fpu: Use validate_xstate_header() to validate the xstate_header " Ingo Molnar
2017-09-26  8:37   ` [tip:x86/fpu] " tip-bot for Eric Biggers
2017-09-24 10:59 ` [PATCH 10/10] x86/fpu: Use using_compacted_format() instead of open coded X86_FEATURE_XSAVES Ingo Molnar
2017-09-26  8:37   ` [tip:x86/fpu] " tip-bot for Eric Biggers
2017-09-24 18:04 ` [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header" Linus Torvalds
2017-09-24 19:01   ` Ingo Molnar
2017-09-26 16:28     ` [RFC GIT PULL] x86 FPU fixes and cleanups Ingo Molnar
2017-09-26 18:17       ` Linus Torvalds
2017-09-27  7:40         ` [RFC GIT PULL, v2] " Ingo Molnar

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).