Preparatory patches for x86 KAISER support
Subsystems (7)
cover Preparatory patches for x86 KAISER support
This is mostly Andy's syscall trampoline stack patches, but also some other preparatory cleanups and enhancements.
Sending this separately to make it (slightly) easier to review.
The Git version of the full queue can be found at:
git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git WIP.x86/mm
Thanks,
Ingo
=============>
Andy Lutomirski (20):
x86/unwinder/orc: Don't bail on stack overflow
x86/irq: Remove an old outdated comment about context tracking races x86/irq/64: Print the offending IP in the stack overflow warning x86/entry/64: Allocate and enable the SYSENTER stack
x86/dumpstack: Add get_stack_info() support for the SYSENTER stack x86/entry/gdt: Put per-CPU GDT remaps in ascending order
x86/mm/fixmap: Generalize the GDT fixmap mechanism, introduce 'struct cpu_entry_area' x86/kasan/64: Teach KASAN about the cpu_entry_area
x86/entry: Fix assumptions that the HW TSS is at the beginning of cpu_tss x86/dumpstack: Handle stack overflow on all stacks
x86/entry: Move SYSENTER_stack to the beginning of struct tss_struct x86/entry: Remap the TSS into the CPU entry area
x86/entry/64: Separate cpu_current_top_of_stack from TSS.sp0 x86/espfix/64: Stop assuming that pt_regs is on the entry stack x86/entry/64: Use a per-CPU trampoline stack for IDT entries x86/entry/64: Return to userspace from the trampoline stack x86/entry/64: Create a per-CPU SYSCALL entry trampoline
x86/entry/64: Move the IST stacks into 'struct cpu_entry_area' x86/entry/64: Remove the SYSENTER stack canary
x86/entry: Clean up the SYSENTER_stack code
Josh Poimboeuf (1):
x86/unwinder: Handle stack overflows more gracefully
arch/x86/entry/entry_32.S | 6 +- arch/x86/entry/entry_64.S | 170 ++++++++++++++++++++++++++++++++----- arch/x86/entry/entry_64_compat.S | 7 +- arch/x86/include/asm/desc.h | 11 +-- arch/x86/include/asm/fixmap.h | 61 ++++++++++++- arch/x86/include/asm/kdebug.h | 1 + arch/x86/include/asm/processor.h | 54 +++++++----- arch/x86/include/asm/stacktrace.h | 3 + arch/x86/include/asm/switch_to.h | 3 +- arch/x86/include/asm/thread_info.h | 2 +- arch/x86/include/asm/traps.h | 1 - arch/x86/include/asm/unwind.h | 7 ++ arch/x86/kernel/asm-offsets.c | 7 ++ arch/x86/kernel/asm-offsets_32.c | 5 -- arch/x86/kernel/asm-offsets_64.c | 1 + arch/x86/kernel/cpu/common.c | 136 +++++++++++++++++++++-------- arch/x86/kernel/doublefault.c | 36 ++++---- arch/x86/kernel/dumpstack.c | 74 ++++++++++++---- arch/x86/kernel/dumpstack_32.c | 6 ++ arch/x86/kernel/dumpstack_64.c | 6 ++ arch/x86/kernel/irq.c | 12 --- arch/x86/kernel/irq_64.c | 4 +- arch/x86/kernel/process.c | 13 ++- arch/x86/kernel/process_64.c | 12 +-- arch/x86/kernel/stacktrace.c | 2 +- arch/x86/kernel/traps.c | 57 ++++++++----- arch/x86/kernel/unwind_orc.c | 88 ++++++++----------- arch/x86/kernel/vmlinux.lds.S | 9 ++ arch/x86/mm/kasan_init_64.c | 18 +++- arch/x86/power/cpu.c | 16 ++-- arch/x86/xen/mmu_pv.c | 2 +- 31 files changed, 590 insertions(+), 240 deletions(-)
Signature · 1 line
2.14.1
PATCH 1/21 x86/unwinder/orc: Don't bail on stack overflow
From: Andy Lutomirski <[email protected]>
If we overflow the stack into a guard page and then try to unwind it with ORC, it should work well: by construction, there can't be any meaningful data in the guard page because no writes to the guard page will have succeeded.
This patch fixes a bug that unwinding from working correctly: if the starting register state has RSP pointing into a stack guard page, the ORC unwinder bails out immediately. This patch fixes that: the ORC unwinder will start the unwind.
I tested this by intentionally overflowing the task stack. The result is an accurate call trace instead of a trace consisting purely of '?' entries.
There are a few other bugs that are triggered if the unwinder encounters a stack overflow after the first step, and Josh has WIP patches to fix those as well.
Signed-off-by Andy Lutomirski <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Link http://lkml.kernel.org/r/927042950d7f1a7007dd0f58538966a593508f8b.1511715954.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/kernel/unwind_orc.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)
arch/x86/kernel/unwind_orc.c +12 -2
@@ -553,8 +553,18 @@ void __unwind_start(struct unwind_state *state, struct task_struct *task, }if (get_stack_info ((unsigned long *)state->sp, state->task, - &state->stack_info, &state->stack_mask)) -return ; + &state->stack_info, &state->stack_mask)) { +/* +* We weren't on a valid stack. It's possible that +* we overflowed a valid stack into a guard page. +* See if the next page up is valid so that we can +* generate some kind of backtrace if this happens. +*/ +void *next_page = (void *)PAGE_ALIGN ((unsigned long )regs->sp); +if (get_stack_info (next_page, state->task, &state->stack_info, + &state->stack_mask)) +return ; + }/* * The caller can provide the address of the first frame directly
Signature · 1 line
2.14.1
Borislav Petkov
maintainerRe: [PATCH 01/21] x86/unwinder/orc: Don't bail on stack overflow
Quoted 8 lines · Ingo Molnar
From: Andy Lutomirski <[email protected]>
If we overflow the stack into a guard page and then try to unwind it with ORC, it should work well: by construction, there can't be any meaningful data in the guard page because no writes to the guard page will have succeeded.
This patch fixes a bug that unwinding from working correctly: if the
^ prevents
Quoted 2 lines
starting register state has RSP pointing into a stack guard page, the ORC unwinder bails out immediately. This patch fixes that: the ORC
I believe here we can kill the second "This patch" :)
Quoted 9 lines
unwinder will start the unwind.
I tested this by intentionally overflowing the task stack. The result is an accurate call trace instead of a trace consisting purely of '?' entries.
There are a few other bugs that are triggered if the unwinder encounters a stack overflow after the first step, and Josh has WIP patches to fix those as well.
I guess we don't need that paragraph.
Quoted 21 lines
Signed-off-by Andy Lutomirski <[email protected]>Cc Borislav Petkov <[email protected]>Cc Brian Gerst <[email protected]>Cc Dave Hansen <[email protected]>Cc Josh Poimboeuf <[email protected]>Cc Linus Torvalds <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Thomas Gleixner <[email protected]>Link http://lkml.kernel.org/r/927042950d7f1a7007dd0f58538966a593508f8b.1511715954.git.luto@kernel.org open hereSigned-off-by Ingo Molnar <[email protected]>--- arch/x86/kernel/unwind_orc.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)arch/x86/kernel/unwind_orc.c +0
-0@@ -553,8 +553,18 @@ void __unwind_start(struct unwind_state *state, struct task_struct *task, }
arch/x86/kernel/unwind_orc.c void __unwind_start(struct unwind_state *state, struct task_struct *task, · line 562
Quoted 10 lines
arch/x86/kernel/unwind_orc.c +7
-2if (get_stack_info ((unsigned long *)state->sp, state->task, - &state->stack_info, &state->stack_mask)) -return ; + &state->stack_info, &state->stack_mask)) { +/* +* We weren't on a valid stack. It's possible that +* we overflowed a valid stack into a guard page. +* See if the next page up is valid so that we can +* generate some kind of backtrace if this happens. +*/
Right, should we issue a marker or somesuch here to denote that we somehow walked into the guard page?
It might be helpful when debugging issues, to see the big picture...
Quoted 9 lines
arch/x86/kernel/unwind_orc.c +5
-0+void *next_page = (void *)PAGE_ALIGN ((unsigned long )regs->sp); +if (get_stack_info (next_page, state->task, &state->stack_info, + &state->stack_mask)) +return ; + }/* * The caller can provide the address of the first frame directly
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.PATCH 2/21 x86/unwinder: Handle stack overflows more gracefully
In reply to Ingo MolnarFrom: Josh Poimboeuf <[email protected]>
There are at least two unwinder bugs hindering the debugging of stack-overflow crashes:
- It doesn't deal gracefully with the case where the stack overflows and the stack pointer itself isn't on a valid stack but the to-be-dereferenced data *is*.
- The ORC oops dump code doesn't know how to print partial pt_regs, for the case where if we get an interrupt/exception in *early* entry code before the full pt_regs have been saved.
Fix both issues.
Signed-off-by Josh Poimboeuf <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/include/asm/kdebug.h | 1 + arch/x86/include/asm/unwind.h | 7 ++++ arch/x86/kernel/dumpstack.c | 32 ++++++++++++++++--- arch/x86/kernel/process_64.c | 11 +++---- arch/x86/kernel/stacktrace.c | 2 +- arch/x86/kernel/unwind_orc.c | 74 +++++++++++++++---------------------------- 6 files changed, 66 insertions(+), 61 deletions(-)
arch/x86/include/asm/kdebug.h +1 -0
@@ -26,6 +26,7 @@ extern void die(const char *, struct pt_regs *,long);extern int __must_check __die (const char *,struct pt_regs *,long );extern void show_stack_regs (struct pt_regs *regs);extern void __show_regs (struct pt_regs *regs,int all); +extern void show_iret_regs (struct pt_regs *regs);extern unsigned long oops_begin (void );extern void oops_end (unsigned long ,struct pt_regs *,int signr);
arch/x86/include/asm/unwind.h +7 -0
@@ -7,6 +7,9 @@#include <asm/ptrace.h> #include <asm/stacktrace.h> +#define IRET_FRAME_OFFSET (offsetof(struct pt_regs, ip)) +#define IRET_FRAME_SIZE (sizeof(struct pt_regs) - IRET_FRAME_OFFSET) +struct unwind_state {struct stack_info stack_info;unsigned long stack_mask;
@@ -52,6 +55,10 @@ void unwind_start(struct unwind_state *state, struct task_struct *task, }#if defined(CONFIG_UNWINDER_ORC ) || defined(CONFIG_UNWINDER_FRAME_POINTER ) +/* +* WARNING: The entire pt_regs may not be safe to dereference. In some cases, +* only the iret registers are accessible. Use with caution! +*/ static inline struct pt_regs *unwind_get_entry_regs (struct unwind_state *state) {if (unwind_done (state))
arch/x86/kernel/dumpstack.c +27 -5
@@ -50,6 +50,28 @@ static void printk_stack_address(unsigned long address, int reliable,printk ("%s %s%pB\n" , log_lvl, reliable ?"" :"? " , (void *)address); } +void show_iret_regs (struct pt_regs *regs) +{ +printk (KERN_DEFAULT "RIP: %04x:%pS\n" , (int )regs->cs, (void *)regs->ip); +printk (KERN_DEFAULT "RSP: %04x:%016lx EFLAGS: %08lx" , (int )regs->ss, + regs->sp, regs->flags); +} + +static void show_regs_safe (struct stack_info *info,struct pt_regs *regs) +{ +if (on_stack (info, regs,sizeof (*regs))) +__show_regs (regs,0 ); +else if (on_stack (info, (void *)regs +IRET_FRAME_OFFSET , +IRET_FRAME_SIZE )) { +/* +* When an interrupt or exception occurs in entry code, the +* full pt_regs might not have been saved yet. In that case +* just print the iret return frame. +*/ +show_iret_regs (regs); + } +} +void show_trace_log_lvl (struct task_struct *task,struct pt_regs *regs,unsigned long *stack,char *log_lvl) {
@@ -94,8 +116,8 @@ void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,if (stack_name)printk ("%s <%s>\n" , log_lvl, stack_name); -if (regs &&on_stack (&stack_info, regs,sizeof (*regs))) -__show_regs (regs,0 ); +if (regs) +show_regs_safe (&stack_info, regs);/* * Scan the stack, printing any text addresses we find. At the
@@ -119,7 +141,7 @@ void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,/* * Don't print regs->ip again if it was already printed -* by __show_regs() below. +* by show_regs_safe() below. */ if (regs && stack == ®s->ip) goto next;
@@ -155,8 +177,8 @@ void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,/* if the frame has entry regs, print them */ regs =unwind_get_entry_regs (&state); -if (regs &&on_stack (&stack_info, regs,sizeof (*regs))) -__show_regs (regs,0 ); +if (regs) +show_regs_safe (&stack_info, regs); }if (stack_name)
arch/x86/kernel/process_64.c +5 -6
@@ -69,9 +69,8 @@ void __show_regs(struct pt_regs *regs, int all)unsigned int fsindex, gsindex;unsigned int ds, cs, es; -printk (KERN_DEFAULT "RIP: %04lx:%pS\n" , regs->cs, (void *)regs->ip); -printk (KERN_DEFAULT "RSP: %04lx:%016lx EFLAGS: %08lx" , regs->ss, - regs->sp, regs->flags); +show_iret_regs (regs); +if (regs->orig_ax !=-1 )pr_cont (" ORIG_RAX: %016lx\n" , regs->orig_ax);else
@@ -88,6 +87,9 @@ void __show_regs(struct pt_regs *regs, int all)printk (KERN_DEFAULT "R13: %016lx R14: %016lx R15: %016lx\n" , regs->r13, regs->r14, regs->r15); +if (!all) +return ; + asm("movl %%ds,%0" :"=r" (ds)); asm("movl %%cs,%0" :"=r" (cs)); asm("movl %%es,%0" :"=r" (es));
@@ -98,9 +100,6 @@ void __show_regs(struct pt_regs *regs, int all)rdmsrl (MSR_GS_BASE , gs);rdmsrl (MSR_KERNEL_GS_BASE , shadowgs); -if (!all) -return ; - cr0 =read_cr0 (); cr2 =read_cr2 (); cr3 =__read_cr3 ();
arch/x86/kernel/stacktrace.c +1 -1
@@ -102,7 +102,7 @@ __save_stack_trace_reliable(struct stack_trace *trace,for (unwind_start (&state, task,NULL ,NULL ); !unwind_done (&state);unwind_next_frame (&state)) { - regs =unwind_get_entry_regs (&state); + regs =unwind_get_entry_regs (&state,NULL );if (regs) {/* * Kernel mode registers on the stack indicate an
arch/x86/kernel/unwind_orc.c +25 -49
123 lines not on this page · raw message
Signature · 1 line
2.14.1
Re: [PATCH 02/21] x86/unwinder: Handle stack overflows more gracefully
Quoted 61 lines · Ingo Molnar
From: Josh Poimboeuf <[email protected]>
There are at least two unwinder bugs hindering the debugging of stack-overflow crashes:
- It doesn't deal gracefully with the case where the stack overflows and the stack pointer itself isn't on a valid stack but the to-be-dereferenced data *is*.
- The ORC oops dump code doesn't know how to print partial
pt_regs, for the case where if we get an interrupt/exception in *early* entry code before the fullpt_regshave been saved.Fix both issues.
Signed-off-by Josh Poimboeuf <[email protected]>Cc Andy Lutomirski <[email protected]>Cc Borislav Petkov <[email protected]>Cc Brian Gerst <[email protected]>Cc Dave Hansen <[email protected]>Cc Linus Torvalds <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Thomas Gleixner <[email protected]>Signed-off-by Ingo Molnar <[email protected]>--- arch/x86/include/asm/kdebug.h | 1 + arch/x86/include/asm/unwind.h | 7 ++++ arch/x86/kernel/dumpstack.c | 32 ++++++++++++++++--- arch/x86/kernel/process_64.c | 11 +++---- arch/x86/kernel/stacktrace.c | 2 +- arch/x86/kernel/unwind_orc.c | 74 +++++++++++++++---------------------------- 6 files changed, 66 insertions(+), 61 deletions(-)arch/x86/include/asm/kdebug.h +1
-0@@ -26,6 +26,7 @@ extern void die(const char *, struct pt_regs *,long);extern int __must_check __die (const char *,struct pt_regs *,long );extern void show_stack_regs (struct pt_regs *regs);extern void __show_regs (struct pt_regs *regs,int all); +extern void show_iret_regs (struct pt_regs *regs);extern unsigned long oops_begin (void );extern void oops_end (unsigned long ,struct pt_regs *,int signr);arch/x86/include/asm/unwind.h +3
-0@@ -7,6 +7,9 @@#include <asm/ptrace.h> #include <asm/stacktrace.h> +#define IRET_FRAME_OFFSET (offsetof(struct pt_regs, ip)) +#define IRET_FRAME_SIZE (sizeof(struct pt_regs) - IRET_FRAME_OFFSET) +struct unwind_state {struct stack_info stack_info;unsigned long stack_mask;
arch/x86/include/asm/unwind.h void unwind_start(struct unwind_state *state, struct task_struct *task, · line 60
Quoted 6 lines
arch/x86/include/asm/unwind.h +3
-0@@ -52,6 +55,10 @@ void unwind_start(struct unwind_state *state, struct task_struct *task, }#if defined(CONFIG_UNWINDER_ORC ) || defined(CONFIG_UNWINDER_FRAME_POINTER ) +/* +* WARNING: The entire pt_regs may not be safe to dereference. In some cases, +* only the iret registers are accessible. Use with caution!
^^^^^^^^^^^^^^^^^^
You mean the interrupt stack frame here, right?
Quoted 19 lines
arch/x86/include/asm/unwind.h +1
-0+*/ static inline struct pt_regs *unwind_get_entry_regs (struct unwind_state *state) {if (unwind_done (state))arch/x86/kernel/dumpstack.c +7
-0@@ -50,6 +50,28 @@ static void printk_stack_address(unsigned long address, int reliable,printk ("%s %s%pB\n" , log_lvl, reliable ?"" :"? " , (void *)address); } +void show_iret_regs (struct pt_regs *regs) +{ +printk (KERN_DEFAULT "RIP: %04x:%pS\n" , (int )regs->cs, (void *)regs->ip); +printk (KERN_DEFAULT "RSP: %04x:%016lx EFLAGS: %08lx" , (int )regs->ss, + regs->sp, regs->flags); +} +
arch/x86/kernel/dumpstack.c static void printk_stack_address(unsigned long address, int reliable, · line 69
Quoted 10 lines
arch/x86/kernel/dumpstack.c +10
-0+static void show_regs_safe (struct stack_info *info,struct pt_regs *regs) +{ +if (on_stack (info, regs,sizeof (*regs))) +__show_regs (regs,0 ); +else if (on_stack (info, (void *)regs +IRET_FRAME_OFFSET , +IRET_FRAME_SIZE )) { +/* +* When an interrupt or exception occurs in entry code, the +* full pt_regs might not have been saved yet. In that case +* just print the iret return frame.
Right, it is the interrupt stack frame. But "iret frame" is shorter so let's stick to that :)
...
Quoted 37 lines
arch/x86/kernel/dumpstack.c +7
-19@@ -283,42 +276,32 @@ static bool deref_stack_reg(struct unwind_state *state, unsigned long addr,return true; } -#define REGS_SIZE (sizeof(struct pt_regs)) -#define SP_OFFSET (offsetof(struct pt_regs, sp)) -#define IRET_REGS_SIZE (REGS_SIZE - offsetof(struct pt_regs, ip)) -#define IRET_SP_OFFSET (SP_OFFSET - offsetof(struct pt_regs, ip)) -static bool deref_stack_regs (struct unwind_state *state,unsigned long addr, -unsigned long *ip,unsigned long *sp,bool full) +unsigned long *ip,unsigned long *sp) { -size_t regs_size = full ?REGS_SIZE :IRET_REGS_SIZE ; -size_t sp_offset = full ?SP_OFFSET :IRET_SP_OFFSET ; -struct pt_regs *regs = (struct pt_regs *)(addr + regs_size -REGS_SIZE ); - -if (IS_ENABLED (CONFIG_X86_64 )) { -if (!stack_access_ok (state, addr, regs_size)) -return false; - - *ip = regs->ip; - *sp = regs->sp; +struct pt_regs *regs = (struct pt_regs *)addr; -return true; - } +/* x86-32 support will be more complicated due to the ®s->sp hack */ +BUILD_BUG_ON (IS_ENABLED (CONFIG_X86_32 )); -if (!stack_access_ok (state, addr, sp_offset)) +if (!stack_access_ok (state, addr,sizeof (struct pt_regs )))return false; *ip = regs->ip; + *sp = regs->sp; +return true;
arch/x86/kernel/dumpstack.c static bool deref_stack_reg(struct unwind_state *state, unsigned long addr, · line 298
Quoted 10 lines
arch/x86/kernel/dumpstack.c +5
-4+} -if (user_mode (regs)) { -if (!stack_access_ok (state, addr + sp_offset, -REGS_SIZE -SP_OFFSET )) -return false; +static bool deref_stack_iret_regs (struct unwind_state *state,unsigned long addr, +unsigned long *ip,unsigned long *sp) +{ +struct pt_regs *regs = (void *)addr -IRET_FRAME_OFFSET ;
I guess those are traditionally done with container_of()...
But yeah, FWIW, looks ok to me:
Reviewed-by Borislav Petkov <[email protected]>
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.Re: [PATCH 02/21] x86/unwinder: Handle stack overflows more gracefully
Quoted 6 lines · Borislav Petkov
Review
Quoted 4 lines
(quoted diff) +4
-0+static bool deref_stack_iret_regs(struct unwind_state *state, unsigned long addr, + unsigned long *ip, unsigned long *sp) +{ + struct pt_regs *regs = (void *)addr - IRET_FRAME_OFFSET;I guess those are traditionally done with
container_of()...
Without modifying pt_regs, there's no container to speak of, but at least IRET_FRAME_OFFSET uses offsetof(), which is similar.
Quoted 3 lines
But yeah, FWIW, looks ok to me:
Reviewed-by Borislav Petkov <[email protected]>
Thanks!
Ingo, can you fold in the below changes based on Boris' comments?
arch/x86/include/asm/unwind.h +1 -1
@@ -57,7 +57,7 @@ void unwind_start(struct unwind_state *state, struct task_struct *task,#if defined(CONFIG_UNWINDER_ORC ) || defined(CONFIG_UNWINDER_FRAME_POINTER )/* * WARNING: The entire pt_regs may not be safe to dereference. In some cases, -* only the iret registers are accessible. Use with caution! +* only the iret frame registers are accessible. Use with caution! */ static inline struct pt_regs *unwind_get_entry_regs (struct unwind_state *state) {
arch/x86/kernel/dumpstack.c +1 -1
@@ -66,7 +66,7 @@ static void show_regs_safe(struct stack_info *info, struct pt_regs *regs)/* * When an interrupt or exception occurs in entry code, the * full pt_regs might not have been saved yet. In that case -* just print the iret return frame. +* just print the iret frame. */ show_iret_regs (regs); }
PATCH 3/21 x86/irq: Remove an old outdated comment about context tracking races
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
That race has been fixed and code cleaned up for a while now.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Reviewed-by Thomas Gleixner <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/12e75976dbbb7ece2b0a64238f1d3892dfed1e16.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/kernel/irq.c | 12 ------------ 1 file changed, 12 deletions(-)
arch/x86/kernel/irq.c +0 -12
@@ -219,18 +219,6 @@ __visible unsigned int __irq_entry do_IRQ(struct pt_regs *regs)/* high bit used in ret_from_ code */ unsigned vector = ~regs->orig_ax; -/* -* NB: Unlike exception entries, IRQ entries do not reliably -* handle context tracking in the low-level entry code. This is -* because syscall entries execute briefly with IRQs on before -* updating context tracking state, so we can take an IRQ from -* kernel mode with CONTEXT_USER. The low-level entry code only -* updates the context if we came from user mode, so we won't -* switch to CONTEXT_KERNEL. We'll fix that once the syscall -* code is cleaned up enough that we can cleanly defer enabling -* IRQs. -*/ -entering_irq ();/* entering_irq() tells RCU that we're not quiescent. Check it. */
Signature · 1 line
2.14.1
PATCH 4/21 x86/irq/64: Print the offending IP in the stack overflow warning
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
In case something goes wrong with unwind (not unlikely in case of overflow), print the offending IP where we detected the overflow.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Reviewed-by Thomas Gleixner <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/6fcf700cc5ee884fb739b67d1246ab4185c41409.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/kernel/irq_64.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/kernel/irq_64.c +2 -2
@@ -57,10 +57,10 @@ static inline void stack_overflow_check(struct pt_regs *regs)if (regs->sp >= estack_top && regs->sp <= estack_bottom)return ; -WARN_ONCE (1 ,"do_IRQ(): %s has overflown the kernel stack (cur:%Lx,sp:%lx,irq stk top-bottom:%Lx-%Lx,exception stk top-bottom:%Lx-%Lx)\n" , +WARN_ONCE (1 ,"do_IRQ(): %s has overflown the kernel stack (cur:%Lx,sp:%lx,irq stk top-bottom:%Lx-%Lx,exception stk top-bottom:%Lx-%Lx,ip:%pF)\n" , current->comm, curbase, regs->sp, irq_stack_top, irq_stack_bottom, - estack_top, estack_bottom); + estack_top, estack_bottom, (void *)regs->ip);if (sysctl_panic_on_stackoverflow)panic ("low stack detected by irq handler - check messages\n" );
Signature · 1 line
2.14.1
PATCH 5/21 x86/entry/64: Allocate and enable the SYSENTER stack
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
This will simplify future changes that want scratch variables early in the SYSENTER handler -- they'll be able to spill registers to the stack. It also lets us get rid of a SWAPGS_UNSAFE_STACK user.
This does not depend on CONFIG_IA32_EMULATION=y because we'll want the stack space even without IA32 emulation.
As far as I can tell, the reason that this wasn't done from day 1 is that we use IST for #DB and #BP, which is IMO rather nasty and causes a lot more problems than it solves. But, since #DB uses IST, we don't actually need a real stack for SYSENTER (because SYSENTER with TF set will invoke #DB on the IST stack rather than the SYSENTER stack).
I want to remove IST usage from these vectors some day, and this patch is a prerequisite for that as well.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Thomas Gleixner <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/c37d6e68a73e1b5b1203e0e95b488fa8092b3cfb.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/entry/entry_64_compat.S | 2 +- arch/x86/include/asm/processor.h | 3 --- arch/x86/kernel/asm-offsets.c | 5 +++++ arch/x86/kernel/asm-offsets_32.c | 5 ----- arch/x86/kernel/cpu/common.c | 4 +++- arch/x86/kernel/process.c | 2 -- arch/x86/kernel/traps.c | 3 +-- 7 files changed, 10 insertions(+), 14 deletions(-)
arch/x86/entry/entry_64_compat.S +1 -1
@@ -48,7 +48,7 @@ */ ENTRY(entry_SYSENTER_compat) /* Interrupts are off on entry. */ - SWAPGS_UNSAFE_STACK + SWAPGS movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp /*
arch/x86/include/asm/processor.h +0 -3
@@ -340,14 +340,11 @@ struct tss_struct {*/ unsigned long io_bitmap[IO_BITMAP_LONGS +1 ]; -#ifdef CONFIG_X86_32 /* * Space for the temporary SYSENTER stack. */ unsigned long SYSENTER_stack_canary;unsigned long SYSENTER_stack[64 ]; -#endif - }____cacheline_aligned ; DECLARE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss);
arch/x86/kernel/asm-offsets.c +5 -0
@@ -93,4 +93,9 @@ void common(void) {BLANK ();DEFINE (PTREGS_SIZE ,sizeof (struct pt_regs )); + +/* Offset from cpu_tss to SYSENTER_stack */ +OFFSET (CPU_TSS_SYSENTER_stack, tss_struct, SYSENTER_stack); +/* Size of SYSENTER_stack */ +DEFINE (SIZEOF_SYSENTER_stack,sizeof (((struct tss_struct *)0 )->SYSENTER_stack)); }
arch/x86/kernel/asm-offsets_32.c +0 -5
@@ -50,11 +50,6 @@ void foo(void) DEFINE(TSS_sysenter_sp0, offsetof(struct tss_struct, x86_tss.sp0) - offsetofend(struct tss_struct, SYSENTER_stack)); -/* Offset from cpu_tss to SYSENTER_stack */ -OFFSET (CPU_TSS_SYSENTER_stack, tss_struct, SYSENTER_stack); -/* Size of SYSENTER_stack */ -DEFINE (SIZEOF_SYSENTER_stack,sizeof (((struct tss_struct *)0 )->SYSENTER_stack)); -#ifdef CONFIG_CC_STACKPROTECTOR BLANK ();OFFSET (stack_canary_offset, stack_canary, canary);
arch/x86/kernel/cpu/common.c +3 -1
@@ -1386,7 +1386,9 @@ void syscall_init(void)* AMD doesn't allow SYSENTER in long mode (either 32- or 64-bit). */ wrmsrl_safe (MSR_IA32_SYSENTER_CS , (u64 )__KERNEL_CS); -wrmsrl_safe (MSR_IA32_SYSENTER_ESP ,0ULL ); +wrmsrl_safe (MSR_IA32_SYSENTER_ESP , + (unsigned long )this_cpu_ptr (&cpu_tss) + + offsetofend(struct tss_struct, SYSENTER_stack));wrmsrl_safe (MSR_IA32_SYSENTER_EIP , (u64 )entry_SYSENTER_compat);#else wrmsrl (MSR_CSTAR , (unsigned long )ignore_sysret);
arch/x86/kernel/process.c +0 -2
@@ -71,9 +71,7 @@ __visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss) = {*/ .io_bitmap = { [0 ... IO_BITMAP_LONGS] = ~0 }, #endif -#ifdef CONFIG_X86_32 .SYSENTER_stack_canary =STACK_END_MAGIC , -#endif };EXPORT_PER_CPU_SYMBOL (cpu_tss);
arch/x86/kernel/traps.c +1 -2
@@ -800,14 +800,13 @@ dotraplinkage void do_debug(struct pt_regs *regs, long error_code)debug_stack_usage_dec (); exit: -#if defined(CONFIG_X86_32 )/* * This is the most likely code path that involves non-trivial use * of the SYSENTER stack. Check that we haven't overrun it. */ WARN (this_cpu_read (cpu_tss.SYSENTER_stack_canary) !=STACK_END_MAGIC ,"Overran or corrupted SYSENTER stack\n" ); -#endif +ist_exit (regs); }NOKPROBE_SYMBOL (do_debug);
Signature · 1 line
2.14.1
PATCH 6/21 x86/dumpstack: Add get_stack_info() support for the SYSENTER stack
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
get_stack_info() doesn't currently know about the SYSENTER stack, so unwinding will fail if we entered the kernel on the SYSENTER stack and haven't fully switched off. Teach get_stack_info() about the SYSENTER stack.
With future patches applied that run part of the entry code on the SYSENTER stack and introduce an intentional BUG(), I would get:
PANIC: double fault, error_code: 0x0
...
RIP: 0010:do_error_trap+0x33/0x1c0 ... Call Trace: Code: ...
With this patch, I get:
PANIC: double fault, error_code: 0x0
...
Call Trace: <SYSENTER> ? async_page_fault+0x36/0x60 ? invalid_op+0x22/0x40 ? async_page_fault+0x36/0x60 ? sync_regs+0x3c/0x40 ? sync_regs+0x2e/0x40 ? error_entry+0x6c/0xd0 ? async_page_fault+0x36/0x60 </SYSENTER> Code: ...
which is a lot more informative.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/c32ce8b363e27fa9b4a4773297d5b4b0f4b39e94.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/include/asm/stacktrace.h | 3 +++ arch/x86/kernel/dumpstack.c | 19 +++++++++++++++++++ arch/x86/kernel/dumpstack_32.c | 6 ++++++ arch/x86/kernel/dumpstack_64.c | 6 ++++++ 4 files changed, 34 insertions(+)
arch/x86/include/asm/stacktrace.h +3 -0
@@ -16,6 +16,7 @@ enum stack_type {STACK_TYPE_TASK ,STACK_TYPE_IRQ ,STACK_TYPE_SOFTIRQ , +STACK_TYPE_SYSENTER ,STACK_TYPE_EXCEPTION , STACK_TYPE_EXCEPTION_LAST = STACK_TYPE_EXCEPTION + N_EXCEPTION_STACKS-1, };
@@ -28,6 +29,8 @@ struct stack_info {bool in_task_stack (unsigned long *stack,struct task_struct *task,struct stack_info *info); +bool in_sysenter_stack (unsigned long *stack,struct stack_info *info); +int get_stack_info (unsigned long *stack,struct task_struct *task,struct stack_info *info,unsigned long *visit_mask);
arch/x86/kernel/dumpstack.c +19 -0
@@ -43,6 +43,25 @@ bool in_task_stack(unsigned long *stack, struct task_struct *task,return true; } +bool in_sysenter_stack (unsigned long *stack,struct stack_info *info) +{ +struct tss_struct *tss =this_cpu_ptr (&cpu_tss); + +/* Treat the canary as part of the stack for unwinding purposes. */ +void *begin = &tss->SYSENTER_stack_canary; +void *end = (void *)&tss->SYSENTER_stack +sizeof (tss->SYSENTER_stack); + +if ((void *)stack < begin || (void *)stack >= end) +return false; + + info->type =STACK_TYPE_SYSENTER ; + info->begin = begin; + info->end = end; + info->next_sp =NULL ; + +return true; +} +static void printk_stack_address (unsigned long address,int reliable,char *log_lvl) {
arch/x86/kernel/dumpstack_32.c +6 -0
@@ -26,6 +26,9 @@ const char *stack_type_name(enum stack_type type)if (type ==STACK_TYPE_SOFTIRQ )return "SOFTIRQ" ; +if (type ==STACK_TYPE_SYSENTER ) +return "SYSENTER" ; +return NULL ; }
@@ -93,6 +96,9 @@ int get_stack_info(unsigned long *stack, struct task_struct *task,if (task != current) goto unknown; +if (in_sysenter_stack (stack, info)) + goto recursion_check; +if (in_hardirq_stack (stack, info)) goto recursion_check;
arch/x86/kernel/dumpstack_64.c +6 -0
@@ -37,6 +37,9 @@ const char *stack_type_name(enum stack_type type)if (type ==STACK_TYPE_IRQ )return "IRQ" ; +if (type ==STACK_TYPE_SYSENTER ) +return "SYSENTER" ; +if (type >=STACK_TYPE_EXCEPTION && type <=STACK_TYPE_EXCEPTION_LAST )return exception_stack_names[type -STACK_TYPE_EXCEPTION ];
@@ -115,6 +118,9 @@ int get_stack_info(unsigned long *stack, struct task_struct *task,if (in_irq_stack (stack, info)) goto recursion_check; +if (in_sysenter_stack (stack, info)) + goto recursion_check; + goto unknown; recursion_check:
Signature · 1 line
2.14.1
PATCH 7/21 x86/entry/gdt: Put per-CPU GDT remaps in ascending order
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
We currently have CPU 0's GDT at the top of the GDT range and higher-numbered CPUs at lower addresses. This happens because the fixmap is upside down (index 0 is the top of the fixmap).
Flip it so that GDTs are in ascending order by virtual address. This will simplify a future patch that will generalize the GDT remap to contain multiple pages.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Reviewed-by Thomas Gleixner <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/3966a6edf6fd45deca4cf52a9b9276402499dda9.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/include/asm/desc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/include/asm/desc.h +1 -1
@@ -63,7 +63,7 @@ static inline struct desc_struct *get_current_gdt_rw(void)/* Get the fixmap index for a specific processor */ static inline unsigned int get_cpu_gdt_ro_index (int cpu) { -return FIX_GDT_REMAP_BEGIN + cpu; +return FIX_GDT_REMAP_END - cpu; }/* Provide the fixmap address of the remapped GDT */
Signature · 1 line
2.14.1
PATCH 8/21 x86/mm/fixmap: Generalize the GDT fixmap mechanism, introduce 'struct cpu_entry_area'
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
Currently, the GDT is an ad-hoc array of pages, one per CPU, in the fixmap. Generalize it to be an array of a new 'struct cpu_entry_area' so that we can cleanly add new things to it.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Thomas Gleixner <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/22571d77ba1f3c714df9fa37db9a58218bc17597.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/include/asm/desc.h | 9 +-------- arch/x86/include/asm/fixmap.h | 37 +++++++++++++++++++++++++++++++++++-- arch/x86/kernel/cpu/common.c | 14 +++++++------- arch/x86/xen/mmu_pv.c | 2 +- 4 files changed, 44 insertions(+), 18 deletions(-)
arch/x86/include/asm/desc.h +1 -8
@@ -60,17 +60,10 @@ static inline struct desc_struct *get_current_gdt_rw(void)return this_cpu_ptr (&gdt_page)->gdt; } -/* Get the fixmap index for a specific processor */ -static inline unsigned int get_cpu_gdt_ro_index (int cpu) -{ -return FIX_GDT_REMAP_END - cpu; -} -/* Provide the fixmap address of the remapped GDT */ static inline struct desc_struct *get_cpu_gdt_ro (int cpu) { -unsigned int idx =get_cpu_gdt_ro_index (cpu); -return (struct desc_struct *)__fix_to_virt (idx); +return (struct desc_struct *)&get_cpu_entry_area (cpu)->gdt; }/* Provide the current read-only GDT */
arch/x86/include/asm/fixmap.h +35 -2
@@ -44,6 +44,19 @@ extern unsigned long __FIXADDR_TOP; PAGE_SIZE)#endif +/* +* cpu_entry_area is a percpu region in the fixmap that contains things +* needed by the CPU and early entry/exit code. Real types aren't used +* for all fields here to avoid circular header dependencies. +* +* Every field is a virtual alias of some other allocated backing store. +* There is no direct allocation of a struct cpu_entry_area. +*/ +struct cpu_entry_area { +char gdt[PAGE_SIZE ]; +}; + +#define CPU_ENTRY_AREA_PAGES (sizeof(struct cpu_entry_area) / PAGE_SIZE)/* * Here we define all the compile-time 'special' virtual
@@ -101,8 +114,8 @@ enum fixed_addresses {FIX_LNW_VRTC , #endif/* Fixmap entries to remap the GDTs, one per processor. */ -FIX_GDT_REMAP_BEGIN , -FIX_GDT_REMAP_END =FIX_GDT_REMAP_BEGIN +NR_CPUS -1 , +FIX_CPU_ENTRY_AREA_TOP , +FIX_CPU_ENTRY_AREA_BOTTOM =FIX_CPU_ENTRY_AREA_TOP + (CPU_ENTRY_AREA_PAGES *NR_CPUS ) -1 , __end_of_permanent_fixed_addresses,
@@ -185,5 +198,25 @@ void __init *early_memremap_decrypted_wp(resource_size_t phys_addr,void __early_set_fixmap (enum fixed_addresses idx,phys_addr_t phys,pgprot_t flags); +static inline unsigned int __get_cpu_entry_area_page_index (int cpu,int page) +{ +BUILD_BUG_ON (sizeof (struct cpu_entry_area ) %PAGE_SIZE !=0 ); + +return FIX_CPU_ENTRY_AREA_BOTTOM - cpu*CPU_ENTRY_AREA_PAGES - page; +} + +#define __get_cpu_entry_area_offset_index (cpu, offset) ({ \ + BUILD_BUG_ON(offset % PAGE_SIZE != 0); \ + __get_cpu_entry_area_page_index(cpu, offset / PAGE_SIZE); \ + }) + +#define get_cpu_entry_area_index (cpu, field) \ + __get_cpu_entry_area_offset_index((cpu), offsetof(struct cpu_entry_area, field)) + +static inline struct cpu_entry_area *get_cpu_entry_area (int cpu) +{ +return (struct cpu_entry_area *)__fix_to_virt (__get_cpu_entry_area_page_index (cpu,0 )); +} +#endif /* !__ASSEMBLY__ */ #endif /* _ASM_X86_FIXMAP_H */
arch/x86/kernel/cpu/common.c +7 -7
@@ -490,12 +490,12 @@ void load_percpu_segment(int cpu)load_stack_canary_segment (); } -/* Setup the fixmap mapping only once per-processor */ -static inline void setup_fixmap_gdt (int cpu) +/* Setup the fixmap mappings only once per-processor */ +static inline void setup_cpu_entry_area (int cpu) {#ifdef CONFIG_X86_64 /* On 64-bit systems, we use a read-only fixmap GDT. */ -pgprot_t prot =PAGE_KERNEL_RO ; +pgprot_t gdt_prot =PAGE_KERNEL_RO ;#else /* * On native 32-bit systems, the GDT cannot be read-only because
@@ -506,11 +506,11 @@ static inline void setup_fixmap_gdt(int cpu)* On Xen PV, the GDT must be read-only because the hypervisor requires * it. */ -pgprot_t prot =boot_cpu_has (X86_FEATURE_XENPV ) ? +pgprot_t gdt_prot =boot_cpu_has (X86_FEATURE_XENPV ) ?PAGE_KERNEL_RO :PAGE_KERNEL ;#endif -__set_fixmap (get_cpu_gdt_ro_index (cpu),get_cpu_gdt_paddr (cpu), prot); +__set_fixmap (get_cpu_entry_area_index (cpu, gdt),get_cpu_gdt_paddr (cpu), gdt_prot); }/* Load the original GDT from the per-cpu structure */
@@ -1614,7 +1614,7 @@ void cpu_init(void)if (is_uv_system ())uv_cpu_init (); -setup_fixmap_gdt (cpu); +setup_cpu_entry_area (cpu);load_fixmap_gdt (cpu); }
@@ -1676,7 +1676,7 @@ void cpu_init(void)fpu__init_cpu (); -setup_fixmap_gdt (cpu); +setup_cpu_entry_area (cpu); load_fixmap_gdt(cpu); }#endif
arch/x86/xen/mmu_pv.c +1 -1
@@ -2272,7 +2272,7 @@ static void xen_set_fixmap(unsigned idx, phys_addr_t phys, pgprot_t prot)#endif case FIX_TEXT_POKE0 :case FIX_TEXT_POKE1 : - case FIX_GDT_REMAP_BEGIN ... FIX_GDT_REMAP_END: + case FIX_CPU_ENTRY_AREA_TOP ... FIX_CPU_ENTRY_AREA_BOTTOM:/* All local page mappings */ pte =pfn_pte (phys, prot);break ;
Signature · 1 line
2.14.1
PATCH 9/21 x86/kasan/64: Teach KASAN about the cpu_entry_area
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
The cpu_entry_area will contain stacks. Make sure that KASAN has appropriate shadow mappings for them.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Andrey Ryabinin <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Cc Alexander Potapenko <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc Dmitry Vyukov <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/8407adf9126440d6467dade88fdb3e3b75fc1019.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/mm/kasan_init_64.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)
arch/x86/mm/kasan_init_64.c +17 -1
@@ -277,6 +277,7 @@ void __init kasan_early_init(void)void __init kasan_init (void ) {int i; +void *shadow_cpu_entry_begin, *shadow_cpu_entry_end;#ifdef CONFIG_KASAN_INLINE register_die_notifier (&kasan_die_notifier);
@@ -329,8 +330,23 @@ void __init kasan_init(void) (unsigned long )kasan_mem_to_shadow (_end), early_pfn_to_nid(__pa(_stext))); + shadow_cpu_entry_begin = (void *)__fix_to_virt (FIX_CPU_ENTRY_AREA_BOTTOM ); + shadow_cpu_entry_begin =kasan_mem_to_shadow (shadow_cpu_entry_begin); + shadow_cpu_entry_begin = (void *)round_down ((unsigned long )shadow_cpu_entry_begin, +PAGE_SIZE ); + + shadow_cpu_entry_end = (void *)(__fix_to_virt (FIX_CPU_ENTRY_AREA_TOP ) +PAGE_SIZE ); + shadow_cpu_entry_end =kasan_mem_to_shadow (shadow_cpu_entry_end); + shadow_cpu_entry_end = (void *)round_up ((unsigned long )shadow_cpu_entry_end, +PAGE_SIZE ); +kasan_populate_zero_shadow (kasan_mem_to_shadow ((void *)MODULES_END ), - (void *)KASAN_SHADOW_END ); + shadow_cpu_entry_begin); + +kasan_populate_shadow ((unsigned long )shadow_cpu_entry_begin, + (unsigned long )shadow_cpu_entry_end,0 ); + +kasan_populate_zero_shadow (shadow_cpu_entry_end, (void *)KASAN_SHADOW_END );load_cr3 (init_top_pgt);__flush_tlb_all ();
Signature · 1 line
2.14.1
PATCH 10/21 x86/entry: Fix assumptions that the HW TSS is at the beginning of cpu_tss
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
A future patch will move SYSENTER_stack to the beginning of cpu_tss to help detect overflow. Before this can happen, fix several code paths that hardcode assumptions about the old layout.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Reviewed-by Dave Hansen <[email protected]>
Reviewed-by Thomas Gleixner <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/d40a2c5ae4539d64090849a374f3169ec492f4e2.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/include/asm/desc.h | 2 +- arch/x86/include/asm/processor.h | 9 +++++++-- arch/x86/kernel/cpu/common.c | 8 ++++---- arch/x86/kernel/doublefault.c | 36 +++++++++++++++++------------------- arch/x86/power/cpu.c | 13 +++++++------ 5 files changed, 36 insertions(+), 32 deletions(-)
arch/x86/include/asm/desc.h +1 -1
@@ -178,7 +178,7 @@ static inline void set_tssldt_descriptor(void *d, unsigned long addr,#endif } -static inline void __set_tss_desc (unsigned cpu,unsigned int entry,void *addr) +static inline void __set_tss_desc (unsigned cpu,unsigned int entry,struct x86_hw_tss *addr) {struct desc_struct *d =get_cpu_gdt_rw (cpu);tss_desc tss;
arch/x86/include/asm/processor.h +7 -2
@@ -163,7 +163,7 @@ enum cpuid_regs_idx {extern struct cpuinfo_x86 boot_cpu_data;extern struct cpuinfo_x86 new_cpu_data; -extern struct tss_struct doublefault_tss; +extern struct x86_hw_tss doublefault_tss;extern __u32 cpu_caps_cleared[NCAPINTS ];extern __u32 cpu_caps_set[NCAPINTS ];
@@ -253,6 +253,11 @@ static inline void load_cr3(pgd_t *pgdir)write_cr3 (__sme_pa (pgdir)); } +/* +* Note that while the legacy 'TSS' name comes from 'Task State Segment', +* on modern x86 CPUs the TSS also holds information important to 64-bit mode, +* unrelated to the task-switch mechanism: +*/ #ifdef CONFIG_X86_32 /* This is the TSS defined by the hardware. */ struct x86_hw_tss {
@@ -323,7 +328,7 @@ struct x86_hw_tss {#define IO_BITMAP_BITS 65536#define IO_BITMAP_BYTES (IO_BITMAP_BITS/8)#define IO_BITMAP_LONGS (IO_BITMAP_BYTES/sizeof(long)) -#define IO_BITMAP_OFFSET offsetof(struct tss_struct, io_bitmap) +#define IO_BITMAP_OFFSET (offsetof(struct tss_struct, io_bitmap) - offsetof(struct tss_struct, x86_tss))#define INVALID_IO_BITMAP_OFFSET 0x8000struct tss_struct {
arch/x86/kernel/cpu/common.c +4 -4
@@ -1582,7 +1582,7 @@ void cpu_init(void) } } - t->x86_tss.io_bitmap_base = offsetof(struct tss_struct , io_bitmap); + t->x86_tss.io_bitmap_base =IO_BITMAP_OFFSET ;/* * <= is required because the CPU will access up to
@@ -1601,7 +1601,7 @@ void cpu_init(void)* Initialize the TSS. Don't bother initializing sp0, as the initial * task never enters user mode. */ -set_tss_desc (cpu, t); +set_tss_desc (cpu, &t->x86_tss);load_TR_desc ();load_mm_ldt (&init_mm);
@@ -1659,12 +1659,12 @@ void cpu_init(void)* Initialize the TSS. Don't bother initializing sp0, as the initial * task never enters user mode. */ -set_tss_desc (cpu, t); +set_tss_desc (cpu, &t->x86_tss);load_TR_desc ();load_mm_ldt (&init_mm); - t->x86_tss.io_bitmap_base = offsetof(struct tss_struct , io_bitmap); + t->x86_tss.io_bitmap_base =IO_BITMAP_OFFSET ;#ifdef CONFIG_DOUBLEFAULT /* Set up doublefault TSS pointer in the GDT */
arch/x86/kernel/doublefault.c +17 -19
@@ -50,25 +50,23 @@ static void doublefault_fn(void)cpu_relax (); } -struct tss_struct doublefault_tss __cacheline_aligned = { - .x86_tss = { - .sp0 =STACK_START , - .ss0 = __KERNEL_DS, - .ldt =0 , - .io_bitmap_base =INVALID_IO_BITMAP_OFFSET , - - .ip = (unsigned long ) doublefault_fn, -/* 0x2 bit is always set */ - .flags =X86_EFLAGS_SF |0x2 , - .sp =STACK_START , - .es = __USER_DS, - .cs = __KERNEL_CS, - .ss = __KERNEL_DS, - .ds = __USER_DS, - .fs = __KERNEL_PERCPU, - - .__cr3 =__pa_nodebug (swapper_pg_dir), - } +struct x86_hw_tss doublefault_tss __cacheline_aligned = { + .sp0 =STACK_START , + .ss0 = __KERNEL_DS, + .ldt =0 , + .io_bitmap_base =INVALID_IO_BITMAP_OFFSET , + + .ip = (unsigned long ) doublefault_fn, +/* 0x2 bit is always set */ + .flags =X86_EFLAGS_SF |0x2 , + .sp =STACK_START , + .es = __USER_DS, + .cs = __KERNEL_CS, + .ss = __KERNEL_DS, + .ds = __USER_DS, + .fs = __KERNEL_PERCPU, + + .__cr3 =__pa_nodebug (swapper_pg_dir), };/* dummy for do_double_fault() call */
arch/x86/power/cpu.c +7 -6
@@ -165,12 +165,13 @@ static void fix_processor_context(void)struct desc_struct *desc =get_cpu_gdt_rw (cpu);tss_desc tss;#endif -set_tss_desc (cpu, t);/* -* This just modifies memory; should not be -* necessary. But... This is necessary, because -* 386 hardware has concept of busy TSS or some -* similar stupidity. -*/ + +/* +* This just modifies memory; should not be necessary. But... This is +* necessary, because 386 hardware has concept of busy TSS or some +* similar stupidity. +*/ +set_tss_desc (cpu, &t->x86_tss);#ifdef CONFIG_X86_64 memcpy (&tss, &desc[GDT_ENTRY_TSS ],sizeof (tss_desc));
Signature · 1 line
2.14.1
PATCH 11/21 x86/dumpstack: Handle stack overflow on all stacks
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
We currently special-case stack overflow on the task stack. We're going to start putting special stacks in the fixmap with a custom layout, so they'll have guard pages, too. Teach the unwinder to be able to unwind an overflow of any of the stacks.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/5454bb325cb30a70457a47b50f22317be65eba7d.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/kernel/dumpstack.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)
arch/x86/kernel/dumpstack.c +14 -10
@@ -112,24 +112,28 @@ void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,* - task stack * - interrupt stack * - HW exception stacks (double fault, nmi, debug, mce) +* - SYSENTER stack * -* x86-32 can have up to three stacks: +* x86-32 can have up to four stacks: * - task stack * - softirq stack * - hardirq stack +* - SYSENTER stack */ for (regs =NULL ; stack; stack =PTR_ALIGN (stack_info.next_sp,sizeof (long ))) {const char *stack_name; -/* -* If we overflowed the task stack into a guard page, jump back -* to the bottom of the usable stack. -*/ -if (task_stack_page (task) - (void *)stack <PAGE_SIZE ) - stack =task_stack_page (task); - -if (get_stack_info (stack, task, &stack_info, &visit_mask)) -break ; +if (get_stack_info (stack, task, &stack_info, &visit_mask)) { +/* +* We weren't on a valid stack. It's possible that +* we overflowed a valid stack into a guard page. +* See if the next page up is valid so that we can +* generate some kind of backtrace if this happens. +*/ + stack = (unsigned long *)PAGE_ALIGN ((unsigned long )stack); +if (get_stack_info (stack, task, &stack_info, &visit_mask)) +break ; + } stack_name =stack_type_name (stack_info.type);if (stack_name)
Signature · 1 line
2.14.1
Re: [PATCH 11/21] x86/dumpstack: Handle stack overflow on all stacks
Quoted 6 lines · Ingo Molnar
From: Andy Lutomirski <[email protected]>
We currently special-case stack overflow on the task stack. We're going to start putting special stacks in the fixmap with a custom layout, so they'll have guard pages, too. Teach the unwinder to be able to unwind an overflow of any of the stacks.
Why isn't this together with 01/21? The two cases seem to be entirely identical and fundamentally the same issue.
In fact, maybe the whole "stack overflow" special cases should be in "get_stack_info()" itself, rather than be special-cased in the callers?
Linus
Re: [PATCH 11/21] x86/dumpstack: Handle stack overflow on all stacks
Quoted 10 lines · Linus Torvalds
Quoted 6 lines · Ingo Molnar
From: Andy Lutomirski <[email protected]>
We currently special-case stack overflow on the task stack. We're going to start putting special stacks in the fixmap with a custom layout, so they'll have guard pages, too. Teach the unwinder to be able to unwind an overflow of any of the stacks.
Why isn't this together with 01/21? The two cases seem to be entirely identical and fundamentally the same issue.
Yeah, they probably do belong in the same patch.
Quoted 3 lines
In fact, maybe the whole "stack overflow" special cases should be in "
get_stack_info()" itself, rather than be special-cased in the callers?
I would be nervous about doing that. Several of the get_stack_info() callers rely on it being honest.
In fact, looking deeper at the above patch, it doesn't seem convincingly safe to me. What if the adjacent page doesn't exist? Then when the oops dumping code dereferences the 'stack' variable, you get an oops in your oops.
Signature · 1 line
Josh
Re: [PATCH 11/21] x86/dumpstack: Handle stack overflow on all stacks
Quoted 18 lines · Josh Poimboeuf
Quoted 10 lines · Linus Torvalds
Quoted 6 lines · Ingo Molnar
From: Andy Lutomirski <[email protected]>
We currently special-case stack overflow on the task stack. We're going to start putting special stacks in the fixmap with a custom layout, so they'll have guard pages, too. Teach the unwinder to be able to unwind an overflow of any of the stacks.
Why isn't this together with 01/21? The two cases seem to be entirely identical and fundamentally the same issue.
Yeah, they probably do belong in the same patch.
Quoted 3 lines
In fact, maybe the whole "stack overflow" special cases should be in "
get_stack_info()" itself, rather than be special-cased in the callers?
Quoted 8 lines
I would be nervous about doing that. Several of the
get_stack_info()callers rely on it being honest.In fact, looking deeper at the above patch, it doesn't seem convincingly safe to me. What if the adjacent page doesn't exist? Then when the oops dumping code dereferences the 'stack' variable, you get an oops in your oops.
Isn't the oops dumping code supposed to dereference everything using a special safe function?
Anyway, get_stack_info() wouldn't really be lying. It would just be returning something where begin..end doesn't contain the requested pointer.
Re: [PATCH 11/21] x86/dumpstack: Handle stack overflow on all stacks
Quoted 19 lines · Andy Lutomirski
Quoted 18 lines · Josh Poimboeuf
Quoted 10 lines · Linus Torvalds
Quoted 6 lines · Ingo Molnar
From: Andy Lutomirski <[email protected]>
We currently special-case stack overflow on the task stack. We're going to start putting special stacks in the fixmap with a custom layout, so they'll have guard pages, too. Teach the unwinder to be able to unwind an overflow of any of the stacks.
Why isn't this together with 01/21? The two cases seem to be entirely identical and fundamentally the same issue.
Yeah, they probably do belong in the same patch.
Quoted 3 lines
In fact, maybe the whole "stack overflow" special cases should be in "
get_stack_info()" itself, rather than be special-cased in the callers?
Quoted 7 lines
Quoted 7 lines
I would be nervous about doing that. Several of the
get_stack_info()callers rely on it being honest.In fact, looking deeper at the above patch, it doesn't seem convincingly safe to me. What if the adjacent page doesn't exist? Then when the oops dumping code dereferences the 'stack' variable, you get an oops in your oops.
I was wrong here. Too much thinking, not enough sleep. Looking at it today, the patch looks safe again.
The 'stack' variable is actually safe to dereference afterwards because it gets updated with the safe aligned address.
Quoted 2 lines
Isn't the oops dumping code supposed to dereference everything using a special safe function?
That's not what it does now. The 'stack' pointer variable is dereferenced normally, unless KASAN is enabled. It relies on get_stack_info() doing the safety check.
Quoted 3 lines
Anyway,
get_stack_info()wouldn't really be lying. It would just be returning something where begin..end doesn't contain the requested pointer.
At the very least, it would be surprising. I prefer the current approach, it's much more explicit.
Signature · 1 line
Josh
PATCH 12/21 x86/entry: Move SYSENTER_stack to the beginning of struct tss_struct
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
SYSENTER_stack should have reliable overflow detection, which means that it needs to be at the bottom of a page, not the top. Move it to the beginning of struct tss_struct and page-align it.
Also add an assertion to make sure that the fixed hardware TSS doesn't cross a page boundary.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Thomas Gleixner <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/8de9901e7c3a6aa8fac95b37b9c7b96f1900f11a.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/include/asm/processor.h | 21 ++++++++++++--------- arch/x86/kernel/cpu/common.c | 21 +++++++++++++++++++++ 2 files changed, 33 insertions(+), 9 deletions(-)
arch/x86/include/asm/processor.h +12 -9
@@ -333,7 +333,16 @@ struct x86_hw_tss {struct tss_struct {/* -* The hardware state: +* Space for the temporary SYSENTER stack, used for SYSENTER +* and the entry trampoline as well. +*/ +unsigned long SYSENTER_stack_canary; +unsigned long SYSENTER_stack[64 ]; + +/* +* The fixed hardware portion. This must not cross a page boundary +* at risk of violating the SDM's advice and potentially triggering +* errata. */ struct x86_hw_tss x86_tss;
@@ -344,15 +353,9 @@ struct tss_struct {* be within the limit. */ unsigned long io_bitmap[IO_BITMAP_LONGS +1 ]; +}__aligned(PAGE_SIZE) ; -/* -* Space for the temporary SYSENTER stack. -*/ -unsigned long SYSENTER_stack_canary; -unsigned long SYSENTER_stack[64 ]; -}____cacheline_aligned ; - -DECLARE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss); +DECLARE_PER_CPU_PAGE_ALIGNED(struct tss_struct, cpu_tss);/* * sizeof(unsigned long) coming from an extra "long" at the end
arch/x86/kernel/cpu/common.c +21 -0
@@ -511,6 +511,27 @@ static inline void setup_cpu_entry_area(int cpu)#endif __set_fixmap (get_cpu_entry_area_index (cpu, gdt),get_cpu_gdt_paddr (cpu), gdt_prot); + +/* +* The Intel SDM says (Volume 3, 7.2.1): +* +* Avoid placing a page boundary in the part of the TSS that the +* processor reads during a task switch (the first 104 bytes). The +* processor may not correctly perform address translations if a +* boundary occurs in this area. During a task switch, the processor +* reads and writes into the first 104 bytes of each TSS (using +* contiguous physical addresses beginning with the physical address +* of the first byte of the TSS). So, after TSS access begins, if +* part of the 104 bytes is not physically contiguous, the processor +* will access incorrect information without generating a page-fault +* exception. +* +* There are also a lot of errata involving the TSS spanning a page +* boundary. Assert that we're not doing that. +*/ +BUILD_BUG_ON ((offsetof(struct tss_struct , x86_tss) ^ + offsetofend(struct tss_struct, x86_tss)) & PAGE_MASK); + }/* Load the original GDT from the per-cpu structure */
Signature · 1 line
2.14.1
PATCH 13/21 x86/entry: Remap the TSS into the CPU entry area
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
This has a secondary purpose: it puts the entry stack into a region with a well-controlled layout. A subsequent patch will take advantage of this to streamline the SYSCALL entry code to be able to find it more easily.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Thomas Gleixner <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/cdcba7e1e82122461b3ca36bb3ef6713ba605e35.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/entry/entry_32.S | 6 ++++-- arch/x86/include/asm/fixmap.h | 7 +++++++ arch/x86/kernel/asm-offsets.c | 3 +++ arch/x86/kernel/cpu/common.c | 38 ++++++++++++++++++++++++++++++++------ arch/x86/kernel/dumpstack.c | 3 ++- arch/x86/power/cpu.c | 11 ++++++----- 6 files changed, 54 insertions(+), 14 deletions(-)
arch/x86/entry/entry_32.S +4 -2
@@ -941,7 +941,8 @@ ENTRY(debug) movl %esp, %eax # pt_regs pointer /* Are we currently on the SYSENTER stack? */ - PER_CPU(cpu_tss + CPU_TSS_SYSENTER_stack + SIZEOF_SYSENTER_stack, %ecx) + movl PER_CPU_VAR(cpu_entry_area), %ecx + addl $CPU_ENTRY_AREA_tss + CPU_TSS_SYSENTER_stack + SIZEOF_SYSENTER_stack, %ecx subl %eax, %ecx /* ecx = (end of SYSENTER_stack) - esp */ cmpl $SIZEOF_SYSENTER_stack, %ecx jb .Ldebug_from_sysenter_stack
@@ -984,7 +985,8 @@ ENTRY(nmi) movl %esp, %eax # pt_regs pointer /* Are we currently on the SYSENTER stack? */ - PER_CPU(cpu_tss + CPU_TSS_SYSENTER_stack + SIZEOF_SYSENTER_stack, %ecx) + movl PER_CPU_VAR(cpu_entry_area), %ecx + addl $CPU_ENTRY_AREA_tss + CPU_TSS_SYSENTER_stack + SIZEOF_SYSENTER_stack, %ecx subl %eax, %ecx /* ecx = (end of SYSENTER_stack) - esp */ cmpl $SIZEOF_SYSENTER_stack, %ecx jb .Lnmi_from_sysenter_stack
arch/x86/include/asm/fixmap.h +7 -0
@@ -54,6 +54,13 @@ extern unsigned long __FIXADDR_TOP;*/ struct cpu_entry_area {char gdt[PAGE_SIZE ]; + +/* +* The GDT is just below cpu_tss and thus serves (on x86_64) as a +* a read-only guard page for the SYSENTER stack at the bottom +* of the TSS region. +*/ +struct tss_struct tss; };#define CPU_ENTRY_AREA_PAGES (sizeof(struct cpu_entry_area) / PAGE_SIZE)
arch/x86/kernel/asm-offsets.c +3 -0
@@ -98,4 +98,7 @@ void common(void) {OFFSET (CPU_TSS_SYSENTER_stack, tss_struct, SYSENTER_stack);/* Size of SYSENTER_stack */ DEFINE (SIZEOF_SYSENTER_stack,sizeof (((struct tss_struct *)0 )->SYSENTER_stack)); + +/* Layout info for cpu_entry_area */ +OFFSET (CPU_ENTRY_AREA_tss, cpu_entry_area, tss); }
arch/x86/kernel/cpu/common.c +32 -6
@@ -490,6 +490,19 @@ void load_percpu_segment(int cpu)load_stack_canary_segment (); } +static void set_percpu_fixmap_pages (int fixmap_index,void *ptr,int pages,pgprot_t prot) +{ +int i; + +for (i =0 ; i < pages; i++) +__set_fixmap (fixmap_index - i,per_cpu_ptr_to_phys (ptr + i*PAGE_SIZE ), prot); +} + +#ifdef CONFIG_X86_32 +/* The 32-bit entry code needs to find cpu_entry_area. */ +DEFINE_PER_CPU(struct cpu_entry_area *, cpu_entry_area); +#endif +/* Setup the fixmap mappings only once per-processor */ static inline void setup_cpu_entry_area (int cpu) {
@@ -531,7 +544,15 @@ static inline void setup_cpu_entry_area(int cpu)*/ BUILD_BUG_ON ((offsetof(struct tss_struct , x86_tss) ^ offsetofend(struct tss_struct, x86_tss)) & PAGE_MASK); +BUILD_BUG_ON (sizeof (struct tss_struct ) %PAGE_SIZE !=0 ); +set_percpu_fixmap_pages (get_cpu_entry_area_index (cpu, tss), + &per_cpu (cpu_tss, cpu), +sizeof (struct tss_struct ) /PAGE_SIZE , +PAGE_KERNEL ); +#ifdef CONFIG_X86_32 +this_cpu_write (cpu_entry_area,get_cpu_entry_area (cpu)); +#endif }/* Load the original GDT from the per-cpu structure */
@@ -1282,7 +1303,8 @@ void enable_sep_cpu(void)wrmsr (MSR_IA32_SYSENTER_CS , tss->x86_tss.ss1,0 );wrmsr (MSR_IA32_SYSENTER_ESP , - (unsigned long)tss + offsetofend(struct tss_struct, SYSENTER_stack), + (unsigned long )&get_cpu_entry_area (cpu)->tss + + offsetofend(struct tss_struct, SYSENTER_stack),0 );wrmsr (MSR_IA32_SYSENTER_EIP , (unsigned long )entry_SYSENTER_32,0 );
@@ -1395,6 +1417,8 @@ static DEFINE_PER_CPU_PAGE_ALIGNED(char, exception_stacks/* May not be marked __init: used by software suspend */ void syscall_init (void ) { +int cpu =smp_processor_id (); +wrmsr (MSR_STAR ,0 , (__USER32_CS <<16 ) | __KERNEL_CS);wrmsrl (MSR_LSTAR , (unsigned long )entry_SYSCALL_64);
@@ -1408,7 +1432,7 @@ void syscall_init(void)*/ wrmsrl_safe (MSR_IA32_SYSENTER_CS , (u64 )__KERNEL_CS);wrmsrl_safe (MSR_IA32_SYSENTER_ESP , - (unsigned long )this_cpu_ptr (&cpu_tss) + + (unsigned long )&get_cpu_entry_area (cpu)->tss + offsetofend(struct tss_struct, SYSENTER_stack));wrmsrl_safe (MSR_IA32_SYSENTER_EIP , (u64 )entry_SYSENTER_compat);#else
@@ -1618,11 +1642,13 @@ void cpu_init(void)initialize_tlbstate_and_flush ();enter_lazy_tlb (&init_mm, me); +setup_cpu_entry_area (cpu); +/* * Initialize the TSS. Don't bother initializing sp0, as the initial * task never enters user mode. */ -set_tss_desc (cpu, &t->x86_tss); +set_tss_desc (cpu, &get_cpu_entry_area (cpu)->tss.x86_tss);load_TR_desc ();load_mm_ldt (&init_mm);
@@ -1635,7 +1661,6 @@ void cpu_init(void)if (is_uv_system ())uv_cpu_init (); -setup_cpu_entry_area (cpu);load_fixmap_gdt (cpu); }
@@ -1676,11 +1701,13 @@ void cpu_init(void)initialize_tlbstate_and_flush ();enter_lazy_tlb (&init_mm, curr); +setup_cpu_entry_area (cpu); +/* * Initialize the TSS. Don't bother initializing sp0, as the initial * task never enters user mode. */ -set_tss_desc (cpu, &t->x86_tss); +set_tss_desc (cpu, &get_cpu_entry_area (cpu)->tss.x86_tss);load_TR_desc ();load_mm_ldt (&init_mm);
@@ -1697,7 +1724,6 @@ void cpu_init(void)fpu__init_cpu (); -setup_cpu_entry_area (cpu); load_fixmap_gdt(cpu); }#endif
arch/x86/kernel/dumpstack.c +2 -1
@@ -45,7 +45,8 @@ bool in_task_stack(unsigned long *stack, struct task_struct *task,bool in_sysenter_stack (unsigned long *stack,struct stack_info *info) { -struct tss_struct *tss =this_cpu_ptr (&cpu_tss); +int cpu =smp_processor_id (); +struct tss_struct *tss = &get_cpu_entry_area (cpu)->tss;/* Treat the canary as part of the stack for unwinding purposes. */ void *begin = &tss->SYSENTER_stack_canary;
arch/x86/power/cpu.c +6 -5
@@ -160,18 +160,19 @@ static void do_fpu_end(void)static void fix_processor_context (void ) {int cpu =smp_processor_id (); -struct tss_struct *t = &per_cpu (cpu_tss, cpu);#ifdef CONFIG_X86_64 struct desc_struct *desc =get_cpu_gdt_rw (cpu);tss_desc tss;#endif /* -* This just modifies memory; should not be necessary. But... This is -* necessary, because 386 hardware has concept of busy TSS or some -* similar stupidity. +* We need to reload TR, which requires that we change the +* GDT entry to indicate "available" first. +* +* XXX: This could probably all be replaced by a call to +* force_reload_TR(). */ -set_tss_desc (cpu, &t->x86_tss); +set_tss_desc (cpu, &get_cpu_entry_area (cpu)->tss.x86_tss);#ifdef CONFIG_X86_64 memcpy (&tss, &desc[GDT_ENTRY_TSS ],sizeof (tss_desc));
Signature · 1 line
2.14.1
PATCH 14/21 x86/entry/64: Separate cpu_current_top_of_stack from TSS.sp0
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
On 64-bit kernels, we used to assume that TSS.sp0 was the current top of stack. With the addition of an entry trampoline, this will no longer be the case. Store the current top of stack in TSS.sp1, which is otherwise unused but shares the same cacheline.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Thomas Gleixner <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/f56634c746a2926eb7bae61e7b80ed51a1940769.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/include/asm/processor.h | 18 +++++++++++++----- arch/x86/include/asm/thread_info.h | 2 +- arch/x86/kernel/asm-offsets_64.c | 1 + arch/x86/kernel/process.c | 10 ++++++++++ arch/x86/kernel/process_64.c | 1 + 5 files changed, 26 insertions(+), 6 deletions(-)
arch/x86/include/asm/processor.h +13 -5
@@ -310,7 +310,13 @@ struct x86_hw_tss {struct x86_hw_tss {u32 reserved1;u64 sp0; + +/* +* We store cpu_current_top_of_stack in sp1 so it's always accessible. +* Linux does not use ring 1, so sp1 is not otherwise needed. +*/ u64 sp1; +u64 sp2;u64 reserved2;u64 ist[7 ];
@@ -369,6 +375,8 @@ DECLARE_PER_CPU_PAGE_ALIGNED(struct tss_struct, cpu_tss);#ifdef CONFIG_X86_32 DECLARE_PER_CPU(unsigned long, cpu_current_top_of_stack); +#else +#define cpu_current_top_of_stack cpu_tss.x86_tss.sp1#endif /*
@@ -540,12 +548,12 @@ static inline void native_swapgs(void)static inline unsigned long current_top_of_stack (void ) { -#ifdef CONFIG_X86_64 -return this_cpu_read_stable (cpu_tss.x86_tss.sp0); -#else -/* sp0 on x86_32 is special in and around vm86 mode. */ +/* +* We can't read directly from tss.sp0: sp0 on x86_32 is special in +* and around vm86 mode and sp0 on x86_64 is special because of the +* entry trampoline. +*/ return this_cpu_read_stable (cpu_current_top_of_stack); -#endif }static inline bool on_thread_stack (void )
arch/x86/include/asm/thread_info.h +1 -1
@@ -207,7 +207,7 @@ static inline int arch_within_stack_frames(const void * const stack,#else /* !__ASSEMBLY__ */ #ifdef CONFIG_X86_64 -# define cpu_current_top_of_stack (cpu_tss + TSS_sp0) +# define cpu_current_top_of_stack (cpu_tss + TSS_sp1)#endif #endif
arch/x86/kernel/asm-offsets_64.c +1 -0
@@ -63,6 +63,7 @@ int main(void)OFFSET (TSS_ist, tss_struct, x86_tss.ist);OFFSET (TSS_sp0, tss_struct, x86_tss.sp0); +OFFSET (TSS_sp1, tss_struct, x86_tss.sp1);BLANK ();#ifdef CONFIG_CC_STACKPROTECTOR
arch/x86/kernel/process.c +10 -0
@@ -56,6 +56,16 @@ __visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss) = {* Poison it. */ .sp0 = (1UL << (BITS_PER_LONG-1)) + 1, + +#ifdef CONFIG_X86_64 +/* +* .sp1 is cpu_current_top_of_stack. The init task never +* runs user code, but cpu_current_top_of_stack should still +* be well defined before the first context switch. +*/ + .sp1 = TOP_OF_INIT_STACK, +#endif +#ifdef CONFIG_X86_32 .ss0 = __KERNEL_DS, .ss1 = __KERNEL_CS,
arch/x86/kernel/process_64.c +1 -0
@@ -461,6 +461,7 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)* Switch the PDA and FPU contexts. */ this_cpu_write (current_task, next_p); +this_cpu_write (cpu_current_top_of_stack,task_top_of_stack (next_p));/* Reload sp0. */ update_sp0 (next_p);
Signature · 1 line
2.14.1
PATCH 15/21 x86/espfix/64: Stop assuming that pt_regs is on the entry stack
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
When we start using an entry trampoline, a #GP from userspace will be delivered on the entry stack, not on the task stack. Fix the espfix64 #DF fixup to set up #GP according to TSS.SP0, rather than assuming that pt_regs + 1 == SP0. This won't change anything without an entry stack, but it will make the code continue to work when an entry stack is added.
While we're at it, improve the comments to explain what's actually going on.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Thomas Gleixner <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/b1ef4136616c6bd2a75d1fd2736d1d54437d65a8.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/kernel/traps.c | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-)
arch/x86/kernel/traps.c +28 -9
@@ -349,9 +349,15 @@ dotraplinkage void do_double_fault(struct pt_regs *regs, long error_code)/* * If IRET takes a non-IST fault on the espfix64 stack, then we -* end up promoting it to a doublefault. In that case, modify -* the stack to make it look like we just entered the #GP -* handler from user space, similar to bad_iret. +* end up promoting it to a doublefault. In that case, take +* advantage of the fact that we're not using the normal (TSS.sp0) +* stack right now. We can write a fake #GP(0) frame at TSS.sp0 +* and then modify our own IRET frame so that, when we return, +* we land directly at the #GP(0) vector with the stack already +* set up according to its expectations. +* +* The net result is that our #GP handler will think that we +* entered from usermode with the bad user context. * * No need for ist_enter here because we don't use RCU. */
@@ -359,13 +365,26 @@ dotraplinkage void do_double_fault(struct pt_regs *regs, long error_code) regs->cs == __KERNEL_CS && regs->ip == (unsigned long)native_irq_return_iret) { -struct pt_regs *normal_regs =task_pt_regs (current); +struct pt_regs *gpregs = (struct pt_regs *)this_cpu_read (cpu_tss.x86_tss.sp0) -1 ; + +/* +* regs->sp points to the failing IRET frame on the +* ESPFIX64 stack. Copy it to the entry stack. This fills +* in gpregs->ss through gpregs->ip. +* +*/ +memmove (&gpregs->ip, (void *)regs->sp,5 *8 ); + gpregs->orig_ax =0 ;/* Missing (lost) #GP error code */ -/* Fake a #GP(0) from userspace. */ -memmove (&normal_regs->ip, (void *)regs->sp,5 *8 ); - normal_regs->orig_ax =0 ;/* Missing (lost) #GP error code */ +/* +* Adjust our frame so that we return straight to the #GP +* vector with the expected RSP value. This is safe because +* we won't enable interupts or schedule before we invoke +* general_protection, so nothing will clobber the stack +* frame we just set up. +*/ regs->ip = (unsigned long )general_protection; - regs->sp = (unsigned long )&normal_regs->orig_ax; + regs->sp = (unsigned long )&gpregs->orig_ax;return ; }
@@ -390,7 +409,7 @@ dotraplinkage void do_double_fault(struct pt_regs *regs, long error_code)* * Processors update CR2 whenever a page fault is detected. If a * second page fault occurs while an earlier page fault is being -* deliv- ered, the faulting linear address of the second fault will +* delivered, the faulting linear address of the second fault will * overwrite the contents of CR2 (replacing the previous * address). These updates to CR2 occur even if the page fault * results in a double fault or occurs during the delivery of a
Signature · 1 line
2.14.1
PATCH 16/21 x86/entry/64: Use a per-CPU trampoline stack for IDT entries
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
Historically, IDT entries from usermode have always gone directly to the running task's kernel stack. Rearrange it so that we enter on a per-CPU trampoline stack and then manually switch to the task's stack. This touches a couple of extra cachelines, but it gives us a chance to run some code before we touch the kernel stack.
The asm isn't exactly beautiful, but I think that fully refactoring it can wait.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Reviewed-by Thomas Gleixner <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/fa3958723a1a85baeaf309c735b775841205800e.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/entry/entry_64.S | 67 ++++++++++++++++++++++++++++++---------- arch/x86/entry/entry_64_compat.S | 5 ++- arch/x86/include/asm/switch_to.h | 3 +- arch/x86/include/asm/traps.h | 1 - arch/x86/kernel/cpu/common.c | 6 ++-- arch/x86/kernel/traps.c | 12 +++---- 6 files changed, 65 insertions(+), 29 deletions(-)
arch/x86/entry/entry_64.S +50 -17
@@ -563,6 +563,13 @@ END(irq_entries_start) /* 0(%rsp): ~(interrupt number) */ .macro interrupt func cld + + testb $3, CS-ORIG_RAX(%rsp) + jz 1f + SWAPGS + call switch_to_thread_stack +1: + ALLOC_PT_GPREGS_ON_STACK SAVE_C_REGS SAVE_EXTRA_REGS
@@ -572,12 +579,8 @@ END(irq_entries_start) jz 1f /* - * IRQ from user mode. Switch to kernel gsbase and inform context - * tracking that we're in kernel mode. - */ - SWAPGS - - /* + * IRQ from user mode. + * * We need to tell lockdep that IRQs are off. We can't do this until * we fix gsbase, and we should do it before enter_from_user_mode * (which can take locks). Since TRACE_IRQS_OFF idempotent,
@@ -831,6 +834,32 @@ apicinterrupt IRQ_WORK_VECTOR irq_work_interrupt smp_irq_work_interrupt */ #define CPU_TSS_IST(x) PER_CPU_VAR(cpu_tss) + (TSS_ist + ((x) - 1) * 8) +/* + * Switch to the thread stack. This is called with the IRET frame and + * orig_ax on the stack. (That is, RDI..R12 are not on the stack and + * space has not been allocated for them.) + */ +ENTRY(switch_to_thread_stack) + UNWIND_HINT_FUNC + + pushq %rdi + movq %rsp, %rdi + movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp + UNWIND_HINT sp_offset=16 sp_reg=ORC_REG_DI + + pushq 7*8(%rdi) /* regs->ss */ + pushq 6*8(%rdi) /* regs->rsp */ + pushq 5*8(%rdi) /* regs->eflags */ + pushq 4*8(%rdi) /* regs->cs */ + pushq 3*8(%rdi) /* regs->ip */ + pushq 2*8(%rdi) /* regs->orig_ax */ + pushq 8(%rdi) /* return address */ + UNWIND_HINT_FUNC + + movq (%rdi), %rdi + ret +END(switch_to_thread_stack) + .macro idtentry sym do_sym has_error_code:req paranoid=0 shift_ist=-1 ENTRY(\sym) UNWIND_HINT_IRET_REGS offset=\has_error_code*8
@@ -848,11 +877,12 @@ ENTRY(\sym) ALLOC_PT_GPREGS_ON_STACK - .if \paranoid - .if \paranoid == 1 + .if \paranoid < 2 testb $3, CS(%rsp) /* If coming from userspace, switch stacks */ - jnz 1f + jnz .Lfrom_usermode_switch_stack_\@ .endif + + .if \paranoid call paranoid_entry .else call error_entry
@@ -894,20 +924,15 @@ ENTRY(\sym) jmp error_exit .endif - .if \paranoid == 1 + .if \paranoid < 2 /* - * Paranoid entry from userspace. Switch stacks and treat it + * Entry from userspace. Switch stacks and treat it * as a normal entry. This means that paranoid handlers * run in real process context if user_mode(regs). */ -1: +.Lfrom_usermode_switch_stack_\@: call error_entry - - movq %rsp, %rdi /* pt_regs pointer */ - call sync_regs - movq %rax, %rsp /* switch stack */ - movq %rsp, %rdi /* pt_regs pointer */ .if \has_error_code
@@ -1170,6 +1195,14 @@ ENTRY(error_entry) SWAPGS .Lerror_entry_from_usermode_after_swapgs: + /* Put us onto the real thread stack. */ + popq %r12 /* save return addr in %12 */ + movq %rsp, %rdi /* arg0 = pt_regs pointer */ + call sync_regs + movq %rax, %rsp /* switch stack */ + ENCODE_FRAME_POINTER + pushq %r12 + /* * We need to tell lockdep that IRQs are off. We can't do this until * we fix gsbase, and we should do it before enter_from_user_mode
arch/x86/entry/entry_64_compat.S +4 -1
@@ -306,8 +306,11 @@ ENTRY(entry_INT80_compat) */ movl %eax, %eax - /* Construct struct pt_regs on stack (iret frame is already on stack) */ pushq %rax /* pt_regs->orig_ax */ + + /* switch to thread stack expects orig_ax to be pushed */ + call switch_to_thread_stack + pushq %rdi /* pt_regs->di */ pushq %rsi /* pt_regs->si */ pushq %rdx /* pt_regs->dx */
arch/x86/include/asm/switch_to.h +1 -2
@@ -90,10 +90,9 @@ static inline void refresh_sysenter_cs(struct thread_struct *thread)/* This is used when switching tasks or entering/exiting vm86 mode. */ static inline void update_sp0 (struct task_struct *task) { +/* On x86_64, sp0 always points to the entry trampoline stack, which is constant: */ #ifdef CONFIG_X86_32 load_sp0 (task->thread.sp0); -#else -load_sp0 (task_top_of_stack (task));#endif }
arch/x86/include/asm/traps.h +0 -1
@@ -75,7 +75,6 @@ dotraplinkage void do_segment_not_present(struct pt_regs *, long); dotraplinkage void do_stack_segment(struct pt_regs *, long);#ifdef CONFIG_X86_64 dotraplinkage void do_double_fault(struct pt_regs *, long); -asmlinkage struct pt_regs *sync_regs(struct pt_regs *);#endif dotraplinkage void do_general_protection(struct pt_regs *, long); dotraplinkage void do_page_fault(struct pt_regs *, unsigned long);
arch/x86/kernel/cpu/common.c +4 -2
@@ -1645,11 +1645,13 @@ void cpu_init(void)setup_cpu_entry_area (cpu);/* -* Initialize the TSS. Don't bother initializing sp0, as the initial -* task never enters user mode. +* Initialize the TSS. sp0 points to the entry trampoline stack +* regardless of what task is running. */ set_tss_desc (cpu, &get_cpu_entry_area (cpu)->tss.x86_tss);load_TR_desc (); +load_sp0 ((unsigned long )&get_cpu_entry_area (cpu)->tss + + offsetofend(struct tss_struct, SYSENTER_stack));load_mm_ldt (&init_mm);
arch/x86/kernel/traps.c +6 -6
@@ -631,7 +631,7 @@ NOKPROBE_SYMBOL(do_int3);*/ asmlinkage __visible notrace struct pt_regs *sync_regs(struct pt_regs *eregs) { -struct pt_regs *regs =task_pt_regs (current); +struct pt_regs *regs = (struct pt_regs *)this_cpu_read (cpu_current_top_of_stack) -1 ; *regs = *eregs;return regs; }
@@ -648,13 +648,13 @@ struct bad_iret_stack *fixup_bad_iret(struct bad_iret_stack *s)/* * This is called from entry_64.S early in handling a fault * caused by a bad iret to user mode. To handle the fault -* correctly, we want move our stack frame to task_pt_regs -* and we want to pretend that the exception came from the -* iret target. +* correctly, we want to move our stack frame to where it would +* be had we entered directly on the entry stack (rather than +* just below the IRET frame) and we want to pretend that the +* exception came from the IRET target. */ struct bad_iret_stack *new_stack = -container_of (task_pt_regs (current), - struct bad_iret_stack, regs); + (struct bad_iret_stack *)this_cpu_read (cpu_tss.x86_tss.sp0) -1 ;/* Copy the IRET target to the new stack. */ memmove (&new_stack->regs.ip, (void *)s->regs.sp,5 *8 );
Signature · 1 line
2.14.1
Dave Hansen
maintainerRe: [PATCH 16/21] x86/entry/64: Use a per-CPU trampoline stack for IDT entries
Quoted 7 lines · Ingo Molnar
arch/x86/kernel/cpu/common.c +0
-1@@ -1645,11 +1645,13 @@ void cpu_init(void)setup_cpu_entry_area (cpu);/* -* Initialize the TSS. Don't bother initializing sp0, as the initial
arch/x86/kernel/cpu/common.c void cpu_init(void) · line 1656
Quoted 10 lines
arch/x86/kernel/cpu/common.c +4
-1-* task never enters user mode. +* Initialize the TSS. sp0 points to the entry trampoline stack +* regardless of what task is running. */ set_tss_desc (cpu, &get_cpu_entry_area (cpu)->tss.x86_tss);load_TR_desc (); +load_sp0 ((unsigned long )&get_cpu_entry_area (cpu)->tss + + offsetofend(struct tss_struct, SYSENTER_stack));load_mm_ldt (&init_mm);
Does this also mean that our stack dumps will say "<SYSENTER>" in oopses?
Quoted 4 lines
[ 30.811750] CR2: fffffffffdeb2f98 CR3: 0000000423fae001 CR4: 00000000001607e0 [ 30.819712] Call Trace: [ 30.822442] <SYSENTER> [ 30.825170] trace_hardirqs_on_thunk+0x1c/0x1c
...
Quoted 2 lines
[ 31.000571] R13: 0000000000000050 R14: 0000000000000076 R15: 00007f59f76f2d60 [ 31.008533] </SYSENTER>
Should we change that string to something more descriptive?
Andy Lutomirski
maintainerRe: [PATCH 16/21] x86/entry/64: Use a per-CPU trampoline stack for IDT entries
Quoted 19 lines · Dave Hansen
Quoted 18 lines · Ingo Molnar
arch/x86/kernel/cpu/common.c +4
-2@@ -1645,11 +1645,13 @@ void cpu_init(void)setup_cpu_entry_area (cpu);/* -* Initialize the TSS. Don't bother initializing sp0, as the initial -* task never enters user mode. +* Initialize the TSS. sp0 points to the entry trampoline stack +* regardless of what task is running. */ set_tss_desc (cpu, &get_cpu_entry_area (cpu)->tss.x86_tss);load_TR_desc (); +load_sp0 ((unsigned long )&get_cpu_entry_area (cpu)->tss + + offsetofend(struct tss_struct, SYSENTER_stack));
Quoted 3 lines
arch/x86/kernel/cpu/common.c void cpu_init(void) · line 1656
Quoted 1 line
arch/x86/kernel/cpu/common.c +0
-0load_mm_ldt (&init_mm);Does this also mean that our stack dumps will say "<SYSENTER>" in oopses?
Only if we oops while we're on the trampoline stack. If the oops is due to a bug that isn't in the entry code, then we won't see it.
Quoted 10 lines
Quoted 5 lines
[ 30.811750] CR2: fffffffffdeb2f98 CR3: 0000000423fae001 CR4: 00000000001607e0 [ 30.819712] Call Trace: [ 30.822442] <SYSENTER> [ 30.825170] trace_hardirqs_on_thunk+0x1c/0x1c...
Quoted 2 lines
[ 31.000571] R13: 0000000000000050 R14: 0000000000000076 R15: 00007f59f76f2d60 [ 31.008533] </SYSENTER>Should we change that string to something more descriptive?
I suppose we could rename it to "ENTRY_TRAMPOLINE" or something like that.
Dave Hansen
maintainerRe: [PATCH 16/21] x86/entry/64: Use a per-CPU trampoline stack for IDT entries
Quoted 11 lines
Quoted 9 lines
Quoted 4 lines
[ 30.811750] CR2: fffffffffdeb2f98 CR3: 0000000423fae001 CR4: 00000000001607e0 [ 30.819712] Call Trace: [ 30.822442] <SYSENTER> [ 30.825170] trace_hardirqs_on_thunk+0x1c/0x1c...
Quoted 2 lines
[ 31.000571] R13: 0000000000000050 R14: 0000000000000076 R15: 00007f59f76f2d60 [ 31.008533] </SYSENTER>Should we change that string to something more descriptive?
I suppose we could rename it to "
ENTRY_TRAMPOLINE" or something like that.
The attached patch does just that. Any objections?
From: Dave Hansen <[email protected]>
The "SYSENTER" stack is used for a lot more than SYSENTER now. Give it a better string to display in stack dumps.
We should probably cleanse the 64-bit code of the remaining "SYSENTER" nomenclature too at some point.
Signed-off-by Dave Hansen <[email protected]>
---
b/arch//x86/kernel/dumpstack_64.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
arch//x86/kernel/dumpstack_64.c renamed from arch//x86/kernel/dumpstack_64.c~SYSENTER-rename +8 -2
@@ -37,8 +37,14 @@ const char *stack_type_name(enum stack_tif (type ==STACK_TYPE_IRQ )return "IRQ" ; -if (type ==STACK_TYPE_SYSENTER ) -return "SYSENTER" ; +if (type ==STACK_TYPE_SYSENTER ) { +/* +* On 64-bit, we have a generic entry stack that we +* use for all the kernel try points, including +* SYSENTER. +*/ +return "ENTRY_TRAMPOLINE" ; + }if (type >=STACK_TYPE_EXCEPTION && type <=STACK_TYPE_EXCEPTION_LAST )return exception_stack_names[type -STACK_TYPE_EXCEPTION ];
_
Re: [PATCH 16/21] x86/entry/64: Use a per-CPU trampoline stack for IDT entries
Quoted 8 lines · Dave Hansen
Quoted 8 lines
Quoted 8 lines
Quoted 4 lines
[ 30.811750] CR2: fffffffffdeb2f98 CR3: 0000000423fae001 CR4: 00000000001607e0 [ 30.819712] Call Trace: [ 30.822442] <SYSENTER> [ 30.825170] trace_hardirqs_on_thunk+0x1c/0x1c...
Quoted 2 lines
[ 31.000571] R13: 0000000000000050 R14: 0000000000000076 R15: 00007f59f76f2d60 [ 31.008533] </SYSENTER>
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
Should we change that string to something more descriptive?
I suppose we could rename it to "
ENTRY_TRAMPOLINE" or something like that.The attached patch does just that. Any objections? <SYSENTER-rename.patch>
I think that, if we do this, we should rename it in the code, too. Calling it one thing in the oops and something else in the code is just going to add confusion.
Re: [PATCH 16/21] x86/entry/64: Use a per-CPU trampoline stack for IDT entries
In reply to Dave HansenQuoted 18 lines
From: Dave Hansen <[email protected]>
The "SYSENTER" stack is used for a lot more than SYSENTER now. Give it a better string to display in stack dumps.
We should probably cleanse the 64-bit code of the remaining "SYSENTER" nomenclature too at some point.
Signed-off-by Dave Hansen <[email protected]>---
b/arch//x86/kernel/dumpstack_64.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)diff -puN
arch//x86/kernel/dumpstack_64.c~SYSENTER-renamearch//x86/kernel/dumpstack_64.c
arch//x86/kernel/dumpstack_64.c const char *stack_type_name(enum stack_t · line 44
Quoted 10 lines
arch//x86/kernel/dumpstack_64.c renamed from arch//x86/kernel/dumpstack_64.c~SYSENTER-rename +5
-2@@ -37,8 +37,14 @@ const char *stack_type_name(enum stack_tif (type ==STACK_TYPE_IRQ )return "IRQ" ; -if (type ==STACK_TYPE_SYSENTER ) -return "SYSENTER" ; +if (type ==STACK_TYPE_SYSENTER ) { +/* +* On 64-bit, we have a generic entry stack that we +* use for all the kernel try points, including +* SYSENTER.
ITYM "kernel entry points".
- Kevin
PATCH 17/21 x86/entry/64: Return to userspace from the trampoline stack
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
By itself, this is useless. It gives us the ability to run some final code before exit that cannnot run on the kernel stack. This could include a CR3 switch a la KAISER or some kernel stack erasing, for example. (Or even weird things like *changing* which kernel stack gets used as an ASLR-strengthening mechanism.)
The SYSRET32 path is not covered yet. It could be in the future or we could just ignore it and force the slow path if needed.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Thomas Gleixner <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/d350017000eed20922c3b2711a2d9229dc809256.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/entry/entry_64.S | 55 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 4 deletions(-)
arch/x86/entry/entry_64.S +51 -4
@@ -330,8 +330,24 @@ GLOBAL(entry_SYSCALL_64_after_hwframe) popq %rsi /* skip rcx */ popq %rdx popq %rsi + + /* + * Now all regs are restored except RSP and RDI. + * Save old stack pointer and switch to trampoline stack. + */ + movq %rsp, %rdi + movq PER_CPU_VAR(cpu_tss + TSS_sp0), %rsp + + pushq RSP-RDI(%rdi) /* RSP */ + pushq (%rdi) /* RDI */ + + /* + * We are on the trampoline stack. All regs except RDI are live. + * We can do future final exit work right here. + */ + popq %rdi - movq RSP-ORIG_RAX(%rsp), %rsp + popq %rsp USERGS_SYSRET64 END(entry_SYSCALL_64)
@@ -633,10 +649,41 @@ GLOBAL(swapgs_restore_regs_and_return_to_usermode) ud2 1: #endif - SWAPGS POP_EXTRA_REGS - POP_C_REGS - addq $8, %rsp /* skip regs->orig_ax */ + popq %r11 + popq %r10 + popq %r9 + popq %r8 + popq %rax + popq %rcx + popq %rdx + popq %rsi + + /* + * The stack is now user RDI, orig_ax, RIP, CS, EFLAGS, RSP, SS. + * Save old stack pointer and switch to trampoline stack. + */ + movq %rsp, %rdi + movq PER_CPU_VAR(cpu_tss + TSS_sp0), %rsp + + /* Copy the IRET frame to the trampoline stack. */ + pushq 6*8(%rdi) /* SS */ + pushq 5*8(%rdi) /* RSP */ + pushq 4*8(%rdi) /* EFLAGS */ + pushq 3*8(%rdi) /* CS */ + pushq 2*8(%rdi) /* RIP */ + + /* Push user RDI on the trampoline stack. */ + pushq (%rdi) + + /* + * We are on the trampoline stack. All regs except RDI are live. + * We can do future final exit work right here. + */ + + /* Restore RDI. */ + popq %rdi + SWAPGS INTERRUPT_RETURN
Signature · 1 line
2.14.1
PATCH 18/21 x86/entry/64: Create a per-CPU SYSCALL entry trampoline
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
Handling SYSCALL is tricky: the SYSCALL handler is entered with every single register (except FLAGS), including RSP, live. It somehow needs to set RSP to point to a valid stack, which means it needs to save the user RSP somewhere and find its own stack pointer. The canonical way to do this is with SWAPGS, which lets us access percpu data using the %gs prefix.
With KAISER-like pagetable switching, this is problematic. Without a scratch register, switching CR3 is impossible, so %gs-based percpu memory would need to be mapped in the user pagetables. Doing that without information leaks is difficult or impossible.
Instead, use a different sneaky trick. Map a copy of the first part of the SYSCALL asm at a different address for each CPU. Now RIP varies depending on the CPU, so we can use RIP-relative memory access to access percpu memory. By putting the relevant information (one scratch slot and the stack address) at a constant offset relative to RIP, we can make SYSCALL work without relying on %gs.
A nice thing about this approach is that we can easily switch it on and off if we want pagetable switching to be configurable.
The compat variant of SYSCALL doesn't have this problem in the first place -- there are plenty of scratch registers, since we don't care about preserving r8-r15. This patch therefore doesn't touch SYSCALL32 at all.
XXX: Whenever we settle how KAISER gets turned on and off, we should do the same to this.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Thomas Gleixner <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/b95ccae0a5a2f090c901e49fce7c9e8ff6acd40d.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/entry/entry_64.S | 48 +++++++++++++++++++++++++++++++++++++++++++ arch/x86/include/asm/fixmap.h | 2 ++ arch/x86/kernel/asm-offsets.c | 1 + arch/x86/kernel/cpu/common.c | 15 +++++++++++++- arch/x86/kernel/vmlinux.lds.S | 9 ++++++++ 5 files changed, 74 insertions(+), 1 deletion(-)
arch/x86/entry/entry_64.S +48 -0
@@ -140,6 +140,54 @@ END(native_usergs_sysret64) * with them due to bugs in both AMD and Intel CPUs. */ + .pushsection .entry_trampoline, "ax" + +/* + * The code in here gets remapped into cpu_entry_area's trampoline. This means + * that the assembler and linker have the wrong idea as to where this code + * lives (and, in fact, it's mapped more than once, so it's not even at a + * fixed address). So we can't reference any symbols outside the entry + * trampoline and expect it to work. + * + * Instead, we carefully abuse %rip-relative addressing. + * _entry_trampoline(%rip) refers to the start of the remapped) entry + * trampoline. We can thus find cpu_entry_area with this macro: + */ + +#define CPU_ENTRY_AREA \ + _entry_trampoline - CPU_ENTRY_AREA_entry_trampoline(%rip) + +/* The top word of the SYSENTER stack is hot and is usable as scratch space. */ +#define RSP_SCRATCH CPU_ENTRY_AREA_tss + CPU_TSS_SYSENTER_stack + \ + SIZEOF_SYSENTER_stack - 8 + CPU_ENTRY_AREA + +ENTRY(entry_SYSCALL_64_trampoline) + UNWIND_HINT_EMPTY + swapgs + + /* Stash the user RSP. */ + movq %rsp, RSP_SCRATCH + + /* Load the top of the task stack into RSP */ + movq CPU_ENTRY_AREA_tss + TSS_sp1 + CPU_ENTRY_AREA, %rsp + + /* Start building the simulated IRET frame. */ + pushq $__USER_DS /* pt_regs->ss */ + pushq RSP_SCRATCH /* pt_regs->sp */ + pushq %r11 /* pt_regs->flags */ + pushq $__USER_CS /* pt_regs->cs */ + pushq %rcx /* pt_regs->ip */ + + /* + * x86 lacks a near absolute jump, and we can't jump to the real + * entry text with a relative jump, so we fake it using retq. + */ + pushq $entry_SYSCALL_64_after_hwframe + retq +END(entry_SYSCALL_64_trampoline) + + .popsection + ENTRY(entry_SYSCALL_64) UNWIND_HINT_EMPTY /*
arch/x86/include/asm/fixmap.h +2 -0
@@ -61,6 +61,8 @@ struct cpu_entry_area {* of the TSS region. */ struct tss_struct tss; + +char entry_trampoline[PAGE_SIZE ]; };#define CPU_ENTRY_AREA_PAGES (sizeof(struct cpu_entry_area) / PAGE_SIZE)
arch/x86/kernel/asm-offsets.c +1 -0
@@ -101,4 +101,5 @@ void common(void) {/* Layout info for cpu_entry_area */ OFFSET (CPU_ENTRY_AREA_tss, cpu_entry_area, tss); +OFFSET (CPU_ENTRY_AREA_entry_trampoline, cpu_entry_area, entry_trampoline); }
arch/x86/kernel/cpu/common.c +14 -1
@@ -507,6 +507,8 @@ DEFINE_PER_CPU(struct cpu_entry_area *, cpu_entry_area);static inline void setup_cpu_entry_area (int cpu) {#ifdef CONFIG_X86_64 +extern char _entry_trampoline[]; +/* On 64-bit systems, we use a read-only fixmap GDT. */ pgprot_t gdt_prot =PAGE_KERNEL_RO ;#else
@@ -553,6 +555,11 @@ static inline void setup_cpu_entry_area(int cpu)#ifdef CONFIG_X86_32 this_cpu_write (cpu_entry_area,get_cpu_entry_area (cpu));#endif + +#ifdef CONFIG_X86_64 +__set_fixmap (get_cpu_entry_area_index (cpu, entry_trampoline), +__pa_symbol (_entry_trampoline),PAGE_KERNEL_RX ); +#endif }/* Load the original GDT from the per-cpu structure */
@@ -1417,10 +1424,16 @@ static DEFINE_PER_CPU_PAGE_ALIGNED(char, exception_stacks/* May not be marked __init: used by software suspend */ void syscall_init (void ) { +extern char _entry_trampoline[]; +extern char entry_SYSCALL_64_trampoline[]; +int cpu =smp_processor_id (); +unsigned long SYSCALL64_entry_trampoline = + (unsigned long )get_cpu_entry_area (cpu)->entry_trampoline + + (entry_SYSCALL_64_trampoline - _entry_trampoline);wrmsr (MSR_STAR ,0 , (__USER32_CS <<16 ) | __KERNEL_CS); -wrmsrl (MSR_LSTAR , (unsigned long )entry_SYSCALL_64); +wrmsrl (MSR_LSTAR , SYSCALL64_entry_trampoline);#ifdef CONFIG_IA32_EMULATION wrmsrl (MSR_CSTAR , (unsigned long )entry_SYSCALL_compat);
arch/x86/kernel/vmlinux.lds.S +9 -0
@@ -107,6 +107,15 @@ SECTIONS SOFTIRQENTRY_TEXT *(.fixup) *(.gnu.warning) + +#ifdef CONFIG_X86_64 + . = ALIGN(PAGE_SIZE); + _entry_trampoline = .; + *(.entry_trampoline) + . = ALIGN(PAGE_SIZE); + ASSERT(. - _entry_trampoline == PAGE_SIZE, "entry trampoline is too big"); +#endif + /* End of text section */ _etext = .; } :text = 0x9090
Signature · 1 line
2.14.1
PATCH 19/21 x86/entry/64: Move the IST stacks into 'struct cpu_entry_area'
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
The IST stacks are needed when an IST exception occurs and are accessed before any kernel code at all runs. Move them into struct cpu_entry_area.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Thomas Gleixner <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/0ffddccdc0ce1953f950a553142662cf68258fb7.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/include/asm/fixmap.h | 10 ++++++++++ arch/x86/kernel/cpu/common.c | 40 +++++++++++++++++++++++++--------------- 2 files changed, 35 insertions(+), 15 deletions(-)
arch/x86/include/asm/fixmap.h +10 -0
@@ -63,6 +63,16 @@ struct cpu_entry_area {struct tss_struct tss;char entry_trampoline[PAGE_SIZE ]; + +#ifdef CONFIG_X86_64 +/* +* Exception stacks used for IST entries. +* +* In the future, this should have a separate slot for each stack +* with guard pages between them. +*/ +char exception_stacks[(N_EXCEPTION_STACKS -1 ) *EXCEPTION_STKSZ +DEBUG_STKSZ ]; +#endif };#define CPU_ENTRY_AREA_PAGES (sizeof(struct cpu_entry_area) / PAGE_SIZE)
arch/x86/kernel/cpu/common.c +25 -15
@@ -503,6 +503,22 @@ static void set_percpu_fixmap_pages(int fixmap_index, void *ptr, int pages, pgpr DEFINE_PER_CPU(struct cpu_entry_area *, cpu_entry_area);#endif +#ifdef CONFIG_X86_64 +/* +* Special IST stacks which the CPU switches to when it calls +* an IST-marked descriptor entry. Up to 7 stacks (hardware +* limit), all of them are 4K, except the debug stack which +* is 8K. +*/ +static const unsigned int exception_stack_sizes[N_EXCEPTION_STACKS ] = { + [0 ...N_EXCEPTION_STACKS -1 ] =EXCEPTION_STKSZ , + [DEBUG_STACK -1 ] =DEBUG_STKSZ +}; + +static DEFINE_PER_CPU_PAGE_ALIGNED(char, exception_stacks + [(N_EXCEPTION_STACKS - 1) * EXCEPTION_STKSZ + DEBUG_STKSZ]); +#endif +/* Setup the fixmap mappings only once per-processor */ static inline void setup_cpu_entry_area (int cpu) {
@@ -557,6 +573,14 @@ static inline void setup_cpu_entry_area(int cpu)#endif #ifdef CONFIG_X86_64 +BUILD_BUG_ON (sizeof (exception_stacks) %PAGE_SIZE !=0 ); +BUILD_BUG_ON (sizeof (exception_stacks) != +sizeof (((struct cpu_entry_area *)0 )->exception_stacks)); +set_percpu_fixmap_pages (get_cpu_entry_area_index (cpu, exception_stacks), + &per_cpu (exception_stacks, cpu), +sizeof (exception_stacks) /PAGE_SIZE , +PAGE_KERNEL ); +__set_fixmap (get_cpu_entry_area_index (cpu, entry_trampoline),__pa_symbol (_entry_trampoline),PAGE_KERNEL_RX );#endif
@@ -1407,20 +1431,6 @@ DEFINE_PER_CPU(unsigned int, irq_count) __visible = -1;DEFINE_PER_CPU (int, __preempt_count) =INIT_PREEMPT_COUNT ;EXPORT_PER_CPU_SYMBOL (__preempt_count); -/* -* Special IST stacks which the CPU switches to when it calls -* an IST-marked descriptor entry. Up to 7 stacks (hardware -* limit), all of them are 4K, except the debug stack which -* is 8K. -*/ -static const unsigned int exception_stack_sizes[N_EXCEPTION_STACKS ] = { - [0 ...N_EXCEPTION_STACKS -1 ] =EXCEPTION_STKSZ , - [DEBUG_STACK -1 ] =DEBUG_STKSZ -}; - -static DEFINE_PER_CPU_PAGE_ALIGNED(char, exception_stacks - [(N_EXCEPTION_STACKS - 1) * EXCEPTION_STKSZ + DEBUG_STKSZ]); -/* May not be marked __init: used by software suspend */ void syscall_init (void ) {
@@ -1629,7 +1639,7 @@ void cpu_init(void)* set up and load the per-CPU TSS */ if (!oist->ist[0 ]) { -char *estacks =per_cpu (exception_stacks, cpu); +char *estacks =get_cpu_entry_area (cpu)->exception_stacks;for (v =0 ; v <N_EXCEPTION_STACKS ; v++) { estacks += exception_stack_sizes[v];
Signature · 1 line
2.14.1
PATCH 20/21 x86/entry/64: Remove the SYSENTER stack canary
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
Now that the SYSENTER stack has a guard page, there's no need for a canary to detect overflow after the fact.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Thomas Gleixner <[email protected]>
Reviewed-by Borislav Petkov <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/be3179c0a38c392fa44ebeb7dd89391ff5c010c3.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/include/asm/processor.h | 1 - arch/x86/kernel/dumpstack.c | 3 +-- arch/x86/kernel/process.c | 1 - arch/x86/kernel/traps.c | 7 ------- 4 files changed, 1 insertion(+), 11 deletions(-)
arch/x86/include/asm/processor.h +0 -1
@@ -342,7 +342,6 @@ struct tss_struct {* Space for the temporary SYSENTER stack, used for SYSENTER * and the entry trampoline as well. */ -unsigned long SYSENTER_stack_canary;unsigned long SYSENTER_stack[64 ];/*
arch/x86/kernel/dumpstack.c +1 -2
@@ -48,8 +48,7 @@ bool in_sysenter_stack(unsigned long *stack, struct stack_info *info)int cpu =smp_processor_id ();struct tss_struct *tss = &get_cpu_entry_area (cpu)->tss; -/* Treat the canary as part of the stack for unwinding purposes. */ -void *begin = &tss->SYSENTER_stack_canary; +void *begin = &tss->SYSENTER_stack;void *end = (void *)&tss->SYSENTER_stack +sizeof (tss->SYSENTER_stack);if ((void *)stack < begin || (void *)stack >= end)
arch/x86/kernel/process.c +0 -1
@@ -81,7 +81,6 @@ __visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss) = {*/ .io_bitmap = { [0 ... IO_BITMAP_LONGS] = ~0 },#endif - .SYSENTER_stack_canary =STACK_END_MAGIC , };EXPORT_PER_CPU_SYMBOL (cpu_tss);
arch/x86/kernel/traps.c +0 -7
@@ -819,13 +819,6 @@ dotraplinkage void do_debug(struct pt_regs *regs, long error_code)debug_stack_usage_dec (); exit: -/* -* This is the most likely code path that involves non-trivial use -* of the SYSENTER stack. Check that we haven't overrun it. -*/ -WARN (this_cpu_read (cpu_tss.SYSENTER_stack_canary) !=STACK_END_MAGIC , -"Overran or corrupted SYSENTER stack\n" ); -ist_exit (regs); }NOKPROBE_SYMBOL (do_debug);
Signature · 1 line
2.14.1
PATCH 21/21 x86/entry: Clean up the SYSENTER_stack code
In reply to Ingo MolnarFrom: Andy Lutomirski <[email protected]>
The existing code was a mess, mainly because C arrays are nasty. Turn SYSENTER_stack into a struct, add a helper to find it, and do all the obvious cleanups this enables.
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Reviewed-by Thomas Gleixner <[email protected]>
Cc Brian Gerst <[email protected]>
Cc Dave Hansen <[email protected]>
Cc Denys Vlasenko <[email protected]>
Cc H. Peter Anvin <[email protected]>
Cc Josh Poimboeuf <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Rik van Riel <[email protected]>
Link https://lkml.kernel.org/r/38ff640712c9b591b32de24a080daf13afaba234.1511497875.git.luto@kernel.org open here
Signed-off-by Ingo Molnar <[email protected]>
--- arch/x86/entry/entry_32.S | 4 ++-- arch/x86/entry/entry_64.S | 2 +- arch/x86/include/asm/fixmap.h | 5 +++++ arch/x86/include/asm/processor.h | 6 +++++- arch/x86/kernel/asm-offsets.c | 6 ++---- arch/x86/kernel/cpu/common.c | 14 +++----------- arch/x86/kernel/dumpstack.c | 7 +++---- 7 files changed, 21 insertions(+), 23 deletions(-)
arch/x86/entry/entry_32.S +2 -2
@@ -942,7 +942,7 @@ ENTRY(debug) /* Are we currently on the SYSENTER stack? */ movl PER_CPU_VAR(cpu_entry_area), %ecx - addl $CPU_ENTRY_AREA_tss + CPU_TSS_SYSENTER_stack + SIZEOF_SYSENTER_stack, %ecx + addl $CPU_ENTRY_AREA_tss + TSS_STRUCT_SYSENTER_stack + SIZEOF_SYSENTER_stack, %ecx subl %eax, %ecx /* ecx = (end of SYSENTER_stack) - esp */ cmpl $SIZEOF_SYSENTER_stack, %ecx jb .Ldebug_from_sysenter_stack
@@ -986,7 +986,7 @@ ENTRY(nmi) /* Are we currently on the SYSENTER stack? */ movl PER_CPU_VAR(cpu_entry_area), %ecx - addl $CPU_ENTRY_AREA_tss + CPU_TSS_SYSENTER_stack + SIZEOF_SYSENTER_stack, %ecx + addl $CPU_ENTRY_AREA_tss + TSS_STRUCT_SYSENTER_stack + SIZEOF_SYSENTER_stack, %ecx subl %eax, %ecx /* ecx = (end of SYSENTER_stack) - esp */ cmpl $SIZEOF_SYSENTER_stack, %ecx jb .Lnmi_from_sysenter_stack
arch/x86/entry/entry_64.S +1 -1
@@ -158,7 +158,7 @@ END(native_usergs_sysret64) _entry_trampoline - CPU_ENTRY_AREA_entry_trampoline(%rip) /* The top word of the SYSENTER stack is hot and is usable as scratch space. */ -#define RSP_SCRATCH CPU_ENTRY_AREA_tss + CPU_TSS_SYSENTER_stack + \ +#define RSP_SCRATCH CPU_ENTRY_AREA_tss + TSS_STRUCT_SYSENTER_stack + \ SIZEOF_SYSENTER_stack - 8 + CPU_ENTRY_AREA ENTRY(entry_SYSCALL_64_trampoline)
arch/x86/include/asm/fixmap.h +5 -0
@@ -237,5 +237,10 @@ static inline struct cpu_entry_area *get_cpu_entry_area(int cpu)return (struct cpu_entry_area *)__fix_to_virt (__get_cpu_entry_area_page_index (cpu,0 )); } +static inline struct SYSENTER_stack *cpu_SYSENTER_stack (int cpu) +{ +return &get_cpu_entry_area ((cpu))->tss.SYSENTER_stack; +} +#endif /* !__ASSEMBLY__ */ #endif /* _ASM_X86_FIXMAP_H */
arch/x86/include/asm/processor.h +5 -1
@@ -337,12 +337,16 @@ struct x86_hw_tss {#define IO_BITMAP_OFFSET (offsetof(struct tss_struct, io_bitmap) - offsetof(struct tss_struct, x86_tss))#define INVALID_IO_BITMAP_OFFSET 0x8000 +struct SYSENTER_stack { +unsigned long words[64 ]; +}; +struct tss_struct {/* * Space for the temporary SYSENTER stack, used for SYSENTER * and the entry trampoline as well. */ -unsigned long SYSENTER_stack[64 ]; +struct SYSENTER_stack SYSENTER_stack;/* * The fixed hardware portion. This must not cross a page boundary
arch/x86/kernel/asm-offsets.c +2 -4
@@ -94,10 +94,8 @@ void common(void) {BLANK ();DEFINE (PTREGS_SIZE ,sizeof (struct pt_regs )); -/* Offset from cpu_tss to SYSENTER_stack */ -OFFSET (CPU_TSS_SYSENTER_stack, tss_struct, SYSENTER_stack); -/* Size of SYSENTER_stack */ -DEFINE (SIZEOF_SYSENTER_stack,sizeof (((struct tss_struct *)0 )->SYSENTER_stack)); +OFFSET (TSS_STRUCT_SYSENTER_stack, tss_struct, SYSENTER_stack); +DEFINE (SIZEOF_SYSENTER_stack,sizeof (struct SYSENTER_stack ));/* Layout info for cpu_entry_area */ OFFSET (CPU_ENTRY_AREA_tss, cpu_entry_area, tss);
arch/x86/kernel/cpu/common.c +3 -11
@@ -1332,12 +1332,7 @@ void enable_sep_cpu(void) tss->x86_tss.ss1 = __KERNEL_CS;wrmsr (MSR_IA32_SYSENTER_CS , tss->x86_tss.ss1,0 ); - -wrmsr (MSR_IA32_SYSENTER_ESP , - (unsigned long )&get_cpu_entry_area (cpu)->tss + - offsetofend(struct tss_struct, SYSENTER_stack), -0 ); - +wrmsr (MSR_IA32_SYSENTER_ESP , (unsigned long )(cpu_SYSENTER_stack (cpu) +1 ),0 );wrmsr (MSR_IA32_SYSENTER_EIP , (unsigned long )entry_SYSENTER_32,0 );put_cpu ();
@@ -1454,9 +1449,7 @@ void syscall_init(void)* AMD doesn't allow SYSENTER in long mode (either 32- or 64-bit). */ wrmsrl_safe (MSR_IA32_SYSENTER_CS , (u64 )__KERNEL_CS); -wrmsrl_safe (MSR_IA32_SYSENTER_ESP , - (unsigned long )&get_cpu_entry_area (cpu)->tss + - offsetofend(struct tss_struct, SYSENTER_stack)); +wrmsrl_safe (MSR_IA32_SYSENTER_ESP , (unsigned long )(cpu_SYSENTER_stack (cpu) +1 ));wrmsrl_safe (MSR_IA32_SYSENTER_EIP , (u64 )entry_SYSENTER_compat);#else wrmsrl (MSR_CSTAR , (unsigned long )ignore_sysret);
@@ -1673,8 +1666,7 @@ void cpu_init(void)*/ set_tss_desc (cpu, &get_cpu_entry_area (cpu)->tss.x86_tss);load_TR_desc (); -load_sp0 ((unsigned long )&get_cpu_entry_area (cpu)->tss + - offsetofend(struct tss_struct, SYSENTER_stack)); +load_sp0 ((unsigned long )(cpu_SYSENTER_stack (cpu) +1 ));load_mm_ldt (&init_mm);
arch/x86/kernel/dumpstack.c +3 -4
@@ -45,11 +45,10 @@ bool in_task_stack(unsigned long *stack, struct task_struct *task,bool in_sysenter_stack (unsigned long *stack,struct stack_info *info) { -int cpu =smp_processor_id (); -struct tss_struct *tss = &get_cpu_entry_area (cpu)->tss; +struct SYSENTER_stack *ss =cpu_SYSENTER_stack (smp_processor_id ()); -void *begin = &tss->SYSENTER_stack; -void *end = (void *)&tss->SYSENTER_stack +sizeof (tss->SYSENTER_stack); +void *begin = ss; +void *end = ss +1 ;if ((void *)stack < begin || (void *)stack >= end)return false;
Signature · 1 line
2.14.1
Borislav Petkov
maintainerRe: [PATCH 21/21] x86/entry: Clean up the SYSENTER_stack code
arch/x86/include/asm/fixmap.h static inline struct cpu_entry_area *get_cpu_entry_area(int cpu) · line 242
Quoted 11 lines · Ingo Molnar
arch/x86/include/asm/fixmap.h +3
-0@@ -237,5 +237,10 @@ static inline struct cpu_entry_area *get_cpu_entry_area(int cpu)return (struct cpu_entry_area *)__fix_to_virt (__get_cpu_entry_area_page_index (cpu,0 )); } +static inline struct SYSENTER_stack *cpu_SYSENTER_stack (int cpu) +{ +return &get_cpu_entry_area ((cpu))->tss.SYSENTER_stack;
^^^^^^^^^
Those inner brackets are not needed, right?
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.