Skip to contentSkip to conversation

x86/asm: Add unwind hint annotations to head code

PATCH Superseded Josh Poimboeuf 2 participants 11 messages Aug 31, 2017 – Sep 12, 2017

Subsystems (3)
x86 Xen hypervisor x86 objtool

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Josh Poimboeuf

author
#0 lore raw

cover x86/asm: Add unwind hint annotations to head code

Add some annotations to allow objtool to understand the head code.

Also do a few cleanups while I'm there.

Josh Poimboeuf (8):
objtool: Don't report end of section error after an empty unwind hint x86/head: Remove confusing comment
x86/head: Remove unused 'bad_address' code
x86/head: Fix head ELF function annotations
x86/boot: Annotate verify_cpu() as a callable function x86/xen: Fix xen head ELF annotations
x86/xen: Add unwind hint annotations
x86/head: Add unwind hint annotations

 arch/x86/kernel/Makefile     |  1 -
 arch/x86/kernel/head_64.S    | 27 +++++++++++++++------------
 arch/x86/kernel/verify_cpu.S |  3 ++-
 arch/x86/xen/xen-head.S      | 10 +++++++---
 tools/objtool/check.c        |  7 +++++--
 5 files changed, 29 insertions(+), 19 deletions(-)
Signature · 1 line
2.13.5
author
#1 lore raw

PATCH 1/8 objtool: Don't report end of section error after an empty unwind hint

If asm code specifies an UNWIND_HINT_EMPTY hint, don't warn if the section ends unexpectedly. This can happen with the xen-head.S code because the hypercall_page is "text" but it's all zeros.

Signed-off-by Josh Poimboeuf <[email protected]>
---
 tools/objtool/check.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
tools/objtool/check.c +5 -2
index f744617c9946..d8683f0fe04d 100644
@@ -1737,11 +1737,14 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
 		if (insn->dead_end)
 			return 0;
 
-		insn = next_insn;
-		if (!insn) {
+		if (!next_insn) {
+			if (state.cfa.base == CFI_UNDEFINED)
+				return 0;
 			WARN("%s: unexpected end of section", sec->name);
 			return 1;
 		}
+
+		insn = next_insn;
 	}
 
 	return 0;
Signature · 1 line
2.13.5
author
#2 lore raw

PATCH 2/8 x86/head: Remove confusing comment

In reply to Josh Poimboeuf

This comment is actively wrong and confusing. It refers to the registers' stack offsets after the pt_regs has been constructed on the stack, but this code is *before* that.

At this point the stack just has the standard iret frame, for which no comment should be needed.

Signed-off-by Josh Poimboeuf <[email protected]>
---
 arch/x86/kernel/head_64.S | 4 ----
 1 file changed, 4 deletions(-)
arch/x86/kernel/head_64.S +0 -4
index 513cbb012ecc..3b04e4c99389 100644
@@ -270,10 +270,6 @@ bad_address:
 
 	__INIT
 ENTRY(early_idt_handler_array)
-	# 104(%rsp) %rflags
-	#  96(%rsp) %cs
-	#  88(%rsp) %rip
-	#  80(%rsp) error code
 	i = 0
 	.rept NUM_EXCEPTION_VECTORS
 	.ifeq (EXCEPTION_ERRCODE_MASK >> i) & 1
Signature · 1 line
2.13.5
author
#3 lore raw

PATCH 3/8 x86/head: Remove unused 'bad_address' code

In reply to Josh Poimboeuf

It's no longer possible for this code to be executed, so remove it.

Signed-off-by Josh Poimboeuf <[email protected]>
---
 arch/x86/kernel/head_64.S | 3 ---
 1 file changed, 3 deletions(-)
arch/x86/kernel/head_64.S +0 -3
index 3b04e4c99389..afb0a1e22d41 100644
@@ -265,9 +265,6 @@ ENDPROC(start_cpu0)
 	.quad  init_thread_union + THREAD_SIZE - SIZEOF_PTREGS
 	__FINITDATA
 
-bad_address:
-	jmp bad_address
-
 	__INIT
 ENTRY(early_idt_handler_array)
 	i = 0
Signature · 1 line
2.13.5
author
#4 lore raw

PATCH 4/8 x86/head: Fix head ELF function annotations

