All of lore.kernel.org
 help / color / mirror / Atom feed
* [kernel-hardening] status: GRKERNSEC_KSTACKOVERFLOW
@ 2015-11-24 19:10 Kees Cook
  2015-11-25 23:45 ` [kernel-hardening] " Quentin Casasnovas
  0 siblings, 1 reply; 15+ messages in thread
From: Kees Cook @ 2015-11-24 19:10 UTC (permalink / raw)
  To: Quentin Casasnovas; +Cc: kernel-hardening

Hi,

I just wanted to check in and see how progress was going on the stack
overflow feature. Anything we can help with?

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [kernel-hardening] Re: status: GRKERNSEC_KSTACKOVERFLOW
  2015-11-24 19:10 [kernel-hardening] status: GRKERNSEC_KSTACKOVERFLOW Kees Cook
@ 2015-11-25 23:45 ` Quentin Casasnovas
  2015-11-26 16:39   ` Quentin Casasnovas
                     ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Quentin Casasnovas @ 2015-11-25 23:45 UTC (permalink / raw)
  To: Kees Cook; +Cc: Quentin Casasnovas, kernel-hardening

On Tue, Nov 24, 2015 at 11:10:09AM -0800, Kees Cook wrote:
> Hi,
>

Hi Kees,

> I just wanted to check in and see how progress was going on the stack
> overflow feature. Anything we can help with?
>

Sorry for not following up on this, I've been busy and haven't had the time
to finish it properly.  I've pushed an initial WIP break up of the
KSTACK_OVERFLOW feature on my github:

  https://github.com/casasnovas/linux/tree/quentin-split-kstackoverflow

This is far from being complete though, and hasn't been cleaned at all.  I
didn't share it earlier because I don't think I fully understand it and
haven't tested it yet.  In "short", there's mention of guard pages in the
Kconfig help:

  If you say Y here, the kernel's process stacks will be allocated with
  vmalloc instead of the kernel's default allocator.  This introduces guard
                                                                      ^^^^^
  pages that in combination with the alloca checking of the STACKLEAK
  ^^^^^
  feature prevents all forms of kernel process stack overflow abuse.  Note
  that this is different from kernel stack buffer overflows. """

And I couldn't find anything about it in the code.  Maybe it's just coming
from a mis-interpretation of the above text, but I was expecting this to
mean there would be a PROT_NONE guard page after the end of the stack, so
that read/writes below it could be trapped.  It could also be that I missed
some parts in my initial break-up or simply am missing something.

It should also be noted that I did not find that the struct thread_info
(which is stuffed at the end of the stack) was protected in any way either.
So even if a write/read _below_ the stack could still be trapped if nothing
is currently mapped there, it looks like deep stack usage could still
overflow it and go unoticed.  Here again, I didn't spend a lot of time on
this and it might just be that I'm missing something.

In the very unlikely event where I didn't miss anything and the struct
thread_info can still be overflown and there isn't any guard page, maybe we
can improve on the current KSTACK_OVERFLOW feature by putting the struct
thread_info on a different page than the kernel stack, and not vmap() it
like the rest of the stack pages, but instead map a PROT_NONE page there.
That would mean the struct thread_info can still be accessed by using its
lowmem mapping (i.e. legit usage from the kernel) but not by deep kernel
stack usage.  Maybe the cost of adding an extra page per kernel stack is
too high though.

Finally, I'd like to find a real life example where one could overflow the
kernel stack, so it can be used to test the feature (once properly split)
and show it can happen, for real, before sending real patches for review.
I might have found such a case because of what appears to me like a gcc
bug, more on this in another follow up :)

tldr; Initial break up done, not even compile-tested yet and am probably
missing some bits.  Might have found a code path to trigger a kernel stack
overflow due to a gcc weirdness, I need to investigate it.

Any comments appreciated :)

Thanks,
Quentin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [kernel-hardening] Re: status: GRKERNSEC_KSTACKOVERFLOW
  2015-11-25 23:45 ` [kernel-hardening] " Quentin Casasnovas
@ 2015-11-26 16:39   ` Quentin Casasnovas
  2015-11-27 20:09     ` Kees Cook
  2015-11-26 17:17   ` Quentin Casasnovas
  2015-11-27 20:23   ` Kees Cook
  2 siblings, 1 reply; 15+ messages in thread
From: Quentin Casasnovas @ 2015-11-26 16:39 UTC (permalink / raw)
  To: kernel-hardening; +Cc: Kees Cook, Quentin Casasnovas

