x86/entry/head: standardize the end of the stack
Subsystems (2)
cover x86/entry/head: standardize the end of the stack
Thanks to all the recent x86 entry code refactoring, most tasks' kernel stacks start at the same offset right below their saved pt_regs, regardless of which syscall was used to enter the kernel. That creates a nice convention which makes it straightforward to identify the end of the stack, which can be useful [*] for the unwinder.
But there a few places where tasks don't yet follow the convention. This patch set finishes the job.
[*] This will be useful for three upcoming proposed features:
- Detecting corrupt stacks in the unwinder (which will also be a force for ensuring this end of stack convention continues to be followed in the future).
- Printing all saved pt_regs on the stack during an oops/warning.
- Validating stacks in the livepatch consistency model.
Josh Poimboeuf (9):
x86/entry/head/32: use local labels
x86/entry/32: rename 'error_code' to 'common_exception' x86/entry/32: fix the end of the stack for newly forked tasks x86/head/32: fix the end of the stack for idle tasks x86/smp: fix initial idle stack location on 32-bit x86/asm/head: use a common function for starting CPUs x86/head: put real return address on idle task stack x86/head: fix the end of the stack for idle tasks
x86: move _stext marker to before head code
arch/x86/entry/entry_32.S | 120 ++++++++++++++++++++++++------------------ arch/x86/kernel/head_32.S | 49 ++++++++++------- arch/x86/kernel/head_64.S | 42 +++++++-------- arch/x86/kernel/smpboot.c | 4 +- arch/x86/kernel/vmlinux.lds.S | 2 +- 5 files changed, 122 insertions(+), 95 deletions(-)
Signature · 1 line
2.7.4
PATCH 1/9 x86/entry/head/32: use local labels
Add the local label prefix to all non-function named labels in head_32.S and entry_32.S. In addition to decluttering the symbol table, it also will help stack traces to be more sensible. For example, the last reported function in the idle task stack trace will be startup_32_smp() instead of is486().
--- arch/x86/entry/entry_32.S | 55 ++++++++++++++++++++++++----------------------- arch/x86/kernel/head_32.S | 32 +++++++++++++-------------- 2 files changed, 44 insertions(+), 43 deletions(-)
arch/x86/entry/entry_32.S +28 -27
@@ -64,7 +64,7 @@ # define preempt_stop(clobbers) DISABLE_INTERRUPTS(clobbers); TRACE_IRQS_OFF #else # define preempt_stop(clobbers) -# define resume_kernel restore_all +# define resume_kernel .Lrestore_all #endif .macro TRACE_IRQS_IRET
@@ -255,7 +255,7 @@ ENTRY(ret_from_fork) /* When we fork, we trace the syscall return in the child, too. */ movl %esp, %eax call syscall_return_slowpath - jmp restore_all + jmp .Lrestore_all /* kernel thread */ 1: movl %edi, %eax
@@ -300,19 +300,19 @@ ENTRY(resume_userspace) TRACE_IRQS_OFF movl %esp, %eax call prepare_exit_to_usermode - jmp restore_all + jmp .Lrestore_all END(ret_from_exception) #ifdef CONFIG_PREEMPT ENTRY(resume_kernel) DISABLE_INTERRUPTS(CLBR_ANY) -need_resched: +.Lneed_resched: cmpl $0, PER_CPU_VAR(__preempt_count) - jnz restore_all + jnz .Lrestore_all testl $X86_EFLAGS_IF, PT_EFLAGS(%esp) # interrupts off (exception path) ? - jz restore_all + jz .Lrestore_all call preempt_schedule_irq - jmp need_resched + jmp .Lneed_resched END(resume_kernel) #endif
@@ -333,7 +333,7 @@ GLOBAL(__begin_SYSENTER_singlestep_region) */ ENTRY(xen_sysenter_target) addl $5*4, %esp /* remove xen-provided frame */ - jmp sysenter_past_esp + jmp .Lsysenter_past_esp #endif /*
@@ -370,7 +370,7 @@ ENTRY(xen_sysenter_target) */ ENTRY(entry_SYSENTER_32) movl TSS_sysenter_sp0(%esp), %esp -sysenter_past_esp: +.Lsysenter_past_esp: pushl $__USER_DS /* pt_regs->ss */ pushl %ebp /* pt_regs->sp (stashed in bp) */ pushfl /* pt_regs->flags (except IF = 0) */
@@ -501,11 +501,11 @@ ENTRY(entry_INT80_32) call do_int80_syscall_32 .Lsyscall_32_done: -restore_all: +.Lrestore_all: TRACE_IRQS_IRET -restore_all_notrace: +.Lrestore_all_notrace: #ifdef CONFIG_X86_ESPFIX32 - ALTERNATIVE "jmp restore_nocheck", "", X86_BUG_ESPFIX + ALTERNATIVE "jmp .Lrestore_nocheck", "", X86_BUG_ESPFIX movl PT_EFLAGS(%esp), %eax # mix EFLAGS, SS and CS /*
@@ -517,22 +517,23 @@ restore_all_notrace: movb PT_CS(%esp), %al andl $(X86_EFLAGS_VM | (SEGMENT_TI_MASK << 8) | SEGMENT_RPL_MASK), %eax cmpl $((SEGMENT_LDT << 8) | USER_RPL), %eax - je ldt_ss # returning to user-space with LDT SS + je .Lldt_ss # returning to user-space with LDT SS #endif -restore_nocheck: +.Lrestore_nocheck: RESTORE_REGS 4 # skip orig_eax/error_code -irq_return: +.Lirq_return: INTERRUPT_RETURN + .section .fixup, "ax" ENTRY(iret_exc ) pushl $0 # no error code pushl $do_iret_error jmp error_code .previous - _ASM_EXTABLE(irq_return, iret_exc) + _ASM_EXTABLE(.Lirq_return, iret_exc) #ifdef CONFIG_X86_ESPFIX32 -ldt_ss: +.Lldt_ss: /* * Setup and switch to ESPFIX stack *
@@ -561,7 +562,7 @@ ldt_ss: */ DISABLE_INTERRUPTS(CLBR_EAX) lss (%esp), %esp /* switch to espfix segment */ - jmp restore_nocheck + jmp .Lrestore_nocheck #endif ENDPROC(entry_INT80_32)
@@ -881,7 +882,7 @@ ftrace_call: popl %edx popl %ecx popl %eax -ftrace_ret: +.Lftrace_ret: #ifdef CONFIG_FUNCTION_GRAPH_TRACER .globl ftrace_graph_call ftrace_graph_call:
@@ -951,7 +952,7 @@ GLOBAL(ftrace_regs_call) popl %gs addl $8, %esp /* Skip orig_ax and ip */ popf /* Pop flags at end (no addl to corrupt flags) */ - jmp ftrace_ret + jmp .Lftrace_ret popf jmp ftrace_stub
@@ -962,7 +963,7 @@ ENTRY(mcount) jb ftrace_stub /* Paging not enabled yet? */ cmpl $ftrace_stub, ftrace_trace_function - jnz trace + jnz .Ltrace #ifdef CONFIG_FUNCTION_GRAPH_TRACER cmpl $ftrace_stub, ftrace_graph_return jnz ftrace_graph_caller
@@ -975,7 +976,7 @@ ftrace_stub: ret /* taken from glibc */ -trace: +.Ltrace: pushl %eax pushl %ecx pushl %edx
@@ -1114,7 +1115,7 @@ ENTRY(nmi) movl %ss, %eax cmpw $__ESPFIX_SS, %ax popl %eax - je nmi_espfix_stack + je .Lnmi_espfix_stack #endif pushl %eax # pt_regs->orig_ax
@@ -1130,7 +1131,7 @@ ENTRY(nmi) /* Not on SYSENTER stack. */ call do_nmi - jmp restore_all_notrace + jmp .Lrestore_all_notrace .Lnmi_from_sysenter_stack: /*
@@ -1141,10 +1142,10 @@ ENTRY(nmi) movl PER_CPU_VAR(cpu_current_top_of_stack), %esp call do_nmi movl %ebp, %esp - jmp restore_all_notrace + jmp .Lrestore_all_notrace #ifdef CONFIG_X86_ESPFIX32 -nmi_espfix_stack: +.Lnmi_espfix_stack: /* * create the pointer to lss back */
@@ -1162,7 +1163,7 @@ nmi_espfix_stack: call do_nmi RESTORE_REGS lss 12+4(%esp), %esp # back to espfix stack - jmp irq_return + jmp .Lirq_return #endif END(nmi)
arch/x86/kernel/head_32.S +16 -16
@@ -247,19 +247,19 @@ page_pde_offset = (__PAGE_OFFSET >> 20); #ifdef CONFIG_PARAVIRT /* This is can only trip for a broken bootloader... */ cmpw $0x207, pa(boot_params + BP_version) - jb default_entry + jb .Ldefault_entry /* Paravirt-compatible boot parameters. Look to see what architecture we're booting under. */ movl pa(boot_params + BP_hardware_subarch), %eax cmpl $num_subarch_entries, %eax - jae bad_subarch + jae .Lbad_subarch movl pa(subarch_entries)(,%eax,4), %eax subl $__PAGE_OFFSET, %eax jmp *%eax -bad_subarch: +.Lbad_subarch: WEAK(lguest_entry) WEAK(xen_entry) /* Unknown implementation; there's really
@@ -269,14 +269,14 @@ WEAK(xen_entry) __INITDATA subarch_entries: - .long default_entry /* normal x86/PC */ + .long .Ldefault_entry /* normal x86/PC */ .long lguest_entry /* lguest hypervisor */ .long xen_entry /* Xen hypervisor */ - .long default_entry /* Moorestown MID */ + .long .Ldefault_entry /* Moorestown MID */ num_subarch_entries = (. - subarch_entries) / 4 .previous #else - jmp default_entry + jmp .Ldefault_entry #endif /* CONFIG_PARAVIRT */ #ifdef CONFIG_HOTPLUG_CPU
@@ -316,7 +316,7 @@ ENTRY(startup_32_smp) call load_ucode_ap #endif -default_entry: +.Ldefault_entry: #define CR0_STATE (X86_CR0_PE | X86_CR0_MP | X86_CR0_ET | \ X86_CR0_NE | X86_CR0_WP | X86_CR0_AM | \ X86_CR0_PG)
@@ -346,7 +346,7 @@ default_entry: pushfl popl %eax # get EFLAGS testl $X86_EFLAGS_ID,%eax # did EFLAGS.ID remained set? - jz enable_paging # hw disallowed setting of ID bit + jz .Lenable_paging # hw disallowed setting of ID bit # which means no CPUID and no CR4 xorl %eax,%eax
@@ -356,13 +356,13 @@ default_entry: movl $1,%eax cpuid andl $~1,%edx # Ignore CPUID.FPU - jz enable_paging # No flags or only CPUID.FPU = no CR4 + jz .Lenable_paging # No flags or only CPUID.FPU = no CR4 movl pa(mmu_cr4_features),%eax movl %eax,%cr4 testb $X86_CR4_PAE, %al # check if PAE is enabled - jz enable_paging + jz .Lenable_paging /* Check if extended functions are implemented */ movl $0x80000000, %eax
@@ -370,7 +370,7 @@ default_entry: /* Value must be in the range 0x80000001 to 0x8000ffff */ subl $0x80000001, %eax cmpl $(0x8000ffff-0x80000001), %eax - ja enable_paging + ja .Lenable_paging /* Clear bogus XD_DISABLE bits */ call verify_cpu
@@ -379,7 +379,7 @@ default_entry: cpuid /* Execute Disable bit supported? */ btl $(X86_FEATURE_NX & 31), %edx - jnc enable_paging + jnc .Lenable_paging /* Setup EFER (Extended Feature Enable Register) */ movl $MSR_EFER, %ecx
@@ -389,7 +389,7 @@ default_entry: /* Make changes effective */ wrmsr -enable_paging: +.Lenable_paging: /* * Enable paging
@@ -418,7 +418,7 @@ enable_paging: */ movb $4,X86 # at least 486 cmpl $-1,X86_CPUID - je is486 + je .Lis486 /* get vendor info */ xorl %eax,%eax # call CPUID with 0 -> return vendor ID
@@ -429,7 +429,7 @@ enable_paging: movl %ecx,X86_VENDOR_ID+8 # last 4 chars orl %eax,%eax # do we have processor info as well? - je is486 + je .Lis486 movl $1,%eax # Use the CPUID instruction to get CPU type cpuid
@@ -443,7 +443,7 @@ enable_paging: movb %cl,X86_MASK movl %edx,X86_CAPABILITY -is486: +.Lis486: movl $0x50022,%ecx # set AM, WP, NE and MP movl %cr0,%eax andl $0x80000011,%eax # Save PG,PE,ET
Signature · 1 line
2.7.4
Re: [PATCH 1/9] x86/entry/head/32: use local labels
Quoted 7 lines · Josh Poimboeuf
Add the local label prefix to all non-function named labels in
head_32.Sandentry_32.S. In addition to decluttering the symbol table, it also will help stack traces to be more sensible. For example, the last reported function in the idle task stack trace will bestartup_32_smp()instead ofis486().
I think that restore_all, at least, should stay. It's a common tail for lots of functions. I haven't checked this patch for other cases where the new label is worse than the old for debugging.
--Andy
Re: [PATCH 1/9] x86/entry/head/32: use local labels
Quoted 9 lines · Andy Lutomirski
Quoted 7 lines · Josh Poimboeuf
Add the local label prefix to all non-function named labels in
head_32.Sandentry_32.S. In addition to decluttering the symbol table, it also will help stack traces to be more sensible. For example, the last reported function in the idle task stack trace will bestartup_32_smp()instead ofis486().
Quoted 3 lines
I think that
restore_all, at least, should stay. It's a common tail for lots of functions. I haven't checked this patch for other cases where the new label is worse than the old for debugging.
Yes, for restore_all, I think you're right. I'll change it back.
Signature · 1 line
Josh
PATCH 2/9 x86/entry/32: rename 'error_code' to 'common_exception'
In reply to Josh PoimboeufThe 'error_code' label is awkwardly named, especially when it shows up in a stack trace. Move it to its own local function and rename it to 'common_exception', analagous to the existing 'common_interrupt'.
This also makes related stack traces more sensible.
--- arch/x86/entry/entry_32.S | 43 +++++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 20 deletions(-)
arch/x86/entry/entry_32.S +23 -20
@@ -528,7 +528,7 @@ ENTRY(entry_INT80_32) ENTRY(iret_exc ) pushl $0 # no error code pushl $do_iret_error - jmp error_code + jmp common_exception .previous _ASM_EXTABLE(.Lirq_return, iret_exc)
@@ -659,7 +659,7 @@ ENTRY(coprocessor_error) ASM_CLAC pushl $0 pushl $do_coprocessor_error - jmp error_code + jmp common_exception END(coprocessor_error) ENTRY(simd_coprocessor_error)
@@ -673,14 +673,14 @@ ENTRY(simd_coprocessor_error) #else pushl $do_simd_coprocessor_error #endif - jmp error_code + jmp common_exception END(simd_coprocessor_error) ENTRY(device_not_available) ASM_CLAC pushl $-1 # mark this as an int pushl $do_device_not_available - jmp error_code + jmp common_exception END(device_not_available) #ifdef CONFIG_PARAVIRT
@@ -694,59 +694,59 @@ ENTRY(overflow) ASM_CLAC pushl $0 pushl $do_overflow - jmp error_code + jmp common_exception END(overflow) ENTRY(bounds) ASM_CLAC pushl $0 pushl $do_bounds - jmp error_code + jmp common_exception END(bounds) ENTRY(invalid_op) ASM_CLAC pushl $0 pushl $do_invalid_op - jmp error_code + jmp common_exception END(invalid_op) ENTRY(coprocessor_segment_overrun) ASM_CLAC pushl $0 pushl $do_coprocessor_segment_overrun - jmp error_code + jmp common_exception END(coprocessor_segment_overrun) ENTRY(invalid_TSS) ASM_CLAC pushl $do_invalid_TSS - jmp error_code + jmp common_exception END(invalid_TSS) ENTRY(segment_not_present) ASM_CLAC pushl $do_segment_not_present - jmp error_code + jmp common_exception END(segment_not_present) ENTRY(stack_segment) ASM_CLAC pushl $do_stack_segment - jmp error_code + jmp common_exception END(stack_segment) ENTRY(alignment_check) ASM_CLAC pushl $do_alignment_check - jmp error_code + jmp common_exception END(alignment_check) ENTRY(divide_error) ASM_CLAC pushl $0 # no error code pushl $do_divide_error - jmp error_code + jmp common_exception END(divide_error) #ifdef CONFIG_X86_MCE
@@ -754,7 +754,7 @@ ENTRY(machine_check) ASM_CLAC pushl $0 pushl machine_check_vector - jmp error_code + jmp common_exception END(machine_check) #endif
@@ -762,7 +762,7 @@ ENTRY(spurious_interrupt_bug) ASM_CLAC pushl $0 pushl $do_spurious_interrupt_bug - jmp error_code + jmp common_exception END(spurious_interrupt_bug) #ifdef CONFIG_XEN
@@ -1026,7 +1026,7 @@ return_to_handler: ENTRY(trace_page_fault) ASM_CLAC pushl $trace_do_page_fault - jmp error_code + jmp common_exception END(trace_page_fault) #endif
@@ -1034,7 +1034,10 @@ ENTRY(page_fault) ASM_CLAC pushl $do_page_fault ALIGN -error_code: + jmp common_exception +END(page_fault) + +common_exception: /* the function address is in %gs's slot on the stack */ pushl %fs pushl %es
@@ -1063,7 +1066,7 @@ error_code: movl %esp, %eax # pt_regs pointer call *%edi jmp ret_from_exception -END(page_fault) +END(common_exception) ENTRY(debug) /*
@@ -1180,14 +1183,14 @@ END(int3) ENTRY(general_protection) pushl $do_general_protection - jmp error_code + jmp common_exception END(general_protection) #ifdef CONFIG_KVM_GUEST ENTRY(async_page_fault) ASM_CLAC pushl $do_async_page_fault - jmp error_code + jmp common_exception END(async_page_fault) #endif
Signature · 1 line
2.7.4
PATCH 3/9 x86/entry/32: fix the end of the stack for newly forked tasks
In reply to Josh PoimboeufThanks to all the recent x86 entry code refactoring, most tasks' kernel stacks start at the same offset right below their saved pt_regs, regardless of which syscall was used to enter the kernel. That creates a nice convention which makes it straightforward to identify the end of the stack, which can be useful for the unwinder to verify the stack is sane.
Calling schedule_tail() directly breaks that convention because its an asmlinkage function so its argument has to be pushed on the stack. Add a wrapper which creates a proper "end of stack" frame header before the call.
--- arch/x86/entry/entry_32.S | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)
arch/x86/entry/entry_32.S +19 -3
@@ -44,6 +44,7 @@ #include <asm/alternative-asm.h> #include <asm/asm.h> #include <asm/smap.h> +#include <asm/frame.h> .section .entry.text, "ax"
@@ -237,6 +238,23 @@ ENTRY(__switch_to_asm) END(__switch_to_asm) /* + * The unwinder expects the last frame on the stack to always be at the same + * offset from the end of the page, which allows it to validate the stack. + * Calling schedule_tail() directly would break that convention because its an + * asmlinkage function so its argument has to be pushed on the stack. This + * wrapper creates a proper "end of stack" frame header before the call. + */ +ENTRY(schedule_tail_wrapper) + FRAME_BEGIN + + pushl %eax + call schedule_tail + popl %eax + + FRAME_END + ret +ENDPROC(schedule_tail_wrapper) +/* * A newly forked process directly context switches into this address. * * eax: prev task we switched from
@@ -244,9 +262,7 @@ END(__switch_to_asm) * edi: kernel thread arg */ ENTRY(ret_from_fork) - pushl %eax - call schedule_tail - popl %eax + call schedule_tail_wrapper testl %ebx, %ebx jnz 1f /* kernel threads are uncommon */
Signature · 1 line
2.7.4
Re: [PATCH 3/9] x86/entry/32: fix the end of the stack for newly forked tasks
Quoted 65 lines · Josh Poimboeuf
Thanks to all the recent x86 entry code refactoring, most tasks' kernel stacks start at the same offset right below their saved
pt_regs, regardless of which syscall was used to enter the kernel. That creates a nice convention which makes it straightforward to identify the end of the stack, which can be useful for the unwinder to verify the stack is sane.Calling
schedule_tail()directly breaks that convention because its an asmlinkage function so its argument has to be pushed on the stack. Add a wrapper which creates a proper "end of stack" frame header before the call.Signed-off-by Josh Poimboeuf <[email protected]>--- arch/x86/entry/entry_32.S | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)arch/x86/entry/entry_32.S +19
-4@@ -44,6 +44,7 @@ #include <asm/alternative-asm.h> #include <asm/asm.h> #include <asm/smap.h> +#include <asm/frame.h> .section .entry.text, "ax"@@ -237,6 +238,23 @@ ENTRY(__switch_to_asm) END(__switch_to_asm) /* + * The unwinder expects the last frame on the stack to always be at the same + * offset from the end of the page, which allows it to validate the stack. + * Calling schedule_tail() directly would break that convention because its an + * asmlinkage function so its argument has to be pushed on the stack. This + * wrapper creates a proper "end of stack" frame header before the call. + */ +ENTRY(schedule_tail_wrapper) + FRAME_BEGIN + + pushl %eax + call schedule_tail + popl %eax + + FRAME_END + ret +ENDPROC(schedule_tail_wrapper) +/* * A newly forked process directly context switches into this address. * * eax: prev task we switched from@@ -244,9 +262,7 @@ END(__switch_to_asm) * edi: kernel thread arg */ ENTRY(ret_from_fork) - pushl %eax - call schedule_tail - popl %eax + call schedule_tail_wrapper testl %ebx, %ebx jnz 1f /* kernel threads are uncommon */ --
Quoted 2 lines
2.7.4
Dropping asmlinkage from schedule_tail() would be a better option if possible.
--
Brian Gerst
Re: [PATCH 3/9] x86/entry/32: fix the end of the stack for newly forked tasks
Quoted 1 line · Brian Gerst
Dropping asmlinkage from
schedule_tail()would be a better option if possible.
My understanding is that it's still needed for ia64. AFAICT, ia64 relies on schedule_tail() having the syscall_linkage function attribute.
Quoted 1 line
From the gcc manual:
This attribute is used to modify the IA64 calling convention by marking all input registers as live at all function exits. This makes it possible to restart a system call after an interrupt without having to save/restore the input registers. This also prevents kernel data from leaking into application code.
And the ia64 entry code has some similar language:
/* * Invoke schedule_tail(task) while preserving in0-in7, which may be needed * in case a system call gets restarted. */ GLOBAL_ENTRY(ia64_invoke_schedule_tail) ...
Signature · 1 line
Josh
Re: [PATCH 3/9] x86/entry/32: fix the end of the stack for newly forked tasks
Quoted 13 lines · Josh Poimboeuf
Quoted 1 line · Brian Gerst
Dropping asmlinkage from
schedule_tail()would be a better option if possible.My understanding is that it's still needed for ia64. AFAICT, ia64 relies on
schedule_tail()having thesyscall_linkagefunction attribute. From the gcc manual:This attribute is used to modify the IA64 calling convention by marking all input registers as live at all function exits. This makes it possible to restart a system call after an interrupt without having to save/restore the input registers. This also prevents kernel data from leaking into application code.
Quoted 8 lines
And the ia64 entry code has some similar language:
/* * Invoke schedule_tail(task) while preserving in0-in7, which may be needed * in case a system call gets restarted. */ GLOBAL_ENTRY(ia64_invoke_schedule_tail) ...
(But I should note that I'm a complete ia64 neophyte, so I could be misreading that...)
Signature · 1 line
Josh
Re: [PATCH 3/9] x86/entry/32: fix the end of the stack for newly forked tasks
In reply to Josh PoimboeufQuoted 8 lines · Josh Poimboeuf
Quoted 1 line · Brian Gerst
Dropping asmlinkage from
schedule_tail()would be a better option if possible.My understanding is that it's still needed for ia64. AFAICT, ia64 relies on
schedule_tail()having thesyscall_linkagefunction attribute. From the gcc manual:
Quoted 5 lines
This attribute is used to modify the IA64 calling convention by marking all input registers as live at all function exits. This makes it possible to restart a system call after an interrupt without having to save/restore the input registers. This also prevents kernel data from leaking into application code.
/me needs to excise this from i386. The amount of BS code involved to avoid a whopping *six* register saves per syscall was absurd.
Quoted 9 lines
And the ia64 entry code has some similar language:
/* * Invoke schedule_tail(task) while preserving in0-in7, which may be needed * in case a system call gets restarted. */ GLOBAL_ENTRY(ia64_invoke_schedule_tail) ...
That comment has to be wrong. What syscall could possibly be restarted across schedule_tail()? It's a brand new thread and has literally never done a syscall.
There may be another reason that the registers are live there, but I generally do my best to never look at ia64 asm code.
Re: [PATCH 3/9] x86/entry/32: fix the end of the stack for newly forked tasks
Quoted 9 lines · Andy Lutomirski
Quoted 8 lines
And the ia64 entry code has some similar language:
/* * Invoke schedule_tail(task) while preserving in0-in7, which may be needed * in case a system call gets restarted. */ GLOBAL_ENTRY(ia64_invoke_schedule_tail) ...
Quoted 3 lines
That comment has to be wrong. What syscall could possibly be restarted across
schedule_tail()? It's a brand new thread and has literally never done a syscall.
Hm, yeah, that comment doesn't make any sense.
Quoted 2 lines
There may be another reason that the registers are live there, but I generally do my best to never look at ia64 asm code.
Yeah, I'm just going to turn around, pretend I never saw it, and slowly walk away...
Signature · 1 line
Josh
PATCH 4/9 x86/head/32: fix the end of the stack for idle tasks
In reply to Josh PoimboeufThe frame at the end of each idle task stack is inconsistent with real task stacks, which have a stack frame header and a real return address before the pt_regs area. This inconsistency can be confusing for stack unwinders. It also hides useful information about what asm code was involved in calling into C.
Fix that by changing the initial code jumps to calls. Also add infinite loops after the calls to make it clear that the calls don't return, and to hang if they do.
--- arch/x86/kernel/head_32.S | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
arch/x86/kernel/head_32.S +5 -3
@@ -288,7 +288,8 @@ num_subarch_entries = (. - subarch_entries) / 4 ENTRY(start_cpu0) movl initial_stack, %ecx movl %ecx, %esp - jmp *(initial_code) + call *(initial_code) +1: jmp 1b ENDPROC(start_cpu0) #endif
@@ -469,8 +470,9 @@ ENTRY(startup_32_smp) xorl %eax,%eax # Clear LDT lldt %ax - pushl $0 # fake return address for unwinder - jmp *(initial_code) + call *(initial_code) +1: jmp 1b +ENDPROC(startup_32_smp) #include "verify_cpu.S"
Signature · 1 line
2.7.4
PATCH 5/9 x86/smp: fix initial idle stack location on 32-bit
In reply to Josh PoimboeufOn 32-bit, the initial idle stack calculation doesn't take into account the TOP_OF_KERNEL_STACK_PADDING, making the stack end address inconsistent with other tasks on 32-bit.
--- arch/x86/kernel/smpboot.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
arch/x86/kernel/smpboot.c +1 -3
@@ -963,9 +963,7 @@ static int do_boot_cpu(int apicid, int cpu, struct task_struct *idle)int cpu0_nmi_registered =0 ;unsigned long timeout; - idle->thread.sp = (unsigned long ) (((struct pt_regs *) - (THREAD_SIZE +task_stack_page (idle))) -1 ); - + idle->thread.sp = (unsigned long )task_pt_regs (idle); early_gdt_descr.address = (unsigned long )get_cpu_gdt_table (cpu); initial_code = (unsigned long )start_secondary; initial_stack = idle->thread.sp;
Signature · 1 line
2.7.4
PATCH 6/9 x86/asm/head: use a common function for starting CPUs
In reply to Josh PoimboeufThere are two different pieces of code for starting a CPU: start_cpu0() and the end of secondary_startup_64(). They're identical except for the stack setup. Combine the common parts into a shared start_cpu() function.
--- arch/x86/kernel/head_64.S | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
arch/x86/kernel/head_64.S +11 -11
@@ -264,13 +264,17 @@ ENTRY(secondary_startup_64) movl $MSR_GS_BASE,%ecx movl initial_gs(%rip),%eax movl initial_gs+4(%rip),%edx - wrmsr + wrmsr /* rsi is pointer to real mode structure with interesting info. pass it to C */ movq %rsi, %rdi - - /* Finally jump to run C code and to be on real kernel address + jmp start_cpu +ENDPROC(secondary_startup_64) + +ENTRY(start_cpu) + /* + * Jump to run C code and to be on a real kernel address. * Since we are running on identity-mapped space we have to jump * to the full 64bit address, this is only possible as indirect * jump. In addition we need to ensure %cs is set so we make this
@@ -299,7 +303,7 @@ ENTRY(secondary_startup_64) pushq $__KERNEL_CS # set correct cs pushq %rax # target address in negative space lretq -ENDPROC(secondary_startup_64) +ENDPROC(start_cpu) #include "verify_cpu.S"
@@ -307,15 +311,11 @@ ENDPROC(secondary_startup_64) /* * Boot CPU0 entry point. It's called from play_dead(). Everything has been set * up already except stack. We just set up stack here. Then call - * start_secondary(). + * start_secondary() via start_cpu(). */ ENTRY(start_cpu0) - movq initial_stack(%rip),%rsp - movq initial_code(%rip),%rax - pushq $0 # fake return address to stop unwinder - pushq $__KERNEL_CS # set correct cs - pushq %rax # target address in negative space - lretq + movq initial_stack(%rip), %rsp + jmp start_cpu ENDPROC(start_cpu0) #endif
Signature · 1 line
2.7.4
PATCH 7/9 x86/head: put real return address on idle task stack
In reply to Josh PoimboeufThe frame at the end of each idle task stack has a zeroed return address. This is inconsistent with real task stacks, which have a real return address at that spot. This inconsistency can be confusing for stack unwinders. It also hides useful information about what asm code was involved in calling into C.
Make it a real address by using the side effect of a call instruction to push the instruction pointer on the stack.
--- arch/x86/kernel/head_64.S | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
arch/x86/kernel/head_64.S +3 -2
@@ -298,8 +298,9 @@ ENTRY(start_cpu) * REX.W + FF /5 JMP m16:64 Jump far, absolute indirect, * address given in m16:64. */ - movq initial_code(%rip),%rax - pushq $0 # fake return address to stop unwinder + call 1f # put return address on stack for unwinder +1: xorq %rbp, %rbp # clear frame pointer + movq initial_code(%rip), %rax pushq $__KERNEL_CS # set correct cs pushq %rax # target address in negative space lretq
Signature · 1 line
2.7.4
PATCH 8/9 x86/head: fix the end of the stack for idle tasks
In reply to Josh PoimboeufThanks to all the recent x86 entry code refactoring, most tasks' kernel stacks start at the same offset right below their saved pt_regs, regardless of which syscall was used to enter the kernel. That creates a nice convention which makes it straightforward to identify the end of the stack, which can be useful for the unwinder to verify the stack is sane.
However, the boot CPU's idle "swapper" task doesn't follow that convention. Fix that by starting its stack at a sizeof(pt_regs) offset from the end of the stack page.
--- arch/x86/kernel/head_32.S | 9 ++++++++- arch/x86/kernel/head_64.S | 15 +++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-)
arch/x86/kernel/head_32.S +8 -1
@@ -62,6 +62,8 @@ #define PAGE_TABLE_SIZE(pages) ((pages) / PTRS_PER_PGD) #endif +#define SIZEOF_PTREGS 17*4 + /* * Number of possible pages in the lowmem region. *
@@ -706,7 +708,12 @@ ENTRY(initial_page_table) .data .balign 4 ENTRY(initial_stack) - .long init_thread_union+THREAD_SIZE + /* + * The SIZEOF_PTREGS gap is a convention which helps the in-kernel + * unwinder reliably detect the end of the stack. + */ + .long init_thread_union + THREAD_SIZE - SIZEOF_PTREGS - \ + TOP_OF_KERNEL_STACK_PADDING; __INITRODATA int_msg:
arch/x86/kernel/head_64.S +7 -8
@@ -65,13 +65,8 @@ startup_64: * tables and then reload them. */ - /* - * Setup stack for verify_cpu(). "-8" because initial_stack is defined - * this way, see below. Our best guess is a NULL ptr for stack - * termination heuristics and we don't want to break anything which - * might depend on it (kgdb, ...). - */ - leaq (__end_init_task - 8)(%rip), %rsp + /* Set up the stack for verify_cpu(), similar to initial_stack below */ + leaq (__end_init_task - SIZEOF_PTREGS)(%rip), %rsp /* Sanitize CPU configuration */ call verify_cpu
@@ -328,7 +323,11 @@ ENDPROC(start_cpu0) GLOBAL(initial_gs) .quad INIT_PER_CPU_VAR(irq_stack_union) GLOBAL(initial_stack) - .quad init_thread_union+THREAD_SIZE-8 + /* + * The SIZEOF_PTREGS gap is a convention which helps the in-kernel + * unwinder reliably detect the end of the stack. + */ + .quad init_thread_union + THREAD_SIZE - SIZEOF_PTREGS __FINITDATA bad_address:
Signature · 1 line
2.7.4
PATCH 9/9 x86: move _stext marker to before head code
In reply to Josh PoimboeufWhen core_kernel_text() is used to determine whether an address on a task's stack trace is a kernel text address, it incorrectly returns false for early text addresses for the head code between the _text and _stext markers. Among other things, this can cause the unwinder to behave incorrectly when unwinding to x86 head code.
Head code is text code too, so mark it as such. This seems to match the intent of other users of the _stext symbol, and it also seems consistent with what other architectures are already doing.
--- arch/x86/kernel/vmlinux.lds.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kernel/vmlinux.lds.S +1 -1
@@ -91,10 +91,10 @@ SECTIONS /* Text and read-only data */ .text : AT(ADDR(.text) - LOAD_OFFSET) { _text = .; + _stext = .; /* bootstrapping code */ HEAD_TEXT . = ALIGN(8); - _stext = .; TEXT_TEXT SCHED_TEXT LOCK_TEXT
Signature · 1 line
2.7.4