In reply to Josh Poimboeuf

These functions aren't callable C-type functions, so don't annotate them as such.

Signed-off-by Josh Poimboeuf <[email protected]>
---
 arch/x86/kernel/head_64.S | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
arch/x86/kernel/head_64.S +3 -3
index afb0a1e22d41..edacd579d504 100644
@@ -234,7 +234,7 @@ ENTRY(secondary_startup_64)
 	pushq	%rax		# target address in negative space
 	lretq
 .Lafter_lret:
-ENDPROC(secondary_startup_64)
+END(secondary_startup_64)
 
 #include "verify_cpu.S"
 

@@ -277,7 +277,7 @@ ENTRY(early_idt_handler_array)
 	i = i + 1
 	.fill early_idt_handler_array + i*EARLY_IDT_HANDLER_SIZE - ., 1, 0xcc
 	.endr
-ENDPROC(early_idt_handler_array)
+END(early_idt_handler_array)
 
 early_idt_handler_common:
 	/*

@@ -320,7 +320,7 @@ early_idt_handler_common:
 20:
 	decl early_recursion_flag(%rip)
 	jmp restore_regs_and_iret
-ENDPROC(early_idt_handler_common)
+END(early_idt_handler_common)
 
 	__INITDATA
Signature · 1 line
2.13.5
author
#5 lore raw

PATCH 5/8 x86/boot: Annotate verify_cpu() as a callable function

In reply to Josh Poimboeuf

verify_cpu() is a callable function. Annotate it as such.

Signed-off-by Josh Poimboeuf <[email protected]>
---
 arch/x86/kernel/verify_cpu.S | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/x86/kernel/verify_cpu.S +2 -1
index 014ea59aa153..3d3c2f71f617 100644
@@ -33,7 +33,7 @@
 #include <asm/cpufeatures.h>
 #include <asm/msr-index.h>
 
-verify_cpu:
+ENTRY(verify_cpu)
 	pushf				# Save caller passed flags
 	push	$0			# Kill any dangerous flags
 	popf

@@ -139,3 +139,4 @@ verify_cpu:
 	popf				# Restore caller passed flags
 	xorl %eax, %eax
 	ret
+ENDPROC(verify_cpu)
Signature · 1 line
2.13.5
author
#6 lore raw

PATCH 6/8 x86/xen: Fix xen head ELF annotations

In reply to Josh Poimboeuf

Mark the ends of the startup_xen and hypercall_page code sections.

Signed-off-by Josh Poimboeuf <[email protected]>
---
 arch/x86/xen/xen-head.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/xen/xen-head.S +2 -2
index a7525e95d53f..9753225289e8 100644
@@ -33,7 +33,7 @@ ENTRY(startup_xen)
 	mov $init_thread_union+THREAD_SIZE, %_ASM_SP
 
 	jmp xen_start_kernel
-
+END(startup_xen)
 	__FINIT
 #endif
 

@@ -47,7 +47,7 @@ ENTRY(hypercall_page)
 	.type xen_hypercall_##n, @function; .size xen_hypercall_##n, 32
 #include <asm/xen-hypercalls.h>
 #undef HYPERCALL
-
+END(hypercall_page)
 .popsection
 
 	ELFNOTE(Xen, XEN_ELFNOTE_GUEST_OS,       .asciz "linux")
Signature · 1 line
2.13.5
author
#7 lore raw

PATCH 7/8 x86/xen: Add unwind hint annotations

In reply to Josh Poimboeuf

Add unwind hint annotations to the xen head code so the ORC unwinder can read head_64.o.

hypercall_page needs empty annotations at 32-byte intervals to match the 'xen_hypercall_*' ELF functions at those locations.

Signed-off-by Josh Poimboeuf <[email protected]>
---
 arch/x86/xen/xen-head.S | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
arch/x86/xen/xen-head.S +5 -1
index 9753225289e8..8f9741cbb32f 100644
@@ -19,6 +19,7 @@
 #ifdef CONFIG_XEN_PV
 	__INIT
 ENTRY(startup_xen)
+	UNWIND_HINT_EMPTY
 	cld
 
 	/* Clear .bss */

@@ -40,7 +41,10 @@ END(startup_xen)
 .pushsection .text
 	.balign PAGE_SIZE
 ENTRY(hypercall_page)
