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

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

On 02/08/2018 03:55 AM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> I would like to see an argument made for CET markup against versioning
>> __pthread_register_cleanup.
> 
> I don't think __pthread_register_cleanup.  It's __sigsetjmp.  We would
> have to version __sigsetjmp.  Changing the name would be ideal, but
> this will be difficult because of its returns-twice nature.

I'm looking for a solution that yields a clearly diagnosable failure
when users mix a compiler which emits CET markup and a glibc which has
the smaller sized cancellation jump buffer.

If we version __sigsetjmp, we need to decide the semantics of the old
and new version.

A naive old version would not save the shadow stack pointer to the
reclaimed space after the newer smaller internal __sigset_t, and a newer
version would save the shadow stack pointer.

This leads to sigsegv when a user mixes as above by accident, and they
will have no warning about the problem. The CET markup will be present
in all objects they have, but when run with a newer glibc it will crash
by corrupting the stack when it writes the shadow stack pointer.

My suggestion with __pthread_register_cancel et.al. (and yes we would 
have to version all the interfaces) was to basically data version the
structure, but my original idea revolved around the idea of shifting
__pad[3] up to the start of the structure and use it for versioning.

However, your idea to version __sigsetjmp has reminded me that
__saved_mask need only be non-zero to work properly and we could
store a version cookie there, which could allow us to do the following:

* Version __sigsetjmp, and store a version cookie in __saved_mask
  indicating the version of the structure.
* Have the old __sigsetjmp disable CET since it clearly indicates
  that we have the wrong sized structure regardless of CET flags.
* Adjust the unwinder to look at __saved_mask version cooke to decide
  which sized structure it is dealing with.

Is this feasible? It would give high quality diagnostics and potentially
allow us to extend the cancellation jump buffer with more data in the
future.

> I also don't have a problem with requiring -fexceptions for
> cancellation handlers with CET support.  For additional safety, we
> could change sigsetjmp to write the shadow stack pointer before the
> kernel signal mask, so that it will still be in bounds for legacy
> cancellation handler allocation.  __pthread_register_cleanup will
> overwrite it, but I don't think we ever need to restore it, so that
> shouldn't be a problem.
 
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?

-- 
Cheers,
Carlos.