linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* WARNING: at arch/x86/kernel/stacktrace.c:132 save_stack_trace_tsk_reliable
@ 2017-08-25 13:54 Jiri Slaby
  2017-08-25 15:01 ` Josh Poimboeuf
  0 siblings, 1 reply; 4+ messages in thread
From: Jiri Slaby @ 2017-08-25 13:54 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: x86, Linux kernel mailing list

Hi,

so we are testing klp with ORC. And we see
WARNING: CPU: 0 PID: 4574 at ../arch/x86/kernel/stacktrace.c:132
save_stack_trace_tsk_reliable+0x130/0x1b0
Modules linked in: ...
...
Call Trace:
 klp_try_switch_task+0x10a/0x2c0
 klp_try_complete_transition+0x121/0x1b0
 __klp_enable_patch+0xb6/0xe0
 klp_enable_patch+0x68/0x70
 livepatch_init+0x28/0x50 [klp_tc_3_live_patch_getpid]




Staring into the code, it is caused by save_stack_trace_tsk of the
CPU0's idle task (task = idle_task(0);).

Its stack trace should look like:
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
ffffffffffffffff 0xffffffffffffffff



1) To have nice secondary_startup_64 there instead of misleading
verify_cpu+0, I had to do:
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -303,6 +305,7 @@ ENTRY(secondary_startup_64)
        pushq   %rax            # target address in negative space
        lretq
 .Lafter_lret:
+       nop
 ENDPROC(secondary_startup_64)

 #include "verify_cpu.S"




2) secondary_startup_64 lives at ffffffff810001bf. It is very near
_stext (+0x1bf) and there are no orc entries for that and neither before
that. But orc_find still returns the very first orc entry to me even
though my address is lower:
ffffffff810002d0: sp:sp+8 bp:(und) type:call

The entry is for run_init_process from init/main.c.

Obviously secondary_startup_64 is from head_64.S which has no orc
entries due to OBJECT_FILES_NON_STANDARD_head_$(BITS).o := y.

So doing this had no chance:
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -169,6 +169,7 @@ startup_64:
        movq    $(early_level4_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.


Anyway, I went the way to fix __orc_find anyway. It should return NULL
if the entry is not found (i.e. the found entry has higher IP), so I did:
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -52,6 +53,8 @@ static struct orc_entry *__orc_find(int
                } else
                        last = mid - 1;
        }
+       if (orc_ip(found) > ip)
+               return NULL;

        return u_table + (found - ip_table);
 }


But now, unwind_init does not succeed, because it cannot find an entry
for LOOKUP_START_IP in the orc_lookup[i] setup loop. Obviously,
head_64.o is at LOOKUP_START_IP with no orc entry in there.

So I did this hack instead of the above:
@@ -52,6 +53,8 @@ static struct orc_entry *__orc_find(int
                } else
                        last = mid - 1;
        }
+       if (orc_init && orc_ip(found) > ip)
+               return NULL;

        return u_table + (found - ip_table);
 }

which produces the nice stack trace on the top.

So now, there are two things:
1) how to fix orc_find & unwind_init properly?
2) how to generate an EMPTY hint for secondary_startup_64 and maybe
later REGS hint after the rsp setup there? The function seems to be a
pretty killer for orc generate (among others in head_64.S):

../orc/arch/x86/kernel/head_64.o: warning: objtool:
secondary_startup_64()+0x25: sibling call from callable instruction with
modified stack frame
../orc/arch/x86/kernel/head_64.o: warning: objtool:
early_idt_handler_array()+0x4: sibling call from callable instruction
with modified stack frame
../orc/arch/x86/kernel/head_64.o: warning: objtool:
early_idt_handler_common()+0x4d: sibling call from callable instruction
with modified stack frame
../orc/arch/x86/kernel/head_64.o: warning: objtool:
secondary_startup_64()+0x9d: unsupported instruction in callable function

thanks,
-- 
js
suse labs

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: WARNING: at arch/x86/kernel/stacktrace.c:132 save_stack_trace_tsk_reliable
  2017-08-25 13:54 WARNING: at arch/x86/kernel/stacktrace.c:132 save_stack_trace_tsk_reliable Jiri Slaby
@ 2017-08-25 15:01 ` Josh Poimboeuf
  2017-08-25 20:31   ` Josh Poimboeuf
  0 siblings, 1 reply; 4+ messages in thread
From: Josh Poimboeuf @ 2017-08-25 15:01 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: x86, Linux kernel mailing list

On Fri, Aug 25, 2017 at 03:54:12PM +0200, Jiri Slaby wrote:
> So now, there are two things:
> 1) how to fix orc_find & unwind_init properly?
> 2) how to generate an EMPTY hint for secondary_startup_64 and maybe
> later REGS hint after the rsp setup there? The function seems to be a
> pretty killer for orc generate (among others in head_64.S):

I think the solution is to get head_64.S annotated properly.  Then there
won't be a gap between _stext and the first ORC entry, and we won't need
to fix __orc_find().

I can work up a patch for you to test with.

-- 
Josh

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: WARNING: at arch/x86/kernel/stacktrace.c:132 save_stack_trace_tsk_reliable
  2017-08-25 15:01 ` Josh Poimboeuf
