[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b8be6867-40aa-86bb-dc9b-ad591f2026f5@linux.com>
Date: Fri, 18 Aug 2017 11:07:08 +0300
From: Alexander Popov <alex.popov@...ux.com>
To: Kees Cook <keescook@...omium.org>
Cc: Laura Abbott <labbott@...hat.com>, Mark Rutland <mark.rutland@....com>,
"kernel-hardening@...ts.openwall.com" <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 Kees and Laura,
On 25.07.2017 06:34, Kees Cook wrote:
> On Mon, Jul 24, 2017 at 1:19 AM, Alexander Popov <alex.popov@...ux.com> wrote:
>> On 22.07.2017 03:23, Laura Abbott wrote:
>>> On 07/21/2017 09:56 AM, Alexander Popov wrote:
>>>> 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?
>>
>> Do you mean this list?
>> http://www.kernsec.org/wiki/index.php/Kernel_Self_Protection_Project/Recommended_Settings
>>
>>> I'm not sure why it requires two words though since the
>>> poison only seems to be 32-bits?
>>
>> On x86_64 end_of_stack() returns the pointer to unsigned long, so we need at
>> least 8 bytes to avoid breaking CONFIG_SCHED_STACK_END_CHECK. Don't know about 8
>> more bytes.
>
> Seems like this should be a random value like the per-frame stack
> canary, but it looks like a relatively cheap check. It's probably not
> in the cache, though, since most stack operations should be pretty far
> from the end of the stack...
It seems that the idea you describe is not implemented in Grsecurity/PaX patch.
On x86_64 the bottom of the stack for the mainline kernel looks like that:
0xffffc900004c8000: 0x57ac6e9d 0x00000000 0x00000000 0x00000000
0xffffc900004c8010: 0xffff4111 0xffffffff 0xffff4111 0xffffffff
...
But for the Grsecurity kernel it looks like that:
0xffffc90000324000: 0x00000000 0x00000000 0x57ac6e9d 0x00000000
0xffffc90000324010: 0xffff4111 0xffffffff 0xffff4111 0xffffffff
...
Because Grsecurity/PaX patch has that change:
static inline unsigned long *end_of_stack(const struct task_struct *task)
{
- return task->stack;
+ return (unsigned long *)task->stack + 1;
}
So that is their reason of having 16 bytes reserved at the bottom of the kernel
stack.
However, right now I don't understand the purpose of patching end_of_stack().
What do you think? Should mainline have it?
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.
