Re: [patch 1/2] x86/process: Add proper bound checks in 64bit get_wchan()

[Thomas Gleixner <tglx@linutronix.de>]

On Fri, 2 Oct 2015, Sasha Levin wrote:
> I'm seeing a different issue with this patch:
> 
> [ 5228.736320] BUG: KASAN: out-of-bounds in get_wchan+0xf9/0x1b0 at addr ffff88049d2b7c50
> [ 5228.737560] Read of size 8 by task killall/22177
> [ 5228.738304] page:ffffea001274adc0 count:0 mapcount:0 mapping:          (null) index:0x0
> [ 5228.739374] flags: 0x6fffff80000000()
> [ 5228.739862] page dumped because: kasan: bad access detected
> [ 5228.741764] CPU: 8 PID: 22177 Comm: killall Not tainted 4.3.0-rc3-next-20151002-sasha-00076-gde7fa56-dirty #2590
> [ 5228.743337]  ffff882c80967828 000000007a901a83 ffff882c80967790 ffffffffacd2c8c8
> [ 5228.744409]  ffff88049d2b7c50 ffff882c80967818 ffffffffab74befb ffff882c8bd00000
> [ 5228.745436]  0000000000000002 0000000000000282 ffff882c8bd00cf8 0000000000000001
> [ 5228.746446] Call Trace:
> [ 5228.746881] dump_stack (lib/dump_stack.c:52)
> [ 5228.747720] kasan_report_error (include/linux/kasan.h:28 mm/kasan/report.c:170 mm/kasan/report.c:237)
> [ 5228.748670] __asan_report_load8_noabort (mm/kasan/report.c:279)
> [ 5228.750563] get_wchan (arch/x86/kernel/process.c:561)
> [ 5228.751378] do_task_stat (fs/proc/array.c:458)
> [ 5228.755912] proc_tgid_stat (fs/proc/array.c:565)
> [ 5228.756770] proc_single_show (./arch/x86/include/asm/atomic.h:118 include/linux/sched.h:2012 fs/proc/base.c:789)
> [ 5228.759066] seq_read (fs/seq_file.c:238)
> [ 5228.762360] __vfs_read (fs/read_write.c:432)
> [ 5228.767957] vfs_read (fs/read_write.c:454)
> [ 5228.769368] SyS_read (fs/read_write.c:570 fs/read_write.c:562)
> [ 5228.778344] entry_SYSCALL_64_fastpath (arch/x86/entry/entry_64.S:186)
> [ 5228.779272] Memory state around the buggy address:
> [ 5228.779971]  ffff88049d2b7b00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 5228.780992]  ffff88049d2b7b80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 5228.782021] >ffff88049d2b7c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 5228.783066]                                                     ^
> [ 5228.783936]  ffff88049d2b7c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 5228.784994]  ffff88049d2b7d00: 00 00 00 00 00 f1 f1 f1 f1 00 f4 f4 f4 f3 f3 f3
> 
>         fp = READ_ONCE(*(unsigned long *)sp);
>         do {
>                 if (fp < bottom || fp > top)
>                         return 0;
>                 ip = READ_ONCE(*(unsigned long *)(fp + sizeof(unsigned long)));
>                 if (!in_sched_functions(ip))
>                         return ip;
>                 fp = READ_ONCE(*(unsigned long *)fp); <=== Here

Weird, we accessed 

     *(unsigned long *)(fp + sizeof(unsigned long))

a few lines above, i.e. ffff88049d2b7c58

But what's more weird is that the memory dump does not really look
like a stack at all.

ffff88049d2b7c50 is stored on the stack:

> [ 5228.744409]  ffff88049d2b7c50 ffff882c80967818 ffffffffab74befb ffff882c8bd00000

But if it is not inside the stack bounds, how do we end up
dereferencing it. Confused....

Too bad that we don't know where the actual stack is. Can you apply
the patch below so we can see where the stack area actually is?

Thanks,

	tglx

diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
index 5f1c6266eb30..1735cbc5e886 100644
--- a/arch/x86/kernel/dumpstack_64.c
+++ b/arch/x86/kernel/dumpstack_64.c
@@ -286,7 +286,7 @@ show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
 		if ((i % STACKSLOTS_PER_LINE) == 0) {
 			if (i != 0)
 				pr_cont("\n");
-			printk("%s %016lx", log_lvl, *stack++);
+			printk("%s %p %016lx", log_lvl, stack, *stack++);
 		} else
 			pr_cont(" %016lx", *stack++);
 		touch_nmi_watchdog();
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index e07c94fbd0ac..473cf68984d0 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -129,6 +129,8 @@ static void print_address_description(struct kasan_access_info *info)
 			pr_err("Address belongs to variable %pS\n", addr);
 	}
 
+	pr_err("kasan: SP: %p\n", task_stack_page(current));
+
 	dump_stack();
 }