@ 2017-08-25 20:31   ` Josh Poimboeuf
  2017-08-31 11:01     ` Jiri Slaby
  0 siblings, 1 reply; 4+ messages in thread
From: Josh Poimboeuf @ 2017-08-25 20:31 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: x86, Linux kernel mailing list

On Fri, Aug 25, 2017 at 10:01:58AM -0500, Josh Poimboeuf wrote:
> On Fri, Aug 25, 2017 at 03:54:12PM +0200, Jiri Slaby wrote:
> > So now, there are two things:
> > 1) how to fix orc_find & unwind_init properly?
> > 2) how to generate an EMPTY hint for secondary_startup_64 and maybe
> > later REGS hint after the rsp setup there? The function seems to be a
> > pretty killer for orc generate (among others in head_64.S):
> 
> I think the solution is to get head_64.S annotated properly.  Then there
> won't be a gap between _stext and the first ORC entry, and we won't need
> to fix __orc_find().
> 
> I can work up a patch for you to test with.

Can you try this?


diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 287eac7d207f..e2315aecc441 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -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
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 513cbb012ecc..a0970b3bfd89 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -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
@@ -234,7 +237,8 @@ ENTRY(secondary_startup_64)
 	pushq	%rax		# target address in negative space
 	lretq
 .Lafter_lret:
-ENDPROC(secondary_startup_64)
+	nop
+END(secondary_startup_64)
 
 #include "verify_cpu.S"
 
@@ -246,6 +250,7 @@ ENDPROC(secondary_startup_64)
  */
 ENTRY(start_cpu0)
 	movq	initial_stack(%rip), %rsp
+	UNWIND_HINT_EMPTY
 	jmp	.Ljump_to_C_code
 ENDPROC(start_cpu0)
 #endif
@@ -265,26 +270,24 @@ ENDPROC(start_cpu0)
 	.quad  init_thread_union + THREAD_SIZE - SIZEOF_PTREGS
 	__FINITDATA
 
-bad_address:
-	jmp 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
-	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
-ENDPROC(early_idt_handler_array)
+	UNWIND_HINT_IRET_REGS offset=16
+END(early_idt_handler_array)
 
 early_idt_handler_common:
 	/*
@@ -312,6 +315,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
@@ -327,7 +331,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
 
@@ -434,7 +438,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
diff --git a/arch/x86/kernel/verify_cpu.S b/arch/x86/kernel/verify_cpu.S
index 014ea59aa153..3d3c2f71f617 100644
--- a/arch/x86/kernel/verify_cpu.S
+++ b/arch/x86/kernel/verify_cpu.S
@@ -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)
diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S
index a7525e95d53f..8f9741cbb32f 100644
--- a/arch/x86/xen/xen-head.S
+++ b/arch/x86/xen/xen-head.S
@@ -19,6 +19,7 @@
 #ifdef CONFIG_XEN_PV
 	__INIT
 ENTRY(startup_xen)
+	UNWIND_HINT_EMPTY
 	cld
 
 	/* Clear .bss */
@@ -33,21 +34,24 @@ ENTRY(startup_xen)
 	mov $init_thread_union+THREAD_SIZE, %_ASM_SP
 
 	jmp xen_start_kernel
-
+END(startup_xen)
 	__FINIT
 #endif
 
 .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; \
 	.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")
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 5d6a3be10aee..21b432e417df 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1765,11 +1765,13 @@ 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;

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: WARNING: at arch/x86/kernel/stacktrace.c:132 save_stack_trace_tsk_reliable
  2017-08-25 20:31   ` Josh Poimboeuf
@ 2017-08-31 11:01     ` Jiri Slaby
  0 siblings, 0 replies; 4+ messages in thread
From: Jiri Slaby @ 2017-08-31 11:01 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: x86, Linux kernel mailing list

On 08/25/2017, 10:31 PM, Josh Poimboeuf wrote:
> On Fri, Aug 25, 2017 at 10:01:58AM -0500, Josh Poimboeuf wrote:
>> On Fri, Aug 25, 2017 at 03:54:12PM +0200, Jiri Slaby wrote:
>>> So now, there are two things:
>>> 1) how to fix orc_find & unwind_init properly?
>>> 2) how to generate an EMPTY hint for secondary_startup_64 and maybe
>>> later REGS hint after the rsp setup there? The function seems to be a
>>> pretty killer for orc generate (among others in head_64.S):
>>
>> I think the solution is to get head_64.S annotated properly.  Then there
>> won't be a gap between _stext and the first ORC entry, and we won't need
>> to fix __orc_find().
>>
>> I can work up a patch for you to test with.
> 
> Can you try this?

Yep, this works for me. Except I left bad_address in place as I tested
against 4.12.

thanks,
-- 
js
suse labs

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-08-31 11:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-25 13:54 WARNING: at arch/x86/kernel/stacktrace.c:132 save_stack_trace_tsk_reliable Jiri Slaby
2017-08-25 15:01 ` Josh Poimboeuf
2017-08-25 20:31   ` Josh Poimboeuf
2017-08-31 11:01     ` Jiri Slaby

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).