* Backtrace after invalid XRSTOR after "x86/fault: BUG() when uaccess helpers fault on kernel addresses"
@ 2018-11-26 16:59 Sebastian Andrzej Siewior
2018-11-26 17:12 ` Jann Horn
2018-11-27 13:32 ` [PATCH v2] x86/fpu: XRSTOR is expected to raise #GP Jann Horn
0 siblings, 2 replies; 4+ messages in thread
From: Sebastian Andrzej Siewior @ 2018-11-26 16:59 UTC (permalink / raw)
To: Jann Horn
Cc: Thomas Gleixner, Andy Lutomirski, kernel-hardening, Naveen N. Rao,
Borislav Petkov, linux-kernel
Commit 75045f77f7a7 ("x86/extable: Introduce _ASM_EXTABLE_UA for uaccess
fixups") made copy_user_to_xregs() -> XSTATE_OP() use _ASM_EXTABLE_UA.
Commit 9da3f2b74054 ("x86/fault: BUG() when uaccess helpers fault on
kernel addresses") then decided that a #GP is not good and has to be
reported loudly.
I had a TC which sets a few invalid bits in xstate which is used by
copy_user_to_xregs() on sig-return. Before that change I had:
| sig-xstate-bum[2253] bad frame in rt_sigreturn frame:0000000056078134 ip:7f9da336c227 sp:7ffc871325e8 orax:ffffffffffffffff in libc-2.27.so[7f9da3358000+146000]
after those two patches are applied:
|BUG: GPF in non-whitelisted uaccess (non-canonical address?)
|general protection fault: 0000 [#1] PREEMPT SMP NOPTI
|CPU: 26 PID: 2236 Comm: sig-xstate-bum Not tainted 4.20.0-rc3 #45
|RIP: 0010:__fpu__restore_sig+0x1c1/0x540
|Code: 02 00 00 48 8b 95 58 ff ff ff 48 f7 d2 48 21 d0 0f 85 6e 03 00 00 0f 01 cb 48 8b 85 58 ff ff ff 48 89 df 48 89 c2 48 c1 ea 20 <48> 0f ae 2f 31 db 0f 01 ca 85 db 0f 84 d7 00 00 00 4c 89 f7 bb ff
|Call Trace:
| fpu__restore_sig+0x28/0x40
| restore_sigcontext+0x13a/0x180
| __ia32_sys_rt_sigreturn+0xae/0x100
| do_syscall_64+0x4f/0x100
| entry_SYSCALL_64_after_hwframe+0x44/0xa9
|RIP: 0033:0x7f9b06aea227
|---[ end trace a45ac23b593e9ab0 ]---
The expected behaviour would that `xrstor' performs a #GP and this does
not a produce a backtrace like that and copy_user_to_fxregs() returns an
error.
copy_user_to_fxregs() / user_insn() does not have this behaviour and
that also might generate a #GP (if invalid bits are set in MCSR).
What do we do?
Sebastian
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: Backtrace after invalid XRSTOR after "x86/fault: BUG() when uaccess helpers fault on kernel addresses" 2018-11-26 16:59 Backtrace after invalid XRSTOR after "x86/fault: BUG() when uaccess helpers fault on kernel addresses" Sebastian Andrzej Siewior @ 2018-11-26 17:12 ` Jann Horn 2018-11-27 13:32 ` [PATCH v2] x86/fpu: XRSTOR is expected to raise #GP Jann Horn 1 sibling, 0 replies; 4+ messages in thread From: Jann Horn @ 2018-11-26 17:12 UTC (permalink / raw) To: bigeasy Cc: Thomas Gleixner, Andy Lutomirski, Kernel Hardening, naveen.n.rao, Borislav Petkov, kernel list On Mon, Nov 26, 2018 at 5:59 PM Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote: > Commit 75045f77f7a7 ("x86/extable: Introduce _ASM_EXTABLE_UA for uaccess > fixups") made copy_user_to_xregs() -> XSTATE_OP() use _ASM_EXTABLE_UA. > Commit 9da3f2b74054 ("x86/fault: BUG() when uaccess helpers fault on > kernel addresses") then decided that a #GP is not good and has to be > reported loudly. > > I had a TC which sets a few invalid bits in xstate which is used by > copy_user_to_xregs() on sig-return. Before that change I had: > | sig-xstate-bum[2253] bad frame in rt_sigreturn frame:0000000056078134 ip:7f9da336c227 sp:7ffc871325e8 orax:ffffffffffffffff in libc-2.27.so[7f9da3358000+146000] > > after those two patches are applied: > |BUG: GPF in non-whitelisted uaccess (non-canonical address?) > |general protection fault: 0000 [#1] PREEMPT SMP NOPTI > |CPU: 26 PID: 2236 Comm: sig-xstate-bum Not tainted 4.20.0-rc3 #45 > |RIP: 0010:__fpu__restore_sig+0x1c1/0x540 > |Code: 02 00 00 48 8b 95 58 ff ff ff 48 f7 d2 48 21 d0 0f 85 6e 03 00 00 0f 01 cb 48 8b 85 58 ff ff ff 48 89 df 48 89 c2 48 c1 ea 20 <48> 0f ae 2f 31 db 0f 01 ca 85 db 0f 84 d7 00 00 00 4c 89 f7 bb ff > |Call Trace: > | fpu__restore_sig+0x28/0x40 > | restore_sigcontext+0x13a/0x180 > | __ia32_sys_rt_sigreturn+0xae/0x100 > | do_syscall_64+0x4f/0x100 > | entry_SYSCALL_64_after_hwframe+0x44/0xa9 > |RIP: 0033:0x7f9b06aea227 > |---[ end trace a45ac23b593e9ab0 ]--- > > The expected behaviour would that `xrstor' performs a #GP and this does > not a produce a backtrace like that and copy_user_to_fxregs() returns an > error. > copy_user_to_fxregs() / user_insn() does not have this behaviour and > that also might generate a #GP (if invalid bits are set in MCSR). > What do we do? Bleh. This code has to use normal _ASM_EXTABLE. _ASM_EXTABLE_UA is (almost, with the exception of stuff like probe_kernel_read() and exact_copy_from_user()) only for code that isn't expected to throw things other than #PF with a userspace address. I must have missed this when looking at the documentation for XRSTOR, or something like that... ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] x86/fpu: XRSTOR is expected to raise #GP @ 2018-11-27 13:32 ` Jann Horn 2018-11-27 17:03 ` [tip:x86/urgent] x86/fpu: Use the correct exception table macro in the XSTATE_OP wrapper tip-bot for Jann Horn 0 siblings, 1 reply; 4+ messages in thread From: Jann Horn @ 2018-11-27 13:32 UTC (permalink / raw) To: Thomas Gleixner, Sebastian Andrzej Siewior, Ingo Molnar, Borislav Petkov, H. Peter Anvin, jannh Cc: Andy Lutomirski, kernel-hardening, Naveen N. Rao, linux-kernel, x86 commit 75045f77f7a7 ("x86/extable: Introduce _ASM_EXTABLE_UA for uaccess fixups") incorrectly replaced the fixup entry for XSTATE_OP with a user-#PF-only fixup. XRSTOR can also raise #GP if the xstate content is invalid, and _ASM_EXTABLE_UA doesn't expect that. Change this fixup back to _ASM_EXTABLE so that #GP gets fixed up. Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Fixes: 75045f77f7a7 ("x86/extable: Introduce _ASM_EXTABLE_UA for uaccess fixups") Signed-off-by: Jann Horn <jannh@google.com> --- arch/x86/include/asm/fpu/internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h index 5f7290e6e954..69dcdf195b61 100644 --- a/arch/x86/include/asm/fpu/internal.h +++ b/arch/x86/include/asm/fpu/internal.h @@ -226,7 +226,7 @@ static inline void copy_fxregs_to_kernel(struct fpu *fpu) "3: movl $-2,%[err]\n\t" \ "jmp 2b\n\t" \ ".popsection\n\t" \ - _ASM_EXTABLE_UA(1b, 3b) \ + _ASM_EXTABLE(1b, 3b) \ : [err] "=r" (err) \ : "D" (st), "m" (*st), "a" (lmask), "d" (hmask) \ : "memory") -- 2.20.0.rc0.387.gc7a69e6b6c-goog ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [tip:x86/urgent] x86/fpu: Use the correct exception table macro in the XSTATE_OP wrapper 2018-11-27 13:32 ` [PATCH v2] x86/fpu: XRSTOR is expected to raise #GP Jann Horn @ 2018-11-27 17:03 ` tip-bot for Jann Horn 0 siblings, 0 replies; 4+ messages in thread From: tip-bot for Jann Horn @ 2018-11-27 17:03 UTC (permalink / raw) To: linux-tip-commits Cc: tglx, mingo, naveen.n.rao, linux-kernel, bigeasy, bp, x86, hpa, mingo, luto, jannh Commit-ID: ac26d1f74cfc19c8dc9d533b5f20e99dbee3d9bd Gitweb: https://git.kernel.org/tip/ac26d1f74cfc19c8dc9d533b5f20e99dbee3d9bd Author: Jann Horn <jannh@google.com> AuthorDate: Tue, 27 Nov 2018 14:32:00 +0100 Committer: Borislav Petkov <bp@suse.de> CommitDate: Tue, 27 Nov 2018 17:55:45 +0100 x86/fpu: Use the correct exception table macro in the XSTATE_OP wrapper Commit 75045f77f7a7 ("x86/extable: Introduce _ASM_EXTABLE_UA for uaccess fixups") incorrectly replaced the fixup entry for XSTATE_OP with a user-#PF-only fixup. XRSTOR can also raise #GP if the xstate content is invalid, and _ASM_EXTABLE_UA doesn't expect that. Change this fixup back to _ASM_EXTABLE so that #GP gets fixed up. Fixes: 75045f77f7a7 ("x86/extable: Introduce _ASM_EXTABLE_UA for uaccess fixups") Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Signed-off-by: Jann Horn <jannh@google.com> Signed-off-by: Borislav Petkov <bp@suse.de> Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> Cc: Andy Lutomirski <luto@kernel.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: kernel-hardening@lists.openwall.com Cc: x86-ml <x86@kernel.org> Link: https://lkml.kernel.org/r/20181126165957.xhsyu2dhyy45mrjo@linutronix.de Link: https://lkml.kernel.org/r/20181127133200.38322-1-jannh@google.com --- arch/x86/include/asm/fpu/internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h index 5f7290e6e954..69dcdf195b61 100644 --- a/arch/x86/include/asm/fpu/internal.h +++ b/arch/x86/include/asm/fpu/internal.h @@ -226,7 +226,7 @@ static inline void copy_fxregs_to_kernel(struct fpu *fpu) "3: movl $-2,%[err]\n\t" \ "jmp 2b\n\t" \ ".popsection\n\t" \ - _ASM_EXTABLE_UA(1b, 3b) \ + _ASM_EXTABLE(1b, 3b) \ : [err] "=r" (err) \ : "D" (st), "m" (*st), "a" (lmask), "d" (hmask) \ : "memory") ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-11-27 17:04 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-11-26 16:59 Backtrace after invalid XRSTOR after "x86/fault: BUG() when uaccess helpers fault on kernel addresses" Sebastian Andrzej Siewior 2018-11-26 17:12 ` Jann Horn 2018-11-27 13:32 ` [PATCH v2] x86/fpu: XRSTOR is expected to raise #GP Jann Horn 2018-11-27 17:03 ` [tip:x86/urgent] x86/fpu: Use the correct exception table macro in the XSTATE_OP wrapper tip-bot for Jann Horn
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.