On Thu, Nov 26, 2015 at 12:45:42AM +0100, Quentin Casasnovas wrote:
> On Tue, Nov 24, 2015 at 11:10:09AM -0800, Kees Cook wrote:
>
> [snip/]
> 
> It should also be noted that I did not find that the struct thread_info
> (which is stuffed at the end of the stack) was protected in any way either.
> So even if a write/read _below_ the stack could still be trapped if nothing
> is currently mapped there, it looks like deep stack usage could still
> overflow it and go unoticed.  Here again, I didn't spend a lot of time on
> this and it might just be that I'm missing something.
> 
> In the very unlikely event where I didn't miss anything and the struct
> thread_info can still be overflown and there isn't any guard page, maybe we
> can improve on the current KSTACK_OVERFLOW feature by putting the struct
> thread_info on a different page than the kernel stack, and not vmap() it
> like the rest of the stack pages, but instead map a PROT_NONE page there.
> That would mean the struct thread_info can still be accessed by using its
> lowmem mapping (i.e. legit usage from the kernel) but not by deep kernel
> stack usage.  Maybe the cost of adding an extra page per kernel stack is
> too high though.

As expected I missed some other changes:

/* Load thread_info address into "reg" */
#define GET_THREAD_INFO(reg) \
-       _ASM_MOV PER_CPU_VAR(cpu_current_top_of_stack),reg ; \
-       _ASM_SUB $(THREAD_SIZE),reg ;
+       _ASM_MOV PER_CPU_VAR(current_tinfo),reg ;

and

+DECLARE_PER_CPU(struct thread_info *, current_tinfo);
+
static inline struct thread_info *current_thread_info(void)
{
-       return (struct thread_info *)(current_top_of_stack() - THREAD_SIZE);
+       return this_cpu_read_stable(current_tinfo);
}

So no more thread_info on the stack in the default configuration, which
isn't correlated with the KSTACKOVERFLOW config option.

Quentin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [kernel-hardening] Re: status: GRKERNSEC_KSTACKOVERFLOW
  2015-11-25 23:45 ` [kernel-hardening] " Quentin Casasnovas
  2015-11-26 16:39   ` Quentin Casasnovas
@ 2015-11-26 17:17   ` Quentin Casasnovas
  2015-11-27 20:18     ` Kees Cook
  2015-11-27 20:23   ` Kees Cook
  2 siblings, 1 reply; 15+ messages in thread
From: Quentin Casasnovas @ 2015-11-26 17:17 UTC (permalink / raw)
  To: Quentin Casasnovas; +Cc: Kees Cook, kernel-hardening

On Thu, Nov 26, 2015 at 12:45:42AM +0100, Quentin Casasnovas wrote:
> On Tue, Nov 24, 2015 at 11:10:09AM -0800, Kees Cook wrote:
> > Hi,
> >
> 
> Hi Kees,
> 
> > I just wanted to check in and see how progress was going on the stack
> > overflow feature. Anything we can help with?
> >
> 
> Sorry for not following up on this, I've been busy and haven't had the time
> to finish it properly.  I've pushed an initial WIP break up of the
> KSTACK_OVERFLOW feature on my github:
> 
>   https://github.com/casasnovas/linux/tree/quentin-split-kstackoverflow
> 
> This is far from being complete though, and hasn't been cleaned at all.  I
> didn't share it earlier because I don't think I fully understand it and
> haven't tested it yet.  In "short", there's mention of guard pages in the
> Kconfig help:
> 
>   If you say Y here, the kernel's process stacks will be allocated with
>   vmalloc instead of the kernel's default allocator.  This introduces guard
>                                                                       ^^^^^
>   pages that in combination with the alloca checking of the STACKLEAK
>   ^^^^^
>   feature prevents all forms of kernel process stack overflow abuse.  Note
>   that this is different from kernel stack buffer overflows. """
> 
> And I couldn't find anything about it in the code.  Maybe it's just coming
> from a mis-interpretation of the above text, but I was expecting this to
> mean there would be a PROT_NONE guard page after the end of the stack, so
> that read/writes below it could be trapped.  It could also be that I missed
> some parts in my initial break-up or simply am missing something.
>

Alright, there's a guard page by default when using vmap() unless
VM_NO_GUARD is in the flags.  I had a feeling I was missing some bits.. ;)

Quentin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [kernel-hardening] Re: status: GRKERNSEC_KSTACKOVERFLOW
  2015-11-26 16:39   ` Quentin Casasnovas
@ 2015-11-27 20:09     ` Kees Cook
  2015-11-28  9:19       ` Quentin Casasnovas
  0 siblings, 1 reply; 15+ messages in thread
From: Kees Cook @ 2015-11-27 20:09 UTC (permalink / raw)
  To: Quentin Casasnovas; +Cc: kernel-hardening

