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

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

On Mon, Dec 18, 2017 at 2:25 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 12/08/2017 03:25 AM, H.J. Lu wrote:
>>
>> Here is call stack during stack unwind:
>>
>> (gdb) bt
>
>
> (snip)
>
>> 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.
>
>
> I still don't understand why you think you have to reset the shadow stack.
>
> I used this test program:
>
> #include <err.h>
> #include <errno.h>
> #include <pthread.h>
> #include <stdbool.h>
> #include <stdio.h>
> #include <unistd.h>
>
> __attribute__ ((noinline, noclone, weak))
> void
> handler1 (void *closure)
> {
>   printf ("handler1 called\n");
> }
>
> __attribute__ ((noinline, noclone, weak))
> void
> handler2 (void *closure)
> {
>   printf ("handler2 called\n");
> }
>
> __attribute__ ((noinline, noclone, weak))
> void
> pausefunc (void)
> {
>   while (true)
>     pause ();
> }
>
> __attribute__ ((noinline, noclone, weak))
> void
> handlerfunc (void)
> {
>   pthread_cleanup_push (handler2, NULL);
>   pausefunc ();
>   pthread_cleanup_pop (1);
> }
>
>
> __attribute__ ((noinline, noclone, weak))
> void *
> threadfunc (void *closure)
> {
>   pthread_cleanup_push (handler1, NULL);
>   handlerfunc ();
>   pthread_cleanup_pop (0);
>   return NULL;
> }
>
> int
> main (void)
> {
>   pthread_t thr;
>   int ret = pthread_create (&thr, NULL, threadfunc, NULL);
>   if (ret != 0)
>     {
>       errno = ret;
>       err (1, "pthread_create");
>     }
>
>   ret = pthread_cancel (thr);
>   if (ret != 0)
>     {
>       errno = ret;
>       err (1, "pthread_cancel");
>     }
>
>   void *result;
>   ret = pthread_join (thr, &result);
>   if (ret != 0)
>     {
>       errno = ret;
>       err (1, "pthread_join");
>     }
>   if (result != PTHREAD_CANCELED)
>     errx (1, "pthread_join did not return PTHREAD_CANCEL, but %p", result);
>
>   return 0;
> }
>
> See the attached GDB log.  As you can see, I set breakpoints on all
> pre-existing RET instructions on the call stack (which would be protected by
> the shadow stack with CET).  None of the RET instructions actually execute,
> ergo we do not have to restore the shadow stack.
>

Shadow stack is invisible to programs.  Shadow stack instructions are
used to maintain shadow stack, like what my patch does.   Any times
there is a "call" instruction, the return address is pushed onto shadow stack.
In your case,

gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) y
Starting program: /tmp/a.out
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Breakpoint 1, main () at x.c:52
52 {
(gdb) ena 2
(gdb) c
Continuing.
[New Thread 0x7ffff77d2700 (LWP 12779)]

Thread 1 "a.out" hit Breakpoint 2, 0x00007ffff780a320 in __sigsetjmp ()
   from /lib64/libc.so.6
(gdb) bt
#0  0x00007ffff780a320 in __sigsetjmp () from /lib64/libc.so.6

We need to save stack stack pointer here in __sigsetjmp,

#1  0x00007ffff7bbd5c0 in start_thread () from /lib64/libpthread.so.0
#2  0x00007ffff78ea88f in clone () from /lib64/libc.so.6
(gdb) dis 2
(gdb) ena 3
(gdb) c
Continuing.
[Switching to Thread 0x7ffff77d2700 (LWP 12779)]

Thread 2 "a.out" hit Breakpoint 3, 0x00007ffff780a410 in __longjmp ()
   from /lib64/libc.so.6
Missing separate debuginfos, use: dnf debuginfo-install
libgcc-7.2.1-4.0.fc27.x86_64
(gdb) bt
#0  0x00007ffff780a410 in __longjmp () from /lib64/libc.so.6

We need to restore shadow stack pointer here so that we can jump back
to the function where __sigsetjmp  is called.

#1  0x00007ffff780a3fb in siglongjmp () from /lib64/libc.so.6
#2  0x00007ffff7bc707d in unwind_stop () from /lib64/libpthread.so.0
#3  0x00007ffff6dcaf2e in ?? () from /lib64/libgcc_s.so.1
#4  0x00007ffff6dcb515 in _Unwind_ForcedUnwind () from /lib64/libgcc_s.so.1
#5  0x00007ffff7bc7180 in __pthread_unwind () from /lib64/libpthread.so.0
#6  0x00007ffff7bbbc82 in sigcancel_handler () from /lib64/libpthread.so.0
#7  <signal handler called>
#8  0x00007ffff7bc8052 in pause () from /lib64/libpthread.so.0
#9  0x000000000040098d in pausefunc () at x.c:27
#10 0x00000000004009af in handlerfunc () at x.c:35
#11 0x00000000004009ff in threadfunc (closure=<optimized out>) at x.c:45
#12 0x00007ffff7bbd5f9 in start_thread () from /lib64/libpthread.so.0
#13 0x00007ffff78ea88f in clone () from /lib64/libc.so.6
(gdb)

As long as there is a call stack, there is a shadow stack if shadow stack is
enabled.

Does it answer your question?

-- 
H.J.