Re: V3 [PATCH 06/12] x86-64/CET: Extend ucontext_t to save shadow stack

[Carlos O'Donell <carlos at redhat dot com>]

On 07/25/2018 08:30 AM, H.J. Lu wrote:
> On Tue, Jul 24, 2018 at 09:47:50PM -0400, Carlos O'Donell wrote:
>> On 07/24/2018 06:38 PM, H.J. Lu wrote:
>>> On Tue, Jul 24, 2018 at 1:48 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>>>> On 07/21/2018 10:20 AM, H.J. Lu wrote:
>>>>> This patch adds a field to ucontext_t to save shadow stack:
>>>>>
>>>>> 1. getcontext and swapcontext are updated to save the caller's shadow
>>>>> stack pointer and return addresses.
>>>>> 2. setcontext and swapcontext are updated to restore shadow stack and
>>>>> jump to new context directly.
>>>>> 3. makecontext is updated to allocate a new shadow stack and set the
>>>>> caller's return address to __start_context.
>>>>>
>>>>> Since makecontext allocates a new shadow stack when making a new
>>>>> context and kernel allocates a new shadow stack for clone/fork/vfork
>>>>> syscalls, we keep track the lowest shadow stack base.  In setcontext
>>>>> and swapcontext, when searching for shadow stack restore token, if the
>>>>> lowest shadow stack base is reached, we assume both the current and
>>>>> target shadow stack pointers are on the same shadow stack.
>>>>>
>>>>> We enable shadow stack at run-time only if program and all used shared
>>>>> objects, including dlopened ones, are shadow stack enabled, which means
>>>>> that they must be compiled with GCC 8 or above and glibc 2.28 or above.
>>>>> We need to save and restore shadow stack only if shadow stack is enabled.
>>>>> When caller of getcontext, setcontext, swapcontext and makecontext is
>>>>> compiled with smaller ucontext_t, shadow stack won't be enabled at
>>>>> run-time.  We check if shadow stack is enabled before accessing the
>>>>> extended field in ucontext_t.
>>>>>
>>>> Right, this is a flag day ABI change.
>>>>
>>>>> 2018-05-21  Vedvyas Shanbhogue  <vedvyas.shanbhogue@intel.com>
>>>>>           H.J. Lu  <hongjiu.lu@intel.com>
>>>>>
>>>>>       * sysdeps/unix/sysv/linux/x86/sys/ucontext.h (ucontext_t): Add
>>>>>       __ssp.
>>>>>       * sysdeps/unix/sysv/linux/x86_64/__start_context.S: Include
>>>>>       <bits/prctl.h> and "ucontext_i.h" when shadow stack is enabled.
>>>>>       (__push___start_context): New.
>>>>>       * sysdeps/unix/sysv/linux/x86_64/getcontext.S (__getcontext):
>>>>>       Save the caller's shadow stack pointer when shadow stack is in
>>>>>       use.
>>>>>       * sysdeps/unix/sysv/linux/x86_64/makecontext.c Include
>>>>>       <pthread.h>, <libc-pointer-arith.h> and <sys/prctl.h>.
>>>>>       (__push___start_context): New prototype.
>>>>>       (__makecontext): Call __push___start_context to allocate a new
>>>>>       shadow stack, push __start_context onto the new stack as well
>>>>>       as the new shadow stack.  Set the lowest shadow stack base.
>>>>>       * sysdeps/unix/sysv/linux/x86_64/setcontext.S: Include
>>>>>       <bits/prctl.h>.
>>>>>       (__setcontext): Use the restore token to restore shadow stack
>>>>>       if available.  Otherwise unwind shadow stack.  Check if the
>>>>>       target shadow stack pointer came from __push___start_context.
>>>>>       Don't search for shadow stack restore token below the lowest
>>>>>       shadow stack base.
>>>>>       * sysdeps/unix/sysv/linux/x86_64/swapcontext.S: Include
>>>>>       <bits/prctl.h>.
>>>>>       (__swapcontext): Save the current shadow stack pointer.
>>>>>       Use the restore token to restore shadow stack if available.
>>>>>       Otherwise unwind shadow stack.  Check if the target shadow
>>>>>       stack pointer came from __push___start_context.  Don't search
>>>>>       for shadow stack restore token below the lowest shadow stack
>>>>>       base.
>>>>>       * sysdeps/unix/sysv/linux/x86_64/sysdep.h
>>>>>       (STACK_SIZE_TO_SHADOW_STACK_SIZE_SHIFT): New.
>>>>>       * sysdeps/unix/sysv/linux/x86_64/ucontext_i.sym (oSSP): New.
>>>>> ---
>>>> OK for 2.28, but should we increase the size we allocate in the
>>>> new structure?
>>>>
>>>> The hardest part to review was the CET token save/restore state
>>>> transitions between context changes and for example I still don't
>>>> clearly understand why we need to check if the restored state is
>>>> on the same shadow stack and unwind? Is this because we are trying
>>>> to unwind the shadow stack as often as possible to avoid overflow
>>>> (as we do in the unwind_shadow_stack/loop sequence). Why do we do
>>>> this unwind process?
>>>>
>>>> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>>>>
>>> Here is a revamp of x86-64 ucontext functions.  It should be
>>> much easier to read now.  The key things are:
>>>
>>> 1. We keep track the base and limit of the current shadow stack
>>> in tcbhead_t.
>>> 2.  In setcontext and swapcontext, if the target shadow stack
>>> pointer is above the current shadow stack pointer and below the upper
>>> limit of the current shadow stack, we unwind the shadow stack.
>>> Otherwise it is a stack switch and we look for a restore token to
>>> restore the target shadow stack.
>>>
>>> OK for master branch?
>>
>> OK for 2.28.
>>
>> The algorithm is basically the same but with a length, so this still
>> seems good to me.
> 
> Furthe similify the algorithm.  There is no need to check length.  If
> the target shadow stack base is the same as the current shadow stack
> base, we unwind the shadow stack.  Otherwise it is a stack switch and
> we look for a restore token to restore the target shadow stack.
> 
>>
>> Please simplify the Changelog, it is too verbose.
>>
>> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> 
> Here is the updated patch with shortened ChangeLog entries.
> 
> OK for master?

Yes, we are back to the original algorithm again for shadow stack
preservation across the *context() calls.

OK for 2.28.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>


> Thanks.
> 
> H.J.
> ---
> This patch adds a field to ucontext_t to save shadow stack:
> 
> 1. getcontext and swapcontext are updated to save the caller's shadow
> stack pointer and return addresses.
> 2. setcontext and swapcontext are updated to restore shadow stack and
> jump to new context directly.
> 3. makecontext is updated to allocate a new shadow stack and set the
> caller's return address to __start_context.
> 
> Since makecontext allocates a new shadow stack when making a new
> context and kernel allocates a new shadow stack for clone/fork/vfork
> syscalls, we track the current shadow stack base.  In setcontext and
> swapcontext, if the target shadow stack base is the same as the current
> shadow stack base, we unwind the shadow stack.  Otherwise it is a stack
> switch and we look for a restore token.
> 
> We enable shadow stack at run-time only if program and all used shared
> objects, including dlopened ones, are shadow stack enabled, which means
> that they must be compiled with GCC 8 or above and glibc 2.28 or above.
> We need to save and restore shadow stack only if shadow stack is enabled.
> When caller of getcontext, setcontext, swapcontext and makecontext is
> compiled with smaller ucontext_t, shadow stack won't be enabled at
> run-time.  We check if shadow stack is enabled before accessing the
> extended field in ucontext_t.
> 
> 2018-05-21  Vedvyas Shanbhogue  <vedvyas.shanbhogue@intel.com>
> 	    H.J. Lu  <hongjiu.lu@intel.com>
> 
> 	* sysdeps/unix/sysv/linux/x86/sys/ucontext.h (ucontext_t): Add
> 	__ssp.
> 	* sysdeps/unix/sysv/linux/x86_64/__start_context.S: Include
> 	<asm/prctl.h> and "ucontext_i.h" when shadow stack is enabled.
> 	(__push___start_context): New.
> 	* sysdeps/unix/sysv/linux/x86_64/getcontext.S: Include
> 	<asm/prctl.h>.
> 	(__getcontext): Record the current shadow stack base.  Save the
> 	caller's shadow stack pointer and base.
> 	* sysdeps/unix/sysv/linux/x86_64/makecontext.c: Include
> 	<pthread.h>, <libc-pointer-arith.h> and <sys/prctl.h>.
> 	(__push___start_context): New prototype.
> 	(__makecontext): Call __push___start_context to allocate a new
> 	shadow stack, push __start_context onto the new stack as well
> 	as the new shadow stack.
> 	* sysdeps/unix/sysv/linux/x86_64/setcontext.S: Include
> 	<asm/prctl.h>.
> 	(__setcontext): Restore the target shadow stack.
> 	* sysdeps/unix/sysv/linux/x86_64/swapcontext.S: Include
> 	<asm/prctl.h>.
> 	(__swapcontext): Record the current shadow stack base.  Save
> 	the caller's shadow stack pointer and base.  Restore the target
> 	shadow stack.
> 	* sysdeps/unix/sysv/linux/x86_64/sysdep.h
> 	(STACK_SIZE_TO_SHADOW_STACK_SIZE_SHIFT): New.
> 	* sysdeps/unix/sysv/linux/x86_64/ucontext_i.sym (oSSP): New.

This looks good to me.

Cheers,
Carlos.