| 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
| ||
|
Message-Id: <20220916201158.8072-2-chang.seok.bae@intel.com>
Date: Fri, 16 Sep 2022 13:11:55 -0700
From: "Chang S. Bae" <chang.seok.bae@...el.com>
To: linux-kernel@...r.kernel.org
Cc: x86@...nel.org, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
dave.hansen@...ux.intel.com, hpa@...or.com, avagin@...il.com,
chang.seok.bae@...el.com
Subject: [PATCH 1/4] x86/fpu: Fix the MXCSR state reshuffling between userspace and kernel buffers
== Hardware Background ==
The MXCSR state, as part of the SSE component, has different treatments
with XSAVE*/XRSTOR* between the two XSAVE formats:
- When the MXCSR state is XSAVEd in the non-compacted format, the feature
bit in XSTATE_BV pertains to the XMM registers. XRSTOR restores the MXCSR
state without referencing XSTATE_BV.
- But, on XSAVE* with the compacted format, the SSE bit is set in XSTATE_BV
if MXCSR is not in the init state on XSAVE*. Then, on XRSTOR*, the state is
restored only when the SSE bit is set in XSTATE_BV.
== Regression ==
The XSTATE copy routine between userspace and kernel buffers used to be
separate for different XSAVE formats. Commit 43be46e89698 ("x86/fpu:
Sanitize xstateregs_set()") combined them together.
This change appears to be a regression on XSAVES-less systems. But, the
merged code is based on the original conversion code with commit
91c3dba7dbc1 ("x86/fpu/xstate: Fix PTRACE frames for XSAVES").
That has such oversight as:
- Mistreating MXCSR as part of the FP state instead of the SSE component.
- Taking the SSE bit in XSTATE_BV always for all the SSE states.
== Correction ==
Update the XSTATE conversion code:
- Refactor the copy routine for legacy states. Treat MXCSR as part of SSE.
- Copying MXCSR, reference XSTATE_BV only with the compacted format.
- Also, flip the SSE bit in XSTATE_BV accordingly to the format.
Reported-by: Andrei Vagin <avagin@...il.com>
Fixes: 91c3dba7dbc1 ("x86/fpu/xstate: Fix PTRACE frames for XSAVES")
Signed-off-by: Chang S. Bae <chang.seok.bae@...el.com>
Cc: x86@...nel.org
Cc: linux-kernel@...r.kernel.org
Link: https://lore.kernel.org/lkml/CANaxB-wkcNKWjyNGFuMn6f6H2DQSGwwQjUgg1eATdUgmM-Kg+A@mail.gmail.com/
---
arch/x86/kernel/fpu/xstate.c | 70 +++++++++++++++++++++++++-----------
1 file changed, 49 insertions(+), 21 deletions(-)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index c8340156bfd2..d7676cfc32eb 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1064,6 +1064,7 @@ void __copy_xstate_to_uabi_buf(struct membuf to, struct fpstate *fpstate,
u32 pkru_val, enum xstate_copy_mode copy_mode)
{
const unsigned int off_mxcsr = offsetof(struct fxregs_state, mxcsr);
+ bool compacted = cpu_feature_enabled(X86_FEATURE_XCOMPACTED);
struct xregs_state *xinit = &init_fpstate.regs.xsave;
struct xregs_state *xsave = &fpstate->regs.xsave;
struct xstate_header header;
@@ -1093,8 +1094,13 @@ void __copy_xstate_to_uabi_buf(struct membuf to, struct fpstate *fpstate,
copy_feature(header.xfeatures & XFEATURE_MASK_FP, &to, &xsave->i387,
&xinit->i387, off_mxcsr);
- /* Copy MXCSR when SSE or YMM are set in the feature mask */
- copy_feature(header.xfeatures & (XFEATURE_MASK_SSE | XFEATURE_MASK_YMM),
+ /*
+ * Copy MXCSR depending on the XSAVE format. If compacted,
+ * reference the feature mask. Otherwise, check if any of related
+ * features is valid.
+ */
+ copy_feature(compacted ? header.xfeatures & XFEATURE_MASK_SSE :
+ fpstate->user_xfeatures & (XFEATURE_MASK_SSE | XFEATURE_MASK_YMM),
&to, &xsave->i387.mxcsr, &xinit->i387.mxcsr,
MXCSR_AND_FLAGS_SIZE);
@@ -1199,6 +1205,11 @@ static int copy_from_buffer(void *dst, unsigned int offset, unsigned int size,
static int copy_uabi_to_xstate(struct fpstate *fpstate, const void *kbuf,
const void __user *ubuf)
{
+ const unsigned int off_stspace = offsetof(struct fxregs_state, st_space);
+ const unsigned int off_xmm = offsetof(struct fxregs_state, xmm_space);
+ const unsigned int off_mxcsr = offsetof(struct fxregs_state, mxcsr);
+ bool compacted = cpu_feature_enabled(X86_FEATURE_XCOMPACTED);
+ struct fxregs_state *fxsave = &fpstate->regs.fxsave;
struct xregs_state *xsave = &fpstate->regs.xsave;
unsigned int offset, size;
struct xstate_header hdr;
@@ -1212,38 +1223,48 @@ static int copy_uabi_to_xstate(struct fpstate *fpstate, const void *kbuf,
if (validate_user_xstate_header(&hdr, fpstate))
return -EINVAL;
- /* Validate MXCSR when any of the related features is in use */
- mask = XFEATURE_MASK_FP | XFEATURE_MASK_SSE | XFEATURE_MASK_YMM;
- if (hdr.xfeatures & mask) {
+ if (hdr.xfeatures & XFEATURE_MASK_FP) {
+ if (copy_from_buffer(fxsave, 0, off_mxcsr, kbuf, ubuf))
+ return -EINVAL;
+ if (copy_from_buffer(fxsave->st_space, off_stspace, sizeof(fxsave->st_space),
+ kbuf, ubuf))
+ return -EINVAL;
+ }
+
+ /* Validate MXCSR when any of the related features is valid. */
+ mask = XFEATURE_MASK_SSE | XFEATURE_MASK_YMM;
+ if (fpstate->user_xfeatures & mask) {
u32 mxcsr[2];
- offset = offsetof(struct fxregs_state, mxcsr);
- if (copy_from_buffer(mxcsr, offset, sizeof(mxcsr), kbuf, ubuf))
+ if (copy_from_buffer(mxcsr, off_mxcsr, sizeof(mxcsr), kbuf, ubuf))
return -EFAULT;
/* Reserved bits in MXCSR must be zero. */
if (mxcsr[0] & ~mxcsr_feature_mask)
return -EINVAL;
- /* SSE and YMM require MXCSR even when FP is not in use. */
- if (!(hdr.xfeatures & XFEATURE_MASK_FP)) {
- xsave->i387.mxcsr = mxcsr[0];
- xsave->i387.mxcsr_mask = mxcsr[1];
- }
+ /*
+ * Copy MXCSR regardless of the feature mask as userspace
+ * uses the uncompacted format.
+ */
+ fxsave->mxcsr = mxcsr[0];
+ fxsave->mxcsr_mask = mxcsr[1];
}
- for (i = 0; i < XFEATURE_MAX; i++) {
- mask = BIT_ULL(i);
+ if (hdr.xfeatures & XFEATURE_MASK_SSE) {
+ if (copy_from_buffer(fxsave->xmm_space, off_xmm, sizeof(fxsave->xmm_space),
+ kbuf, ubuf))
+ return -EINVAL;
+ }
- if (hdr.xfeatures & mask) {
- void *dst = __raw_xsave_addr(xsave, i);
+ for_each_extended_xfeature(i, hdr.xfeatures) {
+ void *dst = __raw_xsave_addr(xsave, i);
- offset = xstate_offsets[i];
- size = xstate_sizes[i];
+ offset = xstate_offsets[i];
+ size = xstate_sizes[i];
- if (copy_from_buffer(dst, offset, size, kbuf, ubuf))
- return -EFAULT;
- }
+ if (copy_from_buffer(dst, offset, size, kbuf, ubuf))
+ return -EFAULT;
}
/*
@@ -1256,6 +1277,13 @@ static int copy_uabi_to_xstate(struct fpstate *fpstate, const void *kbuf,
* Add back in the features that came in from userspace:
*/
xsave->header.xfeatures |= hdr.xfeatures;
+ /*
+ * Convert the SSE bit in the feature mask as it implies
+ * differently between the formats. It indicates the MXCSR state
+ * if compacted; otherwise, it pertains to XMM registers.
+ */
+ if (compacted && fxsave->mxcsr != MXCSR_DEFAULT)
+ xsave->header.xfeatures |= XFEATURE_MASK_SSE;
return 0;
}
--
2.17.1
Powered by blists - more mailing lists