Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

x86/unwind: Handle NULL pointer calls better in frame unwinder

When the frame unwinder is invoked for an oops caused by a call to NULL, it
currently skips the parent function because BP still points to the parent's
stack frame; the (nonexistent) current function only has the first half of
a stack frame, and BP doesn't point to it yet.

Add a special case for IP==0 that calculates a fake BP from SP, then uses
the real BP for the next frame.

Note that this handles first_frame specially: Return information about the
parent function as long as the saved IP is >=first_frame, even if the fake
BP points below it.

With an artificially-added NULL call in prctl_set_seccomp(), before this
patch, the trace is:

Call Trace:
? prctl_set_seccomp+0x3a/0x50
__x64_sys_prctl+0x457/0x6f0
? __ia32_sys_prctl+0x750/0x750
do_syscall_64+0x72/0x160
entry_SYSCALL_64_after_hwframe+0x44/0xa9

After this patch, the trace is:

Call Trace:
prctl_set_seccomp+0x3a/0x50
__x64_sys_prctl+0x457/0x6f0
? __ia32_sys_prctl+0x750/0x750
do_syscall_64+0x72/0x160
entry_SYSCALL_64_after_hwframe+0x44/0xa9

Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: syzbot <syzbot+ca95b2b7aef9e7cbd6ab@syzkaller.appspotmail.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Michal Marek <michal.lkml@markovi.net>
Cc: linux-kbuild@vger.kernel.org
Link: https://lkml.kernel.org/r/20190301031201.7416-1-jannh@google.com

authored by

Jann Horn and committed by
Thomas Gleixner
f4f34e1b f76a16ad

+28 -3
+6
arch/x86/include/asm/unwind.h
··· 23 23 #elif defined(CONFIG_UNWINDER_FRAME_POINTER) 24 24 bool got_irq; 25 25 unsigned long *bp, *orig_sp, ip; 26 + /* 27 + * If non-NULL: The current frame is incomplete and doesn't contain a 28 + * valid BP. When looking for the next frame, use this instead of the 29 + * non-existent saved BP. 30 + */ 31 + unsigned long *next_bp; 26 32 struct pt_regs *regs; 27 33 #else 28 34 unsigned long *sp;
+22 -3
arch/x86/kernel/unwind_frame.c
··· 320 320 } 321 321 322 322 /* Get the next frame pointer: */ 323 - if (state->regs) 323 + if (state->next_bp) { 324 + next_bp = state->next_bp; 325 + state->next_bp = NULL; 326 + } else if (state->regs) { 324 327 next_bp = (unsigned long *)state->regs->bp; 325 - else 328 + } else { 326 329 next_bp = (unsigned long *)READ_ONCE_TASK_STACK(state->task, *state->bp); 330 + } 327 331 328 332 /* Move to the next frame if it's safe: */ 329 333 if (!update_stack_state(state, next_bp)) ··· 402 398 403 399 bp = get_frame_pointer(task, regs); 404 400 401 + /* 402 + * If we crash with IP==0, the last successfully executed instruction 403 + * was probably an indirect function call with a NULL function pointer. 404 + * That means that SP points into the middle of an incomplete frame: 405 + * *SP is a return pointer, and *(SP-sizeof(unsigned long)) is where we 406 + * would have written a frame pointer if we hadn't crashed. 407 + * Pretend that the frame is complete and that BP points to it, but save 408 + * the real BP so that we can use it when looking for the next frame. 409 + */ 410 + if (regs && regs->ip == 0 && 411 + (unsigned long *)kernel_stack_pointer(regs) >= first_frame) { 412 + state->next_bp = bp; 413 + bp = ((unsigned long *)kernel_stack_pointer(regs)) - 1; 414 + } 415 + 405 416 /* Initialize stack info and make sure the frame data is accessible: */ 406 417 get_stack_info(bp, state->task, &state->stack_info, 407 418 &state->stack_mask); ··· 429 410 */ 430 411 while (!unwind_done(state) && 431 412 (!on_stack(&state->stack_info, first_frame, sizeof(long)) || 432 - state->bp < first_frame)) 413 + (state->next_bp == NULL && state->bp < first_frame))) 433 414 unwind_next_frame(state); 434 415 } 435 416 EXPORT_SYMBOL_GPL(__unwind_start);