-	.skip PAGE_SIZE
+	.rept (PAGE_SIZE / 32)
+		UNWIND_HINT_EMPTY
+		.skip 32
+	.endr
 
 #define HYPERCALL(n) \
 	.equ xen_hypercall_##n, hypercall_page + __HYPERVISOR_##n * 32; \
Signature · 1 line
2.13.5
#8 lore raw

Re: [PATCH 7/8] x86/xen: Add unwind hint annotations

Quoted 2 lines · Josh Poimboeuf

Add unwind hint annotations to the xen head code so the ORC unwinder can read head_64.o.

xen-head.S is included also in head_32.S, so we see:
arch/x86/kernel/../../x86/xen/xen-head.S:47: Error: no such instruction:
`unwind_hint_empty'
scripts/Makefile.build:413: recipe for target 'arch/x86/kernel/head_32.o' failed

To fix this, I use:

arch/x86/kernel/head_32.S +1 -0
@@ -25,6 +25,7 @@
 #include <asm/bootparam.h>
 #include <asm/export.h>
 #include <asm/pgtable_32.h>
+#include "../entry/calling.h"
 
 /* Physical address */
 #define pa(X) ((X) - __PAGE_OFFSET)
Quoted 33 lines

hypercall_page needs empty annotations at 32-byte intervals to match the 'xen_hypercall_*' ELF functions at those locations.

Signed-off-by Josh Poimboeuf <[email protected]>
---
 arch/x86/xen/xen-head.S | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
arch/x86/xen/xen-head.S +5 -1
index 9753225289e8..8f9741cbb32f 100644
@@ -19,6 +19,7 @@
 #ifdef CONFIG_XEN_PV
 	__INIT
 ENTRY(startup_xen)
+	UNWIND_HINT_EMPTY
 	cld
 
 	/* Clear .bss */

@@ -40,7 +41,10 @@ END(startup_xen)
 .pushsection .text
 	.balign PAGE_SIZE
 ENTRY(hypercall_page)
-	.skip PAGE_SIZE
+	.rept (PAGE_SIZE / 32)
+		UNWIND_HINT_EMPTY
+		.skip 32
+	.endr
 
 #define HYPERCALL(n) \
 	.equ xen_hypercall_##n, hypercall_page + __HYPERVISOR_##n * 32; \
Signature · 2 lines
js
suse labs
author
#9 lore raw

Re: [PATCH 7/8] x86/xen: Add unwind hint annotations

Quoted 15 lines · Jiri Slaby
Quoted 2 lines · Josh Poimboeuf

Add unwind hint annotations to the xen head code so the ORC unwinder can read head_64.o.

xen-head.S is included also in head_32.S, so we see:
arch/x86/kernel/../../x86/xen/xen-head.S:47: Error: no such instruction:
`unwind_hint_empty'
scripts/Makefile.build:413: recipe for target 'arch/x86/kernel/head_32.o' failed

To fix this, I use:

arch/x86/kernel/head_32.S line 31

Quoted 7 lines
arch/x86/kernel/head_32.S +1 -0
@@ -25,6 +25,7 @@
 #include <asm/bootparam.h>
 #include <asm/export.h>
 #include <asm/pgtable_32.h>
+#include "../entry/calling.h"
 
 /* Physical address */
 #define pa(X) ((X) - __PAGE_OFFSET)

Thanks. 0-day reported this as well. Will fix this in v2.

Signature · 1 line
Josh
author
#10 lore raw

PATCH 8/8 x86/head: Add unwind hint annotations

In reply to Josh Poimboeuf

Jiri Slaby reported an ORC issue when unwinding from an idle task. The stack was:

    ffffffff811083c2 do_idle+0x142/0x1e0
    ffffffff8110861d cpu_startup_entry+0x5d/0x60
    ffffffff82715f58 start_kernel+0x3ff/0x407
    ffffffff827153e8 x86_64_start_kernel+0x14e/0x15d
    ffffffff810001bf secondary_startup_64+0x9f/0xa0

The ORC unwinder errored out at secondary_startup_64 because the head code isn't annotated yet so there wasn't a corresponding ORC entry.

Fix that and any other head-related unwinding issues by adding unwind hints to the head code.

