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

[Carlos O'Donell <carlos@redhat.com>]

On 02/25/2018 07:55 PM, H.J. Lu wrote:
> On Sun, Feb 25, 2018 at 6:13 AM, Florian Weimer <fw@deneb.enyo.de> wrote:
>> * H. J. Lu:
>>
>>> On Sun, Feb 25, 2018 at 5:49 AM, Florian Weimer <fw@deneb.enyo.de> wrote:
>>>> * H. J. Lu:
>>>>
>>>>> On Sun, Feb 25, 2018 at 5:31 AM, Florian Weimer <fw@deneb.enyo.de> wrote:
>>>>>> * H. J. Lu:
>>>>>>
>>>>>>> libpthread cancellation implementation passes cancel_jmp_buf to
>>>>>>> libgcc unwinder,
>>>>>>
>>>>>> Oh.  Where does it do that?  If you mean _Unwind_ForcedUnwind, I think
>>>>>> that's just an opaque closure argument for the callback.
>>>>>
>>>>> Yes.  Libgcc unwinder needs to deal with it.
>>>>
>>>> Please point me to the code.  Thanks.
>>>
>>> sysdeps/nptl/unwind-forcedunwind.c has
>>>
>>> _Unwind_Reason_Code
>>> _Unwind_ForcedUnwind (struct _Unwind_Exception *exc, _Unwind_Stop_Fn stop,
>>>       void *stop_argument)
>>> {
>>>   if (__glibc_unlikely (libgcc_s_handle == NULL))
>>>     pthread_cancel_init ();
>>>   else
>>>     atomic_read_barrier ();
>>>
>>>   _Unwind_Reason_Code (*forcedunwind)
>>>     (struct _Unwind_Exception *, _Unwind_Stop_Fn, void *)
>>>     = libgcc_s_forcedunwind;
>>>   PTR_DEMANGLE (forcedunwind);
>>>   return forcedunwind (exc, stop, stop_argument);
>>> }
>>
>> Thanks.  I think stop_argument ends up in the private_2 member inside
>> unwind.inc, which is only passed back to the callback (the stop
>> function pointer) in _Unwind_ForcedUnwind_Phase2, and not interpreted
>> by libgcc itself.  So this shouldn't be a problem.
> 
> Please take a look at hjl/setjmp/cancel branch:
> 
> https://github.com/hjl-tools/glibc/tree/hjl/setjmp/cancel

OK.

> Functions, like LIBC_START_MAIN, START_THREAD_DEFN as well as these
> with thread cancellation, call setjmp, but never return to their
> callers after longjmp returns.  This patch adds <bits/setjmp-cancel.h>
> and <setjmp-cancelP.h> to provide a version of setjmp family functions,
> __setjmp_cancel and __sigsetjmp_cancel, which are used to implement
> thread cancellation.  The default __setjmp_cancel and __sigsetjmp_cancel
> are defined as setjmp and __sigsetjmp, respectively, which are the same
> as before.

> On x86, a different version is added to avoid saving and restoring
> shadow stack register.  __libc_longjmp, which is a private interface
> for cancellation implementation in libpthread, is changed to call
> __longjmp_cancel instead of __longjmp.

The consequence of this solution is that any function calls after the
first longjmp will continue to extend the shadow stack because there is
no restoration? I think this is OK, you effectively need to have enough
shadow stack to run through all of the unwind functions as if they were
nested frames on the shadow stack.

Do we see that being a problem?
 
> This leaves cancel_jmp_buf unchanged.  But is it worth it?

Absolutely it is worth it. This new design looks much simpler to support.

The technical trade is as follows:

* Three new versioned symbols for __sigsetjmp, __sigsetjmp_cancel,
  and __setjmp_cancel.

  * You can avoid versioning by placing the shadow stack pointer such
    that it is written into the pad[4] of the cancel_jmp_buf, and thus
    it would always have space to be written. However, it is cleaner
    to have cancel versions of these functions that do less.

vs.

* An ABI change that requires coordination across glibc, gcc, and
  binutils to ensure there is a "flag day" where the ABI can be correctly
  selected at runtime.

The latter is not easy to execute for downstream, and the versioned
symbols provide a good barrier in terms of compatibility and errors
reported by the dynamic loader.
 
> 1. I have to add  __setjmp_cancel and __sigsetjmp_cancel which won't
> save and restore shadow stack register.

OK.

> 2. I still need yet to add the new version of __sigsetjmp for older binaries.

OK.

> 3, Older .o files compiled against glibc 2.27 are still incompatible with
> glibc 2.28.
 
This is normal, and has never been assured.

-- 
Cheers,
Carlos.