Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 367ccafb authored by Jann Horn's avatar Jann Horn Committed by Greg Kroah-Hartman
Browse files

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



commit f4f34e1b82eb4219d8eaa1c7e2e17ca219a6a2b5 upstream.

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: default avatarJann Horn <jannh@google.com>
Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
Acked-by: default avatarJosh 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


Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 3254dd30

arch/x86/include/asm/unwind.h

+6 −0
Original line number Original line Diff line number Diff line
@@ -23,6 +23,12 @@ struct unwind_state { 
#elif defined(CONFIG_UNWINDER_FRAME_POINTER)

#elif defined(CONFIG_UNWINDER_FRAME_POINTER)

	bool got_irq;

	bool got_irq;

	unsigned long *bp, *orig_sp, ip;

	unsigned long *bp, *orig_sp, ip;

	/*

	 * If non-NULL: The current frame is incomplete and doesn't contain a

	 * valid BP. When looking for the next frame, use this instead of the

	 * non-existent saved BP.

	 */

	unsigned long *next_bp;

	struct pt_regs *regs;

	struct pt_regs *regs;

#else

#else

	unsigned long *sp;

	unsigned long *sp;

arch/x86/kernel/unwind_frame.c

+22 −3
Original line number Original line Diff line number Diff line
@@ -320,10 +320,14 @@ bool unwind_next_frame(struct unwind_state *state) 
	}

	}





	/* Get the next frame pointer: */

	/* Get the next frame pointer: */

	if (state->regs)

	if (state->next_bp) {

		next_bp = state->next_bp;

		state->next_bp = NULL;

	} else if (state->regs) {

		next_bp = (unsigned long *)state->regs->bp;

		next_bp = (unsigned long *)state->regs->bp;

	else

	} else {

		next_bp = (unsigned long *)READ_ONCE_TASK_STACK(state->task, *state->bp);

		next_bp = (unsigned long *)READ_ONCE_TASK_STACK(state->task, *state->bp);

	}





	/* Move to the next frame if it's safe: */

	/* Move to the next frame if it's safe: */

	if (!update_stack_state(state, next_bp))

	if (!update_stack_state(state, next_bp))
@@ -398,6 +402,21 @@ void __unwind_start(struct unwind_state *state, struct task_struct *task, 




	bp = get_frame_pointer(task, regs);

	bp = get_frame_pointer(task, regs);





	/*

	 * If we crash with IP==0, the last successfully executed instruction

	 * was probably an indirect function call with a NULL function pointer.

	 * That means that SP points into the middle of an incomplete frame:

	 * *SP is a return pointer, and *(SP-sizeof(unsigned long)) is where we

	 * would have written a frame pointer if we hadn't crashed.

	 * Pretend that the frame is complete and that BP points to it, but save

	 * the real BP so that we can use it when looking for the next frame.

	 */

	if (regs && regs->ip == 0 &&

	    (unsigned long *)kernel_stack_pointer(regs) >= first_frame) {

		state->next_bp = bp;

		bp = ((unsigned long *)kernel_stack_pointer(regs)) - 1;

	}



	/* Initialize stack info and make sure the frame data is accessible: */

	/* Initialize stack info and make sure the frame data is accessible: */

	get_stack_info(bp, state->task, &state->stack_info,

	get_stack_info(bp, state->task, &state->stack_info,

		       &state->stack_mask);

		       &state->stack_mask);
@@ -410,7 +429,7 @@ void __unwind_start(struct unwind_state *state, struct task_struct *task, 
	 */

	 */

	while (!unwind_done(state) &&

	while (!unwind_done(state) &&

	       (!on_stack(&state->stack_info, first_frame, sizeof(long)) ||

	       (!on_stack(&state->stack_info, first_frame, sizeof(long)) ||

			state->bp < first_frame))

			(state->next_bp == NULL && state->bp < first_frame)))

		unwind_next_frame(state);

		unwind_next_frame(state);

}

}

EXPORT_SYMBOL_GPL(__unwind_start);
 
EXPORT_SYMBOL_GPL(__unwind_start);