[PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header"

[Ingo Molnar <mingo@kernel.org>]

As mentioned before, the patch was too big and too complex, and I've split it
up into 10 smaller, bisectable patches:

Eric Biggers (10):
  x86/fpu: Introduce validate_xstate_header()
  x86/fpu: Use validate_xstate_header() to validate the xstate_header in xstateregs_set()
  x86/fpu: Use validate_xstate_header() to validate the xstate_header in sanitize_restored_xstate()
  x86/fpu: Copy the full state_header in copy_kernel_to_xstate()
  x86/fpu: Eliminate the 'xfeatures' local variable in copy_kernel_to_xstate()
  x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_kernel_to_xstate()
  x86/fpu: Copy the full header in copy_user_to_xstate()
  x86/fpu: Eliminate the 'xfeatures' local variable in copy_user_to_xstate()
  x86/fpu: Use validate_xstate_header() to validate the xstate_header in copy_user_to_xstate()
  x86/fpu: Use using_compacted_format() instead of open coded X86_FEATURE_XSAVES

 arch/x86/include/asm/fpu/xstate.h |  4 ++++
 arch/x86/kernel/fpu/regset.c      | 21 ++++++-----------
 arch/x86/kernel/fpu/signal.c      | 17 ++++++-------
 arch/x86/kernel/fpu/xstate.c      | 76 +++++++++++++++++++++++++++++++++--------------------------
 4 files changed, 63 insertions(+), 55 deletions(-)

I kept the attribution in place, because the end result is almost the same.

Below is the interdiff from the original version, I uninlined validate_xstate_header(),
because it's way too large to be inlined everywhere, plus I removed a couple of
spurious "!= 0" patterns.

The latest x86/fpu bits can be found in -tip:

    git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git master
    git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git WIP.x86/fpu

Thanks,

    Ingo

===
 include/asm/fpu/xstate.h |   23 +----------------------
 kernel/fpu/xstate.c      |   28 ++++++++++++++++++++++++++--
 2 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
index 3d79d0ee4d30..83fee2469eb7 100644
--- a/arch/x86/include/asm/fpu/xstate.h
+++ b/arch/x86/include/asm/fpu/xstate.h
@@ -54,27 +54,6 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf);
 int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf);
 
 /* Validate an xstate header supplied by userspace (ptrace or sigreturn) */
-static inline int validate_xstate_header(const struct xstate_header *hdr)
-{
-	/* No unknown or supervisor features may be set */
-	if (hdr->xfeatures & (~xfeatures_mask | XFEATURE_MASK_SUPERVISOR))
-		return -EINVAL;
-
-	/* Userspace must use the uncompacted format */
-	if (hdr->xcomp_bv)
-		return -EINVAL;
-
-	/*
-	 * If 'reserved' is shrunken to add a new field, make sure to validate
-	 * that new field here!
-	 */
-	BUILD_BUG_ON(sizeof(hdr->reserved) != 48);
-
-	/* No reserved bits may be set */
-	if (memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
-		return -EINVAL;
-
-	return 0;
-}
+extern int validate_xstate_header(const struct xstate_header *hdr);
 
 #endif
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 7ebd1a0811b6..f1d5476c9022 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -483,6 +483,30 @@ int using_compacted_format(void)
 	return boot_cpu_has(X86_FEATURE_XSAVES);
 }
 
+/* Validate an xstate header supplied by userspace (ptrace or sigreturn) */
+int validate_xstate_header(const struct xstate_header *hdr)
+{
+	/* No unknown or supervisor features may be set */
+	if (hdr->xfeatures & (~xfeatures_mask | XFEATURE_MASK_SUPERVISOR))
+		return -EINVAL;
+
+	/* Userspace must use the uncompacted format */
+	if (hdr->xcomp_bv)
+		return -EINVAL;
+
+	/*
+	 * If 'reserved' is shrunken to add a new field, make sure to validate
+	 * that new field here!
+	 */
+	BUILD_BUG_ON(sizeof(hdr->reserved) != 48);
+
+	/* No reserved bits may be set */
+	if (memchr_inv(hdr->reserved, 0, sizeof(hdr->reserved)))
+		return -EINVAL;
+
+	return 0;
+}
+
 static void __xstate_dump_leaves(void)
 {
 	int i;
@@ -1127,7 +1151,7 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf)
 
 	memcpy(&hdr, kbuf + offset, size);
 
-	if (validate_xstate_header(&hdr) != 0)
+	if (validate_xstate_header(&hdr))
 		return -EINVAL;
 
 	for (i = 0; i < XFEATURE_MAX; i++) {
@@ -1181,7 +1205,7 @@ int copy_user_to_xstate(struct xregs_state *xsave, const void __user *ubuf)
 	if (__copy_from_user(&hdr, ubuf + offset, size))
 		return -EFAULT;
 
-	if (validate_xstate_header(&hdr) != 0)
+	if (validate_xstate_header(&hdr))
 		return -EINVAL;
 
 	for (i = 0; i < XFEATURE_MAX; i++) {