Skip to content

Commit 44b979f

Browse files
author
Peter Zijlstra
committed
x86/mm/64: Improve stack overflow warnings
Current code has an explicit check for hitting the task stack guard; but overflowing any of the other stacks will get you a non-descript general #DF warning. Improve matters by using get_stack_info_noinstr() to detetrmine if and which stack guard page got hit, enabling a better stack warning. In specific, Michael Wang reported what turned out to be an NMI exception stack overflow, which is now clearly reported as such: [] BUG: NMI stack guard page was hit at 0000000085fd977b (stack is 000000003a55b09e..00000000d8cce1a5) Reported-by: Michael Wang <[email protected]> Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Tested-by: Michael Wang <[email protected]> Link: https://lkml.kernel.org/r/YUTE/[email protected]
1 parent b968e84 commit 44b979f

File tree

6 files changed

+67
-37
lines changed

6 files changed

+67
-37
lines changed

arch/x86/include/asm/irq_stack.h

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,11 @@
7777
* Function calls can clobber anything except the callee-saved
7878
* registers. Tell the compiler.
7979
*/
80-
#define call_on_irqstack(func, asm_call, argconstr...) \
80+
#define call_on_stack(stack, func, asm_call, argconstr...) \
8181
{ \
8282
register void *tos asm("r11"); \
8383
\
84-
tos = ((void *)__this_cpu_read(hardirq_stack_ptr)); \
84+
tos = ((void *)(stack)); \
8585
\
8686
asm_inline volatile( \
8787
"movq %%rsp, (%[tos]) \n" \
@@ -98,6 +98,25 @@
9898
); \
9999
}
100100

101+
#define ASM_CALL_ARG0 \
102+
"call %P[__func] \n"
103+
104+
#define ASM_CALL_ARG1 \
105+
"movq %[arg1], %%rdi \n" \
106+
ASM_CALL_ARG0
107+
108+
#define ASM_CALL_ARG2 \
109+
"movq %[arg2], %%rsi \n" \
110+
ASM_CALL_ARG1
111+
112+
#define ASM_CALL_ARG3 \
113+
"movq %[arg3], %%rdx \n" \
114+
ASM_CALL_ARG2
115+
116+
#define call_on_irqstack(func, asm_call, argconstr...) \
117+
call_on_stack(__this_cpu_read(hardirq_stack_ptr), \
118+
func, asm_call, argconstr)
119+
101120
/* Macros to assert type correctness for run_*_on_irqstack macros */
102121
#define assert_function_type(func, proto) \
103122
static_assert(__builtin_types_compatible_p(typeof(&func), proto))
@@ -147,8 +166,7 @@
147166
*/
148167
#define ASM_CALL_SYSVEC \
149168
"call irq_enter_rcu \n" \
150-
"movq %[arg1], %%rdi \n" \
151-
"call %P[__func] \n" \
169+
ASM_CALL_ARG1 \
152170
"call irq_exit_rcu \n"
153171

154172
#define SYSVEC_CONSTRAINTS , [arg1] "r" (regs)
@@ -168,12 +186,10 @@
168186
*/
169187
#define ASM_CALL_IRQ \
170188
"call irq_enter_rcu \n" \
171-
"movq %[arg1], %%rdi \n" \
172-
"movl %[arg2], %%esi \n" \
173-
"call %P[__func] \n" \
189+
ASM_CALL_ARG2 \
174190
"call irq_exit_rcu \n"
175191

176-
#define IRQ_CONSTRAINTS , [arg1] "r" (regs), [arg2] "r" (vector)
192+
#define IRQ_CONSTRAINTS , [arg1] "r" (regs), [arg2] "r" ((unsigned long)vector)
177193

178194
#define run_irq_on_irqstack_cond(func, regs, vector) \
179195
{ \
@@ -185,9 +201,6 @@
185201
IRQ_CONSTRAINTS, regs, vector); \
186202
}
187203

188-
#define ASM_CALL_SOFTIRQ \
189-
"call %P[__func] \n"
190-
191204
/*
192205
* Macro to invoke __do_softirq on the irq stack. This is only called from
193206
* task context when bottom halves are about to be reenabled and soft
@@ -197,7 +210,7 @@
197210
#define do_softirq_own_stack() \
198211
{ \
199212
__this_cpu_write(hardirq_stack_inuse, true); \
200-
call_on_irqstack(__do_softirq, ASM_CALL_SOFTIRQ); \
213+
call_on_irqstack(__do_softirq, ASM_CALL_ARG0); \
201214
__this_cpu_write(hardirq_stack_inuse, false); \
202215
}
203216

arch/x86/include/asm/stacktrace.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,16 @@ int get_stack_info(unsigned long *stack, struct task_struct *task,
3838
bool get_stack_info_noinstr(unsigned long *stack, struct task_struct *task,
3939
struct stack_info *info);
4040

41+
static __always_inline
42+
bool get_stack_guard_info(unsigned long *stack, struct stack_info *info)
43+
{
44+
/* make sure it's not in the stack proper */
45+
if (get_stack_info_noinstr(stack, current, info))
46+
return false;
47+
/* but if it is in the page below it, we hit a guard */
48+
return get_stack_info_noinstr((void *)stack + PAGE_SIZE, current, info);
49+
}
50+
4151
const char *stack_type_name(enum stack_type type);
4252

4353
static inline bool on_stack(struct stack_info *info, void *addr, size_t len)

arch/x86/include/asm/traps.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@ void math_emulate(struct math_emu_info *);
4040
bool fault_in_kernel_space(unsigned long address);
4141

