Re: [PATCH 0/2] nptl: Update struct pthread_unwind_buf

["H.J. Lu" <hjl.tools@gmail.com>]

On Fri, Feb 9, 2018 at 2:48 AM, Florian Weimer <fw@deneb.enyo.de> wrote:
> * Carlos O'Donell:
>
>> In the truncated sigjmp_buf used by pthread cancellation we have only
>> void * which is not large enough on x32, where you need a 64-bit
>> shadow stack pointer. It would work on every other machine though.
>> HJ has mentioned this problem already IIRC.
>>
>> Why would __pthread_register_cancel overwrite it?
>
> __pthread_unwind_buf_t is defined as:
>
> typedef struct
> {
>   struct
>   {
>     __jmp_buf __cancel_jmp_buf;
>     int __mask_was_saved;
>   } __cancel_jmp_buf[1];
>   void *__pad[4];
> } __pthread_unwind_buf_t __attribute__ ((__aligned__));
>
> pthread_cleanup_push does this:
>
> # 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 */                                                      \
>       }                                                                       \
>                                                                               \
>     __pthread_register_cancel (&__cancel_buf);                                \
>     do {
>
> __pthread_register_cancel overwrites __cancel_buf.__pad.  If
> __sigsetjmp were to write to that memory area, it would not matter, as
> long as we skip restoring the shadow stack pointer during unwinding
> (which we do not need to do because we never return along the regular
> execution path recorded on the shadow stack).

That is true.

> In short, the only thing need to ensure is that the over-write from
> __sigsetjmp stays within __cancel_buf.  Then we are good, without
> changing the stack layout for cancellation.

That is correct.

> My proposal is still rather hackish, but so is the existing code (the

A pointer to a buffer in user program is passed to libpthread.  There is a
jmp buf in the buffer followed by other fields.  Since the size of jmp buf is
increased in glibc 2.28, we need to know the offset of other fields. Otherwise
libpthread may write beyond the buffer in user program.  I don't see how
symbol versioning can help us here since the INTERNAL libpthread functions
don't know the layout of __pthread_unwind_buf_t of USER programs.

> truncated jump buffer), and HJ's approach of storing the shadow stack
> pointer in the signal save area of the non-truncated jump buffer.  But
> I think we can make it work.



-- 
H.J.