| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 02 Oct 2015 21:15:46 -0400
From: Sasha Levin <sasha.levin@...cle.com>
To: Thomas Gleixner <tglx@...utronix.de>,
LKML <linux-kernel@...r.kernel.org>
CC: Andrey Ryabinin <ryabinin.a.a@...il.com>,
Andy Lutomirski <luto@...capital.net>,
Andrey Konovalov <andreyknvl@...gle.com>,
Kostya Serebryany <kcc@...gle.com>,
Alexander Potapenko <glider@...gle.com>,
kasan-dev <kasan-dev@...glegroups.com>,
Borislav Petkov <bp@...en8.de>,
Denys Vlasenko <dvlasenk@...hat.com>,
Andi Kleen <ak@...ux.intel.com>, x86@...nel.org,
Dmitry Vyukov <dvyukov@...gle.com>,
Wolfram Gloger <wmglo@...t.med.uni-muenchen.de>
Subject: Re: [patch 1/2] x86/process: Add proper bound checks in 64bit get_wchan()
On 09/30/2015 04:38 AM, Thomas Gleixner wrote:
> Dmitry Vyukov reported the following using trinity and the memory
> error detector AddressSanitizer
> (https://code.google.com/p/address-sanitizer/wiki/AddressSanitizerForKernel).
>
> [ 124.575597] ERROR: AddressSanitizer: heap-buffer-overflow on
> address ffff88002e280000
> [ 124.576801] ffff88002e280000 is located 131938492886538 bytes to
> the left of 28857600-byte region [ffffffff81282e0a, ffffffff82e0830a)
> [ 124.578633] Accessed by thread T10915:
> [ 124.579295] inlined in describe_heap_address
> ./arch/x86/mm/asan/report.c:164
> [ 124.579295] #0 ffffffff810dd277 in asan_report_error
> ./arch/x86/mm/asan/report.c:278
> [ 124.580137] #1 ffffffff810dc6a0 in asan_check_region
> ./arch/x86/mm/asan/asan.c:37
> [ 124.581050] #2 ffffffff810dd423 in __tsan_read8 ??:0
> [ 124.581893] #3 ffffffff8107c093 in get_wchan
> ./arch/x86/kernel/process_64.c:444
>
> The address checks in the 64bit implementation of get_wchan() are
> wrong in several ways:
>
> - The lower bound of the stack is not the start of the stack
> page. It's the start of the stack page plus sizeof (struct
> thread_info)
>
> - The upper bound must be:
>
> top_of_stack - TOP_OF_KERNEL_STACK_PADDING - 2 * sizeof(unsigned long).
>
> The 2 * sizeof(unsigned long) is required because the stack pointer
> points at the frame pointer. The layout on the stack is: ... IP FP
> ... IP FP. So we need to make sure that both IP and FP are in the
> bounds.
>
> Fix the bound checks and get rid of the mix of numeric constants, u64
> and unsigned long. Making all unsigned long allows us to use the same
> function for 32bit as well.
>
> Use READ_ONCE() when accessing the stack. This does not prevent a
> concurrent wakeup of the task and the stack changing, but at least it
> avoids TOCTOU.
>
> Also check task state at the end of the loop. Again that does not
> prevent concurrent changes, but it avoids walking for nothing.
>
> Add proper comments while at it.
>
> Reported-by: Dmitry Vyukov <dvyukov@...gle.com>
> Reported-by: Sasha Levin <sasha.levin@...cle.com>
> Based-on-patch-from: Wolfram Gloger <wmglo@...t.med.uni-muenchen.de>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
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
} while (count++ < 16 && p->state != TASK_RUNNING);
Thanks,
Sasha
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists