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

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-next>] [day] [month] [year] [list]
Date:   Sun, 24 Sep 2017 12:59:03 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     linux-kernel@...r.kernel.org
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Eric Biggers <ebiggers3@...il.com>,
        Andy Lutomirski <luto@...capital.net>,
        Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        "H . Peter Anvin" <hpa@...or.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Oleg Nesterov <oleg@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Rik van Riel <riel@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Yu-cheng Yu <yu-cheng.yu@...el.com>
Subject: [PATCH 00/10] x86/fpu: Split up "x86/fpu: Tighten validation of user-supplied xstate_header"

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++) {