4242
#ifdef CONFIG_VMAP_STACK
43-
void __noreturn handle_stack_overflow(const char *message,
44-
struct pt_regs *regs,
45-
unsigned long fault_address);
43+
void __noreturn handle_stack_overflow(struct pt_regs *regs,
44+
unsigned long fault_address,
45+
struct stack_info *info);
4646
#endif
4747

4848
#endif /* _ASM_X86_TRAPS_H */

arch/x86/kernel/dumpstack_64.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,15 @@ const char *stack_type_name(enum stack_type type)
3232
{
3333
BUILD_BUG_ON(N_EXCEPTION_STACKS != 6);
3434

35+
if (type == STACK_TYPE_TASK)
36+
return "TASK";
37+
3538
if (type == STACK_TYPE_IRQ)
3639
return "IRQ";
3740

41+
if (type == STACK_TYPE_SOFTIRQ)
42+
return "SOFTIRQ";
43+
3844
if (type == STACK_TYPE_ENTRY) {
3945
/*
4046
* On 64-bit, we have a generic entry stack that we

arch/x86/kernel/traps.c

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -313,17 +313,19 @@ DEFINE_IDTENTRY_ERRORCODE(exc_alignment_check)
313313
}
314314

315315
#ifdef CONFIG_VMAP_STACK
316-
__visible void __noreturn handle_stack_overflow(const char *message,
317-
struct pt_regs *regs,
318-
unsigned long fault_address)
316+
__visible void __noreturn handle_stack_overflow(struct pt_regs *regs,
317+
unsigned long fault_address,
318+
struct stack_info *info)
319319
{
320-
printk(KERN_EMERG "BUG: stack guard page was hit at %p (stack is %p..%p)\n",
321-
(void *)fault_address, current->stack,
322-
(char *)current->stack + THREAD_SIZE - 1);
323-
die(message, regs, 0);
320+
const char *name = stack_type_name(info->type);
321+
322+
printk(KERN_EMERG "BUG: %s stack guard page was hit at %p (stack is %p..%p)\n",
323+
name, (void *)fault_address, info->begin, info->end);
324+
325+
die("stack guard page", regs, 0);
324326

325327
/* Be absolutely certain we don't return. */
326-
panic("%s", message);
328+
panic("%s stack guard hit", name);
327329
}
328330
#endif
329331

@@ -353,6 +355,7 @@ DEFINE_IDTENTRY_DF(exc_double_fault)
353355

354356
#ifdef CONFIG_VMAP_STACK
355357
unsigned long address = read_cr2();
358+
struct stack_info info;
356359
#endif
357360

358361
#ifdef CONFIG_X86_ESPFIX64
@@ -455,10 +458,8 @@ DEFINE_IDTENTRY_DF(exc_double_fault)
455458
* stack even if the actual trigger for the double fault was
456459
* something else.
457460
*/
458-
if ((unsigned long)task_stack_page(tsk) - 1 - address < PAGE_SIZE) {
459-
handle_stack_overflow("kernel stack overflow (double-fault)",
460-
regs, address);
461-
}
461+
if (get_stack_guard_info((void *)address, &info))
462+
handle_stack_overflow(regs, address, &info);
462463
#endif
463464

464465
pr_emerg("PANIC: double fault, error_code: 0x%lx\n", error_code);

arch/x86/mm/fault.c

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
#include <asm/pgtable_areas.h> /* VMALLOC_START, ... */
3333
#include <asm/kvm_para.h> /* kvm_handle_async_pf */
3434
#include <asm/vdso.h> /* fixup_vdso_exception() */
35+
#include <asm/irq_stack.h>
3536

3637
#define CREATE_TRACE_POINTS
3738
#include <asm/trace/exceptions.h>
@@ -631,6 +632,9 @@ static noinline void
631632
page_fault_oops(struct pt_regs *regs, unsigned long error_code,
632633
unsigned long address)
633634
{
635+
#ifdef CONFIG_VMAP_STACK
636+
struct stack_info info;
637+
#endif
634638
unsigned long flags;
635639
int sig;
636640

@@ -649,9 +653,7 @@ page_fault_oops(struct pt_regs *regs, unsigned long error_code,
649653
* that we're in vmalloc space to avoid this.
650654
*/
651655
if (is_vmalloc_addr((void *)address) &&
652-
(((unsigned long)current->stack - 1 - address < PAGE_SIZE) ||
653-
address - ((unsigned long)current->stack + THREAD_SIZE) < PAGE_SIZE)) {
654-
unsigned long stack = __this_cpu_ist_top_va(DF) - sizeof(void *);
656+
get_stack_guard_info((void *)address, &info)) {
655657
/*
656658
* We're likely to be running with very little stack space
657659
* left. It's plausible that we'd hit this condition but
@@ -662,13 +664,11 @@ page_fault_oops(struct pt_regs *regs, unsigned long error_code,
662664
* and then double-fault, though, because we're likely to
663665
* break the console driver and lose most of the stack dump.
664666
*/
665-
asm volatile ("movq %[stack], %%rsp\n\t"
666-
"call handle_stack_overflow\n\t"
667-
"1: jmp 1b"
668-
: ASM_CALL_CONSTRAINT
669-
: "D" ("kernel stack overflow (page fault)"),
670-
"S" (regs), "d" (address),
671-
[stack] "rm" (stack));
667+
call_on_stack(__this_cpu_ist_top_va(DF) - sizeof(void*),
668+
handle_stack_overflow,
669+
ASM_CALL_ARG3,
670+
, [arg1] "r" (regs), [arg2] "r" (address), [arg3] "r" (&info));
671+
672672
unreachable();
673673
}
674674
#endif

0 commit comments

Comments
 (0)