From mboxrd@z Thu Jan 1 00:00:00 1970 References: <1499724283-30719-1-git-send-email-labbott@redhat.com> <1499724283-30719-3-git-send-email-labbott@redhat.com> <20170711195154.GA7124@leverpostej> <2f24df9e-64b1-ffef-ddbb-08f7fe8d2a96@redhat.com> <160e3088-b8e0-513b-8c1a-ce22ce598d5a@redhat.com> <44d0ad13-6362-6224-ae64-d1848d217f0b@linux.com> From: Alexander Popov Message-ID: Date: Fri, 18 Aug 2017 11:07:08 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: [kernel-hardening] Re: [RFC][PATCH 2/2] arm64: Clear the stack To: Kees Cook Cc: Laura Abbott , Mark Rutland , "kernel-hardening@lists.openwall.com" , Ard Biesheuvel , Tycho Andersen , PaX Team List-ID: Hello Kees and Laura, On 25.07.2017 06:34, Kees Cook wrote: > On Mon, Jul 24, 2017 at 1:19 AM, Alexander Popov 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