On Thu, Nov 26, 2015 at 8:39 AM, Quentin Casasnovas
<quentin.casasnovas@oracle.com> wrote:
> On Thu, Nov 26, 2015 at 12:45:42AM +0100, Quentin Casasnovas wrote:
>> On Tue, Nov 24, 2015 at 11:10:09AM -0800, Kees Cook wrote:
>>
>> [snip/]
>>
>> It should also be noted that I did not find that the struct thread_info
>> (which is stuffed at the end of the stack) was protected in any way either.
>> So even if a write/read _below_ the stack could still be trapped if nothing
>> is currently mapped there, it looks like deep stack usage could still
>> overflow it and go unoticed.  Here again, I didn't spend a lot of time on
>> this and it might just be that I'm missing something.
>>
>> In the very unlikely event where I didn't miss anything and the struct
>> thread_info can still be overflown and there isn't any guard page, maybe we
>> can improve on the current KSTACK_OVERFLOW feature by putting the struct
>> thread_info on a different page than the kernel stack, and not vmap() it
>> like the rest of the stack pages, but instead map a PROT_NONE page there.
>> That would mean the struct thread_info can still be accessed by using its
>> lowmem mapping (i.e. legit usage from the kernel) but not by deep kernel
>> stack usage.  Maybe the cost of adding an extra page per kernel stack is
>> too high though.
>
> As expected I missed some other changes:
>
> /* Load thread_info address into "reg" */
> #define GET_THREAD_INFO(reg) \
> -       _ASM_MOV PER_CPU_VAR(cpu_current_top_of_stack),reg ; \
> -       _ASM_SUB $(THREAD_SIZE),reg ;
> +       _ASM_MOV PER_CPU_VAR(current_tinfo),reg ;
>
> and
>
> +DECLARE_PER_CPU(struct thread_info *, current_tinfo);
> +
> static inline struct thread_info *current_thread_info(void)
> {
> -       return (struct thread_info *)(current_top_of_stack() - THREAD_SIZE);
> +       return this_cpu_read_stable(current_tinfo);
> }
>
> So no more thread_info on the stack in the default configuration, which
> isn't correlated with the KSTACKOVERFLOW config option.

Good find! This seems like it should be its own patch, distinct from
KSTACKOVERFLOW?

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [kernel-hardening] Re: status: GRKERNSEC_KSTACKOVERFLOW
  2015-11-26 17:17   ` Quentin Casasnovas
@ 2015-11-27 20:18     ` Kees Cook
  2015-11-28  9:16       ` Quentin Casasnovas
  0 siblings, 1 reply; 15+ messages in thread
From: Kees Cook @ 2015-11-27 20:18 UTC (permalink / raw)
  To: Quentin Casasnovas; +Cc: kernel-hardening

On Thu, Nov 26, 2015 at 9:17 AM, Quentin Casasnovas
<quentin.casasnovas@oracle.com> wrote:
> On Thu, Nov 26, 2015 at 12:45:42AM +0100, Quentin Casasnovas wrote:
>> On Tue, Nov 24, 2015 at 11:10:09AM -0800, Kees Cook wrote:
>> > Hi,
>> >
>>
>> Hi Kees,
>>
>> > I just wanted to check in and see how progress was going on the stack
>> > overflow feature. Anything we can help with?
>> >
>>
>> Sorry for not following up on this, I've been busy and haven't had the time
>> to finish it properly.  I've pushed an initial WIP break up of the
>> KSTACK_OVERFLOW feature on my github:
>>
>>   https://github.com/casasnovas/linux/tree/quentin-split-kstackoverflow
>>
>> This is far from being complete though, and hasn't been cleaned at all.  I
>> didn't share it earlier because I don't think I fully understand it and
>> haven't tested it yet.  In "short", there's mention of guard pages in the
>> Kconfig help:
>>
>>   If you say Y here, the kernel's process stacks will be allocated with
>>   vmalloc instead of the kernel's default allocator.  This introduces guard
>>                                                                       ^^^^^
>>   pages that in combination with the alloca checking of the STACKLEAK
>>   ^^^^^
>>   feature prevents all forms of kernel process stack overflow abuse.  Note
>>   that this is different from kernel stack buffer overflows. """
>>
>> And I couldn't find anything about it in the code.  Maybe it's just coming
>> from a mis-interpretation of the above text, but I was expecting this to
>> mean there would be a PROT_NONE guard page after the end of the stack, so
>> that read/writes below it could be trapped.  It could also be that I missed
>> some parts in my initial break-up or simply am missing something.
>>
>
> Alright, there's a guard page by default when using vmap() unless
> VM_NO_GUARD is in the flags.  I had a feeling I was missing some bits.. ;)

Is this documented anywhere in Documentation? Reading the code
quickly, I don't understand what actually makes this a guard page. It
looks like the requested size is just grown. Is it actually set up in
a way that'll trap if it that page gets touched?

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [kernel-hardening] Re: status: GRKERNSEC_KSTACKOVERFLOW
  2015-11-25 23:45 ` [kernel-hardening] " Quentin Casasnovas
  2015-11-26 16:39   ` Quentin Casasnovas
  2015-11-26 17:17   ` Quentin Casasnovas
