From bf190800d0285393dffd04bd9998aaddd7f45d8e Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 2 Oct 2017 20:58:00 +0200 Subject: [PATCH] 4.9-stable patches added patches: x86-fpu-don-t-let-userspace-set-bogus-xcomp_bv.patch --- queue-4.9/series | 1 + ...n-t-let-userspace-set-bogus-xcomp_bv.patch | 187 ++++++++++++++++++ 2 files changed, 188 insertions(+) create mode 100644 queue-4.9/x86-fpu-don-t-let-userspace-set-bogus-xcomp_bv.patch diff --git a/queue-4.9/series b/queue-4.9/series index 27b1a6488b8..92257aa3283 100644 --- a/queue-4.9/series +++ b/queue-4.9/series @@ -53,3 +53,4 @@ btrfs-fix-null-pointer-dereference-from-free_reloc_roots.patch btrfs-propagate-error-to-btrfs_cmp_data_prepare-caller.patch btrfs-prevent-to-set-invalid-default-subvolid.patch x86-mm-fix-fault-error-path-using-unsafe-vma-pointer.patch +x86-fpu-don-t-let-userspace-set-bogus-xcomp_bv.patch diff --git a/queue-4.9/x86-fpu-don-t-let-userspace-set-bogus-xcomp_bv.patch b/queue-4.9/x86-fpu-don-t-let-userspace-set-bogus-xcomp_bv.patch new file mode 100644 index 00000000000..a32fbd53b7b --- /dev/null +++ b/queue-4.9/x86-fpu-don-t-let-userspace-set-bogus-xcomp_bv.patch @@ -0,0 +1,187 @@ +From ebiggers3@gmail.com Mon Oct 2 20:54:14 2017 +From: Eric Biggers +Date: Mon, 2 Oct 2017 11:01:40 -0700 +Subject: x86/fpu: Don't let userspace set bogus xcomp_bv +To: stable@vger.kernel.org, Greg Kroah-Hartman +Cc: Eric Biggers , Andrew Morton , Andy Lutomirski , Andy Lutomirski , Borislav Petkov , Eric Biggers , Fenghua Yu , Kevin Hao , Linus Torvalds , Michael Halcrow , Oleg Nesterov , Peter Zijlstra , Thomas Gleixner , Wanpeng Li , Yu-cheng Yu , kernel-hardening@lists.openwall.com, Ingo Molnar +Message-ID: <20171002180140.142854-1-ebiggers3@gmail.com> + + +From: Eric Biggers + +commit 814fb7bb7db5433757d76f4c4502c96fc53b0b5e upstream. + +On x86, userspace can use the ptrace() or rt_sigreturn() system calls to +set a task's extended state (xstate) or "FPU" registers. ptrace() can +set them for another task using the PTRACE_SETREGSET request with +NT_X86_XSTATE, while rt_sigreturn() can set them for the current task. +In either case, registers can be set to any value, but the kernel +assumes that the XSAVE area itself remains valid in the sense that the +CPU can restore it. + +However, in the case where the kernel is using the uncompacted xstate +format (which it does whenever the XSAVES instruction is unavailable), +it was possible for userspace to set the xcomp_bv field in the +xstate_header to an arbitrary value. However, all bits in that field +are reserved in the uncompacted case, so when switching to a task with +nonzero xcomp_bv, the XRSTOR instruction failed with a #GP fault. This +caused the WARN_ON_FPU(err) in copy_kernel_to_xregs() to be hit. In +addition, since the error is otherwise ignored, the FPU registers from +the task previously executing on the CPU were leaked. + +Fix the bug by checking that the user-supplied value of xcomp_bv is 0 in +the uncompacted case, and returning an error otherwise. + +The reason for validating xcomp_bv rather than simply overwriting it +with 0 is that we want userspace to see an error if it (incorrectly) +provides an XSAVE area in compacted format rather than in uncompacted +format. + +Note that as before, in case of error we clear the task's FPU state. +This is perhaps non-ideal, especially for PTRACE_SETREGSET; it might be +better to return an error before changing anything. But it seems the +"clear on error" behavior is fine for now, and it's a little tricky to +do otherwise because it would mean we couldn't simply copy the full +userspace state into kernel memory in one __copy_from_user(). + +This bug was found by syzkaller, which hit the above-mentioned +WARN_ON_FPU(): + + WARNING: CPU: 1 PID: 0 at ./arch/x86/include/asm/fpu/internal.h:373 __switch_to+0x5b5/0x5d0 + CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.13.0 #453 + Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 + task: ffff9ba2bc8e42c0 task.stack: ffffa78cc036c000 + RIP: 0010:__switch_to+0x5b5/0x5d0 + RSP: 0000:ffffa78cc08bbb88 EFLAGS: 00010082 + RAX: 00000000fffffffe RBX: ffff9ba2b8bf2180 RCX: 00000000c0000100 + RDX: 00000000ffffffff RSI: 000000005cb10700 RDI: ffff9ba2b8bf36c0 + RBP: ffffa78cc08bbbd0 R08: 00000000929fdf46 R09: 0000000000000001 + R10: 0000000000000000 R11: 0000000000000000 R12: ffff9ba2bc8e42c0 + R13: 0000000000000000 R14: ffff9ba2b8bf3680 R15: ffff9ba2bf5d7b40 + FS: 00007f7e5cb10700(0000) GS:ffff9ba2bf400000(0000) knlGS:0000000000000000 + CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 + CR2: 00000000004005cc CR3: 0000000079fd5000 CR4: 00000000001406e0 + Call Trace: + Code: 84 00 00 00 00 00 e9 11 fd ff ff 0f ff 66 0f 1f 84 00 00 00 00 00 e9 e7 fa ff ff 0f ff 66 0f 1f 84 00 00 00 00 00 e9 c2 fa ff ff <0f> ff 66 0f 1f 84 00 00 00 00 00 e9 d4 fc ff ff 66 66 2e 0f 1f + +Here is a C reproducer. The expected behavior is that the program spin +forever with no output. However, on a buggy kernel running on a +processor with the "xsave" feature but without the "xsaves" feature +(e.g. Sandy Bridge through Broadwell for Intel), within a second or two +the program reports that the xmm registers were corrupted, i.e. were not +restored correctly. With CONFIG_X86_DEBUG_FPU=y it also hits the above +kernel warning. + + #define _GNU_SOURCE + #include + #include + #include + #include + #include + #include + #include + #include + + int main(void) + { + int pid = fork(); + uint64_t xstate[512]; + struct iovec iov = { .iov_base = xstate, .iov_len = sizeof(xstate) }; + + if (pid == 0) { + bool tracee = true; + for (int i = 0; i < sysconf(_SC_NPROCESSORS_ONLN) && tracee; i++) + tracee = (fork() != 0); + uint32_t xmm0[4] = { [0 ... 3] = tracee ? 0x00000000 : 0xDEADBEEF }; + asm volatile(" movdqu %0, %%xmm0\n" + " mov %0, %%rbx\n" + "1: movdqu %%xmm0, %0\n" + " mov %0, %%rax\n" + " cmp %%rax, %%rbx\n" + " je 1b\n" + : "+m" (xmm0) : : "rax", "rbx", "xmm0"); + printf("BUG: xmm registers corrupted! tracee=%d, xmm0=%08X%08X%08X%08X\n", + tracee, xmm0[0], xmm0[1], xmm0[2], xmm0[3]); + } else { + usleep(100000); + ptrace(PTRACE_ATTACH, pid, 0, 0); + wait(NULL); + ptrace(PTRACE_GETREGSET, pid, NT_X86_XSTATE, &iov); + xstate[65] = -1; + ptrace(PTRACE_SETREGSET, pid, NT_X86_XSTATE, &iov); + ptrace(PTRACE_CONT, pid, 0, 0); + wait(NULL); + } + return 1; + } + +Note: the program only tests for the bug using the ptrace() system call. +The bug can also be reproduced using the rt_sigreturn() system call, but +only when called from a 32-bit program, since for 64-bit programs the +kernel restores the FPU state from the signal frame by doing XRSTOR +directly from userspace memory (with proper error checking). + +Reported-by: Dmitry Vyukov +Signed-off-by: Eric Biggers +Reviewed-by: Kees Cook +Reviewed-by: Rik van Riel +Acked-by: Dave Hansen +Cc: Andrew Morton +Cc: Andy Lutomirski +Cc: Andy Lutomirski +Cc: Borislav Petkov +Cc: Eric Biggers +Cc: Fenghua Yu +Cc: Kevin Hao +Cc: Linus Torvalds +Cc: Michael Halcrow +Cc: Oleg Nesterov +Cc: Peter Zijlstra +Cc: Thomas Gleixner +Cc: Wanpeng Li +Cc: Yu-cheng Yu +Cc: kernel-hardening@lists.openwall.com +Fixes: 0b29643a5843 ("x86/xsaves: Change compacted format xsave area header") +Link: http://lkml.kernel.org/r/20170922174156.16780-2-ebiggers3@gmail.com +Link: http://lkml.kernel.org/r/20170923130016.21448-25-mingo@kernel.org +Signed-off-by: Ingo Molnar +Signed-off-by: Greg Kroah-Hartman +--- + arch/x86/kernel/fpu/regset.c | 9 +++++++-- + arch/x86/kernel/fpu/signal.c | 4 ++++ + 2 files changed, 11 insertions(+), 2 deletions(-) + +--- a/arch/x86/kernel/fpu/regset.c ++++ b/arch/x86/kernel/fpu/regset.c +@@ -130,11 +130,16 @@ int xstateregs_set(struct task_struct *t + + fpu__activate_fpstate_write(fpu); + +- if (boot_cpu_has(X86_FEATURE_XSAVES)) ++ if (boot_cpu_has(X86_FEATURE_XSAVES)) { + ret = copyin_to_xsaves(kbuf, ubuf, xsave); +- else ++ } else { + ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, xsave, 0, -1); + ++ /* xcomp_bv must be 0 when using uncompacted format */ ++ if (!ret && xsave->header.xcomp_bv) ++ ret = -EINVAL; ++ } ++ + /* + * In case of failure, mark all states as init: + */ +--- a/arch/x86/kernel/fpu/signal.c ++++ b/arch/x86/kernel/fpu/signal.c +@@ -329,6 +329,10 @@ static int __fpu__restore_sig(void __use + } else { + err = __copy_from_user(&fpu->state.xsave, + buf_fx, state_size); ++ ++ /* xcomp_bv must be 0 when using uncompacted format */ ++ if (!err && state_size > offsetof(struct xregs_state, header) && fpu->state.xsave.header.xcomp_bv) ++ err = -EINVAL; + } + + if (err || __copy_from_user(&env, buf, sizeof(env))) { -- 2.47.3