[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 21 Jul 2017 19:56:13 +0300
From: Alexander Popov <alex.popov@...ux.com>
To: Laura Abbott <labbott@...hat.com>, Mark Rutland <mark.rutland@....com>
Cc: Kees Cook <keescook@...omium.org>, kernel-hardening@...ts.openwall.com,
Ard Biesheuvel <ard.biesheuvel@...aro.org>, Tycho Andersen
<tycho@...ker.com>, PaX Team <pageexec@...email.hu>
Subject: Re: [RFC][PATCH 2/2] arm64: Clear the stack
Hello Laura and Mark,
[+ Tycho Andersen and Pax Team]
On 14.07.2017 23:51, Laura Abbott wrote:
> On 07/11/2017 12:51 PM, Mark Rutland wrote:
>> On Mon, Jul 10, 2017 at 03:04:43PM -0700, Laura Abbott wrote:
>>> + /* The poison function */
>>> +4:
>>> + /* Generate the address we found */
>>> + add x5, x1, x3, lsl #3
>>> + orr x5, x5, #16
>>
>> Have you figured out what the forced 16 byte offset is for?
>>
>> I've not managed to figure out why it's there, and it looks like
>> Alexander couldn't either, judging by his comments in the x86 asm.
>
> From get_wchan in arch/x86/kernel/process.c, it might be be to
> account for the start of the frame correctly? I don't have a
> definitive answer though and plan on just removing this for the
> next version.
I've investigated it carefully: we should not remove this 16-byte offset.
I looked at the bottom of the kernel stack with the debugger and found a
non-zero value 0x57AC6E9D. It is STACK_END_MAGIC, which is defined in
include/uapi/linux/magic.h. This value is checked if we enable
CONFIG_SCHED_STACK_END_CHECK.
Then I removed this 16-byte offset in stackleak patch (including the OR
instruction in erase_kstack()) to see how the first erase_kstack() call happily
overwrites that magic value:
[ 1.574655] Freeing unused kernel memory: 1244K
[ 1.575026] Kernel memory protection disabled.
[ 1.578575] Kernel panic - not syncing: corrupted stack end detected inside
scheduler
[ 1.578575]
[ 1.579420] CPU: 0 PID: 1 Comm: init Not tainted 4.12.0+ #9
[ 1.579420] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
Ubuntu-1.8.2-1ubuntu1 04/01/2014
[ 1.579420] Call Trace:
[ 1.579420] dump_stack+0x63/0x81
[ 1.579420] panic+0xd0/0x212
[ 1.579420] __schedule+0x6d8/0x6e0
[ 1.579420] schedule+0x31/0x80
[ 1.579420] io_schedule+0x11/0x40
[ 1.579420] __lock_page_or_retry+0x17d/0x2b0
[ 1.579420] ? page_cache_tree_insert+0x90/0x90
[ 1.579420] filemap_fault+0x3aa/0x5c0
[ 1.579420] ? filemap_map_pages+0x1a6/0x280
[ 1.579420] ext4_filemap_fault+0x2d/0x40
[ 1.579420] __do_fault+0x1b/0x60
[ 1.579420] __handle_mm_fault+0x641/0x8b0
[ 1.579420] handle_mm_fault+0x87/0x130
[ 1.579420] __do_page_fault+0x25f/0x4a0
[ 1.579420] trace_do_page_fault+0x37/0xd0
[ 1.579420] do_async_page_fault+0x23/0x80
[ 1.579420] async_page_fault+0x28/0x30
[ 1.579420] RIP: 0033:0x7f81be514ab0
[ 1.579420] RSP: 002b:00007ffc8de73768 EFLAGS: 00010202
[ 1.579420] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
[ 1.579420] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 00007ffc8de73770
[ 1.579420] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
[ 1.579420] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[ 1.579420] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 1.579420] Dumping ftrace buffer:
[ 1.579420] (ftrace buffer empty)
[ 1.579420] Kernel Offset: disabled
[ 1.579420] Rebooting in 86400 seconds..
So let's keep it not to break CONFIG_SCHED_STACK_END_CHECK.
Again, I want to say kudos to PaX Team for his awesome code.
Best regards,
Alexander
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