@ 2015-11-27 20:23   ` Kees Cook
  2015-11-28  9:25     ` Quentin Casasnovas
  2016-01-20  0:03     ` Kees Cook
  2 siblings, 2 replies; 15+ messages in thread
From: Kees Cook @ 2015-11-27 20:23 UTC (permalink / raw)
  To: Quentin Casasnovas; +Cc: kernel-hardening

On Wed, Nov 25, 2015 at 3:45 PM, Quentin Casasnovas
<quentin.casasnovas@oracle.com> wrote:
> On Tue, Nov 24, 2015 at 11:10:09AM -0800, Kees Cook wrote:
>> Hi,
>>
>
> Hi Kees,
>
>> I just wanted to check in and see how progress was going on the stack
>> overflow feature. Anything we can help with?
>>
>
> Sorry for not following up on this, I've been busy and haven't had the time
> to finish it properly.  I've pushed an initial WIP break up of the
> KSTACK_OVERFLOW feature on my github:
>
>   https://github.com/casasnovas/linux/tree/quentin-split-kstackoverflow

Great! Thanks for the update!

> This is far from being complete though, and hasn't been cleaned at all.  I
> didn't share it earlier because I don't think I fully understand it and
> haven't tested it yet.  In "short", there's mention of guard pages in the
> Kconfig help:
>
>   If you say Y here, the kernel's process stacks will be allocated with
>   vmalloc instead of the kernel's default allocator.  This introduces guard
>                                                                       ^^^^^
>   pages that in combination with the alloca checking of the STACKLEAK
>   ^^^^^
>   feature prevents all forms of kernel process stack overflow abuse.  Note
>   that this is different from kernel stack buffer overflows. """
>
> And I couldn't find anything about it in the code.  Maybe it's just coming
> from a mis-interpretation of the above text, but I was expecting this to
> mean there would be a PROT_NONE guard page after the end of the stack, so
> that read/writes below it could be trapped.  It could also be that I missed
> some parts in my initial break-up or simply am missing something.
>
> It should also be noted that I did not find that the struct thread_info
> (which is stuffed at the end of the stack) was protected in any way either.
> So even if a write/read _below_ the stack could still be trapped if nothing
> is currently mapped there, it looks like deep stack usage could still
> overflow it and go unoticed.  Here again, I didn't spend a lot of time on
> this and it might just be that I'm missing something.
>
> In the very unlikely event where I didn't miss anything and the struct
> thread_info can still be overflown and there isn't any guard page, maybe we
> can improve on the current KSTACK_OVERFLOW feature by putting the struct
> thread_info on a different page than the kernel stack, and not vmap() it
> like the rest of the stack pages, but instead map a PROT_NONE page there.
> That would mean the struct thread_info can still be accessed by using its
> lowmem mapping (i.e. legit usage from the kernel) but not by deep kernel
> stack usage.  Maybe the cost of adding an extra page per kernel stack is
> too high though.
>
> Finally, I'd like to find a real life example where one could overflow the
> kernel stack, so it can be used to test the feature (once properly split)
> and show it can happen, for real, before sending real patches for review.
> I might have found such a case because of what appears to me like a gcc
> bug, more on this in another follow up :)

I think at least some of the past flaws have been related to having
dynamic stack allocations, where an attacker could control the size of
the object. And "alloca" overflow style attack.

I will see about getting an lkdtm test written too. Though testing
this when there is no guard page seems like a bad idea. I'll have to
get creative on the overwrite. Hmm.

> tldr; Initial break up done, not even compile-tested yet and am probably
> missing some bits.  Might have found a code path to trigger a kernel stack
> overflow due to a gcc weirdness, I need to investigate it.

Yay bugs! :)

>
> Any comments appreciated :)
>
> Thanks,
> Quentin

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [kernel-hardening] Re: status: GRKERNSEC_KSTACKOVERFLOW
  2015-11-27 20:18     ` Kees Cook
