Commit fb799447 authored by Josh Poimboeuf's avatar Josh Poimboeuf Committed by Peter Zijlstra
Browse files

x86,objtool: Split UNWIND_HINT_EMPTY in two 



Mark reported that the ORC unwinder incorrectly marks an unwind as
reliable when the unwind terminates prematurely in the dark corners of
return_to_handler() due to lack of information about the next frame.

The problem is UNWIND_HINT_EMPTY is used in two different situations:

  1) The end of the kernel stack unwind before hitting user entry, boot
     code, or fork entry

  2) A blind spot in ORC coverage where the unwinder has to bail due to
     lack of information about the next frame

The ORC unwinder has no way to tell the difference between the two.
When it encounters an undefined stack state with 'end=1', it blindly
marks the stack reliable, which can break the livepatch consistency
model.

Fix it by splitting UNWIND_HINT_EMPTY into UNWIND_HINT_UNDEFINED and
UNWIND_HINT_END_OF_STACK.

Reported-by: default avatarMark Rutland <mark.rutland@arm.com>
Signed-off-by: default avatarJosh Poimboeuf <jpoimboe@kernel.org>
Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: default avatarSteven Rostedt (Google) <rostedt@goodmis.org>
Acked-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/fd6212c8b450d3564b855e1cb48404d6277b4d9f.1677683419.git.jpoimboe@kernel.org
parent 4708ea14

Documentation/livepatch/reliable-stacktrace.rst

+1 −1
Original line number Diff line number Diff line
@@ -183,7 +183,7 @@ trampoline or return trampoline. For example, considering the x86_64 
.. code-block:: none



   SYM_CODE_START(return_to_handler)

           UNWIND_HINT_EMPTY

           UNWIND_HINT_UNDEFINED

           subq  $24, %rsp



           /* Save the return values */

arch/x86/entry/entry_64.S

+6 −6
Original line number Diff line number Diff line
@@ -205,7 +205,7 @@ syscall_return_via_sysret: 
	 */

	movq	%rsp, %rdi

	movq	PER_CPU_VAR(cpu_tss_rw + TSS_sp0), %rsp

	UNWIND_HINT_EMPTY

	UNWIND_HINT_END_OF_STACK



	pushq	RSP-RDI(%rdi)	/* RSP */

	pushq	(%rdi)		/* RDI */
@@ -286,7 +286,7 @@ SYM_FUNC_END(__switch_to_asm) 
.pushsection .text, "ax"

	__FUNC_ALIGN

SYM_CODE_START_NOALIGN(ret_from_fork)

	UNWIND_HINT_EMPTY

	UNWIND_HINT_END_OF_STACK

	ANNOTATE_NOENDBR // copy_thread

	CALL_DEPTH_ACCOUNT

	movq	%rax, %rdi
@@ -303,7 +303,7 @@ SYM_CODE_START_NOALIGN(ret_from_fork) 


1:

	/* kernel thread */

	UNWIND_HINT_EMPTY

	UNWIND_HINT_END_OF_STACK

	movq	%r12, %rdi

	CALL_NOSPEC rbx

	/*
@@ -643,7 +643,7 @@ SYM_INNER_LABEL(swapgs_restore_regs_and_return_to_usermode, SYM_L_GLOBAL) 
	 */

	movq	%rsp, %rdi

	movq	PER_CPU_VAR(cpu_tss_rw + TSS_sp0), %rsp

	UNWIND_HINT_EMPTY

	UNWIND_HINT_END_OF_STACK



	/* Copy the IRET frame to the trampoline stack. */

	pushq	6*8(%rdi)	/* SS */
@@ -869,7 +869,7 @@ SYM_CODE_END(exc_xen_hypervisor_callback) 
 */

	__FUNC_ALIGN

SYM_CODE_START_NOALIGN(xen_failsafe_callback)

	UNWIND_HINT_EMPTY

	UNWIND_HINT_UNDEFINED

	ENDBR

	movl	%ds, %ecx

	cmpw	%cx, 0x10(%rsp)
@@ -1520,7 +1520,7 @@ SYM_CODE_END(asm_exc_nmi) 
 * MSRs to fully disable 32-bit SYSCALL.

 */

SYM_CODE_START(ignore_sysret)

	UNWIND_HINT_EMPTY

	UNWIND_HINT_END_OF_STACK

	ENDBR

	mov	$-ENOSYS, %eax

	sysretl

arch/x86/include/asm/orc_types.h

+7 −7
Original line number Diff line number Diff line
@@ -39,9 +39,11 @@ 
#define ORC_REG_SP_INDIRECT		9

#define ORC_REG_MAX			15



#define ORC_TYPE_CALL			0

#define ORC_TYPE_REGS			1

#define ORC_TYPE_REGS_PARTIAL		2

#define ORC_TYPE_UNDEFINED		0

#define ORC_TYPE_END_OF_STACK		1

#define ORC_TYPE_CALL			2

#define ORC_TYPE_REGS			3

#define ORC_TYPE_REGS_PARTIAL		4



#ifndef __ASSEMBLY__

#include <asm/byteorder.h>
@@ -60,16 +62,14 @@ struct orc_entry { 
#if defined(__LITTLE_ENDIAN_BITFIELD)

	unsigned	sp_reg:4;

	unsigned	bp_reg:4;

	unsigned	type:2;

	unsigned	type:3;

	unsigned	signal:1;

	unsigned	end:1;

#elif defined(__BIG_ENDIAN_BITFIELD)

	unsigned	bp_reg:4;

	unsigned	sp_reg:4;

	unsigned	unused:4;

	unsigned	end:1;

	unsigned	signal:1;

	unsigned	type:2;

	unsigned	type:3;

#endif

} __packed;

arch/x86/include/asm/unwind_hints.h

+8 −4
Original line number Diff line number Diff line
@@ -7,13 +7,17 @@ 


#ifdef __ASSEMBLY__



.macro UNWIND_HINT_EMPTY

	UNWIND_HINT type=UNWIND_HINT_TYPE_CALL end=1

.macro UNWIND_HINT_END_OF_STACK

	UNWIND_HINT type=UNWIND_HINT_TYPE_END_OF_STACK

.endm



.macro UNWIND_HINT_UNDEFINED

	UNWIND_HINT type=UNWIND_HINT_TYPE_UNDEFINED

.endm



.macro UNWIND_HINT_ENTRY

	VALIDATE_UNRET_BEGIN

	UNWIND_HINT_EMPTY

	UNWIND_HINT_END_OF_STACK

.endm



.macro UNWIND_HINT_REGS base=%rsp offset=0 indirect=0 extra=1 partial=0 signal=1
@@ -73,7 +77,7 @@ 
#else



#define UNWIND_HINT_FUNC \

	UNWIND_HINT(UNWIND_HINT_TYPE_FUNC, ORC_REG_SP, 8, 0, 0)

	UNWIND_HINT(UNWIND_HINT_TYPE_FUNC, ORC_REG_SP, 8, 0)



#endif /* __ASSEMBLY__ */

arch/x86/kernel/ftrace_64.S

+1 −1
Original line number Diff line number Diff line
@@ -340,7 +340,7 @@ STACK_FRAME_NON_STANDARD_FP(__fentry__) 


#ifdef CONFIG_FUNCTION_GRAPH_TRACER

SYM_CODE_START(return_to_handler)

	UNWIND_HINT_EMPTY

	UNWIND_HINT_UNDEFINED

	ANNOTATE_NOENDBR

	subq  $16, %rsp