[PATCH v2] arch/x86: fix out-of-bounds in get_wchan()

[Dmitry Vyukov]

get_wchan() checks that fp is within stack bounds,
but then dereferences fp+8. This can crash kernel
or leak sensitive information. Also the function
operates on a potentially running stack, but does
not use READ_ONCE. As the result it can check that
one value is within stack bounds, but then deref
another value.

Fix the bounds check and use READ_ONCE for all
volatile data.

The bug was discovered with KASAN.

Signed-off-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
---
v2: Use "fp+sizeof(u64)" instead of "fp" in upper bound
comparison because we are going to read 8 bytes at fp.
As per Andrey Ryabinin comment.

FTR, here is the KASAN report:

[  124.575597] ERROR: AddressSanitizer: heap-buffer-overflow on address ffff88002e280000
[  124.578633] Accessed by thread T10915:
[  124.581050]   #2 ffffffff810dd423 in __tsan_read8 ??:0
[  124.581893]   #3 ffffffff8107c093 in get_wchan ./arch/x86/kernel/process_64.c:444
[  124.582763]   #4 ffffffff81342108 in do_task_stat array.c:0
[  124.583634]   #5 ffffffff81342dcc in proc_tgid_stat ??:0
[  124.584548]   #6 ffffffff8133c984 in proc_single_show base.c:0
[  124.585461]   #7 ffffffff812d18cc in seq_read ./fs/seq_file.c:222
[  124.586313]   #8 ffffffff8129e503 in vfs_read ??:0
[  124.587137]   #9 ffffffff8129f800 in SyS_read ??:0
[  124.587827]   #10 ffffffff81929bf5 in sysenter_dispatch ./arch/x86/ia32/ia32entry.S:164
[  124.588738]
[  124.593434] Shadow bytes around the buggy address:
[  124.594270]   ffff88002e27fd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  124.595339]   ffff88002e27fe00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  124.596453]   ffff88002e27fe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  124.597466]   ffff88002e27ff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  124.598501]   ffff88002e27ff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  124.599629] =>ffff88002e280000:[fa]fa fa fa fa fa fa fa fa fa 00 00 00 00 00 00
[  124.600873]   ffff88002e280080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  124.601892]   ffff88002e280100: 00 fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
[  124.603037]   ffff88002e280180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
[  124.604047]   ffff88002e280200: fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd fd
[  124.605054]   ffff88002e280280: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa
[  124.605993] Shadow byte legend (one shadow byte represents 8 application bytes):
[  124.606958]   Addressable:   00
[  124.607483]   Partially addressable: 01 02 03 04 05 06 07
[  124.608219]   Heap redzone:  fa
[  124.608724]   Heap kmalloc redzone:  fb
[  124.609249]   Freed heap region: fd
[  124.609753]   Shadow gap:fe
[  124.610292] =========================================================================
---
 arch/x86/kernel/process_64.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 71d7849..0885eed 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -506,17 +506,19 @@ unsigned long get_wchan(struct task_struct *p)
 	if (!p || p == current || p->state == TASK_RUNNING)
 		return 0;
 	stack = (unsigned long)task_stack_page(p);
-	if (p->thread.sp < stack || p->thread.sp >= stack+THREAD_SIZE)
+	/* The task can be already running at this point, so tread carefully. */
+	fp = READ_ONCE(p->thread.sp);
+	if (fp < stack || fp+sizeof(u64) > stack+THREAD_SIZE)
 		return 0;
-	fp = *(u64 *)(p->thread.sp);
+	fp = READ_ONCE(*(u64 *)fp);
 	do {
 		if (fp < (unsigned long)stack ||
-		    fp >= (unsigned long)stack+THREAD_SIZE)
+		    fp+8+sizeof(u64) > (unsigned long)stack+THREAD_SIZE)
 			return 0;
-		ip = *(u64 *)(fp+8);
+		ip = READ_ONCE(*(u64 *)(fp+8));
 		if (!in_sched_functions(ip))
 			return ip;
-		fp = *(u64 *)fp;
+		fp = READ_ONCE(*(u64 *)fp);
 	} while (count++ < 16);
 	return 0;
 }
-- 
2.6.0.rc2.230.g3dd15c0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/