@ 2015-11-28  9:16       ` Quentin Casasnovas
  0 siblings, 0 replies; 15+ messages in thread
From: Quentin Casasnovas @ 2015-11-28  9:16 UTC (permalink / raw)
  To: Kees Cook; +Cc: Quentin Casasnovas, kernel-hardening

On Fri, Nov 27, 2015 at 12:18:55PM -0800, Kees Cook wrote:
> On Thu, Nov 26, 2015 at 9:17 AM, Quentin Casasnovas
> <quentin.casasnovas@oracle.com> wrote:
> >
> > Alright, there's a guard page by default when using vmap() unless
> > VM_NO_GUARD is in the flags.  I had a feeling I was missing some bits.. ;)
> 
> Is this documented anywhere in Documentation?
>

I couldn't find anything in Documentation/ but the flag is documented in
include/linux/vmalloc.h.  I found it by looking at the vmap() code
directly.

> 
> Reading the code quickly, I don't understand what actually makes this a
> guard page. It looks like the requested size is just grown. Is it
> actually set up in a way that'll trap if it that page gets touched?
> 

AFAIU (which might not be very far!), the size is grown in
get_vm_area_caller() but then only the *requested* size is mapped in
map_vm_area().

On x86-64, where we have the IST stack, that means as soon as we access
below the currently mapped stack, we'll get a page fault, which will be
handled by vmalloc_fault(), and since no PTE is present for that page, that
should cause an Oops.

On x86, I believe a page fault when accessing below the stack would cause a
double fault, as there won't be any stack switching since we're coming from
ring0 (and there isn't a stack switch on exception when there isn't a
privilege change), so the first page fault would cause a second fault when
the CPU tries to push the context on the same stack.

Quentin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [kernel-hardening] Re: status: GRKERNSEC_KSTACKOVERFLOW
  2015-11-27 20:09     ` Kees Cook
@ 2015-11-28  9:19       ` Quentin Casasnovas
  2015-11-30 20:12         ` Kees Cook
  0 siblings, 1 reply; 15+ messages in thread
From: Quentin Casasnovas @ 2015-11-28  9:19 UTC (permalink / raw)
  To: kernel-hardening; +Cc: Quentin Casasnovas

On Fri, Nov 27, 2015 at 12:09:31PM -0800, Kees Cook wrote:
> On Thu, Nov 26, 2015 at 8:39 AM, Quentin Casasnovas
> <quentin.casasnovas@oracle.com> wrote:
> > On Thu, Nov 26, 2015 at 12:45:42AM +0100, Quentin Casasnovas wrote:
> >> On Tue, Nov 24, 2015 at 11:10:09AM -0800, Kees Cook wrote:
> >>
> >> [snip/]
> >>
> >> It should also be noted that I did not find that the struct thread_info
> >> (which is stuffed at the end of the stack) was protected in any way either.
> >> So even if a write/read _below_ the stack could still be trapped if nothing
> >> is currently mapped there, it looks like deep stack usage could still
> >> overflow it and go unoticed.  Here again, I didn't spend a lot of time on
> >> this and it might just be that I'm missing something.
> >>
> >> In the very unlikely event where I didn't miss anything and the struct
> >> thread_info can still be overflown and there isn't any guard page, maybe we
> >> can improve on the current KSTACK_OVERFLOW feature by putting the struct
> >> thread_info on a different page than the kernel stack, and not vmap() it
> >> like the rest of the stack pages, but instead map a PROT_NONE page there.
> >> That would mean the struct thread_info can still be accessed by using its
> >> lowmem mapping (i.e. legit usage from the kernel) but not by deep kernel
> >> stack usage.  Maybe the cost of adding an extra page per kernel stack is
> >> too high though.
> >
> > As expected I missed some other changes:
> >
> > /* Load thread_info address into "reg" */
> > #define GET_THREAD_INFO(reg) \
> > -       _ASM_MOV PER_CPU_VAR(cpu_current_top_of_stack),reg ; \
> > -       _ASM_SUB $(THREAD_SIZE),reg ;
> > +       _ASM_MOV PER_CPU_VAR(current_tinfo),reg ;
> >
> > and
> >
> > +DECLARE_PER_CPU(struct thread_info *, current_tinfo);
> > +
> > static inline struct thread_info *current_thread_info(void)
> > {
> > -       return (struct thread_info *)(current_top_of_stack() - THREAD_SIZE);
> > +       return this_cpu_read_stable(current_tinfo);
> > }
> >
> > So no more thread_info on the stack in the default configuration, which
> > isn't correlated with the KSTACKOVERFLOW config option.
> 
> Good find! This seems like it should be its own patch, distinct from
> KSTACKOVERFLOW?
>

We should probably make KSTACKOVERFLOW depend on moving the thread_info off
the stack, since otherwise the thread_info could still be hijacked and that
would counterfeit the KSTACKOVERFLOW purpose.