Reported-by Jiri Slaby <[email protected]>
Tested-by Jiri Slaby <[email protected]>
Signed-off-by Josh Poimboeuf <[email protected]>
---
 arch/x86/kernel/Makefile  |  1 -
 arch/x86/kernel/head_64.S | 14 ++++++++++++--
 2 files changed, 12 insertions(+), 3 deletions(-)
arch/x86/kernel/Makefile +0 -1
index fd0a7895b63f..d8e2b700d1db 100644
@@ -26,7 +26,6 @@ KASAN_SANITIZE_dumpstack.o				:= n
 KASAN_SANITIZE_dumpstack_$(BITS).o			:= n
 KASAN_SANITIZE_stacktrace.o := n
 
-OBJECT_FILES_NON_STANDARD_head_$(BITS).o		:= y
 OBJECT_FILES_NON_STANDARD_relocate_kernel_$(BITS).o	:= y
 OBJECT_FILES_NON_STANDARD_ftrace_$(BITS).o		:= y
 OBJECT_FILES_NON_STANDARD_test_nx.o			:= y
arch/x86/kernel/head_64.S +12 -2
index edacd579d504..42e32c2e51bb 100644
@@ -49,6 +49,7 @@ L3_START_KERNEL = pud_index(__START_KERNEL_map)
 	.code64
 	.globl startup_64
 startup_64:
+	UNWIND_HINT_EMPTY
 	/*
 	 * At this point the CPU runs in 64bit mode CS.L = 1 CS.D = 0,
 	 * and someone has loaded an identity mapped page table

@@ -88,6 +89,7 @@ startup_64:
 	addq	$(early_top_pgt - __START_KERNEL_map), %rax
 	jmp 1f
 ENTRY(secondary_startup_64)
+	UNWIND_HINT_EMPTY
 	/*
 	 * At this point the CPU runs in 64bit mode CS.L = 1 CS.D = 0,
 	 * and someone has loaded a mapped page table.

@@ -132,6 +134,7 @@ ENTRY(secondary_startup_64)
 	movq	$1f, %rax
 	jmp	*%rax
 1:
+	UNWIND_HINT_EMPTY
 
 	/* Check if nx is implemented */
 	movl	$0x80000001, %eax

@@ -246,6 +249,7 @@ END(secondary_startup_64)
  */
 ENTRY(start_cpu0)
 	movq	initial_stack(%rip), %rsp
+	UNWIND_HINT_EMPTY
 	jmp	.Ljump_to_C_code
 ENDPROC(start_cpu0)
 #endif

@@ -270,13 +274,18 @@ ENTRY(early_idt_handler_array)
 	i = 0
 	.rept NUM_EXCEPTION_VECTORS
 	.ifeq (EXCEPTION_ERRCODE_MASK >> i) & 1
-	pushq $0		# Dummy error code, to make stack frame uniform
+		UNWIND_HINT_IRET_REGS
+		pushq $0	# Dummy error code, to make stack frame uniform
+	.else
+		UNWIND_HINT_IRET_REGS offset=8
 	.endif
 	pushq $i		# 72(%rsp) Vector number
 	jmp early_idt_handler_common
+	UNWIND_HINT_IRET_REGS
 	i = i + 1
 	.fill early_idt_handler_array + i*EARLY_IDT_HANDLER_SIZE - ., 1, 0xcc
 	.endr
+	UNWIND_HINT_IRET_REGS offset=16
 END(early_idt_handler_array)
 
 early_idt_handler_common:

@@ -305,6 +314,7 @@ early_idt_handler_common:
 	pushq %r13				/* pt_regs->r13 */
 	pushq %r14				/* pt_regs->r14 */
 	pushq %r15				/* pt_regs->r15 */
+	UNWIND_HINT_REGS
 
 	cmpq $14,%rsi		/* Page fault? */
 	jnz 10f

@@ -427,7 +437,7 @@ ENTRY(phys_base)
 EXPORT_SYMBOL(phys_base)
 
 #include "../../x86/xen/xen-head.S"
-	
+
 	__PAGE_ALIGNED_BSS
 NEXT_PAGE(empty_zero_page)
 	.skip PAGE_SIZE
Signature · 1 line
2.13.5