From: Greg Kroah-Hartman Date: Mon, 2 Oct 2017 18:58:57 +0000 (+0200) Subject: 3.18-stable patches X-Git-Tag: v3.18.73~12 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f5b1527572d062fffb70a5a6149064e8b891812a;p=thirdparty%2Fkernel%2Fstable-queue.git 3.18-stable patches added patches: x86-fpu-don-t-let-userspace-set-bogus-xcomp_bv.patch --- diff --git a/queue-3.18/series b/queue-3.18/series index 3eadc2da5f3..2cdd08cc92d 100644 --- a/queue-3.18/series +++ b/queue-3.18/series @@ -19,3 +19,4 @@ arm64-make-sure-spsel-is-always-set.patch kvm-nvmx-don-t-allow-l2-to-access-the-hardware-cr8.patch pci-fix-race-condition-with-driver_override.patch btrfs-prevent-to-set-invalid-default-subvolid.patch +x86-fpu-don-t-let-userspace-set-bogus-xcomp_bv.patch diff --git a/queue-3.18/x86-fpu-don-t-let-userspace-set-bogus-xcomp_bv.patch b/queue-3.18/x86-fpu-don-t-let-userspace-set-bogus-xcomp_bv.patch new file mode 100644 index 00000000000..b5c1ae5657d --- /dev/null +++ b/queue-3.18/x86-fpu-don-t-let-userspace-set-bogus-xcomp_bv.patch @@ -0,0 +1,196 @@ +From ebiggers3@gmail.com Mon Oct 2 20:57:38 2017 +From: Eric Biggers +Date: Mon, 2 Oct 2017 11:10:07 -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: <20171002181007.19371-1-ebiggers3@gmail.com> + + +From: Eric Biggers + +commit 814fb7bb7db5433757d76f4c4502c96fc53b0b5e upstream. + +[Please apply to 3.18-stable. Note: the backport includes the +fpu_finit() call in xstateregs_set(), since fix is useless without it. +It was added by commit 91c3dba7dbc1 ("x86/fpu/xstate: Fix PTRACE frames +for XSAVES"), but it doesn't make sense to backport that whole commit.] + +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/i387.c | 11 +++++++++++ + arch/x86/kernel/xsave.c | 4 +++- + 2 files changed, 14 insertions(+), 1 deletion(-) + +--- a/arch/x86/kernel/i387.c ++++ b/arch/x86/kernel/i387.c +@@ -388,11 +388,22 @@ int xstateregs_set(struct task_struct *t + xsave_hdr = &target->thread.fpu.state->xsave.xsave_hdr; + + xsave_hdr->xstate_bv &= pcntxt_mask; ++ ++ /* xcomp_bv must be 0 when using uncompacted format */ ++ if (!ret && xsave_hdr->xcomp_bv) ++ ret = -EINVAL; ++ + /* + * These bits must be zero. + */ + memset(xsave_hdr->reserved, 0, 48); + ++ /* ++ * In case of failure, mark all states as init: ++ */ ++ if (ret) ++ fpu_finit(&target->thread.fpu); ++ + return ret; + } + +--- a/arch/x86/kernel/xsave.c ++++ b/arch/x86/kernel/xsave.c +@@ -394,7 +394,9 @@ int __restore_xstate_sig(void __user *bu + drop_fpu(tsk); + + if (__copy_from_user(&fpu->state->xsave, buf_fx, state_size) || +- __copy_from_user(&env, buf, sizeof(env))) { ++ __copy_from_user(&env, buf, sizeof(env)) || ++ (state_size > offsetof(struct xsave_struct, xsave_hdr) && ++ fpu->state->xsave.xsave_hdr.xcomp_bv)) { + fpu_finit(fpu); + err = -1; + } else {