[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180504162557.iodmglq3duomz6c2@treble>
Date: Fri, 4 May 2018 11:25:57 -0500
From: Josh Poimboeuf <jpoimboe@...hat.com>
To: Vince Weaver <vincent.weaver@...ne.edu>
Cc: Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Andy Lutomirski <luto@...nel.org>
Subject: Re: perf: fuzzer causes stack going in wrong direction warnings
On Fri, May 04, 2018 at 10:35:53AM -0400, Vince Weaver wrote:
> On Wed, 2 May 2018, Josh Poimboeuf wrote:
>
> > After looking closer, I realized that at least some of these warnings
> > are due to bad unwind hints in the entry code. Can you try this patch
> > instead of the last one?
>
> with just this new patch applied I still get warnings such as this:
>
> [ 469.436218] WARNING: can't dereference registers at 00000000886d9235 for ip apic_timer_interrupt+0xa/0x20
> [ 790.499655] WARNING: stack recursion on stack type 2
> [ 790.907092] WARNING: stack going in the wrong direction? ip=native_sched_clock+0x9/0x90
> [ 3632.876656] WARNING: can't dereference iret registers at 000000001754e5aa for ip nmi_restore+0x16/0x2b
> [ 3650.161250] WARNING: missing regs for base reg R10 at ip native_sched_clock+0xd/0x90
The 'nmi_restore' warning points to a bug in my patch, but the others
are head scratchers. Here's a patch which combines the first two
patches, plus improves the existing warnings a bit. Can you try it?
Also, any tips for reproducing this locally? I cloned the perf fuzzer
github. Is it as simple as just "make" and "./run_tests.sh"?
diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index be63330c5511..73f5d4c10304 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -165,6 +165,7 @@ For 32-bit we have the following conventions - kernel is built with
.endif
popq %rdx
popq %rsi
+ UNWIND_HINT_IRET_REGS offset=16
.if \pop_rdi
popq %rdi
.endif
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 805f52703ee3..2908ed2ef698 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -306,7 +306,6 @@ GLOBAL(entry_SYSCALL_64_after_hwframe)
*/
syscall_return_via_sysret:
/* rcx and r11 are already restored (see code above) */
- UNWIND_HINT_EMPTY
POP_REGS pop_rdi=0 skip_r11rcx=1
/*
@@ -315,6 +314,7 @@ syscall_return_via_sysret:
*/
movq %rsp, %rdi
movq PER_CPU_VAR(cpu_tss_rw + TSS_sp0), %rsp
+ UNWIND_HINT_EMPTY
pushq RSP-RDI(%rdi) /* RSP */
pushq (%rdi) /* RDI */
@@ -666,6 +666,7 @@ GLOBAL(swapgs_restore_regs_and_return_to_usermode)
*/
movq %rsp, %rdi
movq PER_CPU_VAR(cpu_tss_rw + TSS_sp0), %rsp
+ UNWIND_HINT_EMPTY
/* Copy the IRET frame to the trampoline stack. */
pushq 6*8(%rdi) /* SS */
diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index 563e28d14f2c..d8dbf7c3c2f1 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -137,7 +137,9 @@ int get_stack_info(unsigned long *stack, struct task_struct *task,
*/
if (visit_mask) {
if (*visit_mask & (1UL << info->type)) {
- printk_deferred_once(KERN_WARNING "WARNING: stack recursion on stack type %d\n", info->type);
+ if (task == current)
+ printk_deferred(KERN_WARNING "WARNING: stack recursion on stack type %d\n",
+ info->type);
goto unknown;
}
*visit_mask |= 1UL << info->type;
diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index feb28fee6cea..1324ffdb861d 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -7,7 +7,14 @@
#include <asm/orc_lookup.h>
#define orc_warn(fmt, ...) \
- printk_deferred_once(KERN_WARNING pr_fmt("WARNING: " fmt), ##__VA_ARGS__)
+ printk_deferred(KERN_WARNING pr_fmt("WARNING: " fmt), ##__VA_ARGS__)
+
+#define orc_warn_current(fmt, ...) \
+({ \
+ if (state->task == current) \
+ printk_deferred(KERN_WARNING "WARNING: " fmt, \
+ ##__VA_ARGS__); \
+})
extern int __start_orc_unwind_ip[];
extern int __stop_orc_unwind_ip[];
@@ -400,8 +407,8 @@ bool unwind_next_frame(struct unwind_state *state)
case ORC_REG_R10:
if (!state->regs || !state->full_regs) {
- orc_warn("missing regs for base reg R10 at ip %pB\n",
- (void *)state->ip);
+ orc_warn_current("missing regs for base reg R10 at ip %pB\n",
+ (void *)state->ip);
goto done;
}
sp = state->regs->r10;
@@ -409,8 +416,8 @@ bool unwind_next_frame(struct unwind_state *state)
case ORC_REG_R13:
if (!state->regs || !state->full_regs) {
- orc_warn("missing regs for base reg R13 at ip %pB\n",
- (void *)state->ip);
+ orc_warn_current("missing regs for base reg R13 at ip %pB\n",
+ (void *)state->ip);
goto done;
}
sp = state->regs->r13;
@@ -418,8 +425,8 @@ bool unwind_next_frame(struct unwind_state *state)
case ORC_REG_DI:
if (!state->regs || !state->full_regs) {
- orc_warn("missing regs for base reg DI at ip %pB\n",
- (void *)state->ip);
+ orc_warn_current("missing regs for base reg DI at ip %pB\n",
+ (void *)state->ip);
goto done;
}
sp = state->regs->di;
@@ -427,8 +434,8 @@ bool unwind_next_frame(struct unwind_state *state)
case ORC_REG_DX:
if (!state->regs || !state->full_regs) {
- orc_warn("missing regs for base reg DX at ip %pB\n",
- (void *)state->ip);
+ orc_warn_current("missing regs for base reg DX at ip %pB\n",
+ (void *)state->ip);
goto done;
}
sp = state->regs->dx;
@@ -463,8 +470,8 @@ bool unwind_next_frame(struct unwind_state *state)
case ORC_TYPE_REGS:
if (!deref_stack_regs(state, sp, &state->ip, &state->sp)) {
- orc_warn("can't dereference registers at %p for ip %pB\n",
- (void *)sp, (void *)orig_ip);
+ orc_warn_current("can't dereference registers at %p for ip %pB\n",
+ (void *)sp, (void *)orig_ip);
goto done;
}
@@ -475,8 +482,8 @@ bool unwind_next_frame(struct unwind_state *state)
case ORC_TYPE_REGS_IRET:
if (!deref_stack_iret_regs(state, sp, &state->ip, &state->sp)) {
- orc_warn("can't dereference iret registers at %p for ip %pB\n",
- (void *)sp, (void *)orig_ip);
+ orc_warn_current("can't dereference iret registers at %p for ip %pB\n",
+ (void *)sp, (void *)orig_ip);
goto done;
}
@@ -518,8 +525,8 @@ bool unwind_next_frame(struct unwind_state *state)
if (state->stack_info.type == prev_type &&
on_stack(&state->stack_info, (void *)state->sp, sizeof(long)) &&
state->sp <= prev_sp) {
- orc_warn("stack going in the wrong direction? ip=%pB\n",
- (void *)orig_ip);
+ orc_warn_current("stack going in the wrong direction? ip=%pB\n",
+ (void *)orig_ip);
goto done;
}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 92b6a2c21631..d66f14bb6c50 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1239,7 +1239,7 @@ static int update_insn_state_regs(struct instruction *insn, struct insn_state *s
struct cfi_reg *cfa = &state->cfa;
struct stack_op *op = &insn->stack_op;
- if (cfa->base != CFI_SP)
+ if (cfa->base != CFI_SP && cfa->base != CFI_SP_INDIRECT)
return 0;
/* push */
Powered by blists - more mailing lists