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

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

On Fri, Feb 23, 2018 at 9:46 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 02/09/2018 04:34 AM, H.J. Lu wrote:
>> On Fri, Feb 9, 2018 at 4:11 AM, Florian Weimer <fw@deneb.enyo.de> wrote:
>>> * H. J. Lu:
>>>
>>>>> 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.
>>>
>>> I suggest *not* to increase the size of the jump buffer.
>>
>> Where do we save shadow stack pointer?
>
> typedef struct
> {
>   struct
>   {
>     __jmp_buf __cancel_jmp_buf;
>     int __mask_was_saved;
>   } __cancel_jmp_buf[1];
>
>
>   void *__pad[4];
>   ^^^^^^^^^^^^^^^ Save the shadow stack pointer here.
>
>
> } __pthread_unwind_buf_t __attribute__ ((__aligned__));
>
> Save the shadow stack pointer to __pad[4] by making the
> internal sigset_t smaller and moving it down.
>
> The key aspect of Florian's recommendation is a realization
> that a pthread_cleanup_pop can only restore you to the *same*
> function e.g. the earlier pthread_cleanup_push, and therefore
> does not need to change the shadow stack pointer.

PLEASE take a closer look:

Yes, there are

void *__pad[4];

But the name is misleading.   It isn't real padding.  This is
an opaque array:

/* Private data in the cleanup buffer.  */
union pthread_unwind_buf_data
{
  /* This is the placeholder of the public version.  */
  void *pad[4];

  struct
  {
    /* Pointer to the previous cleanup buffer.  */
    struct pthread_unwind_buf *prev;

    /* Backward compatibility: state of the old-style cleanup
       handler at the time of the previous new-style cleanup handler
       installment.  */
    struct _pthread_cleanup_buffer *cleanup;

    /* Cancellation type before the push call.  */
    int canceltype;
  } data;
};

Only the last element in __pad[4] is unused.  There is

---
Note: There is an unused pointer space in pthread_unwind_buf_data.  But
it isn't suitable for saving and restoring shadow stack register since
x32 is a 64-bit process with 32-bit software pointer and kernel may
place x32 shadow stack above 4GB.  We need to save and restore 64-bit
shadow stack register for x32.
---

in my commit log to explain why it isn't suitable for shadow stack.

-- 
H.J.