All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Andy Lutomirski <luto@kernel.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Fenghua Yu <fenghua.yu@intel.com>,
	Tony Luck <tony.luck@intel.com>,
	Yu-cheng Yu <yu-cheng.yu@intel.com>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Borislav Petkov <bp@suse.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Kan Liang <kan.liang@linux.intel.com>
Subject: [patch 22/41] x86/fpu/xstate: Sanitize handling of independent features
Date: Fri, 11 Jun 2021 18:15:45 +0200	[thread overview]
Message-ID: <20210611163113.307062186@linutronix.de> (raw)
In-Reply-To: 20210611161523.508908024@linutronix.de

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@infradead.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Kan Liang <kan.liang@linux.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
@@ -107,7 +107,8 @@ struct membuf;
 void copy_uabi_xstate_to_membuf(struct membuf to, struct xregs_state *xsave);
 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);
 
 #endif
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1107,75 +1107,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


  parent reply	other threads:[~2021-06-11 16:45 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-11 16:15 [patch 00/41] x86/fpu: Spring cleaning and PKRU sanitizing Thomas Gleixner
2021-06-11 16:15 ` [patch 01/41] Revert a5eff7259790 ("x86/pkeys: Add PKRU value to init_fpstate") Thomas Gleixner
2021-06-11 17:04   ` Borislav Petkov
2021-06-11 16:15 ` [patch 02/41] x86/fpu: Mark various FPU states __ro_after_init Thomas Gleixner
2021-06-11 17:21   ` Borislav Petkov
2021-06-11 18:35   ` Andy Lutomirski
2021-06-11 16:15 ` [patch 03/41] x86/fpu: Remove unused get_xsave_field_ptr() Thomas Gleixner
2021-06-11 18:35   ` Andy Lutomirski
2021-06-11 16:15 ` [patch 04/41] x86/fpu: Move inlines where they belong Thomas Gleixner
2021-06-11 16:15 ` [patch 05/41] x86/fpu: Limit xstate copy size in xstateregs_set() Thomas Gleixner
2021-06-11 18:15   ` Borislav Petkov
2021-06-11 18:37   ` Andy Lutomirski
2021-06-11 19:37     ` Thomas Gleixner
2021-06-11 16:15 ` [patch 06/41] x86/fpu: Sanitize xstateregs_set() Thomas Gleixner
2021-06-11 18:45   ` Andy Lutomirski
2021-06-11 20:23     ` Thomas Gleixner
2021-06-11 16:15 ` [patch 07/41] x86/fpu: Simplify PTRACE_GETREGS code Thomas Gleixner
2021-06-11 18:47   ` Andy Lutomirski
2021-06-12  9:13   ` Borislav Petkov
2021-06-11 16:15 ` [patch 08/41] x86/fpu: Restrict fpstate sanitizing to legacy components Thomas Gleixner
2021-06-11 19:03   ` Andy Lutomirski
2021-06-11 19:18     ` Andy Lutomirski
2021-06-11 20:33       ` Thomas Gleixner
2021-06-11 20:34         ` Thomas Gleixner
2021-06-11 20:27     ` Thomas Gleixner
2021-06-11 22:12     ` Thomas Gleixner
2021-06-12 13:15       ` Thomas Gleixner
2021-06-12 22:05       ` Thomas Gleixner
2021-06-11 16:15 ` [patch 09/41] x86/kvm: Avoid looking up PKRU in XSAVE buffer Thomas Gleixner
2021-06-14 10:26   ` Borislav Petkov
2021-06-14 19:34     ` Dave Hansen
2021-06-15 10:09       ` Borislav Petkov
2021-06-11 16:15 ` [patch 10/41] x86/fpu: Cleanup arch_set_user_pkey_access() Thomas Gleixner
2021-06-11 16:15 ` [patch 11/41] x86/fpu: Get rid of copy_supervisor_to_kernel() Thomas Gleixner
2021-06-11 19:42   ` Andy Lutomirski
2021-06-11 16:15 ` [patch 12/41] x86/fpu: Rename copy_xregs_to_kernel() and copy_kernel_to_xregs() Thomas Gleixner
2021-06-11 16:15 ` [patch 13/41] x86/fpu: Rename copy_user_to_xregs() and copy_xregs_to_user() Thomas Gleixner
2021-06-11 16:15 ` [patch 14/41] x86/fpu: Rename fxregs related copy functions Thomas Gleixner
2021-06-11 16:15 ` [patch 15/41] x86/fpu: Rename fregs " Thomas Gleixner
2021-06-11 16:15 ` [patch 16/41] x86/fpu: Rename xstate copy functions which are related to UABI Thomas Gleixner
2021-06-11 16:15 ` [patch 17/41] x86/fpu: Deduplicate copy_uabi_from_user/kernel_to_xstate() Thomas Gleixner
2021-06-11 16:15 ` [patch 18/41] x86/fpu: Rename copy_fpregs_to_fpstate() to save_fpregs_to_fpstate() Thomas Gleixner
2021-06-11 16:15 ` [patch 19/41] x86/fpu: Rename copy_kernel_to_fpregs() to restore_fpregs_from_kernel() Thomas Gleixner
2021-06-11 16:15 ` [patch 20/41] x86/fpu: Rename initstate copy functions Thomas Gleixner
2021-06-11 16:15 ` [patch 21/41] x86/fpu: Rename "dynamic" XSTATEs to "independent" Thomas Gleixner
2021-06-11 16:15 ` Thomas Gleixner [this message]
2021-06-11 16:15 ` [patch 23/41] x86/pkeys: Move read_pkru() and write_pkru() Thomas Gleixner
2021-06-11 16:15 ` [patch 24/41] x86/fpu: Differentiate "copy" versus "move" of fpregs Thomas Gleixner
2021-06-11 16:15 ` [patch 25/41] x86/cpu: Sanitize X86_FEATURE_OSPKE Thomas Gleixner
2021-06-11 16:15 ` [patch 26/41] x86/pkru: Provide pkru_get_init_value() Thomas Gleixner
2021-06-11 16:15 ` [patch 27/41] x86/pkru: Provide pkru_write_default() Thomas Gleixner
2021-06-11 16:15 ` [patch 28/41] x86/cpu: Write the default PKRU value when enabling PKE Thomas Gleixner
2021-06-11 16:15 ` [patch 29/41] x86/fpu: Use pkru_write_default() in copy_init_fpstate_to_fpregs() Thomas Gleixner
2021-06-11 16:15 ` [patch 30/41] x86/fpu: Rename fpu__clear_all() to fpu_flush_thread() Thomas Gleixner
2021-06-11 16:15 ` [patch 31/41] x86/fpu: Clean up the fpu__clear() variants Thomas Gleixner
2021-06-11 16:15 ` [patch 32/41] x86/fpu: Rename __fpregs_load_activate() to fpregs_restore_userregs() Thomas Gleixner
2021-06-11 16:15 ` [patch 33/41] x86/fpu: Move FXSAVE_LEAK quirk info __copy_kernel_to_fpregs() Thomas Gleixner
2021-06-11 16:15 ` [patch 34/41] x86/fpu: Rename xfeatures_mask_user() to xfeatures_mask_uabi() Thomas Gleixner
2021-06-11 16:15 ` [patch 35/41] x86/fpu: Dont restore PKRU in fpregs_restore_userspace() Thomas Gleixner
2021-06-11 16:15 ` [patch 36/41] x86/fpu: Add PKRU storage outside of task XSAVE buffer Thomas Gleixner
2021-06-11 16:16 ` [patch 37/41] x86/fpu: Hook up PKRU into ptrace() Thomas Gleixner
2021-06-11 16:16 ` [patch 38/41] x86/fpu: Mask PKRU from kernel XRSTOR[S] operations Thomas Gleixner
2021-06-11 16:16 ` [patch 39/41] x86/fpu: Remove PKRU handling from switch_fpu_finish() Thomas Gleixner
2021-06-11 16:16 ` [patch 40/41] x86/fpu: Dont store PKRU in xstate in fpu_reset_fpstate() Thomas Gleixner
2021-06-11 16:16 ` [patch 41/41] x86/pkru: Remove xstate fiddling from write_pkru() Thomas Gleixner
2021-06-12  0:24 ` [patch 00/41] x86/fpu: Spring cleaning and PKRU sanitizing Thomas Gleixner
2021-06-12  0:40   ` Dave Hansen
2021-06-16 20:55   ` Dave Hansen
2021-06-17  7:06     ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210611163113.307062186@linutronix.de \
    --to=tglx@linutronix.de \
    --cc=bigeasy@linutronix.de \
    --cc=bp@suse.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=fenghua.yu@intel.com \
    --cc=kan.liang@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tony.luck@intel.com \
    --cc=yu-cheng.yu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.