linux-kernel - [PATCH v2] x86/fpu: copy MXCSR & MXCSR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170209184347.2ef977b9@annuminas.surriel.com>
Date:   Thu, 9 Feb 2017 18:43:47 -0500
From:   Rik van Riel <riel@...hat.com>
To:     mingo@...nel.org
Cc:     linux-kernel@...r.kernel.org, luto@...nel.org,
        dave.hansen@...ux.intel.com, yu-cheng.yu@...el.com, bp@...e.de,
        hpa@...or.com
Subject: [PATCH v2] x86/fpu: copy MXCSR & MXCSR_FLAGS with SSE/YMM state

On Skylake CPUs I noticed that XRSTOR is unable to deal with xsave areas
created by copyout_from_xsaves if the xstate has only SSE/YMM state, but
no FP state. That is, xfeatures had XFEATURE_MASK_SSE set, but not
XFEATURE_MASK_FP.

The reason is that part of the SSE/YMM state lives in the MXCSR and
MXCSR_FLAGS fields of the FP area.

Ensure that whenever we copy SSE or YMM state around, the MXCSR and
MXCSR_FLAGS fields are also copied around.

Signed-off-by: Rik van Riel <riel@...hat.com>
---
 arch/x86/kernel/fpu/xstate.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 772a069f8fbf..97d485157564 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -920,6 +920,23 @@ 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_need_mxcsr_copy(u64 xfeatures)
+{
+	if (!(xfeatures & (XFEATURE_MASK_SSE|XFEATURE_MASK_YMM)))
+		return 0;
+
+	if (xfeatures & XFEATURE_MASK_FP)
+		return 0;
+
+	return 1;
+}
+
+/*
  * This is similar to user_regset_copyout(), but will not add offset to
  * the source data pointer or increment pos, count, kbuf, and ubuf.
  */
@@ -987,6 +1004,13 @@ int copy_xstate_to_kernel(void *kbuf, struct xregs_state *xsave, unsigned int of
 
 	}
 
+	if (xfeatures_need_mxcsr_copy(header.xfeatures)) {
+		offset = offsetof(struct fxregs_state, mxcsr);
+		size = sizeof(u64); // copy mxcsr & mxcsr_flags
+		__copy_xstate_to_kernel(kbuf, &xsave->i387.mxcsr, offset,
+					size, size_total);
+	}
+
 	/*
 	 * Fill xsave->i387.sw_reserved value for ptrace frame:
 	 */
@@ -1069,6 +1093,13 @@ int copy_xstate_to_user(void __user *ubuf, struct xregs_state *xsave, unsigned i
 
 	}
 
+	if (xfeatures_need_mxcsr_copy(header.xfeatures)) {
+		offset = offsetof(struct fxregs_state, mxcsr);
+		size = sizeof(u64); // copy mxcsr & mxcsr_flags
+		__copy_xstate_to_user(ubuf, &xsave->i387.mxcsr, offset,
+					size, size_total);
+	}
+
 	/*
 	 * Fill xsave->i387.sw_reserved value for ptrace frame:
 	 */
@@ -1121,6 +1152,12 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 		}
 	}
 
+	if (xfeatures_need_mxcsr_copy(xfeatures)) {
+		offset = offsetof(struct fxregs_state, mxcsr);
+		size = sizeof(u64); // copy mxcsr & mxcsr_flags
+		memcpy(&xsave->i387.mxcsr, kbuf + offset, size);
+	}
+
 	/*
 	 * The state that came in from userspace was user-state only.
 	 * Mask all the user states out of 'xfeatures':
@@ -1176,6 +1213,13 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 		}
 	}
 
+	if (xfeatures_need_mxcsr_copy(xfeatures)) {
+		offset = offsetof(struct fxregs_state, mxcsr);
+		size = sizeof(u64); // copy mxcsr & mxcsr_flags
+		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':