Quentin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [kernel-hardening] Re: status: GRKERNSEC_KSTACKOVERFLOW
  2015-11-27 20:23   ` Kees Cook
@ 2015-11-28  9:25     ` Quentin Casasnovas
  2016-01-20  0:03     ` Kees Cook
  1 sibling, 0 replies; 15+ messages in thread
From: Quentin Casasnovas @ 2015-11-28  9:25 UTC (permalink / raw)
  To: Kees Cook; +Cc: Quentin Casasnovas, kernel-hardening

On Fri, Nov 27, 2015 at 12:23:58PM -0800, Kees Cook wrote:
> On Wed, Nov 25, 2015 at 3:45 PM, Quentin Casasnovas
> <quentin.casasnovas@oracle.com> wrote:
> >
> > Finally, I'd like to find a real life example where one could overflow the
> > kernel stack, so it can be used to test the feature (once properly split)
> > and show it can happen, for real, before sending real patches for review.
> > I might have found such a case because of what appears to me like a gcc
> > bug, more on this in another follow up :)
> 
> I think at least some of the past flaws have been related to having
> dynamic stack allocations, where an attacker could control the size of
> the object. And "alloca" overflow style attack.
> 

It turns out there isn't any gcc problems and huge stack usage (sometimes
even 5Kb, like for nl80211_send_wiphy(), or 2Kb in interrupt context for
kbd_event()) in functions not supposed to require a lot of stack was caused
by KASAN on my tests.  The THREAD_SIZE is properly augmented when using
KASAN so that shouldn't cause any problems, sorry for the noise.

> 
> I will see about getting an lkdtm test written too. Though testing
> this when there is no guard page seems like a bad idea. I'll have to
> get creative on the overwrite. Hmm.
> 

:)

Quentin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [kernel-hardening] Re: status: GRKERNSEC_KSTACKOVERFLOW
  2015-11-28  9:19       ` Quentin Casasnovas
@ 2015-11-30 20:12         ` Kees Cook
  0 siblings, 0 replies; 15+ messages in thread
From: Kees Cook @ 2015-11-30 20:12 UTC (permalink / raw)
  To: kernel-hardening; +Cc: Quentin Casasnovas

On Sat, Nov 28, 2015 at 1:19 AM, Quentin Casasnovas
<quentin.casasnovas@oracle.com> wrote:
> On Fri, Nov 27, 2015 at 12:09:31PM -0800, Kees Cook wrote:
>> On Thu, Nov 26, 2015 at 8:39 AM, Quentin Casasnovas
>> <quentin.casasnovas@oracle.com> wrote:
>> > On Thu, Nov 26, 2015 at 12:45:42AM +0100, Quentin Casasnovas wrote:
>> >> On Tue, Nov 24, 2015 at 11:10:09AM -0800, Kees Cook wrote:
>> >>
>> >> [snip/]
>> >>
>> >> It should also be noted that I did not find that the struct thread_info
>> >> (which is stuffed at the end of the stack) was protected in any way either.
>> >> So even if a write/read _below_ the stack could still be trapped if nothing
>> >> is currently mapped there, it looks like deep stack usage could still
>> >> overflow it and go unoticed.  Here again, I didn't spend a lot of time on
>> >> this and it might just be that I'm missing something.
>> >>
>> >> In the very unlikely event where I didn't miss anything and the struct
>> >> thread_info can still be overflown and there isn't any guard page, maybe we
>> >> can improve on the current KSTACK_OVERFLOW feature by putting the struct
>> >> thread_info on a different page than the kernel stack, and not vmap() it
>> >> like the rest of the stack pages, but instead map a PROT_NONE page there.
>> >> That would mean the struct thread_info can still be accessed by using its
>> >> lowmem mapping (i.e. legit usage from the kernel) but not by deep kernel
>> >> stack usage.  Maybe the cost of adding an extra page per kernel stack is
>> >> too high though.
>> >
>> > As expected I missed some other changes:
>> >
>> > /* Load thread_info address into "reg" */
>> > #define GET_THREAD_INFO(reg) \
>> > -       _ASM_MOV PER_CPU_VAR(cpu_current_top_of_stack),reg ; \
>> > -       _ASM_SUB $(THREAD_SIZE),reg ;
>> > +       _ASM_MOV PER_CPU_VAR(current_tinfo),reg ;
>> >
>> > and
>> >
>> > +DECLARE_PER_CPU(struct thread_info *, current_tinfo);
>> > +
>> > static inline struct thread_info *current_thread_info(void)
>> > {
>> > -       return (struct thread_info *)(current_top_of_stack() - THREAD_SIZE);
>> > +       return this_cpu_read_stable(current_tinfo);
>> > }
>> >
>> > So no more thread_info on the stack in the default configuration, which
>> > isn't correlated with the KSTACKOVERFLOW config option.
>>
>> Good find! This seems like it should be its own patch, distinct from
>> KSTACKOVERFLOW?
>>
>
> We should probably make KSTACKOVERFLOW depend on moving the thread_info off
> the stack, since otherwise the thread_info could still be hijacked and that
> would counterfeit the KSTACKOVERFLOW purpose.

Right. Either a "select" or "depends" in the Kconfig seems appropriate.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [kernel-hardening] Re: status: GRKERNSEC_KSTACKOVERFLOW
  2015-11-27 20:23   ` Kees Cook
  2015-11-28  9:25     ` Quentin Casasnovas
@ 2016-01-20  0:03     ` Kees Cook
  2016-01-22 22:04       ` Quentin Casasnovas
  1 sibling, 1 reply; 15+ messages in thread
From: Kees Cook @ 2016-01-20  0:03 UTC (permalink / raw)
  To: Quentin Casasnovas; +Cc: kernel-hardening

On Fri, Nov 27, 2015 at 12:23 PM, Kees Cook <keescook@chromium.org> wrote:
> On Wed, Nov 25, 2015 at 3:45 PM, Quentin Casasnovas
> <quentin.casasnovas@oracle.com> wrote:
>> On Tue, Nov 24, 2015 at 11:10:09AM -0800, Kees Cook wrote:
>>> Hi,
>>>
>>
>> Hi Kees,
>>
>>> I just wanted to check in and see how progress was going on the stack
>>> overflow feature. Anything we can help with?
>>>
>>
>> Sorry for not following up on this, I've been busy and haven't had the time
>> to finish it properly.  I've pushed an initial WIP break up of the
>> KSTACK_OVERFLOW feature on my github:
>>
>>   https://github.com/casasnovas/linux/tree/quentin-split-kstackoverflow
>
> Great! Thanks for the update!

Hi Quentin,

Has anything moved on KSTACK_OVERFLOW? I'd love to start getting some
code tested if it's ready.

Thanks!

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [kernel-hardening] Re: status: GRKERNSEC_KSTACKOVERFLOW
  2016-01-20  0:03     ` Kees Cook
