Re: x86: warning in unwind_get_return_address
From: Josh Poimboeuf <hidden>
Date: 2017-01-05 20:38:24
Also in:
lkml
On Thu, Jan 05, 2017 at 09:23:14PM +0100, Dmitry Vyukov wrote:
quoted hunk ↗ jump to hunk
On Thu, Jan 5, 2017 at 6:03 PM, Josh Poimboeuf [off-list ref] wrote:quoted
On Thu, Jan 05, 2017 at 09:17:00AM -0600, Josh Poimboeuf wrote:quoted
On Thu, Jan 05, 2017 at 03:59:52PM +0100, Dmitry Vyukov wrote:quoted
On Thu, Jan 5, 2017 at 3:49 PM, Josh Poimboeuf [off-list ref] wrote:quoted
On Tue, Dec 27, 2016 at 05:38:59PM +0100, Dmitry Vyukov wrote:quoted
On Thu, Dec 22, 2016 at 6:17 AM, Josh Poimboeuf [off-list ref] wrote:quoted
On Wed, Dec 21, 2016 at 01:46:36PM +0100, Andrey Konovalov wrote:quoted
On Wed, Dec 21, 2016 at 12:36 AM, Josh Poimboeuf [off-list ref] wrote:quoted
Thanks. Looking at the stack trace, my guess is that an interrupt hit while running in generated BPF code, and the unwinder got confused because regs->ip points to the generated code. I may need to disable that warning until we figure out a better solution. Can you share your .config file?Sure, attached.Ok, I was able to recreate with your config. The culprit was generated code, as I suspected, though it wasn't BPF, it was a kprobe (created by dccpprobe_init()). I'll make a patch to disable the warning.Hi, I am also seeing the following warnings: [ 281.889259] WARNING: kernel stack regs at ffff8801c29a7ea8 in syz-executor8:1302 has bad 'bp' value ffff8801c29a7f28 [ 833.994878] WARNING: kernel stack regs at ffff8801c4e77ea8 in syz-executor1:13094 has bad 'bp' value ffff8801c4e77f28 Can it also be caused by bpf/kprobe?This is a different warning. I suspect it's due to unwinding the stack of another CPU while it's running, which is still possible in a few places. I'm going to have to disable all these warnings for now.I also have the following diff locally. These loads trigger episodic KASAN warnings about stack-of-bounds reads on rcu stall warnings when it does backtrace of all cpus. If it looks correct to you, can you please also incorporate it into your patch?Ok, will do. BTW, I think there's an issue with your mail client. Your last two replies to me didn't have me on To/Cc.Would you mind testing if the following patch fixes it? It's based on the latest linus/master.diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c index 4443e49..05adf9a 100644 --- a/arch/x86/kernel/unwind_frame.c +++ b/arch/x86/kernel/unwind_frame.c@@ -6,6 +6,21 @@ #define FRAME_HEADER_SIZE (sizeof(long) * 2) +/* + * This disables KASAN checking when reading a value from another task's stack, + * since the other task could be running on another CPU and could have poisoned + * the stack in the meantime. + */ +#define UNWIND_READ_ONCE(state, x) \ +({ \ + unsigned long val; \ + if (state->task == current) \ + val = READ_ONCE(x); \ + else \ + val = READ_ONCE_NOCHECK(x); \ + val; \ +}) + static void unwind_dump(struct unwind_state *state, unsigned long *sp) { static bool dumped_before = false;@@ -48,7 +63,8 @@ unsigned long unwind_get_return_address(struct unwind_state *state) if (state->regs && user_mode(state->regs)) return 0; - addr = ftrace_graph_ret_addr(state->task, &state->graph_idx, *addr_p, + addr = UNWIND_READ_ONCE(state, *addr_p); + addr = ftrace_graph_ret_addr(state->task, &state->graph_idx, addr, addr_p); return __kernel_text_address(addr) ? addr : 0;@@ -162,7 +178,7 @@ bool unwind_next_frame(struct unwind_state *state) if (state->regs) next_bp = (unsigned long *)state->regs->bp; else - next_bp = (unsigned long *)*state->bp; + next_bp = (unsigned long *)UNWIND_READ_ONCE(state, *state->bp); /* is the next frame pointer an encoded pointer to pt_regs? */ regs = decode_frame_pointer(next_bp);@@ -207,6 +223,16 @@ bool unwind_next_frame(struct unwind_state *state) return true; bad_address: + /* + * When dumping a task other than current, the task might actually be + * running on another CPU, in which case it could be modifying its + * stack while we're reading it. This is generally not a problem and + * can be ignored as long as the caller understands that unwinding + * another task will not always succeed. + */ + if (state->task != current) + goto the_end; + if (state->regs) { printk_deferred_once(KERN_WARNING "WARNING: kernel stack regs at %p in %s:%d has bad 'bp' value %p\n",Applied locally for testing. What about this part?diff --git a/arch/x86/include/asm/stacktrace.hb/arch/x86/include/asm/stacktrace.h index a3269c897ec5..d8d4fc66ffec 100644--- a/arch/x86/include/asm/stacktrace.h +++ b/arch/x86/include/asm/stacktrace.h@@ -58,7 +58,7 @@ get_frame_pointer(struct task_struct *task, structpt_regs *regs) if (task == current) return __builtin_frame_address(0); - return (unsigned long *)((struct inactive_task_frame*)task->thread.sp)->bp; + return (unsigned long *)READ_ONCE_NOCHECK(((struct inactive_task_frame *)task->thread.sp)->bp); } #else static inline unsigned long *
Oops, I missed that part. That's needed too. BTW, I'm still not on your email To: list. -- Josh -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>