This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/2] Linux/x86: Update cancel_jmp_buf to match __jmp_buf_tag [BZ #22563]


On Thu, Dec 7, 2017 at 6:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Dec 7, 2017 at 11:35 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Thu, Dec 7, 2017 at 11:25 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>> On 12/07/2017 08:19 PM, H.J. Lu wrote:
>>>>
>>>> On Thu, Dec 7, 2017 at 11:14 AM, Florian Weimer <fweimer@redhat.com>
>>>> wrote:
>>>>>
>>>>> On 12/07/2017 08:12 PM, H.J. Lu wrote:
>>>>>>>
>>>>>>>
>>>>>>> Sorry, what exactly is stored on the shadow stack?  I assumed it was
>>>>>>> for
>>>>>>> verification of the targets of ret instructions.
>>>>>>>
>>>>>>> In this case, don't need to unwind the shadow stack (or preserve its
>>>>>>> contents) because there are no returns from existing stack frames once
>>>>>>> cancellation has started.
>>>>>>>
>>>>>> Shadow stack is the similar to normal call stack without local
>>>>>> variables.
>>>>>> SHSTK checks  the return address of EACH  "RET"  instruction against
>>>>>> shadow stack.
>>>>>
>>>>>
>>>>>
>>>>> Then the shadow stack contents at the time of cancellation does not
>>>>> matter
>>>>> because all future RET instructions on this thread will match CALLs which
>>>>> happened *after* cancellation.  (In other words, I still think I'm right
>>>>> about this.)
>>>>>
>>>>
>>>> We are updating setjmp/lonjmp to save and restore shadow stack pointer:
>>>>
>>>>
>>>> https://sourceware.org/git/?p=glibc.git;a=commit;h=ac195a2d554e3fb577e44474faf3ed7f4521de9f
>>>
>>>
>>> Please try to understand what I wrote.  You don't need the restore during
>>> cancellation handling:
>>>
>>>     if (__glibc_unlikely (__not_first_call))          \
>>>       {                                               \
>>>         __cancel_routine (__cancel_arg);              \
>>>         __pthread_unwind_next (&__cancel_buf);        \
>>>         /* NOTREACHED */                              \
>>>       }                                               \
>>>
>>
>> Who will sync shadow stack with call stack?
>>
>
> Here is call stack during stack unwind:
>
> (gdb) bt
> #0  __longjmp () at ../sysdeps/unix/sysv/linux/x86_64/__longjmp.S:30
> #1  0x00007ffff7837f5f in __libc_siglongjmp (env=env@entry=0x7ffff7800da0,
>     val=val@entry=1) at longjmp.c:39
> #2  0x00007ffff7bc899d in unwind_stop (version=<optimized out>,
>     actions=<optimized out>, exc_class=<optimized out>,
>     exc_obj=<optimized out>, context=<optimized out>,
>     stop_parameter=0x7ffff7800da0) at unwind.c:94
> #3  0x00007ffff6df9b6e in _Unwind_ForcedUnwind_Phase2 (
>     exc=exc@entry=0x7ffff7801d70, context=context@entry=0x7ffff78005d0,
>     frames_p=frames_p@entry=0x7ffff78004d8)
>     at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:170
> #4  0x00007ffff6dfa1c0 in _Unwind_ForcedUnwind (exc=0x7ffff7801d70,
>     stop=stop@entry=0x7ffff7bc88e0 <unwind_stop>,
>     stop_argument=<optimized out>)
>     at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:217
> #5  0x00007ffff7bc8a84 in __GI___pthread_unwind (buf=<optimized out>)
>     at unwind.c:121
> #6  0x00007ffff7bbe46a in __do_cancel () at ./pthreadP.h:297
> #7  sigcancel_handler (sig=<optimized out>, si=0x7ffff7800870,
>     ctx=<optimized out>) at nptl-init.c:216
> #8  <signal handler called>
> #9  0x00007ffff7bc8f04 in __libc_read (fd=fd@entry=3,
>     buf=buf@entry=0x7ffff7800d30, nbytes=nbytes@entry=100)
> ---Type <return> to continue, or q <return> to quit---
>    sv/linux/read.c:27
> #10 0x00000000004064a0 in tf_read (arg=<optimized out>) at tst-cancel4.c:102
> #11 0x00007ffff7bbfcde in start_thread (arg=<optimized out>)
>     at pthread_create.c:463
> #12 0x00007ffff78f5f73 in clone ()
>     at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
> (gdb) f 10
> #10 0x00000000004064a0 in tf_read (arg=<optimized out>) at tst-cancel4.c:102
> 102   s = read (fd, buf, sizeof (buf));
> (gdb) list
> 97
> 98   ssize_t s;
> 99   pthread_cleanup_push (cl, NULL);
> 100
> 101   char buf[100];
> 102   s = read (fd, buf, sizeof (buf));
> 103
> 104   pthread_cleanup_pop (0);
> 105
> 106   FAIL_EXIT1 ("read returns with %zd", s);
> (gdb)
>
> # define pthread_cleanup_push(routine, arg) \
>   do {                                                                        \
>     __pthread_unwind_buf_t __cancel_buf;                                      \
>     void (*__cancel_routine) (void *) = (routine);                            \
>     void *__cancel_arg = (arg);                                               \
>     int __not_first_call = __sigsetjmp ((struct __jmp_buf_tag *) (void *)     \
>                                         __cancel_buf.__cancel_jmp_buf, 0);    \
>     if (__glibc_unlikely (__not_first_call))                                  \
>       {                                                                       \
>         __cancel_routine (__cancel_arg);                                      \
>         __pthread_unwind_next (&__cancel_buf);                                \
>         /* NOTREACHED */                                                      \
>       }
>
> To unwind shadow stack, we need to save shadow stack pointer in
> __cancel_buf.   This updated patch adds bits/types/__cancel_jmp_buf_tag.h
> to define struct __cancel_jmp_buf_tag so that Linux/x86 can add saved_mask
> to __cancel_jmp_buf.   We will check if shadow stack is enabled before saving
> and restoring shadow stack pointer so that it works with the old smaller
> cancel_jmp_buf which doesn't have space for shadow stack pointer.
>

Any comments?

-- 
H.J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]