[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 16 Aug 2017 23:47:44 +0300
From: Alexander Popov <alex.popov@...ux.com>
To: Tycho Andersen <tycho@...ker.com>
Cc: kernel-hardening@...ts.openwall.com, keescook@...omium.org,
pageexec@...email.hu, spender@...ecurity.net,
Laura Abbott <labbott@...hat.com>, Mark Rutland <mark.rutland@....com>,
Ard Biesheuvel <ard.biesheuvel@...aro.org>, x86@...nel.org,
Andy Lutomirski <luto@...capital.net>
Subject: Re: [PATCH RFC v3 1/1] gcc-plugins: Add stackleak feature erasing the
kernel stack at the end of syscalls
Hello Tycho,
On 15.08.2017 06:38, Tycho Andersen wrote:
> On Wed, Jul 12, 2017 at 09:17:51PM +0300, Alexander Popov wrote:
>> +#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
>> +void __used track_stack(void)
>> +{
>> + unsigned long sp = (unsigned long)&sp;
>> +
>> + if (sp < current->thread.lowest_stack &&
>> + sp >= (unsigned long)task_stack_page(current) +
>> + 2 * sizeof(unsigned long)) {
>> + current->thread.lowest_stack = sp;
>> + }
>> +
>> + if (unlikely((sp & ~(THREAD_SIZE - 1)) < (THREAD_SIZE / 16)))
>
> I think this check is wrong, the lhs should be
> (sp & (THREAD_SIZE - 1)). Otherwise, we just check that the upper bits
> of the stack are < THREAD_SIZE / 16, which they never will be.
Thank you, I think you are right!
I can additionally notice that this erroneous check is not a part of PaX patch,
it is introduced by Grsecurity patch.
Thanks again, I'll fix and annotate it in the next version of the patch.
Did you manage to create a test for the correct check which hits the BUG()?
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.
