linux-kernel - [PATCH 2/2] x86, fpu: fix boot time GPF with eagerfpu=off

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

Message-Id: <20150428170010.D2827334@viggo.jf.intel.com>
Date:	Tue, 28 Apr 2015 10:00:10 -0700
From:	Dave Hansen <dave@...1.net>
To:	linux-kernel@...r.kernel.org
Cc:	Dave Hansen <dave@...1.net>, dave.hansen@...ux.intel.com,
	x86@...nel.org, fenghua.yu@...el.com, bp@...en8.de,
	oleg@...hat.com, luto@...capital.net
Subject: [PATCH 2/2] x86, fpu: fix boot time GPF with eagerfpu=off


From: Dave Hansen <dave.hansen@...ux.intel.com>

I'm seeing some widespread failures to boot if I boot with
eagerfpu=off.  This only happens on hardware that supports
'xsaves', which is not widely available, so it is not
seen widely in practice.

The failure comes from init hitting a GPF very early in its
execution.  The GPF comes from a 'xrstors' operation that
the kernel performs which kills init.

The problem is essentially that "fpu_finit()" does not have
any knowledge about how to initialize an FPU buffer
(tsk->fpu->state) which will be restored with xsaves.  This
does not hit the use_eager_fpu() xsaves code because it
always does a __save_fpu() to initialize the FPU state from
the the process doing the fork()'s *registers* instead of
the 'init_xstate_buf' via init_fpu().

The code calls fx_finit(&fpu->state->fxsave) on the FPU state
to initialize the 'fxsave' style FPU buffer and then soon
after does an 'xrstors'-style restore.  That promptly GPF's.

As far as I can tell init_fpu() is only called when a task
has !used_math().  That never happens in the eagerfpu=on
case.

We never noticed this problem because this commit:

e7d820a5: x86, xsave: Support eager-only xsave features, add MPX support

effectively disabled lazy FPU mode on all hardware that
supports xsaves.  I think that commit itself is wrong
and there is no fundamental reason we can not support
lazy FPU mode with the MPX registers being xsave'd.  But,
that's another patch.

Signed-off-by: Dave Hansen <dave.hansen@...ux.intel.com>
Cc: x86@...nel.org
Cc: Fenghua Yu <fenghua.yu@...el.com>
Cc: Borislav Petkov <bp@...en8.de>
Cc: Oleg Nesterov <oleg@...hat.com>
Cc: Andy Lutomirski <luto@...capital.net>
---

 b/arch/x86/kernel/i387.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff -puN arch/x86/kernel/i387.c~x86-fpu-fix-boot-time-gpf-with-eagerfpuoff arch/x86/kernel/i387.c
--- a/arch/x86/kernel/i387.c~x86-fpu-fix-boot-time-gpf-with-eagerfpuoff	2015-04-28 09:51:55.634326943 -0700
+++ b/arch/x86/kernel/i387.c	2015-04-28 09:53:23.065270248 -0700
@@ -19,6 +19,7 @@
 #include <asm/i387.h>
 #include <asm/fpu-internal.h>
 #include <asm/user.h>
+#include <asm/xcr.h>
 
 static DEFINE_PER_CPU(bool, in_kernel_fpu);
 
@@ -218,6 +219,41 @@ void fpu_init(void)
 	eager_fpu_init();
 }
 
+/*
+ * This will take the 'xsave' portion of a 'thread_xstate'
+ * and get it in to a state where an 'xrstor' or 'xrstors'
+ * of the buffer will result in all of the xsave state
+ * components being set to their initialized state.
+ *
+ * We initialize the minimal set of fields in the header
+ * and rely on the CPU to initialize the registers
+ * themselves.
+ */
+static void xsave_fpu_buf_init(struct xsave_hdr_struct *xhdr)
+{
+	/*
+	 * This will cause the xrstor[s] to initialize
+	 * all the xsave state components.
+	 */
+	xhdr->xstate_bv = 0;
+	/*
+	 * xrstors will GPF if this bit is not set
+	 */
+	if (boot_cpu_has(X86_FEATURE_XSAVEC))
+		xhdr->xcomp_bv = XCOMP_BV_COMPACTION_ENABLED;
+	else
+		xhdr->xcomp_bv = 0;
+	/*
+	 * SDM Says we will GPF:
+	 * "If bytes 63:16 of the XSAVE header are not all zero."
+	 * If this reserved size ever changes, we will need to
+	 * update this code to look at and conditionally
+	 * initialize the new fields, just like ->xcomp_bv above.
+	 */
+	BUILD_BUG_ON(sizeof(xhdr->reserved) != 48);
+	memset(xhdr->reserved, 0, sizeof(xhdr->reserved));
+}
+
 void fpu_finit(struct fpu *fpu)
 {
 	if (!cpu_has_fpu) {
@@ -225,6 +261,15 @@ void fpu_finit(struct fpu *fpu)
 		return;
 	}
 
+	/*
+	 * The xsave code does not require the entire buffer
+	 * to be zeroed, only a chunk of the header.
+	 */
+	if (cpu_has_xsave) {
+		xsave_fpu_buf_init(&fpu->state->xsave.xsave_hdr);
+		return;
+	}
+
 	memset(fpu->state, 0, xstate_size);
 
 	if (cpu_has_fxsr) {
_
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/