Skip to contentSkip to conversation

x86: remove the confusing entry in call trace

PATCH Applied jia zhang 3 participants 3 messages Nov 23, 2008 x86

lore
Share & tools
lore

jia zhang

author
#0 lore raw

PATCH 1/1 x86: remove the confusing entry in call trace

avoid the confusion in call trace because of the lack of padding at the tail of function.

---
When do_exit get call, the return address behind call instruction is pushed into stack. If something get wrong in do_exit, for x86_64, the entry "kernel_execve +0x00/0xXX" rather than "child_rip +0xYY/0xZZ" is remained in call trace. It looks confused.

 b/arch/x86/kernel/entry_32.S |    1 +
 b/arch/x86/kernel/entry_64.S |    1 +
 2 files changed, 2 insertions(+)
arch/x86/kernel/entry_32.S +1 -0
index 28b597e..3290819 100644
@@ -1051,6 +1051,7 @@ ENTRY(kernel_thread_helper)
 	push %eax
 	CFI_ADJUST_CFA_OFFSET 4
 	call do_exit
+	nop			# padding for call trace
 	CFI_ENDPROC
 ENDPROC(kernel_thread_helper)
 
arch/x86/kernel/entry_64.S +1 -0
index b86f332..6f677a6 100644
@@ -1170,6 +1170,7 @@ child_rip:
 	# exit
 	mov %eax, %edi
 	call do_exit
+	nop			# padding for call trace
 	CFI_ENDPROC
 ENDPROC(child_rip)
 
Acked-by on v1 1/1
#1 lore raw
Quoted 8 lines · jia zhang

avoid the confusion in call trace because of the lack of padding at the tail of function.

the return address behind call instruction is pushed into stack. If something get wrong in do_exit, for x86_64, the entry "kernel_execve +0x00/0xXX" rather than "child_rip +0xYY/0xZZ" is remained in call trace. It looks confused.

It's valuable to know from where do_exit is called, so that's the only reason why using "call" is important. Otherwise it could just be changed to a jmp as do_exit does not return. It's maybe a bit ugly to add the padding, but it does give better traces. Maybe a 'ret', 'int3', or 'ud2' would be slightly less offensive?

Anyhow, I don't have a problem with the added padding, so:

Quoted 27 lines
 b/arch/x86/kernel/entry_32.S |    1 +
 b/arch/x86/kernel/entry_64.S |    1 +
 2 files changed, 2 insertions(+)
arch/x86/kernel/entry_32.S +1 -0
index 28b597e..3290819 100644
@@ -1051,6 +1051,7 @@ ENTRY(kernel_thread_helper)
 	push %eax
 	CFI_ADJUST_CFA_OFFSET 4
 	call do_exit
+	nop			# padding for call trace
 	CFI_ENDPROC
 ENDPROC(kernel_thread_helper)
 
arch/x86/kernel/entry_64.S +1 -0
index b86f332..6f677a6 100644
@@ -1170,6 +1170,7 @@ child_rip:
 	# exit
 	mov %eax, %edi
 	call do_exit
+	nop			# padding for call trace
 	CFI_ENDPROC
 ENDPROC(child_rip)
Applied
#2 lore raw
Quoted 16 lines · Alexander van Heukelum
Quoted 8 lines · jia zhang

avoid the confusion in call trace because of the lack of padding at the tail of function.

the return address behind call instruction is pushed into stack. If something get wrong in do_exit, for x86_64, the entry "kernel_execve +0x00/0xXX" rather than "child_rip +0xYY/0xZZ" is remained in call trace. It looks confused.

It's valuable to know from where do_exit is called, so that's the only reason why using "call" is important. Otherwise it could just be changed to a jmp as do_exit does not return. It's maybe a bit ugly to add the padding, but it does give better traces. Maybe a 'ret', 'int3', or 'ud2' would be slightly less offensive?

Quoted 3 lines

Anyhow, I don't have a problem with the added padding, so:

applied to tip/x86/debug, thanks guys!

i changed it to u2d based on your suggestion - that makes it not only easier to understand, but also more robust, should do_exit() return. (which it should never in practice)

Ingo