| 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: <20210614155356.961460910@linutronix.de>
Date: Mon, 14 Jun 2021 17:44:40 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: LKML <linux-kernel@...r.kernel.org>
Cc: Andy Lutomirski <luto@...nel.org>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Fenghua Yu <fenghua.yu@...el.com>,
Tony Luck <tony.luck@...el.com>,
Yu-cheng Yu <yu-cheng.yu@...el.com>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Borislav Petkov <bp@...e.de>,
Peter Zijlstra <peterz@...radead.org>,
Kan Liang <kan.liang@...ux.intel.com>
Subject: [patch V2 32/52] x86/fpu/xstate: Sanitize handling of independent features
The copy functions for the independent features are horribly named and the
supervisor and independent part is just overengineered.
The point is that the supplied mask has either to be a subset of the
independent feature or a subset of the task->fpu.xstate managed features.
Rewrite it so it checks check for invalid overlaps of these areas in the
caller supplied feature mask. Rename it so it follows the new naming
convention for these operations. Mop up the function documentation.
This allows to use that function for other purposes as well.
Suggested-by: Peter Zijlstra <peterz@...radead.org>
Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
Cc: Kan Liang <kan.liang@...ux.intel.com>
---
arch/x86/events/intel/lbr.c | 6 +-
arch/x86/include/asm/fpu/xstate.h | 5 +-
arch/x86/kernel/fpu/xstate.c | 93 +++++++++++++++++++-------------------
3 files changed, 53 insertions(+), 51 deletions(-)
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -491,7 +491,7 @@ static void intel_pmu_arch_lbr_xrstors(v
{
struct x86_perf_task_context_arch_lbr_xsave *task_ctx = ctx;
- copy_kernel_to_independent_supervisor(&task_ctx->xsave, XFEATURE_MASK_LBR);
+ xrstors_from_kernel(&task_ctx->xsave, XFEATURE_MASK_LBR);
}
static __always_inline bool lbr_is_reset_in_cstate(void *ctx)
@@ -576,7 +576,7 @@ static void intel_pmu_arch_lbr_xsaves(vo
{
struct x86_perf_task_context_arch_lbr_xsave *task_ctx = ctx;
- copy_independent_supervisor_to_kernel(&task_ctx->xsave, XFEATURE_MASK_LBR);
+ xsaves_to_kernel(&task_ctx->xsave, XFEATURE_MASK_LBR);
}
static void __intel_pmu_lbr_save(void *ctx)
@@ -992,7 +992,7 @@ static void intel_pmu_arch_lbr_read_xsav
intel_pmu_store_lbr(cpuc, NULL);
return;
}
- copy_independent_supervisor_to_kernel(&xsave->xsave, XFEATURE_MASK_LBR);
+ xsaves_to_kernel(&xsave->xsave, XFEATURE_MASK_LBR);
intel_pmu_store_lbr(cpuc, xsave->lbr.entries);
}
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -104,8 +104,9 @@ void *get_xsave_addr(struct xregs_state
int xfeature_size(int xfeature_nr);
int copy_uabi_from_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf);
int copy_sigframe_from_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf);
-void copy_independent_supervisor_to_kernel(struct xregs_state *xstate, u64 mask);
-void copy_kernel_to_independent_supervisor(struct xregs_state *xstate, u64 mask);
+
+void xsaves_to_kernel(struct xregs_state *xsave, u64 mask);
+void xrstors_from_kernel(struct xregs_state *xsave, u64 mask);
enum xstate_copy_mode {
XSTATE_COPY_FP,
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1163,75 +1163,76 @@ int copy_sigframe_from_user_to_xstate(st
}
/**
- * copy_independent_supervisor_to_kernel() - Save independent supervisor states to
- * an xsave area
- * @xstate: A pointer to an xsave area
- * @mask: Represent the independent supervisor features saved into the xsave area
+ * xsaves_to_kernel - Save selected components to a kernel xstate buffer
+ * @xstate: Pointer to the buffer
+ * @mask: Feature mask to select the components to save
*
- * Only the independent supervisor states sets in the mask are saved into the xsave
- * area (See the comment in XFEATURE_MASK_INDEPENDENT for the details of independent
- * supervisor feature). Besides the independent supervisor states, the legacy
- * region and XSAVE header are also saved into the xsave area. The supervisor
- * features in the XFEATURE_MASK_SUPERVISOR_SUPPORTED and
- * XFEATURE_MASK_SUPERVISOR_UNSUPPORTED are not saved.
+ * The @xstate buffer must be 64 byte aligned and correctly initialized as
+ * XSAVES does not write the full xstate header. Before first use the
+ * buffer should be zeroed otherwise a consecutive XRSTORS from that buffer
+ * can #GP.
*
- * The xsave area must be 64-bytes aligned.
+ * The feature mask must either be a subset of the independent features or
+ * a subset of the task->fpstate related features
*/
-void copy_independent_supervisor_to_kernel(struct xregs_state *xstate, u64 mask)
+void xsaves_to_kernel(struct xregs_state *xstate, u64 mask)
{
- u64 independent_mask = xfeatures_mask_independent() & mask;
- u32 lmask, hmask;
+ u64 xchk;
int err;
- if (WARN_ON_FPU(!boot_cpu_has(X86_FEATURE_XSAVES)))
+ if (WARN_ON_FPU(!cpu_feature_enabled(X86_FEATURE_XSAVES)))
return;
+ /*
+ * Validate that this is either a task->fpstate related component
+ * subset or an independent one.
+ */
+ if (mask & xfeatures_mask_independent())
+ xchk = ~xfeatures_mask_independent();
+ else
+ xchk = ~xfeatures_mask_all;
- if (WARN_ON_FPU(!independent_mask))
+ if (WARN_ON_ONCE(!mask || mask & xchk))
return;
- lmask = independent_mask;
- hmask = independent_mask >> 32;
-
- XSTATE_OP(XSAVES, xstate, lmask, hmask, err);
-
- /* Should never fault when copying to a kernel buffer */
- WARN_ON_FPU(err);
+ XSTATE_OP(XSAVES, xstate, (u32)mask, (u32)(mask >> 32), err);
+ WARN_ON_ONCE(err);
}
/**
- * copy_kernel_to_independent_supervisor() - Restore independent supervisor states from
- * an xsave area
- * @xstate: A pointer to an xsave area
- * @mask: Represent the independent supervisor features restored from the xsave area
+ * xrstors_from_kernel - Restore selected components from a kernel xstate buffer
+ * @xstate: Pointer to the buffer
+ * @mask: Feature mask to select the components to restore
+ *
+ * The @xstate buffer must be 64 byte aligned and correctly initialized
+ * otherwise XRSTORS from that buffer can #GP.
*
- * Only the independent supervisor states sets in the mask are restored from the
- * xsave area (See the comment in XFEATURE_MASK_INDEPENDENT for the details of
- * independent supervisor feature). Besides the independent supervisor states, the
- * legacy region and XSAVE header are also restored from the xsave area. The
- * supervisor features in the XFEATURE_MASK_SUPERVISOR_SUPPORTED and
- * XFEATURE_MASK_SUPERVISOR_UNSUPPORTED are not restored.
+ * Proper usage is to restore the state which was saved with
+ * xsaves_to_kernel() into @xstate.
*
- * The xsave area must be 64-bytes aligned.
+ * The feature mask must either be a subset of the independent features or
+ * a subset of the task->fpstate related features
*/
-void copy_kernel_to_independent_supervisor(struct xregs_state *xstate, u64 mask)
+void xrstors_from_kernel(struct xregs_state *xstate, u64 mask)
{
- u64 independent_mask = xfeatures_mask_independent() & mask;
- u32 lmask, hmask;
+ u64 xchk;
int err;
- if (WARN_ON_FPU(!boot_cpu_has(X86_FEATURE_XSAVES)))
+ if (WARN_ON_FPU(!cpu_feature_enabled(X86_FEATURE_XSAVES)))
return;
+ /*
+ * Validate that this is either a task->fpstate related component
+ * subset or an independent one.
+ */
+ if (mask & xfeatures_mask_independent())
+ xchk = ~xfeatures_mask_independent();
+ else
+ xchk = ~xfeatures_mask_all;
- if (WARN_ON_FPU(!independent_mask))
+ if (WARN_ON_ONCE(!mask || mask & xchk))
return;
- lmask = independent_mask;
- hmask = independent_mask >> 32;
-
- XSTATE_OP(XRSTORS, xstate, lmask, hmask, err);
-
- /* Should never fault when copying from a kernel buffer */
- WARN_ON_FPU(err);
+ XSTATE_OP(XRSTORS, xstate, (u32)mask, (u32)(mask >> 32), err);
+ WARN_ON_ONCE(err);
}
#ifdef CONFIG_PROC_PID_ARCH_STATUS
Powered by blists - more mailing lists