@ 2016-01-22 22:04       ` Quentin Casasnovas
  2016-04-21 20:31         ` Kees Cook
  0 siblings, 1 reply; 15+ messages in thread
From: Quentin Casasnovas @ 2016-01-22 22:04 UTC (permalink / raw)
  To: Kees Cook; +Cc: Quentin Casasnovas, kernel-hardening

On Tue, Jan 19, 2016 at 04:03:12PM -0800, Kees Cook wrote:
> On Fri, Nov 27, 2015 at 12:23 PM, Kees Cook <keescook@chromium.org> wrote:
> > On Wed, Nov 25, 2015 at 3:45 PM, Quentin Casasnovas
> > <quentin.casasnovas@oracle.com> wrote:
> >> On Tue, Nov 24, 2015 at 11:10:09AM -0800, Kees Cook wrote:
> >>> Hi,
> >>>
> >>
> >> Hi Kees,
> >>
> >>> I just wanted to check in and see how progress was going on the stack
> >>> overflow feature. Anything we can help with?
> >>>
> >>
> >> Sorry for not following up on this, I've been busy and haven't had the time
> >> to finish it properly.  I've pushed an initial WIP break up of the
> >> KSTACK_OVERFLOW feature on my github:
> >>
> >>   https://github.com/casasnovas/linux/tree/quentin-split-kstackoverflow
> >
> > Great! Thanks for the update!
> 
> Hi Quentin,
> 
> Has anything moved on KSTACK_OVERFLOW? I'd love to start getting some
> code tested if it's ready.
> 

Hi Kees,

Sorry I've been very bad at finding free time recenlty, being a new dad
takes more time than expected :)

I've got next week off so hopefully I can spend some time on this, starting
with moving the thread_info off the stack and then splitting properly the
rest of the KSTACK_OVERFLOW.

