linux-kernel - [PATCH 2/2] x86/fpu: Improve FPU APIs for independent features

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-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <657f57d4a50f0bdab7c74bf8ca47b413583b6db4.1623388344.git.luto@kernel.org>
Date:   Thu, 10 Jun 2021 22:13:37 -0700
From:   Andy Lutomirski <luto@...nel.org>
To:     x86@...nel.org
Cc:     Dave Hansen <dave.hansen@...el.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Andy Lutomirski <luto@...nel.org>
Subject: [PATCH 2/2] x86/fpu: Improve FPU APIs for independent features

Having the functions that save and restore independent features accept,
but ignore, non-independent features is confusing, so instead require
that only independent features be independently saved and restored.

Remove a bunch of nonsense comments from the save and restore functions:
saving and restoring the XSAVE header makes no sense.

Document that fpu__clear_all() does not clear independent features.

Signed-off-by: Andy Lutomirski <luto@...nel.org>
---
 arch/x86/include/asm/fpu/xstate.h |  5 ++++-
 arch/x86/kernel/fpu/core.c        |  3 +++
 arch/x86/kernel/fpu/xstate.c      | 33 +++++++++++--------------------
 3 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 5802b1715c7f..3bc5e6c9e47a 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -34,7 +34,10 @@
 				      XFEATURE_MASK_BNDREGS | \
 				      XFEATURE_MASK_BNDCSR)
 
-/* All currently supported supervisor features */
+/*
+ * All currently supported supervisor features that are saved and
+ * restored as part of the main task XSAVE buffers.
+ */
 #define XFEATURE_MASK_SUPERVISOR_SUPPORTED (XFEATURE_MASK_PASID)
 
 /*
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 571220ac8bea..9af361464c66 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -390,6 +390,9 @@ void fpu__clear_user_states(struct fpu *fpu)
 	fpu__clear(fpu, true);
 }
 
+/*
+ * This does not affect independent features -- see XFEATURE_MASK_INDEPENDENT.
+ */
 void fpu__clear_all(struct fpu *fpu)
 {
 	fpu__clear(fpu, false);
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index d582275164ad..15bb87f4f510 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1292,29 +1292,26 @@ void copy_supervisor_to_kernel(struct xregs_state *xstate)
  * @xstate: A pointer to an xsave area
  * @mask: Represent the independent supervisor features saved into the xsave area
  *
- * 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 XSAVE header in the target buffer must already be initialized, as the
+ * XSAVES instruction may not fully initialize the XSAVE header.
  *
  * The xsave area must be 64-bytes aligned.
  */
 void copy_independent_supervisor_to_kernel(struct xregs_state *xstate, u64 mask)
 {
-	u64 independent_mask = xfeatures_mask_independent() & mask;
 	u32 lmask, hmask;
 	int err;
 
+	WARN_ON_FPU(mask & ~xfeatures_mask_independent());
+
 	if (WARN_ON_FPU(!boot_cpu_has(X86_FEATURE_XSAVES)))
 		return;
 
-	if (WARN_ON_FPU(!independent_mask))
+	if (WARN_ON_FPU(!mask))
 		return;
 
-	lmask = independent_mask;
-	hmask = independent_mask >> 32;
+	lmask = mask;
+	hmask = mask >> 32;
 
 	XSTATE_OP(XSAVES, xstate, lmask, hmask, err);
 
@@ -1328,29 +1325,23 @@ void copy_independent_supervisor_to_kernel(struct xregs_state *xstate, u64 mask)
  * @xstate: A pointer to an xsave area
  * @mask: Represent the independent supervisor features restored from the xsave area
  *
- * 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.
- *
  * The xsave area must be 64-bytes aligned.
  */
 void copy_kernel_to_independent_supervisor(struct xregs_state *xstate, u64 mask)
 {
-	u64 independent_mask = xfeatures_mask_independent() & mask;
 	u32 lmask, hmask;
 	int err;
 
+	WARN_ON_FPU(mask & ~xfeatures_mask_independent());
+
 	if (WARN_ON_FPU(!boot_cpu_has(X86_FEATURE_XSAVES)))
 		return;
 
-	if (WARN_ON_FPU(!independent_mask))
+	if (WARN_ON_FPU(!mask))
 		return;
 
-	lmask = independent_mask;
-	hmask = independent_mask >> 32;
+	lmask = mask;
+	hmask = mask >> 32;
 
 	XSTATE_OP(XRSTORS, xstate, lmask, hmask, err);
 
-- 
2.31.1