[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 21 Jul 2017 17:23:06 -0700
From: Laura Abbott <labbott@...hat.com>
To: alex.popov@...ux.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
On 07/21/2017 09:56 AM, Alexander Popov wrote:
> 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.
>
That makes sense, good find! I wonder if CONFIG_SCHED_STACK_END_CHECK
should go on the list of hardening options and/or if we can enhance
it somehow?
I'm not sure why it requires two words though since the
poison only seems to be 32-bits?
> Again, I want to say kudos to PaX Team for his awesome code.
Agreed!
> Best regards,
> Alexander
>
Thanks,
Laura
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.