Quentin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [kernel-hardening] Re: status: GRKERNSEC_KSTACKOVERFLOW
  2016-01-22 22:04       ` Quentin Casasnovas
@ 2016-04-21 20:31         ` Kees Cook
  2016-04-27  8:30           ` Quentin Casasnovas
  0 siblings, 1 reply; 15+ messages in thread
From: Kees Cook @ 2016-04-21 20:31 UTC (permalink / raw)
  To: Quentin Casasnovas, Michael Leibowitz; +Cc: kernel-hardening

On Fri, Jan 22, 2016 at 2:04 PM, Quentin Casasnovas
<quentin.casasnovas@oracle.com> wrote:
> On Tue, Jan 19, 2016 at 04:03:12PM -0800, Kees Cook wrote:
>> On Fri, Nov 27, 2015 at 12:23 PM, Kees Cook <keescook@chromium.org> wrote:
>> > On Wed, Nov 25, 2015 at 3:45 PM, Quentin Casasnovas
>> > <quentin.casasnovas@oracle.com> wrote:
>> >> On Tue, Nov 24, 2015 at 11:10:09AM -0800, Kees Cook wrote:
>> >>> Hi,
>> >>>
>> >>
>> >> Hi Kees,
>> >>
>> >>> I just wanted to check in and see how progress was going on the stack
>> >>> overflow feature. Anything we can help with?
>> >>>
>> >>
>> >> Sorry for not following up on this, I've been busy and haven't had the time
>> >> to finish it properly.  I've pushed an initial WIP break up of the
>> >> KSTACK_OVERFLOW feature on my github:
>> >>
>> >>   https://github.com/casasnovas/linux/tree/quentin-split-kstackoverflow
>> >
>> > Great! Thanks for the update!
>>
>> Hi Quentin,
>>
>> Has anything moved on KSTACK_OVERFLOW? I'd love to start getting some
>> code tested if it's ready.
>>
>
> Hi Kees,
>
> Sorry I've been very bad at finding free time recenlty, being a new dad
> takes more time than expected :)

Congratulations! Yeah, seems to be a very time-consuming endeavor. But
you get a person out of it, so that's good. :)

> I've got next week off so hopefully I can spend some time on this, starting

I hope you used your week off for non-work things. :)

> with moving the thread_info off the stack and then splitting properly the
> rest of the KSTACK_OVERFLOW.

It seems like moving thread_info off the stack (and the gcc plugin
infrastructure) is a prerequisite for KRANDSTRUCT too, which Michael
has been looking at. If either of you have patches for this, I'd love
to get them on the list.

If you want to rebase off this, I've been tracking Emese's plugin work here:
http://git.kernel.org/cgit/linux/kernel/git/kees/linux.git/log/?h=kspp/gcc-plugins

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [kernel-hardening] Re: status: GRKERNSEC_KSTACKOVERFLOW
  2016-04-21 20:31         ` Kees Cook
@ 2016-04-27  8:30           ` Quentin Casasnovas
  0 siblings, 0 replies; 15+ messages in thread
From: Quentin Casasnovas @ 2016-04-27  8:30 UTC (permalink / raw)
  To: kernel-hardening; +Cc: Quentin Casasnovas, Michael Leibowitz

Hi Kees,

Wow, time just flew by.. !!  How are things?

On Thu, Apr 21, 2016 at 01:31:33PM -0700, Kees Cook wrote:
> On Fri, Jan 22, 2016 at 2:04 PM, Quentin Casasnovas
> >
> > Sorry I've been very bad at finding free time recenlty, being a new dad
> > takes more time than expected :)
> 
> Congratulations! Yeah, seems to be a very time-consuming endeavor. But
> you get a person out of it, so that's good. :)
>

Thanks! Oh yes. Right, it's the greatest feeling :)

> > I've got next week off so hopefully I can spend some time on this, starting
> 
> I hope you used your week off for non-work things. :)
>

I did, whoops.

> > with moving the thread_info off the stack and then splitting properly the
> > rest of the KSTACK_OVERFLOW.
> 
> It seems like moving thread_info off the stack (and the gcc plugin
> infrastructure) is a prerequisite for KRANDSTRUCT too, which Michael
> has been looking at. If either of you have patches for this, I'd love
> to get them on the list.

Gah I'm really sorry, I have no progress to communicate so far.  I didn't
really find the time in the past few months but I do intend to work on this
if nobody beats me to it!

To bo honest I was a bit put off by Brad's comments on his twitter account,
making fun of the few things I initially missed.  I realise now, given his
own reaction to mistakes, that I should not have.

> 
> If you want to rebase off this, I've been tracking Emese's plugin work here:
> http://git.kernel.org/cgit/linux/kernel/git/kees/linux.git/log/?h=kspp/gcc-plugins
>

Great thanks!  Pretty cool Dmitry's gcc patch was ported to its own plugin,
as I've mentioned in the past we've been using that here to run AFL on the
kernel, so it's great if it makes it to mainline!

Quentin

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-04-27  8:30 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-24 19:10 [kernel-hardening] status: GRKERNSEC_KSTACKOVERFLOW Kees Cook
2015-11-25 23:45 ` [kernel-hardening] " Quentin Casasnovas
2015-11-26 16:39   ` Quentin Casasnovas
2015-11-27 20:09     ` Kees Cook
2015-11-28  9:19       ` Quentin Casasnovas
2015-11-30 20:12         ` Kees Cook
2015-11-26 17:17   ` Quentin Casasnovas
2015-11-27 20:18     ` Kees Cook
2015-11-28  9:16       ` Quentin Casasnovas
2015-11-27 20:23   ` Kees Cook
2015-11-28  9:25     ` Quentin Casasnovas
2016-01-20  0:03     ` Kees Cook
2016-01-22 22:04       ` Quentin Casasnovas
2016-04-21 20:31         ` Kees Cook
2016-04-27  8:30           ` Quentin Casasnovas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.