aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJosh Poimboeuf <jpoimboe@redhat.com>2017-05-23 11:37:29 -0400
committerIngo Molnar <mingo@kernel.org>2017-05-24 03:05:16 -0400
commitebd574994c63164d538a197172157318f58ac647 (patch)
tree0016f12f367965af38a23c8d8c84cfad2dd9ea8f
parent3780578761921f094179c6289072a74b2228c602 (diff)
Revert "x86/entry: Fix the end of the stack for newly forked tasks"
Petr Mladek reported the following warning when loading the livepatch sample module: WARNING: CPU: 1 PID: 3699 at arch/x86/kernel/stacktrace.c:132 save_stack_trace_tsk_reliable+0x133/0x1a0 ... Call Trace: __schedule+0x273/0x820 schedule+0x36/0x80 kthreadd+0x305/0x310 ? kthread_create_on_cpu+0x80/0x80 ? icmp_echo.part.32+0x50/0x50 ret_from_fork+0x2c/0x40 That warning means the end of the stack is no longer recognized as such for newly forked tasks. The problem was introduced with the following commit: ff3f7e2475bb ("x86/entry: Fix the end of the stack for newly forked tasks") ... which was completely misguided. It only partially fixed the reported issue, and it introduced another bug in the process. None of the other entry code saves the frame pointer before calling into C code, so it doesn't make sense for ret_from_fork to do so either. Contrary to what I originally thought, the original issue wasn't related to newly forked tasks. It was actually related to ftrace. When entry code calls into a function which then calls into an ftrace handler, the stack frame looks different than normal. The original issue will be fixed in the unwinder, in a subsequent patch. Reported-by: Petr Mladek <pmladek@suse.com> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Acked-by: Thomas Gleixner <tglx@linutronix.de> Cc: Dave Jones <davej@codemonkey.org.uk> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Steven Rostedt <rostedt@goodmis.org> Cc: live-patching@vger.kernel.org Fixes: ff3f7e2475bb ("x86/entry: Fix the end of the stack for newly forked tasks") Link: http://lkml.kernel.org/r/f350760f7e82f0750c8d1dd093456eb212751caa.1495553739.git.jpoimboe@redhat.com Signed-off-by: Ingo Molnar <mingo@kernel.org>
-rw-r--r--arch/x86/entry/entry_32.S30
-rw-r--r--arch/x86/entry/entry_64.S11
2 files changed, 23 insertions, 18 deletions
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index 50bc26949e9e..48ef7bb32c42 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -252,6 +252,23 @@ ENTRY(__switch_to_asm)
252END(__switch_to_asm) 252END(__switch_to_asm)
253 253
254/* 254/*
255 * The unwinder expects the last frame on the stack to always be at the same
256 * offset from the end of the page, which allows it to validate the stack.
257 * Calling schedule_tail() directly would break that convention because its an
258 * asmlinkage function so its argument has to be pushed on the stack. This
259 * wrapper creates a proper "end of stack" frame header before the call.
260 */
261ENTRY(schedule_tail_wrapper)
262 FRAME_BEGIN
263
264 pushl %eax
265 call schedule_tail
266 popl %eax
267
268 FRAME_END
269 ret
270ENDPROC(schedule_tail_wrapper)
271/*
255 * A newly forked process directly context switches into this address. 272 * A newly forked process directly context switches into this address.
256 * 273 *
257 * eax: prev task we switched from 274 * eax: prev task we switched from
@@ -259,24 +276,15 @@ END(__switch_to_asm)
259 * edi: kernel thread arg 276 * edi: kernel thread arg
260 */ 277 */
261ENTRY(ret_from_fork) 278ENTRY(ret_from_fork)
262 FRAME_BEGIN /* help unwinder find end of stack */ 279 call schedule_tail_wrapper
263
264 /*
265 * schedule_tail() is asmlinkage so we have to put its 'prev' argument
266 * on the stack.
267 */
268 pushl %eax
269 call schedule_tail
270 popl %eax
271 280
272 testl %ebx, %ebx 281 testl %ebx, %ebx
273 jnz 1f /* kernel threads are uncommon */ 282 jnz 1f /* kernel threads are uncommon */
274 283
2752: 2842:
276 /* When we fork, we trace the syscall return in the child, too. */ 285 /* When we fork, we trace the syscall return in the child, too. */
277 leal FRAME_OFFSET(%esp), %eax 286 movl %esp, %eax
278 call syscall_return_slowpath 287 call syscall_return_slowpath
279 FRAME_END
280 jmp restore_all 288 jmp restore_all
281 289
282 /* kernel thread */ 290 /* kernel thread */
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 607d72c4a485..4a4c0834f965 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -36,7 +36,6 @@
36#include <asm/smap.h> 36#include <asm/smap.h>
37#include <asm/pgtable_types.h> 37#include <asm/pgtable_types.h>
38#include <asm/export.h> 38#include <asm/export.h>
39#include <asm/frame.h>
40#include <linux/err.h> 39#include <linux/err.h>
41 40
42.code64 41.code64
@@ -406,19 +405,17 @@ END(__switch_to_asm)
406 * r12: kernel thread arg 405 * r12: kernel thread arg
407 */ 406 */
408ENTRY(ret_from_fork) 407ENTRY(ret_from_fork)
409 FRAME_BEGIN /* help unwinder find end of stack */
410 movq %rax, %rdi 408 movq %rax, %rdi
411 call schedule_tail /* rdi: 'prev' task parameter */ 409 call schedule_tail /* rdi: 'prev' task parameter */
412 410
413 testq %rbx, %rbx /* from kernel_thread? */ 411 testq %rbx, %rbx /* from kernel_thread? */
414 jnz 1f /* kernel threads are uncommon */ 412 jnz 1f /* kernel threads are uncommon */
415 413
4162: 4142:
417 leaq FRAME_OFFSET(%rsp),%rdi /* pt_regs pointer */ 415 movq %rsp, %rdi
418 call syscall_return_slowpath /* returns with IRQs disabled */ 416 call syscall_return_slowpath /* returns with IRQs disabled */
419 TRACE_IRQS_ON /* user mode is traced as IRQS on */ 417 TRACE_IRQS_ON /* user mode is traced as IRQS on */
420 SWAPGS 418 SWAPGS
421 FRAME_END
422 jmp restore_regs_and_iret 419 jmp restore_regs_and_iret
423 420
4241: 4211: