Message ID | CAMe9rOqkygr2gvrr6wjV-s0NEaw5juDyx9eju352C0wRb2_r2A@mail.gmail.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 8994 invoked by alias); 8 Dec 2017 02:25:53 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 8983 invoked by uid 89); 8 Dec 2017 02:25:52 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-oi0-f65.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=v4xn+UQ/0pO3KmNYhIPYOlTf7tZ8R/tVGMp4ofqHhe0=; b=fiUBgwrZzqE70ASjW0pVikHjTLJzwMEpAANCpj0OxCu0t+Y48dp+ZucVmetBRSj9Sp CzNYlk2BQw8hIoPt0He0rTezqoIhozs8TIUlh8C3IZLumkdfx9/G6n+WC5DEtXzGRePB CZPz63L+Rm3r3bA7nskM2uPtVL4W3elQr9CNQctVOETv2MWcaDsE3zDU8aNDGFCRZdwi S4bCNOUtPtWGZNgWQazUvUr/GxjXu3Cc52n5Y8+RTl50qScUWzrhOBVShUoMLnzOIrkF yAUj881Gu5sI6GUhAd4x85x5KYHUlxjWOHpIDtcnAfUEaTWtEh+ukrY/pptGF1ccCz+A 0PiA== X-Gm-Message-State: AJaThX7Gjy0TsVIEigO6FGLUjDnbg7KQ4euoVtS1dwEXXxWFhizwB1Xq 7Ki1Ngq1TbrkrzUdj9rm1fWiXLszsddf9UFS1Mg= X-Google-Smtp-Source: AGs4zMZgzKYDH0pst/MD4t3L2D4UMZ/ZgR+I94zT3MIP2s/5ck9qJBGnwXsGrUA2LtY1FsS4/FLTsC5FoqIRRxV4vRM= X-Received: by 10.202.253.73 with SMTP id b70mr24822669oii.279.1512699947743; Thu, 07 Dec 2017 18:25:47 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <CAMe9rOp2V3ZkdfwCyO6c7-574sQHNdkMiUC1JVtvAZwanbTsow@mail.gmail.com> References: <20171207174057.GA32196@gmail.com> <e3100ba9-587a-cbad-1b49-0b57f3d2457c@redhat.com> <CAMe9rOoMjoZOO-rVHeJWGh_jWmRqw4aLS_RDWCfE2L18iUdOGQ@mail.gmail.com> <7e890f53-c331-d86e-ad13-b380a69d99eb@redhat.com> <CAMe9rOrBtpjUsXcnbnFMZi5dc46RWvRd6cMH3qnSFbHbprkvZg@mail.gmail.com> <814691a7-d946-1794-d6d8-7861f9ed2067@redhat.com> <CAMe9rOqYvgRYifxq8CGFsBCGcVS0FAcoa+gwZ5m4CyMQVpTWWg@mail.gmail.com> <08cf7c54-8bf0-27c0-863a-65cb76dd0728@redhat.com> <CAMe9rOp2V3ZkdfwCyO6c7-574sQHNdkMiUC1JVtvAZwanbTsow@mail.gmail.com> From: "H.J. Lu" <hjl.tools@gmail.com> Date: Thu, 7 Dec 2017 18:25:47 -0800 Message-ID: <CAMe9rOqkygr2gvrr6wjV-s0NEaw5juDyx9eju352C0wRb2_r2A@mail.gmail.com> Subject: Re: [PATCH 1/2] Linux/x86: Update cancel_jmp_buf to match __jmp_buf_tag [BZ #22563] To: Florian Weimer <fweimer@redhat.com> Cc: GNU C Library <libc-alpha@sourceware.org> Content-Type: multipart/mixed; boundary="001a1147780009a550055fcae7b9" |
Commit Message
H.J. Lu
Dec. 8, 2017, 2:25 a.m. UTC
On Thu, Dec 7, 2017 at 11:35 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Dec 7, 2017 at 11:25 AM, Florian Weimer <fweimer@redhat.com> wrote: >> On 12/07/2017 08:19 PM, H.J. Lu wrote: >>> >>> On Thu, Dec 7, 2017 at 11:14 AM, Florian Weimer <fweimer@redhat.com> >>> wrote: >>>> >>>> On 12/07/2017 08:12 PM, H.J. Lu wrote: >>>>>> >>>>>> >>>>>> Sorry, what exactly is stored on the shadow stack? I assumed it was >>>>>> for >>>>>> verification of the targets of ret instructions. >>>>>> >>>>>> In this case, don't need to unwind the shadow stack (or preserve its >>>>>> contents) because there are no returns from existing stack frames once >>>>>> cancellation has started. >>>>>> >>>>> Shadow stack is the similar to normal call stack without local >>>>> variables. >>>>> SHSTK checks the return address of EACH "RET" instruction against >>>>> shadow stack. >>>> >>>> >>>> >>>> Then the shadow stack contents at the time of cancellation does not >>>> matter >>>> because all future RET instructions on this thread will match CALLs which >>>> happened *after* cancellation. (In other words, I still think I'm right >>>> about this.) >>>> >>> >>> We are updating setjmp/lonjmp to save and restore shadow stack pointer: >>> >>> >>> https://sourceware.org/git/?p=glibc.git;a=commit;h=ac195a2d554e3fb577e44474faf3ed7f4521de9f >> >> >> Please try to understand what I wrote. You don't need the restore during >> cancellation handling: >> >> if (__glibc_unlikely (__not_first_call)) \ >> { \ >> __cancel_routine (__cancel_arg); \ >> __pthread_unwind_next (&__cancel_buf); \ >> /* NOTREACHED */ \ >> } \ >> > > Who will sync shadow stack with call stack? > Here is call stack during stack unwind: (gdb) bt #0 __longjmp () at ../sysdeps/unix/sysv/linux/x86_64/__longjmp.S:30 #1 0x00007ffff7837f5f in __libc_siglongjmp (env=env@entry=0x7ffff7800da0, val=val@entry=1) at longjmp.c:39 #2 0x00007ffff7bc899d in unwind_stop (version=<optimized out>, actions=<optimized out>, exc_class=<optimized out>, exc_obj=<optimized out>, context=<optimized out>, stop_parameter=0x7ffff7800da0) at unwind.c:94 #3 0x00007ffff6df9b6e in _Unwind_ForcedUnwind_Phase2 ( exc=exc@entry=0x7ffff7801d70, context=context@entry=0x7ffff78005d0, frames_p=frames_p@entry=0x7ffff78004d8) at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:170 #4 0x00007ffff6dfa1c0 in _Unwind_ForcedUnwind (exc=0x7ffff7801d70, stop=stop@entry=0x7ffff7bc88e0 <unwind_stop>, stop_argument=<optimized out>) at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:217 #5 0x00007ffff7bc8a84 in __GI___pthread_unwind (buf=<optimized out>) at unwind.c:121 #6 0x00007ffff7bbe46a in __do_cancel () at ./pthreadP.h:297 #7 sigcancel_handler (sig=<optimized out>, si=0x7ffff7800870, ctx=<optimized out>) at nptl-init.c:216 #8 <signal handler called> #9 0x00007ffff7bc8f04 in __libc_read (fd=fd@entry=3, buf=buf@entry=0x7ffff7800d30, nbytes=nbytes@entry=100) ---Type <return> to continue, or q <return> to quit--- sv/linux/read.c:27 #10 0x00000000004064a0 in tf_read (arg=<optimized out>) at tst-cancel4.c:102 #11 0x00007ffff7bbfcde in start_thread (arg=<optimized out>) at pthread_create.c:463 #12 0x00007ffff78f5f73 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 (gdb) f 10 #10 0x00000000004064a0 in tf_read (arg=<optimized out>) at tst-cancel4.c:102 102 s = read (fd, buf, sizeof (buf)); (gdb) list 97 98 ssize_t s; 99 pthread_cleanup_push (cl, NULL); 100 101 char buf[100]; 102 s = read (fd, buf, sizeof (buf)); 103 104 pthread_cleanup_pop (0); 105 106 FAIL_EXIT1 ("read returns with %zd", s); (gdb) # 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 */ \ } 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.
Comments
On Thu, Dec 7, 2017 at 6:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Dec 7, 2017 at 11:35 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Thu, Dec 7, 2017 at 11:25 AM, Florian Weimer <fweimer@redhat.com> wrote: >>> On 12/07/2017 08:19 PM, H.J. Lu wrote: >>>> >>>> On Thu, Dec 7, 2017 at 11:14 AM, Florian Weimer <fweimer@redhat.com> >>>> wrote: >>>>> >>>>> On 12/07/2017 08:12 PM, H.J. Lu wrote: >>>>>>> >>>>>>> >>>>>>> Sorry, what exactly is stored on the shadow stack? I assumed it was >>>>>>> for >>>>>>> verification of the targets of ret instructions. >>>>>>> >>>>>>> In this case, don't need to unwind the shadow stack (or preserve its >>>>>>> contents) because there are no returns from existing stack frames once >>>>>>> cancellation has started. >>>>>>> >>>>>> Shadow stack is the similar to normal call stack without local >>>>>> variables. >>>>>> SHSTK checks the return address of EACH "RET" instruction against >>>>>> shadow stack. >>>>> >>>>> >>>>> >>>>> Then the shadow stack contents at the time of cancellation does not >>>>> matter >>>>> because all future RET instructions on this thread will match CALLs which >>>>> happened *after* cancellation. (In other words, I still think I'm right >>>>> about this.) >>>>> >>>> >>>> We are updating setjmp/lonjmp to save and restore shadow stack pointer: >>>> >>>> >>>> https://sourceware.org/git/?p=glibc.git;a=commit;h=ac195a2d554e3fb577e44474faf3ed7f4521de9f >>> >>> >>> Please try to understand what I wrote. You don't need the restore during >>> cancellation handling: >>> >>> if (__glibc_unlikely (__not_first_call)) \ >>> { \ >>> __cancel_routine (__cancel_arg); \ >>> __pthread_unwind_next (&__cancel_buf); \ >>> /* NOTREACHED */ \ >>> } \ >>> >> >> Who will sync shadow stack with call stack? >> > > Here is call stack during stack unwind: > > (gdb) bt > #0 __longjmp () at ../sysdeps/unix/sysv/linux/x86_64/__longjmp.S:30 > #1 0x00007ffff7837f5f in __libc_siglongjmp (env=env@entry=0x7ffff7800da0, > val=val@entry=1) at longjmp.c:39 > #2 0x00007ffff7bc899d in unwind_stop (version=<optimized out>, > actions=<optimized out>, exc_class=<optimized out>, > exc_obj=<optimized out>, context=<optimized out>, > stop_parameter=0x7ffff7800da0) at unwind.c:94 > #3 0x00007ffff6df9b6e in _Unwind_ForcedUnwind_Phase2 ( > exc=exc@entry=0x7ffff7801d70, context=context@entry=0x7ffff78005d0, > frames_p=frames_p@entry=0x7ffff78004d8) > at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:170 > #4 0x00007ffff6dfa1c0 in _Unwind_ForcedUnwind (exc=0x7ffff7801d70, > stop=stop@entry=0x7ffff7bc88e0 <unwind_stop>, > stop_argument=<optimized out>) > at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:217 > #5 0x00007ffff7bc8a84 in __GI___pthread_unwind (buf=<optimized out>) > at unwind.c:121 > #6 0x00007ffff7bbe46a in __do_cancel () at ./pthreadP.h:297 > #7 sigcancel_handler (sig=<optimized out>, si=0x7ffff7800870, > ctx=<optimized out>) at nptl-init.c:216 > #8 <signal handler called> > #9 0x00007ffff7bc8f04 in __libc_read (fd=fd@entry=3, > buf=buf@entry=0x7ffff7800d30, nbytes=nbytes@entry=100) > ---Type <return> to continue, or q <return> to quit--- > sv/linux/read.c:27 > #10 0x00000000004064a0 in tf_read (arg=<optimized out>) at tst-cancel4.c:102 > #11 0x00007ffff7bbfcde in start_thread (arg=<optimized out>) > at pthread_create.c:463 > #12 0x00007ffff78f5f73 in clone () > at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 > (gdb) f 10 > #10 0x00000000004064a0 in tf_read (arg=<optimized out>) at tst-cancel4.c:102 > 102 s = read (fd, buf, sizeof (buf)); > (gdb) list > 97 > 98 ssize_t s; > 99 pthread_cleanup_push (cl, NULL); > 100 > 101 char buf[100]; > 102 s = read (fd, buf, sizeof (buf)); > 103 > 104 pthread_cleanup_pop (0); > 105 > 106 FAIL_EXIT1 ("read returns with %zd", s); > (gdb) > > # 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 */ \ > } > > 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. > Any comments?
On Thu, Dec 14, 2017 at 5:06 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Dec 7, 2017 at 6:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Thu, Dec 7, 2017 at 11:35 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> On Thu, Dec 7, 2017 at 11:25 AM, Florian Weimer <fweimer@redhat.com> wrote: >>>> On 12/07/2017 08:19 PM, H.J. Lu wrote: >>>>> >>>>> On Thu, Dec 7, 2017 at 11:14 AM, Florian Weimer <fweimer@redhat.com> >>>>> wrote: >>>>>> >>>>>> On 12/07/2017 08:12 PM, H.J. Lu wrote: >>>>>>>> >>>>>>>> >>>>>>>> Sorry, what exactly is stored on the shadow stack? I assumed it was >>>>>>>> for >>>>>>>> verification of the targets of ret instructions. >>>>>>>> >>>>>>>> In this case, don't need to unwind the shadow stack (or preserve its >>>>>>>> contents) because there are no returns from existing stack frames once >>>>>>>> cancellation has started. >>>>>>>> >>>>>>> Shadow stack is the similar to normal call stack without local >>>>>>> variables. >>>>>>> SHSTK checks the return address of EACH "RET" instruction against >>>>>>> shadow stack. >>>>>> >>>>>> >>>>>> >>>>>> Then the shadow stack contents at the time of cancellation does not >>>>>> matter >>>>>> because all future RET instructions on this thread will match CALLs which >>>>>> happened *after* cancellation. (In other words, I still think I'm right >>>>>> about this.) >>>>>> >>>>> >>>>> We are updating setjmp/lonjmp to save and restore shadow stack pointer: >>>>> >>>>> >>>>> https://sourceware.org/git/?p=glibc.git;a=commit;h=ac195a2d554e3fb577e44474faf3ed7f4521de9f >>>> >>>> >>>> Please try to understand what I wrote. You don't need the restore during >>>> cancellation handling: >>>> >>>> if (__glibc_unlikely (__not_first_call)) \ >>>> { \ >>>> __cancel_routine (__cancel_arg); \ >>>> __pthread_unwind_next (&__cancel_buf); \ >>>> /* NOTREACHED */ \ >>>> } \ >>>> >>> >>> Who will sync shadow stack with call stack? >>> >> >> Here is call stack during stack unwind: >> >> (gdb) bt >> #0 __longjmp () at ../sysdeps/unix/sysv/linux/x86_64/__longjmp.S:30 >> #1 0x00007ffff7837f5f in __libc_siglongjmp (env=env@entry=0x7ffff7800da0, >> val=val@entry=1) at longjmp.c:39 >> #2 0x00007ffff7bc899d in unwind_stop (version=<optimized out>, >> actions=<optimized out>, exc_class=<optimized out>, >> exc_obj=<optimized out>, context=<optimized out>, >> stop_parameter=0x7ffff7800da0) at unwind.c:94 >> #3 0x00007ffff6df9b6e in _Unwind_ForcedUnwind_Phase2 ( >> exc=exc@entry=0x7ffff7801d70, context=context@entry=0x7ffff78005d0, >> frames_p=frames_p@entry=0x7ffff78004d8) >> at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:170 >> #4 0x00007ffff6dfa1c0 in _Unwind_ForcedUnwind (exc=0x7ffff7801d70, >> stop=stop@entry=0x7ffff7bc88e0 <unwind_stop>, >> stop_argument=<optimized out>) >> at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:217 >> #5 0x00007ffff7bc8a84 in __GI___pthread_unwind (buf=<optimized out>) >> at unwind.c:121 >> #6 0x00007ffff7bbe46a in __do_cancel () at ./pthreadP.h:297 >> #7 sigcancel_handler (sig=<optimized out>, si=0x7ffff7800870, >> ctx=<optimized out>) at nptl-init.c:216 >> #8 <signal handler called> >> #9 0x00007ffff7bc8f04 in __libc_read (fd=fd@entry=3, >> buf=buf@entry=0x7ffff7800d30, nbytes=nbytes@entry=100) >> ---Type <return> to continue, or q <return> to quit--- >> sv/linux/read.c:27 >> #10 0x00000000004064a0 in tf_read (arg=<optimized out>) at tst-cancel4.c:102 >> #11 0x00007ffff7bbfcde in start_thread (arg=<optimized out>) >> at pthread_create.c:463 >> #12 0x00007ffff78f5f73 in clone () >> at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 >> (gdb) f 10 >> #10 0x00000000004064a0 in tf_read (arg=<optimized out>) at tst-cancel4.c:102 >> 102 s = read (fd, buf, sizeof (buf)); >> (gdb) list >> 97 >> 98 ssize_t s; >> 99 pthread_cleanup_push (cl, NULL); >> 100 >> 101 char buf[100]; >> 102 s = read (fd, buf, sizeof (buf)); >> 103 >> 104 pthread_cleanup_pop (0); >> 105 >> 106 FAIL_EXIT1 ("read returns with %zd", s); >> (gdb) >> >> # 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 */ \ >> } >> >> 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. >> > > Any comments? > I will check it in next Monday.
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. Thanks, Florian gdb ./simple-cancel GNU gdb (GDB) Fedora 8.0.1-33.fc26 Copyright (C) 2017 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Type "show copying" and "show warranty" for details. This GDB was configured as "x86_64-redhat-linux-gnu". Type "show configuration" for configuration details. For bug reporting instructions, please see: <http://www.gnu.org/software/gdb/bugs/>. Find the GDB manual and other documentation resources online at: <http://www.gnu.org/software/gdb/documentation/>. For help, type "help". Type "apropos word" to search for commands related to "word"... Reading symbols from ./simple-cancel...done. (gdb) r Starting program: /home/fweimer/tmp/simple-cancel [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib64/libthread_db.so.1". [New Thread 0x7ffff77e1700 (LWP 12256)] handler2 called handler1 called [Thread 0x7ffff77e1700 (LWP 12256) exited] [Inferior 1 (process 12252) exited normally] Missing separate debuginfos, use: dnf debuginfo-install libgcc-7.2.1-2.fc26.x86_64 (gdb) break sigcancel_handler Breakpoint 1 at 0x7ffff7bbc960: file nptl-init.c, line 187. (gdb) r Starting program: /home/fweimer/tmp/simple-cancel [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib64/libthread_db.so.1". [New Thread 0x7ffff77e1700 (LWP 12258)] [Switching to Thread 0x7ffff77e1700 (LWP 12258)] Thread 2 "simple-cancel" hit Breakpoint 1, sigcancel_handler (sig=32, si=0x7ffff77e09b0, ctx=0x7ffff77e0880) at nptl-init.c:187 187 if (sig != SIGCANCEL (gdb) bt #0 sigcancel_handler (sig=32, si=0x7ffff77e09b0, ctx=0x7ffff77e0880) at nptl-init.c:187 #1 <signal handler called> #2 0x00007ffff7bc89ed in pause () at ../sysdeps/unix/syscall-template.S:84 #3 0x000000000040098d in pausefunc () at simple-cancel.c:27 #4 0x00000000004009af in handlerfunc () at simple-cancel.c:35 #5 0x00000000004009ff in threadfunc (closure=<optimized out>) at simple-cancel.c:45 #6 0x00007ffff7bbe36d in start_thread (arg=0x7ffff77e1700) at pthread_create.c:456 #7 0x00007ffff78f2e1f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:97 (gdb) up #1 <signal handler called> (gdb) down #0 sigcancel_handler (sig=32, si=0x7ffff77e09b0, ctx=0x7ffff77e0880) at nptl-init.c:187 187 if (sig != SIGCANCEL (gdb) disas Dump of assembler code for function sigcancel_handler: => 0x00007ffff7bbc960 <+0>: cmp $0x20,%edi 0x00007ffff7bbc963 <+3>: je 0x7ffff7bbc970 <sigcancel_handler+16> 0x00007ffff7bbc965 <+5>: repz retq 0x00007ffff7bbc967 <+7>: nopw 0x0(%rax,%rax,1) 0x00007ffff7bbc970 <+16>: push %rbp 0x00007ffff7bbc971 <+17>: push %rbx 0x00007ffff7bbc972 <+18>: mov %rsi,%rbx 0x00007ffff7bbc975 <+21>: sub $0x8,%rsp 0x00007ffff7bbc979 <+25>: mov 0x10(%rsi),%ebp 0x00007ffff7bbc97c <+28>: callq 0x7ffff7bbc670 0x00007ffff7bbc981 <+33>: cmp %eax,%ebp 0x00007ffff7bbc983 <+35>: je 0x7ffff7bbc990 <sigcancel_handler+48> 0x00007ffff7bbc985 <+37>: add $0x8,%rsp 0x00007ffff7bbc989 <+41>: pop %rbx 0x00007ffff7bbc98a <+42>: pop %rbp 0x00007ffff7bbc98b <+43>: retq 0x00007ffff7bbc98c <+44>: nopl 0x0(%rax) 0x00007ffff7bbc990 <+48>: cmpl $0xfffffffa,0x8(%rbx) 0x00007ffff7bbc994 <+52>: jne 0x7ffff7bbc985 <sigcancel_handler+37> 0x00007ffff7bbc996 <+54>: mov %fs:0x308,%edx 0x00007ffff7bbc99e <+62>: jmp 0x7ffff7bbc9b7 <sigcancel_handler+87> 0x00007ffff7bbc9a0 <+64>: test $0x10,%dl 0x00007ffff7bbc9a3 <+67>: jne 0x7ffff7bbc985 <sigcancel_handler+37> 0x00007ffff7bbc9a5 <+69>: mov %edx,%eax 0x00007ffff7bbc9a7 <+71>: lock cmpxchg %ecx,%fs:0x308 0x00007ffff7bbc9b1 <+81>: cmp %eax,%edx 0x00007ffff7bbc9b3 <+83>: je 0x7ffff7bbc9c8 <sigcancel_handler+104> 0x00007ffff7bbc9b5 <+85>: mov %eax,%edx 0x00007ffff7bbc9b7 <+87>: mov %edx,%ecx 0x00007ffff7bbc9b9 <+89>: or $0xc,%ecx 0x00007ffff7bbc9bc <+92>: cmp %ecx,%edx 0x00007ffff7bbc9be <+94>: jne 0x7ffff7bbc9a0 <sigcancel_handler+64> 0x00007ffff7bbc9c0 <+96>: jmp 0x7ffff7bbc985 <sigcancel_handler+37> 0x00007ffff7bbc9c2 <+98>: nopw 0x0(%rax,%rax,1) 0x00007ffff7bbc9c8 <+104>: movq $0xffffffffffffffff,%fs:0x630 0x00007ffff7bbc9d5 <+117>: and $0x2,%edx 0x00007ffff7bbc9d8 <+120>: je 0x7ffff7bbc985 <sigcancel_handler+37> 0x00007ffff7bbc9da <+122>: lock orl $0x10,%fs:0x308 0x00007ffff7bbc9e4 <+132>: mov %fs:0x300,%rdi 0x00007ffff7bbc9ed <+141>: callq 0x7ffff7bc7e60 <__GI___pthread_unwind> End of assembler dump. (gdb) break *0x00007ffff7bbc965 Breakpoint 2 at 0x7ffff7bbc965: file nptl-init.c, line 187. (gdb) break *0x00007ffff7bbc98b Breakpoint 3 at 0x7ffff7bbc98b: file nptl-init.c, line 223. (gdb) up #1 <signal handler called> (gdb) disas Dump of assembler code for function __restore_rt: => 0x00007ffff7bc93b0 <+0>: mov $0xf,%rax 0x00007ffff7bc93b7 <+7>: syscall 0x00007ffff7bc93b9 <+9>: nopl 0x0(%rax) End of assembler dump. (gdb) up #2 0x00007ffff7bc89ed in pause () at ../sysdeps/unix/syscall-template.S:84 84 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) (gdb) disas Dump of assembler code for function pause: 0x00007ffff7bc89c0 <+0>: cmpl $0x0,0x20c7b9(%rip) # 0x7ffff7dd5180 <__pthread_multiple_threads> 0x00007ffff7bc89c7 <+7>: jne 0x7ffff7bc89d9 <pause+25> 0x00007ffff7bc89c9 <+0>: mov $0x22,%eax 0x00007ffff7bc89ce <+5>: syscall 0x00007ffff7bc89d0 <+7>: cmp $0xfffffffffffff001,%rax 0x00007ffff7bc89d6 <+13>: jae 0x7ffff7bc8a09 <pause+73> 0x00007ffff7bc89d8 <+15>: retq 0x00007ffff7bc89d9 <+25>: sub $0x8,%rsp 0x00007ffff7bc89dd <+29>: callq 0x7ffff7bc7f90 <__pthread_enable_asynccancel> 0x00007ffff7bc89e2 <+34>: mov %rax,(%rsp) 0x00007ffff7bc89e6 <+38>: mov $0x22,%eax 0x00007ffff7bc89eb <+43>: syscall => 0x00007ffff7bc89ed <+45>: mov (%rsp),%rdi 0x00007ffff7bc89f1 <+49>: mov %rax,%rdx 0x00007ffff7bc89f4 <+52>: callq 0x7ffff7bc7ff0 <__pthread_disable_asynccancel> 0x00007ffff7bc89f9 <+57>: mov %rdx,%rax 0x00007ffff7bc89fc <+60>: add $0x8,%rsp 0x00007ffff7bc8a00 <+64>: cmp $0xfffffffffffff001,%rax 0x00007ffff7bc8a06 <+70>: jae 0x7ffff7bc8a09 <pause+73> 0x00007ffff7bc8a08 <+72>: retq 0x00007ffff7bc8a09 <+73>: mov 0x208370(%rip),%rcx # 0x7ffff7dd0d80 0x00007ffff7bc8a10 <+80>: neg %eax 0x00007ffff7bc8a12 <+82>: mov %eax,%fs:(%rcx) 0x00007ffff7bc8a15 <+85>: or $0xffffffffffffffff,%rax 0x00007ffff7bc8a19 <+89>: retq End of assembler dump. (gdb) break *0x00007ffff7bc89d8 Breakpoint 4 at 0x7ffff7bc89d8: file ../sysdeps/unix/syscall-template.S, line 84. (gdb) break *0x00007ffff7bc8a08 Breakpoint 5 at 0x7ffff7bc8a08: file ../sysdeps/unix/syscall-template.S, line 85. (gdb) break *0x00007ffff7bc8a19 Breakpoint 6 at 0x7ffff7bc8a19: file ../sysdeps/unix/syscall-template.S, line 86. (gdb) up #3 0x000000000040098d in pausefunc () at simple-cancel.c:27 27 pause (); (gdb) disas Dump of assembler code for function pausefunc: 0x0000000000400980 <+0>: sub $0x8,%rsp 0x0000000000400984 <+4>: nopl 0x0(%rax) 0x0000000000400988 <+8>: callq 0x400780 <pause@plt> => 0x000000000040098d <+13>: jmp 0x400988 <pausefunc+8> End of assembler dump. (gdb) up #4 0x00000000004009af in handlerfunc () at simple-cancel.c:35 35 pausefunc (); (gdb) disas Dump of assembler code for function handlerfunc: 0x0000000000400990 <+0>: sub $0x78,%rsp 0x0000000000400994 <+4>: xor %esi,%esi 0x0000000000400996 <+6>: mov %rsp,%rdi 0x0000000000400999 <+9>: callq 0x4007c0 <__sigsetjmp@plt> 0x000000000040099e <+14>: test %eax,%eax 0x00000000004009a0 <+16>: jne 0x4009c8 <handlerfunc+56> 0x00000000004009a2 <+18>: mov %rsp,%rdi 0x00000000004009a5 <+21>: callq 0x400750 <__pthread_register_cancel@plt> 0x00000000004009aa <+26>: callq 0x400980 <pausefunc> => 0x00000000004009af <+31>: mov %rsp,%rdi 0x00000000004009b2 <+34>: callq 0x400770 <__pthread_unregister_cancel@plt> 0x00000000004009b7 <+39>: xor %edi,%edi 0x00000000004009b9 <+41>: callq 0x400970 <handler2> 0x00000000004009be <+46>: add $0x78,%rsp 0x00000000004009c2 <+50>: retq 0x00000000004009c3 <+51>: nopl 0x0(%rax,%rax,1) 0x00000000004009c8 <+56>: xor %edi,%edi 0x00000000004009ca <+58>: callq 0x400970 <handler2> 0x00000000004009cf <+63>: mov %rsp,%rdi 0x00000000004009d2 <+66>: callq 0x4007b0 <__pthread_unwind_next@plt> End of assembler dump. (gdb) break *0x00000000004009c2 Breakpoint 7 at 0x4009c2: file simple-cancel.c, line 37. (gdb) up #5 0x00000000004009ff in threadfunc (closure=<optimized out>) at simple-cancel.c:45 45 handlerfunc (); (gdb) disas Dump of assembler code for function threadfunc: 0x00000000004009e0 <+0>: sub $0x78,%rsp 0x00000000004009e4 <+4>: xor %esi,%esi 0x00000000004009e6 <+6>: mov %rsp,%rdi 0x00000000004009e9 <+9>: callq 0x4007c0 <__sigsetjmp@plt> 0x00000000004009ee <+14>: test %eax,%eax 0x00000000004009f0 <+16>: jne 0x400a10 <threadfunc+48> 0x00000000004009f2 <+18>: mov %rsp,%rdi 0x00000000004009f5 <+21>: callq 0x400750 <__pthread_register_cancel@plt> 0x00000000004009fa <+26>: callq 0x400990 <handlerfunc> => 0x00000000004009ff <+31>: mov %rsp,%rdi 0x0000000000400a02 <+34>: callq 0x400770 <__pthread_unregister_cancel@plt> 0x0000000000400a07 <+39>: xor %eax,%eax 0x0000000000400a09 <+41>: add $0x78,%rsp 0x0000000000400a0d <+45>: retq 0x0000000000400a0e <+46>: xchg %ax,%ax 0x0000000000400a10 <+48>: xor %edi,%edi 0x0000000000400a12 <+50>: callq 0x400960 <handler1> 0x0000000000400a17 <+55>: mov %rsp,%rdi 0x0000000000400a1a <+58>: callq 0x4007b0 <__pthread_unwind_next@plt> End of assembler dump. (gdb) break *0x0000000000400a0d Breakpoint 8 at 0x400a0d: file simple-cancel.c, line 48. (gdb) up #6 0x00007ffff7bbe36d in start_thread (arg=0x7ffff77e1700) at pthread_create.c:456 456 THREAD_SETMEM (pd, result, CALL_THREAD_FCT (pd)); (gdb) disas Dump of assembler code for function start_thread: 0x00007ffff7bbe290 <+0>: push %rbx 0x00007ffff7bbe291 <+1>: mov %rdi,%rbx 0x00007ffff7bbe294 <+4>: sub $0xa0,%rsp 0x00007ffff7bbe29b <+11>: mov %rdi,0x8(%rsp) 0x00007ffff7bbe2a0 <+16>: mov %fs:0x28,%rax 0x00007ffff7bbe2a9 <+25>: mov %rax,0x98(%rsp) 0x00007ffff7bbe2b1 <+33>: xor %eax,%eax 0x00007ffff7bbe2b3 <+35>: rdtsc 0x00007ffff7bbe2b5 <+37>: shl $0x20,%rdx 0x00007ffff7bbe2b9 <+41>: mov %eax,%eax 0x00007ffff7bbe2bb <+43>: or %rax,%rdx 0x00007ffff7bbe2be <+46>: mov %rdx,%fs:0x620 0x00007ffff7bbe2c7 <+55>: mov 0x212ada(%rip),%rax # 0x7ffff7dd0da8 0x00007ffff7bbe2ce <+62>: lea 0x6b8(%rdi),%rdx 0x00007ffff7bbe2d5 <+69>: mov %rdx,%fs:(%rax) 0x00007ffff7bbe2d9 <+73>: callq 0x7ffff7bbc780 0x00007ffff7bbe2de <+78>: xor %eax,%eax 0x00007ffff7bbe2e0 <+80>: xchg %eax,0x61c(%rbx) 0x00007ffff7bbe2e6 <+86>: cmp $0xfffffffe,%eax 0x00007ffff7bbe2e9 <+89>: je 0x7ffff7bbe46b <start_thread+475> 0x00007ffff7bbe2ef <+95>: mov 0x8(%rsp),%rbx 0x00007ffff7bbe2f4 <+100>: mov $0x18,%esi 0x00007ffff7bbe2f9 <+105>: mov $0x111,%eax 0x00007ffff7bbe2fe <+110>: lea 0x2e0(%rbx),%rdi 0x00007ffff7bbe305 <+117>: syscall 0x00007ffff7bbe307 <+119>: testb $0x4,0x614(%rbx) 0x00007ffff7bbe30e <+126>: jne 0x7ffff7bbe432 <start_thread+418> 0x00007ffff7bbe314 <+132>: lea 0x10(%rsp),%rdi 0x00007ffff7bbe319 <+137>: movq $0x0,0x58(%rsp) 0x00007ffff7bbe322 <+146>: movq $0x0,0x60(%rsp) 0x00007ffff7bbe32b <+155>: callq 0x7ffff7bbc6e0 0x00007ffff7bbe330 <+160>: test %eax,%eax 0x00007ffff7bbe332 <+162>: mov %eax,%ebx 0x00007ffff7bbe334 <+164>: jne 0x7ffff7bbe376 <start_thread+230> 0x00007ffff7bbe336 <+166>: lea 0x10(%rsp),%rax 0x00007ffff7bbe33b <+171>: mov %rax,%fs:0x300 0x00007ffff7bbe344 <+180>: mov 0x8(%rsp),%rax 0x00007ffff7bbe349 <+185>: cmpb $0x0,0x613(%rax) 0x00007ffff7bbe350 <+192>: jne 0x7ffff7bbe4d4 <start_thread+580> 0x00007ffff7bbe356 <+198>: mov 0x8(%rsp),%rax 0x00007ffff7bbe35b <+203>: nop 0x00007ffff7bbe35c <+204>: mov %fs:0x648,%rdi 0x00007ffff7bbe365 <+213>: callq *%fs:0x640 => 0x00007ffff7bbe36d <+221>: mov %rax,%fs:0x630 0x00007ffff7bbe376 <+230>: callq 0x7ffff7bbc6d0 0x00007ffff7bbe37b <+235>: xor %eax,%eax 0x00007ffff7bbe37d <+237>: mov %fs:0x610,%al 0x00007ffff7bbe385 <+245>: test %al,%al 0x00007ffff7bbe387 <+247>: jne 0x7ffff7bbe428 <start_thread+408> 0x00007ffff7bbe38d <+253>: callq 0x7ffff7bbc710 0x00007ffff7bbe392 <+258>: lock decl 0x212c87(%rip) # 0x7ffff7dd1020 <__nptl_nthreads> 0x00007ffff7bbe399 <+265>: sete %al 0x00007ffff7bbe39c <+268>: test %al,%al 0x00007ffff7bbe39e <+270>: jne 0x7ffff7bbe5cd <start_thread+829> 0x00007ffff7bbe3a4 <+276>: mov 0x8(%rsp),%rax 0x00007ffff7bbe3a9 <+281>: cmpb $0x0,0x611(%rax) 0x00007ffff7bbe3b0 <+288>: jne 0x7ffff7bbe59f <start_thread+783> 0x00007ffff7bbe3b6 <+294>: mov 0x8(%rsp),%rbx 0x00007ffff7bbe3bb <+299>: lock orl $0x10,0x308(%rbx) 0x00007ffff7bbe3c3 <+307>: callq 0x7ffff7bbc758 0x00007ffff7bbe3c8 <+312>: mov 0x690(%rbx),%rdi 0x00007ffff7bbe3cf <+319>: neg %eax 0x00007ffff7bbe3d1 <+321>: mov %rsp,%rdx 0x00007ffff7bbe3d4 <+324>: cltq 0x00007ffff7bbe3d6 <+326>: sub %rdi,%rdx 0x00007ffff7bbe3d9 <+329>: and %rdx,%rax 0x00007ffff7bbe3dc <+332>: cmp %rax,0x698(%rbx) 0x00007ffff7bbe3e3 <+339>: jbe 0x7ffff7bbe4b5 <start_thread+549> 0x00007ffff7bbe3e9 <+345>: cmp $0x4000,%rax 0x00007ffff7bbe3ef <+351>: ja 0x7ffff7bbe617 <start_thread+903> 0x00007ffff7bbe3f5 <+357>: mov 0x8(%rsp),%rax 0x00007ffff7bbe3fa <+362>: cmp %rax,0x628(%rax) 0x00007ffff7bbe401 <+369>: je 0x7ffff7bbe608 <start_thread+888> 0x00007ffff7bbe407 <+375>: mov 0x8(%rsp),%rax 0x00007ffff7bbe40c <+380>: testb $0x40,0x308(%rax) 0x00007ffff7bbe413 <+387>: jne 0x7ffff7bbe53a <start_thread+682> 0x00007ffff7bbe419 <+393>: mov $0x3c,%edx 0x00007ffff7bbe41e <+398>: xchg %ax,%ax 0x00007ffff7bbe420 <+400>: xor %edi,%edi 0x00007ffff7bbe422 <+402>: mov %edx,%eax 0x00007ffff7bbe424 <+404>: syscall 0x00007ffff7bbe426 <+406>: jmp 0x7ffff7bbe420 <start_thread+400> 0x00007ffff7bbe428 <+408>: callq 0x7ffff7bbd020 <__nptl_deallocate_tsd> 0x00007ffff7bbe42d <+413>: jmpq 0x7ffff7bbe38d <start_thread+253> 0x00007ffff7bbe432 <+418>: lea 0x18(%rsp),%rdx 0x00007ffff7bbe437 <+423>: xor %eax,%eax 0x00007ffff7bbe439 <+425>: mov $0x1e,%ecx 0x00007ffff7bbe43e <+430>: lea 0x10(%rsp),%rsi 0x00007ffff7bbe443 <+435>: mov $0x8,%r10d 0x00007ffff7bbe449 <+441>: mov %rdx,%rdi 0x00007ffff7bbe44c <+444>: xor %edx,%edx 0x00007ffff7bbe44e <+446>: rep stos %eax,%es:(%rdi) 0x00007ffff7bbe450 <+448>: mov $0x80000000,%eax 0x00007ffff7bbe455 <+453>: mov $0x1,%edi 0x00007ffff7bbe45a <+458>: mov %rax,0x10(%rsp) 0x00007ffff7bbe45f <+463>: mov $0xe,%eax 0x00007ffff7bbe464 <+468>: syscall 0x00007ffff7bbe466 <+470>: jmpq 0x7ffff7bbe314 <start_thread+132> 0x00007ffff7bbe46b <+475>: mov 0x8(%rsp),%rax 0x00007ffff7bbe470 <+480>: xor %r10d,%r10d 0x00007ffff7bbe473 <+483>: mov $0x1,%edx 0x00007ffff7bbe478 <+488>: mov $0x81,%esi 0x00007ffff7bbe47d <+493>: lea 0x61c(%rax),%rdi 0x00007ffff7bbe484 <+500>: mov $0xca,%eax 0x00007ffff7bbe489 <+505>: syscall 0x00007ffff7bbe48b <+507>: cmp $0xfffffffffffff000,%rax 0x00007ffff7bbe491 <+513>: jbe 0x7ffff7bbe2ef <start_thread+95> 0x00007ffff7bbe497 <+519>: cmp $0xffffffea,%eax 0x00007ffff7bbe49a <+522>: je 0x7ffff7bbe2ef <start_thread+95> 0x00007ffff7bbe4a0 <+528>: cmp $0xfffffff2,%eax 0x00007ffff7bbe4a3 <+531>: je 0x7ffff7bbe2ef <start_thread+95> 0x00007ffff7bbe4a9 <+537>: lea 0xc770(%rip),%rdi # 0x7ffff7bcac20 0x00007ffff7bbe4b0 <+544>: callq 0x7ffff7bbc638 0x00007ffff7bbe4b5 <+549>: lea 0xc914(%rip),%rcx # 0x7ffff7bcadd0 <__PRETTY_FUNCTION__.11908> 0x00007ffff7bbe4bc <+556>: lea 0xc962(%rip),%rsi # 0x7ffff7bcae25 0x00007ffff7bbe4c3 <+563>: lea 0xc8b6(%rip),%rdi # 0x7ffff7bcad80 0x00007ffff7bbe4ca <+570>: mov $0x22a,%edx 0x00007ffff7bbe4cf <+575>: callq 0x7ffff7bbc6a0 0x00007ffff7bbe4d4 <+580>: callq 0x7ffff7bc7f90 <__pthread_enable_asynccancel> 0x00007ffff7bbe4d9 <+585>: mov $0x1,%esi 0x00007ffff7bbe4de <+590>: mov %eax,%edx 0x00007ffff7bbe4e0 <+592>: mov %ebx,%eax 0x00007ffff7bbe4e2 <+594>: mov 0x8(%rsp),%rbx 0x00007ffff7bbe4e7 <+599>: lock cmpxchg %esi,0x618(%rbx) 0x00007ffff7bbe4ef <+607>: je 0x7ffff7bbe50b <start_thread+635> 0x00007ffff7bbe4f1 <+609>: lea 0x618(%rbx),%rdi 0x00007ffff7bbe4f8 <+616>: sub $0x80,%rsp 0x00007ffff7bbe4ff <+623>: callq 0x7ffff7bc8050 <__lll_lock_wait_private> 0x00007ffff7bbe504 <+628>: add $0x80,%rsp 0x00007ffff7bbe50b <+635>: lock decl 0x618(%rbx) 0x00007ffff7bbe512 <+642>: je 0x7ffff7bbe52e <start_thread+670> 0x00007ffff7bbe514 <+644>: lea 0x618(%rbx),%rdi 0x00007ffff7bbe51b <+651>: sub $0x80,%rsp 0x00007ffff7bbe522 <+658>: callq 0x7ffff7bc8100 <__lll_unlock_wake_private> 0x00007ffff7bbe527 <+663>: add $0x80,%rsp 0x00007ffff7bbe52e <+670>: mov %edx,%edi 0x00007ffff7bbe530 <+672>: callq 0x7ffff7bc7ff0 <__pthread_disable_asynccancel> 0x00007ffff7bbe535 <+677>: jmpq 0x7ffff7bbe356 <start_thread+198> 0x00007ffff7bbe53a <+682>: lea 0x61c(%rax),%rbx 0x00007ffff7bbe541 <+689>: mov $0xca,%r9d 0x00007ffff7bbe547 <+695>: mov $0x1,%r8d 0x00007ffff7bbe54d <+701>: jmp 0x7ffff7bbe561 <start_thread+721> 0x00007ffff7bbe54f <+703>: mov 0x8(%rsp),%rax 0x00007ffff7bbe554 <+708>: testb $0x40,0x308(%rax) 0x00007ffff7bbe55b <+715>: je 0x7ffff7bbe62d <start_thread+925> 0x00007ffff7bbe561 <+721>: xor %r10d,%r10d 0x00007ffff7bbe564 <+724>: xor %edx,%edx 0x00007ffff7bbe566 <+726>: mov $0x80,%esi 0x00007ffff7bbe56b <+731>: mov %rbx,%rdi 0x00007ffff7bbe56e <+734>: mov %r9d,%eax 0x00007ffff7bbe571 <+737>: syscall 0x00007ffff7bbe573 <+739>: cmp $0xfffffffffffff000,%rax 0x00007ffff7bbe579 <+745>: jbe 0x7ffff7bbe54f <start_thread+703> 0x00007ffff7bbe57b <+747>: add $0xb,%eax 0x00007ffff7bbe57e <+750>: cmp $0xb,%eax 0x00007ffff7bbe581 <+753>: ja 0x7ffff7bbe4a9 <start_thread+537> 0x00007ffff7bbe587 <+759>: mov %eax,%ecx 0x00007ffff7bbe589 <+761>: mov %r8,%rsi 0x00007ffff7bbe58c <+764>: shl %cl,%rsi 0x00007ffff7bbe58f <+767>: mov %rsi,%rax 0x00007ffff7bbe592 <+770>: test $0x881,%eax 0x00007ffff7bbe597 <+775>: je 0x7ffff7bbe4a9 <start_thread+537> 0x00007ffff7bbe59d <+781>: jmp 0x7ffff7bbe54f <start_thread+703> 0x00007ffff7bbe59f <+783>: mov 0x8(%rsp),%rcx 0x00007ffff7bbe5a4 <+788>: mov 0x216b36(%rip),%eax # 0x7ffff7dd50e0 <__nptl_threads_events> 0x00007ffff7bbe5aa <+794>: or 0x650(%rcx),%eax 0x00007ffff7bbe5b0 <+800>: test $0x1,%ah 0x00007ffff7bbe5b3 <+803>: je 0x7ffff7bbe3b6 <start_thread+294> 0x00007ffff7bbe5b9 <+809>: cmpq $0x0,0x668(%rcx) 0x00007ffff7bbe5c1 <+817>: je 0x7ffff7bbe5d4 <start_thread+836> 0x00007ffff7bbe5c3 <+819>: callq 0x7ffff7bbced0 <__nptl_death_event> 0x00007ffff7bbe5c8 <+824>: jmpq 0x7ffff7bbe3b6 <start_thread+294> 0x00007ffff7bbe5cd <+829>: xor %edi,%edi 0x00007ffff7bbe5cf <+831>: callq 0x7ffff7bbc810 0x00007ffff7bbe5d4 <+836>: mov %rcx,%rax 0x00007ffff7bbe5d7 <+839>: movl $0x9,0x658(%rcx) 0x00007ffff7bbe5e1 <+849>: mov %rcx,0x660(%rax) 0x00007ffff7bbe5e8 <+856>: mov 0x216ae9(%rip),%rax # 0x7ffff7dd50d8 <__nptl_last_event> 0x00007ffff7bbe5ef <+863>: mov 0x8(%rsp),%rsi 0x00007ffff7bbe5f4 <+868>: mov %rax,0x668(%rsi) 0x00007ffff7bbe5fb <+875>: lock cmpxchg %rsi,0x216ad4(%rip) # 0x7ffff7dd50d8 <__nptl_last_event> 0x00007ffff7bbe604 <+884>: je 0x7ffff7bbe5c3 <start_thread+819> 0x00007ffff7bbe606 <+886>: jmp 0x7ffff7bbe5e8 <start_thread+856> 0x00007ffff7bbe608 <+888>: mov 0x8(%rsp),%rdi 0x00007ffff7bbe60d <+893>: callq 0x7ffff7bbe0a0 <__free_tcb> 0x00007ffff7bbe612 <+898>: jmpq 0x7ffff7bbe419 <start_thread+393> 0x00007ffff7bbe617 <+903>: lea -0x4000(%rax),%rsi 0x00007ffff7bbe61e <+910>: mov $0x4,%edx 0x00007ffff7bbe623 <+915>: callq 0x7ffff7bbc7b8 0x00007ffff7bbe628 <+920>: jmpq 0x7ffff7bbe3f5 <start_thread+357> 0x00007ffff7bbe62d <+925>: movl $0x0,0x61c(%rax) 0x00007ffff7bbe637 <+935>: jmpq 0x7ffff7bbe419 <start_thread+393> End of assembler dump. (gdb) up #7 0x00007ffff78f2e1f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:97 97 call *%rax (gdb) disas Dump of assembler code for function clone: 0x00007ffff78f2de0 <+0>: mov $0xffffffffffffffea,%rax 0x00007ffff78f2de7 <+7>: test %rdi,%rdi 0x00007ffff78f2dea <+10>: je 0x7ffff78f2e27 <clone+71> 0x00007ffff78f2dec <+12>: test %rsi,%rsi 0x00007ffff78f2def <+15>: je 0x7ffff78f2e27 <clone+71> 0x00007ffff78f2df1 <+17>: sub $0x10,%rsi 0x00007ffff78f2df5 <+21>: mov %rcx,0x8(%rsi) 0x00007ffff78f2df9 <+25>: mov %rdi,(%rsi) 0x00007ffff78f2dfc <+28>: mov %rdx,%rdi 0x00007ffff78f2dff <+31>: mov %r8,%rdx 0x00007ffff78f2e02 <+34>: mov %r9,%r8 0x00007ffff78f2e05 <+37>: mov 0x8(%rsp),%r10 0x00007ffff78f2e0a <+42>: mov $0x38,%eax 0x00007ffff78f2e0f <+47>: syscall 0x00007ffff78f2e11 <+49>: test %rax,%rax 0x00007ffff78f2e14 <+52>: jl 0x7ffff78f2e27 <clone+71> 0x00007ffff78f2e16 <+54>: je 0x7ffff78f2e19 <clone+57> 0x00007ffff78f2e18 <+56>: retq 0x00007ffff78f2e19 <+57>: xor %ebp,%ebp 0x00007ffff78f2e1b <+59>: pop %rax 0x00007ffff78f2e1c <+60>: pop %rdi 0x00007ffff78f2e1d <+61>: callq *%rax => 0x00007ffff78f2e1f <+63>: mov %rax,%rdi 0x00007ffff78f2e22 <+66>: callq 0x7ffff78b6fc0 <__GI__exit> 0x00007ffff78f2e27 <+71>: mov 0x2be03a(%rip),%rcx # 0x7ffff7bb0e68 0x00007ffff78f2e2e <+78>: neg %eax 0x00007ffff78f2e30 <+80>: mov %eax,%fs:(%rcx) 0x00007ffff78f2e33 <+83>: or $0xffffffffffffffff,%rax 0x00007ffff78f2e37 <+87>: retq End of assembler dump. (gdb) break *0x00007ffff78f2e37 Breakpoint 9 at 0x7ffff78f2e37: file ../sysdeps/unix/sysv/linux/x86_64/clone.S, line 104. (gdb) up Initial frame selected; you cannot go up. (gdb) c Continuing. handler2 called handler1 called [Thread 0x7ffff77e1700 (LWP 12258) exited] [Inferior 1 (process 12257) exited normally] (gdb)
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?
On 12/18/2017 12:42 PM, H.J. Lu wrote: > We need to restore shadow stack pointer here so that we can jump back > to the function where __sigsetjmp is called. But neither __sigsetjmp (when called the second time) nor the function that calls it return normally during cancellation, so it is still completely unclear to me what issue you are observing. Could you post a backtrace from the CET verification failure, please? Thanks, Florian
On Mon, Dec 18, 2017 at 3:49 AM, Florian Weimer <fweimer@redhat.com> wrote: > On 12/18/2017 12:42 PM, H.J. Lu wrote: >> >> We need to restore shadow stack pointer here so that we can jump back >> to the function where __sigsetjmp is called. > > > But neither __sigsetjmp (when called the second time) nor the function that > calls it return normally during cancellation, so it is still completely > unclear to me what issue you are observing. > > Could you post a backtrace from the CET verification failure, please? > Here is your testcase with full debug info: (gdb) r The program being debugged has been started already. Start it from the beginning? (y or n) y Starting program: /export/build/gnu/glibc-cet/build-x86_64-linux/nptl/tst-foo warning: Unable to find libthread_db matching inferior's thread library, thread debugging will not be available. Breakpoint 1, main () at tst-foo.c:52 52 { (gdb) ena 2 (gdb) c Continuing. [Switching to LWP 18711] Thread 2 "tst-foo" hit Breakpoint 2, __sigsetjmp () at ../sysdeps/unix/sysv/linux/x86_64/setjmp.S:26 26 ENTRY (__sigsetjmp) (gdb) bt #0 __sigsetjmp () at ../sysdeps/unix/sysv/linux/x86_64/setjmp.S:26 #1 0x0000000000400e15 in threadfunc (closure=<optimized out>) at tst-foo.c:44 #2 0x00007ffff7bbfcde in start_thread (arg=<optimized out>) at pthread_create.c:463 #3 0x00007ffff78f5f73 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 (gdb) f 1 #1 0x0000000000400e15 in threadfunc (closure=<optimized out>) at tst-foo.c:44 44 pthread_cleanup_push (handler1, NULL); Here we call __sigsetjmp with cancel_jmp_buf. There is a shadow stack for the normal call stack. We need to save shadow stack pointer so that we can lonjmp back here later. (gdb) dis 2 (gdb) ena 3 (gdb) c Continuing. Thread 2 "tst-foo" hit Breakpoint 3, __longjmp () at ../sysdeps/unix/sysv/linux/x86_64/__longjmp.S:30 30 ENTRY(__longjmp) (gdb) bt #0 __longjmp () at ../sysdeps/unix/sysv/linux/x86_64/__longjmp.S:30 If we don't restore shadow stack pointer, when we jump back to tst-foo.c:45, shadow stack won't match call stack when threadfunc () returns. #1 0x00007ffff7837f5f in __libc_siglongjmp (env=env@entry=0x7ffff7800ca0, val=val@entry=1) at longjmp.c:39 #2 0x00007ffff7bc899d in unwind_stop (version=<optimized out>, actions=<optimized out>, exc_class=<optimized out>, exc_obj=<optimized out>, context=<optimized out>, stop_parameter=0x7ffff7800ca0) at unwind.c:94 #3 0x00007ffff6df9b1e in _Unwind_ForcedUnwind_Phase2 (exc=exc@entry=0x7ffff7801d70, context=context@entry=0x7ffff7800550, frames_p=frames_p@entry=0x7ffff7800458) at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:170 #4 0x00007ffff6dfa170 in _Unwind_ForcedUnwind (exc=0x7ffff7801d70, stop=stop@entry=0x7ffff7bc88e0 <unwind_stop>, stop_argument=<optimized out>) at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:217 #5 0x00007ffff7bc8a84 in __GI___pthread_unwind (buf=<optimized out>) at unwind.c:121 #6 0x00007ffff7bbe46a in __do_cancel () at ./pthreadP.h:297 #7 sigcancel_handler (sig=<optimized out>, si=0x7ffff78007f0, ctx=<optimized out>) at nptl-init.c:216 #8 <signal handler called> #9 0x00007ffff7bc99b2 in __libc_pause () at ../sysdeps/unix/sysv/linux/pause.c:30 #10 0x0000000000400d95 in pausefunc () at tst-foo.c:27 #11 0x0000000000400dca in handlerfunc () at tst-foo.c:35 #12 0x0000000000400e2a in threadfunc (closure=<optimized out>) at tst-foo.c:45 #13 0x00007ffff7bbfcde in start_thread (arg=<optimized out>) at pthread_create.c:463 #14 0x00007ffff78f5f73 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 (gdb) f 6 #6 0x00007ffff7bbe46a in __do_cancel () at ./pthreadP.h:297 297 __pthread_unwind ((__pthread_unwind_buf_t *) (gdb) list 292 struct pthread *self = THREAD_SELF; 293 294 /* Make sure we get no more cancellations. */ 295 THREAD_ATOMIC_BIT_SET (self, cancelhandling, EXITING_BIT); 296 297 __pthread_unwind ((__pthread_unwind_buf_t *) 298 THREAD_GETMEM (self, cleanup_jmp_buf)); 299 } 300 301 (gdb) Does it answer your question?
On 12/18/2017 01:25 PM, H.J. Lu wrote: > If we don't restore shadow stack pointer, when we jump back to tst-foo.c:45, > shadow stack won't match call stack when threadfunc () returns. But threadfunc never returns if the thread is canceled, so I'm still as puzzled as before. Sorry. Florian
On Mon, Dec 18, 2017 at 4:52 AM, Florian Weimer <fweimer@redhat.com> wrote: > On 12/18/2017 01:25 PM, H.J. Lu wrote: >> >> If we don't restore shadow stack pointer, when we jump back to >> tst-foo.c:45, >> shadow stack won't match call stack when threadfunc () returns. > > > But threadfunc never returns if the thread is canceled, so I'm still as > puzzled as before. Sorry. True, threadfunc never returns. Instead, it lonjmps back to start_thread: Thread 2 "tst-foo" hit Breakpoint 2, __longjmp () at ../sysdeps/unix/sysv/linux/x86_64/__longjmp.S:30 30 ENTRY(__longjmp) (gdb) bt #0 __longjmp () at ../sysdeps/unix/sysv/linux/x86_64/__longjmp.S:30 #1 0x00007ffff7837f5f in __libc_siglongjmp (env=env@entry=0x7ffff7800eb0, val=val@entry=1) at longjmp.c:39 #2 0x00007ffff7bc899d in unwind_stop (version=<optimized out>, actions=<optimized out>, exc_class=<optimized out>, exc_obj=<optimized out>, context=<optimized out>, stop_parameter=0x7ffff7800eb0) at unwind.c:94 #3 0x00007ffff6df9b1e in _Unwind_ForcedUnwind_Phase2 ( exc=exc@entry=0x7ffff7801d70, context=context@entry=0x7ffff7800c40, frames_p=frames_p@entry=0x7ffff7800b48) at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:170 #4 0x00007ffff6dfa170 in _Unwind_ForcedUnwind (exc=0x7ffff7801d70, stop=stop@entry=0x7ffff7bc88e0 <unwind_stop>, stop_argument=<optimized out>) at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:217 #5 0x00007ffff7bc8a84 in __GI___pthread_unwind (buf=<optimized out>) at unwind.c:121 #6 0x00007ffff7bc8aa4 in __GI___pthread_unwind_next ( buf=buf@entry=0x7ffff7800da0) at unwind.c:136 #7 0x0000000000400e4f in threadfunc (closure=<optimized out>) at tst-foo.c:44 #8 0x00007ffff7bbfcde in start_thread (arg=<optimized out>) at pthread_create.c:463 #9 0x00007ffff78f5f73 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 (gdb) 104 jmpq *%rdx (gdb) next start_thread (arg=<optimized out>) at pthread_create.c:436 436 if (__glibc_likely (! not_first_call)) (gdb) bt #0 start_thread (arg=<optimized out>) at pthread_create.c:436 #1 0x00007ffff78f5f73 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 (gdb) list 431 unwind_buf.priv.data.prev = NULL; 432 unwind_buf.priv.data.cleanup = NULL; 433 434 int not_first_call; 435 not_first_call = setjmp ((struct __jmp_buf_tag *) unwind_buf.cancel_jmp_buf); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This has to save and restore shadow stack pointer. Since we only have one __sigsetjmp and one __longjmp, when shadow stack is enabled, they have to save and restore shadow stack pointer. It means cancel_jmp_buf has to match __jmp_buf_tag. 436 if (__glibc_likely (! not_first_call)) 437 { 438 /* Store the new cleanup handler info. */ 439 THREAD_SETMEM (pd, cleanup_jmp_buf, &unwind_buf); 440 (gdb) Does it answer your question?
On Mon, Dec 18, 2017 at 5:19 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Mon, Dec 18, 2017 at 4:52 AM, Florian Weimer <fweimer@redhat.com> wrote: >> On 12/18/2017 01:25 PM, H.J. Lu wrote: >>> >>> If we don't restore shadow stack pointer, when we jump back to >>> tst-foo.c:45, >>> shadow stack won't match call stack when threadfunc () returns. >> >> >> But threadfunc never returns if the thread is canceled, so I'm still as >> puzzled as before. Sorry. > > True, threadfunc never returns. Instead, it lonjmps back to > start_thread: > > Thread 2 "tst-foo" hit Breakpoint 2, __longjmp () > at ../sysdeps/unix/sysv/linux/x86_64/__longjmp.S:30 > 30 ENTRY(__longjmp) > (gdb) bt > #0 __longjmp () at ../sysdeps/unix/sysv/linux/x86_64/__longjmp.S:30 > #1 0x00007ffff7837f5f in __libc_siglongjmp (env=env@entry=0x7ffff7800eb0, > val=val@entry=1) at longjmp.c:39 > #2 0x00007ffff7bc899d in unwind_stop (version=<optimized out>, > actions=<optimized out>, exc_class=<optimized out>, > exc_obj=<optimized out>, context=<optimized out>, > stop_parameter=0x7ffff7800eb0) at unwind.c:94 > #3 0x00007ffff6df9b1e in _Unwind_ForcedUnwind_Phase2 ( > exc=exc@entry=0x7ffff7801d70, context=context@entry=0x7ffff7800c40, > frames_p=frames_p@entry=0x7ffff7800b48) > at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:170 > #4 0x00007ffff6dfa170 in _Unwind_ForcedUnwind (exc=0x7ffff7801d70, > stop=stop@entry=0x7ffff7bc88e0 <unwind_stop>, > stop_argument=<optimized out>) > at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:217 > #5 0x00007ffff7bc8a84 in __GI___pthread_unwind (buf=<optimized out>) > at unwind.c:121 > #6 0x00007ffff7bc8aa4 in __GI___pthread_unwind_next ( > buf=buf@entry=0x7ffff7800da0) at unwind.c:136 > #7 0x0000000000400e4f in threadfunc (closure=<optimized out>) at tst-foo.c:44 > #8 0x00007ffff7bbfcde in start_thread (arg=<optimized out>) > at pthread_create.c:463 > #9 0x00007ffff78f5f73 in clone () > at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 > (gdb) > 104 jmpq *%rdx > (gdb) next > start_thread (arg=<optimized out>) at pthread_create.c:436 > 436 if (__glibc_likely (! not_first_call)) > (gdb) bt > #0 start_thread (arg=<optimized out>) at pthread_create.c:436 > #1 0x00007ffff78f5f73 in clone () > at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 > (gdb) list > 431 unwind_buf.priv.data.prev = NULL; > 432 unwind_buf.priv.data.cleanup = NULL; > 433 > 434 int not_first_call; > 435 not_first_call = setjmp ((struct __jmp_buf_tag *) > unwind_buf.cancel_jmp_buf); > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > This has to save and restore shadow stack pointer. Since we only have > one __sigsetjmp and one __longjmp, when shadow stack is enabled, they > have to save and restore shadow stack pointer. It means cancel_jmp_buf > has to match __jmp_buf_tag. > > 436 if (__glibc_likely (! not_first_call)) > 437 { > 438 /* Store the new cleanup handler info. */ > 439 THREAD_SETMEM (pd, cleanup_jmp_buf, &unwind_buf); > 440 > (gdb) > > Does it answer your question? > Here is the updated patch with commit message: On x86, padding in struct __jmp_buf_tag is used for shadow stack pointer to support shadow stack in Intel Control-flow Enforcemen Technology. Since the cancel_jmp_buf array is passed to setjmp and longjmp by casting it to pointer to struct __jmp_buf_tag, it should be as large as struct __jmp_buf_tag. Otherwise when shadow stack is enabled, setjmp and longjmp will write and read beyond cancel_jmp_buf when saving and restoring shadow stack pointer. Does it look OK? Thanks.
On Dez 18 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: > This 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. Isn't that an ABI change? Andreas.
On Mon, Dec 18, 2017 at 6:44 AM, Andreas Schwab <schwab@suse.de> wrote: > On Dez 18 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: > >> This 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. > > Isn't that an ABI change? > Yes, this change is exposed to application via <phread.h>. The backward binary compatibility is provided by https://sourceware.org/ml/libc-alpha/2017-12/msg00208.html
On Mon, Dec 18, 2017 at 6:48 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Mon, Dec 18, 2017 at 6:44 AM, Andreas Schwab <schwab@suse.de> wrote: >> On Dez 18 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: >> >>> This 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. >> >> Isn't that an ABI change? >> > > Yes, this change is exposed to application via <phread.h>. The backward > binary compatibility is provided by > > https://sourceware.org/ml/libc-alpha/2017-12/msg00208.html > Here is the patch of shadow stack enabled setjmp/longjmp: https://sourceware.org/ml/libc-alpha/2017-12/msg00559.html which is backward binary compatible with existing binaries.
I think any change applied to sysdeps/nptl/pthread.h should also be applied to sysdeps/unix/sysv/linux/hppa/pthread.h to avoid divergence between those header versions.
On 12/18/2017 03:48 PM, H.J. Lu wrote: > On Mon, Dec 18, 2017 at 6:44 AM, Andreas Schwab <schwab@suse.de> wrote: >> On Dez 18 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: >> >>> This 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. >> >> Isn't that an ABI change? >> > > Yes, this change is exposed to application via <phread.h>. The backward > binary compatibility is provided by > > https://sourceware.org/ml/libc-alpha/2017-12/msg00208.html This doesn't seem to work: <https://sourceware.org/ml/libc-alpha/2018-01/msg00178.html> I have no idea what is going on, but if we can't find the root cause, I think we should revert all the setjmp changes. Thanks, Florian
On Tue, Jan 9, 2018 at 2:47 AM, Florian Weimer <fweimer@redhat.com> wrote: > On 12/18/2017 03:48 PM, H.J. Lu wrote: >> >> On Mon, Dec 18, 2017 at 6:44 AM, Andreas Schwab <schwab@suse.de> wrote: >>> >>> On Dez 18 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: >>> >>>> This 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. >>> >>> >>> Isn't that an ABI change? >>> >> >> Yes, this change is exposed to application via <phread.h>. The backward >> binary compatibility is provided by >> >> https://sourceware.org/ml/libc-alpha/2017-12/msg00208.html > > > This doesn't seem to work: > > <https://sourceware.org/ml/libc-alpha/2018-01/msg00178.html> > > I have no idea what is going on, but if we can't find the root cause, I > think we should revert all the setjmp changes. Hi Andrew, Igor, Please investigate to find out what happened. Thanks.
> -----Original Message----- > From: H.J. Lu [mailto:hjl.tools@gmail.com] > Sent: Tuesday, January 9, 2018 13:18 > To: Florian Weimer <fweimer@redhat.com>; Tsimbalist, Igor V > <igor.v.tsimbalist@intel.com>; Senkevich, Andrew > <andrew.senkevich@intel.com> > Cc: Andreas Schwab <schwab@suse.de>; GNU C Library <libc- > alpha@sourceware.org> > Subject: Re: [PATCH 1/2] Linux/x86: Update cancel_jmp_buf to match > __jmp_buf_tag [BZ #22563] > > On Tue, Jan 9, 2018 at 2:47 AM, Florian Weimer <fweimer@redhat.com> > wrote: > > On 12/18/2017 03:48 PM, H.J. Lu wrote: > >> > >> On Mon, Dec 18, 2017 at 6:44 AM, Andreas Schwab <schwab@suse.de> > wrote: > >>> > >>> On Dez 18 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: > >>> > >>>> This 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. > >>> > >>> > >>> Isn't that an ABI change? > >>> > >> > >> Yes, this change is exposed to application via <phread.h>. The > >> backward binary compatibility is provided by > >> > >> https://sourceware.org/ml/libc-alpha/2017-12/msg00208.html > > > > > > This doesn't seem to work: > > > > <https://sourceware.org/ml/libc-alpha/2018-01/msg00178.html> > > > > I have no idea what is going on, but if we can't find the root cause, > > I think we should revert all the setjmp changes. > > Hi Andrew, Igor, > > Please investigate to find out what happened. > > Thanks. We have connected with Tom Englund who reported issue and we will work on this issue. -- Andrew
On 2018-01-09 11:47, Florian Weimer wrote: > On 12/18/2017 03:48 PM, H.J. Lu wrote: > > On Mon, Dec 18, 2017 at 6:44 AM, Andreas Schwab <schwab@suse.de> wrote: > > > On Dez 18 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: > > > > > > > This 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. > > > > > > Isn't that an ABI change? > > > > > > > Yes, this change is exposed to application via <phread.h>. The backward > > binary compatibility is provided by > > > > https://sourceware.org/ml/libc-alpha/2017-12/msg00208.html > > This doesn't seem to work: > > <https://sourceware.org/ml/libc-alpha/2018-01/msg00178.html> > > I have no idea what is going on, but if we can't find the root cause, I > think we should revert all the setjmp changes. Commit f81ddabffd also breaks software like vlc or amarok, they crash with a segmentation fault during startup. Reverting the commit f81ddabffd fixes the issue. See debian bugs #887078 and #887886.
On Sun, Jan 21, 2018 at 8:15 AM, Aurelien Jarno <aurelien@aurel32.net> wrote: > On 2018-01-09 11:47, Florian Weimer wrote: >> On 12/18/2017 03:48 PM, H.J. Lu wrote: >> > On Mon, Dec 18, 2017 at 6:44 AM, Andreas Schwab <schwab@suse.de> wrote: >> > > On Dez 18 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: >> > > >> > > > This 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. >> > > >> > > Isn't that an ABI change? >> > > >> > >> > Yes, this change is exposed to application via <phread.h>. The backward >> > binary compatibility is provided by >> > >> > https://sourceware.org/ml/libc-alpha/2017-12/msg00208.html >> >> This doesn't seem to work: >> >> <https://sourceware.org/ml/libc-alpha/2018-01/msg00178.html> >> >> I have no idea what is going on, but if we can't find the root cause, I >> think we should revert all the setjmp changes. > > Commit f81ddabffd also breaks software like vlc or amarok, they crash > with a segmentation fault during startup. Reverting the commit > f81ddabffd fixes the issue. > > See debian bugs #887078 and #887886. > Hi Andrew, This may be easier to track. Please investigate.
On 01/21/2018 08:27 AM, H.J. Lu wrote: > On Sun, Jan 21, 2018 at 8:15 AM, Aurelien Jarno <aurelien@aurel32.net> wrote: >> On 2018-01-09 11:47, Florian Weimer wrote: >>> On 12/18/2017 03:48 PM, H.J. Lu wrote: >>>> On Mon, Dec 18, 2017 at 6:44 AM, Andreas Schwab <schwab@suse.de> wrote: >>>>> On Dez 18 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: >>>>> >>>>>> This 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. >>>>> >>>>> Isn't that an ABI change? >>>>> >>>> >>>> Yes, this change is exposed to application via <phread.h>. The backward >>>> binary compatibility is provided by >>>> >>>> https://sourceware.org/ml/libc-alpha/2017-12/msg00208.html >>> >>> This doesn't seem to work: >>> >>> <https://sourceware.org/ml/libc-alpha/2018-01/msg00178.html> >>> >>> I have no idea what is going on, but if we can't find the root cause, I >>> think we should revert all the setjmp changes. >> >> Commit f81ddabffd also breaks software like vlc or amarok, they crash >> with a segmentation fault during startup. Reverting the commit >> f81ddabffd fixes the issue. >> >> See debian bugs #887078 and #887886. >> > > Hi Andrew, > > This may be easier to track. Please investigate. This is now the last remaining release blocker IMO. I'm going to start investigating this on Monday.
> -----Original Message----- > From: H.J. Lu [mailto:hjl.tools@gmail.com] > Sent: Sunday, January 21, 2018 17:27 > To: Florian Weimer <fweimer@redhat.com>; Andreas Schwab > <schwab@suse.de>; GNU C Library <libc-alpha@sourceware.org>; > Senkevich, Andrew <andrew.senkevich@intel.com> > Subject: Re: [PATCH 1/2] Linux/x86: Update cancel_jmp_buf to match > __jmp_buf_tag [BZ #22563] > > On Sun, Jan 21, 2018 at 8:15 AM, Aurelien Jarno <aurelien@aurel32.net> > wrote: > > On 2018-01-09 11:47, Florian Weimer wrote: > >> On 12/18/2017 03:48 PM, H.J. Lu wrote: > >> > On Mon, Dec 18, 2017 at 6:44 AM, Andreas Schwab <schwab@suse.de> > wrote: > >> > > On Dez 18 2017, "H.J. Lu" <hjl.tools@gmail.com> wrote: > >> > > > >> > > > This 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. > >> > > > >> > > Isn't that an ABI change? > >> > > > >> > > >> > Yes, this change is exposed to application via <phread.h>. The > >> > backward binary compatibility is provided by > >> > > >> > https://sourceware.org/ml/libc-alpha/2017-12/msg00208.html > >> > >> This doesn't seem to work: > >> > >> <https://sourceware.org/ml/libc-alpha/2018-01/msg00178.html> > >> > >> I have no idea what is going on, but if we can't find the root cause, > >> I think we should revert all the setjmp changes. > > > > Commit f81ddabffd also breaks software like vlc or amarok, they crash > > with a segmentation fault during startup. Reverting the commit > > f81ddabffd fixes the issue. > > > > See debian bugs #887078 and #887886. > > > > Hi Andrew, > > This may be easier to track. Please investigate. Looks like that. I will have ability to investigate it during tomorrow. -- Andrew
On 01/22/2018 06:44 AM, Senkevich, Andrew wrote: > Looks like that. > I will have ability to investigate it during tomorrow. Any idea what's going wrong?
> -----Original Message----- > From: Carlos O'Donell [mailto:carlos@redhat.com] > Sent: Tuesday, January 23, 2018 20:35 > To: Senkevich, Andrew <andrew.senkevich@intel.com>; H.J. Lu > <hjl.tools@gmail.com>; Florian Weimer <fweimer@redhat.com>; Andreas > Schwab <schwab@suse.de> > Cc: GNU C Library <libc-alpha@sourceware.org> > Subject: Re: [PATCH 1/2] Linux/x86: Update cancel_jmp_buf to match > __jmp_buf_tag [BZ #22563] > > On 01/22/2018 06:44 AM, Senkevich, Andrew wrote: > > Looks like that. > > I will have ability to investigate it during tomorrow. > > Any idea what's going wrong? Currently not clear why but I see return from var_AddCallback () (from /usr/lib/x86_64-linux-gnu/libvlccore.so.9) to wrong address. And setjmp/longjmp doesn't break. Will continue tomorrow. -- Andrew
On Tue, Jan 23, 2018 at 1:13 PM, Senkevich, Andrew <andrew.senkevich@intel.com> wrote: >> -----Original Message----- >> From: Carlos O'Donell [mailto:carlos@redhat.com] >> Sent: Tuesday, January 23, 2018 20:35 >> To: Senkevich, Andrew <andrew.senkevich@intel.com>; H.J. Lu >> <hjl.tools@gmail.com>; Florian Weimer <fweimer@redhat.com>; Andreas >> Schwab <schwab@suse.de> >> Cc: GNU C Library <libc-alpha@sourceware.org> >> Subject: Re: [PATCH 1/2] Linux/x86: Update cancel_jmp_buf to match >> __jmp_buf_tag [BZ #22563] >> >> On 01/22/2018 06:44 AM, Senkevich, Andrew wrote: >> > Looks like that. >> > I will have ability to investigate it during tomorrow. >> >> Any idea what's going wrong? > > Currently not clear why but I see return from var_AddCallback () (from /usr/lib/x86_64-linux-gnu/libvlccore.so.9) to wrong address. > And setjmp/longjmp doesn't break. Will continue tomorrow. > We opened a bug: https://sourceware.org/bugzilla/show_bug.cgi?id=22743 Any help to track down the root cause is appreciated.
On 01/24/2018 07:08 PM, H.J. Lu wrote: > On Tue, Jan 23, 2018 at 1:13 PM, Senkevich, Andrew > <andrew.senkevich@intel.com> wrote: >>> -----Original Message----- >>> From: Carlos O'Donell [mailto:carlos@redhat.com] >>> Sent: Tuesday, January 23, 2018 20:35 >>> To: Senkevich, Andrew <andrew.senkevich@intel.com>; H.J. Lu >>> <hjl.tools@gmail.com>; Florian Weimer <fweimer@redhat.com>; Andreas >>> Schwab <schwab@suse.de> >>> Cc: GNU C Library <libc-alpha@sourceware.org> >>> Subject: Re: [PATCH 1/2] Linux/x86: Update cancel_jmp_buf to match >>> __jmp_buf_tag [BZ #22563] >>> >>> On 01/22/2018 06:44 AM, Senkevich, Andrew wrote: >>>> Looks like that. >>>> I will have ability to investigate it during tomorrow. >>> >>> Any idea what's going wrong? >> >> Currently not clear why but I see return from var_AddCallback () (from /usr/lib/x86_64-linux-gnu/libvlccore.so.9) to wrong address. >> And setjmp/longjmp doesn't break. Will continue tomorrow. >> > > We opened a bug: > > https://sourceware.org/bugzilla/show_bug.cgi?id=22743 > > Any help to track down the root cause is appreciated. Doesn't the bug report clearly show the root cause? The offset of priv.data.cleanup changed, and old binaries have an insufficiently large stack allocation for the new offset. (Congratulations for tracking it down, by the way. I know that such bugs are hard.) You need to add a symbol version for pthread_register_cancel. It's too late for that now, so I recommend reverting the faulty commit. Thanks, Florian
On 01/24/2018 10:23 AM, Florian Weimer wrote: > On 01/24/2018 07:08 PM, H.J. Lu wrote: >> We opened a bug: >> >> https://sourceware.org/bugzilla/show_bug.cgi?id=22743 >> >> Any help to track down the root cause is appreciated. > > Doesn't the bug report clearly show the root cause? The offset of > priv.data.cleanup changed, and old binaries have an insufficiently > large stack allocation for the new offset. > > (Congratulations for tracking it down, by the way. I know that such > bugs are hard.) > > You need to add a symbol version for pthread_register_cancel. It's > too late for that now, so I recommend reverting the faulty commit. I have finished analyzing this and debugging the root cause myself, and I agree with Florian, we need to revert: commit f81ddabffd76ac9dd600b02adbf3e1dac4bb10ec commit cba595c350e52194e10c0006732e1991e3d0803b At a minimum. I am testing with them reverted locally. To be honest I'm surprised that this passed review and was checked in, because the __pthread_unwind_buf_t has only at most 4-bytes of space left before it is an ABI change. In the future please ping me if you have any doubts and I'll review. The addition of __sigset_t saved_mask moves pthread_unwind_buf's priv.data.cleanup forward by 124-bytes. The on-stack allocation of the pthread_cleanup_push's __pthread_unwind_buf_t is not that big and so __pthread_register_cancel writes to other structures which are allocated on the stack. You cannot expand struct pthread_unwind_buf because the on-stack allocated __pthread_unwind_buf_t is not large enough in existing applications. You *might* have used feature_1 to change between two different layouts of struct pthread_unwind_buf, but that will have to wait for 2.28. As Florian suggests though it is cleaner to version __pthread_register_cancel for x86 and the older version expects the smaller non-CET-enabled structure.
On Wed, 24 Jan 2018, Carlos O'Donell wrote: > I have finished analyzing this and debugging the root cause myself, > and I agree with Florian, we need to revert: > > commit f81ddabffd76ac9dd600b02adbf3e1dac4bb10ec > commit cba595c350e52194e10c0006732e1991e3d0803b > > At a minimum. I am testing with them reverted locally. > > To be honest I'm surprised that this passed review and was checked > in, because the __pthread_unwind_buf_t has only at most 4-bytes of It didn't pass review. It was checked in with no evidence of consensus (and even with ABI concerns raised in the original discussion).
On Wed, Jan 24, 2018 at 4:32 PM, Carlos O'Donell <carlos@redhat.com> wrote: > On 01/24/2018 10:23 AM, Florian Weimer wrote: >> On 01/24/2018 07:08 PM, H.J. Lu wrote: >>> We opened a bug: >>> >>> https://sourceware.org/bugzilla/show_bug.cgi?id=22743 >>> >>> Any help to track down the root cause is appreciated. >> >> Doesn't the bug report clearly show the root cause? The offset of >> priv.data.cleanup changed, and old binaries have an insufficiently >> large stack allocation for the new offset. >> >> (Congratulations for tracking it down, by the way. I know that such >> bugs are hard.) >> >> You need to add a symbol version for pthread_register_cancel. It's >> too late for that now, so I recommend reverting the faulty commit. > > I have finished analyzing this and debugging the root cause myself, > and I agree with Florian, we need to revert: > > commit f81ddabffd76ac9dd600b02adbf3e1dac4bb10ec > commit cba595c350e52194e10c0006732e1991e3d0803b > > At a minimum. I am testing with them reverted locally. > > To be honest I'm surprised that this passed review and was checked > in, because the __pthread_unwind_buf_t has only at most 4-bytes of > space left before it is an ABI change. In the future please ping > me if you have any doubts and I'll review. > > The addition of __sigset_t saved_mask moves pthread_unwind_buf's > priv.data.cleanup forward by 124-bytes. The on-stack allocation of > the pthread_cleanup_push's __pthread_unwind_buf_t is not that big > and so __pthread_register_cancel writes to other structures which > are allocated on the stack. > > You cannot expand struct pthread_unwind_buf because the on-stack > allocated __pthread_unwind_buf_t is not large enough in existing > applications. > > You *might* have used feature_1 to change between two different > layouts of struct pthread_unwind_buf, but that will have to wait > for 2.28. As Florian suggests though it is cleaner to version > __pthread_register_cancel for x86 and the older version expects > the smaller non-CET-enabled structure. > I will try to fix it by next Monday.
On 01/24/2018 05:09 PM, H.J. Lu wrote: > On Wed, Jan 24, 2018 at 4:32 PM, Carlos O'Donell <carlos@redhat.com> wrote: >> On 01/24/2018 10:23 AM, Florian Weimer wrote: >>> On 01/24/2018 07:08 PM, H.J. Lu wrote: >>>> We opened a bug: >>>> >>>> https://sourceware.org/bugzilla/show_bug.cgi?id=22743 >>>> >>>> Any help to track down the root cause is appreciated. >>> >>> Doesn't the bug report clearly show the root cause? The offset of >>> priv.data.cleanup changed, and old binaries have an insufficiently >>> large stack allocation for the new offset. >>> >>> (Congratulations for tracking it down, by the way. I know that such >>> bugs are hard.) >>> >>> You need to add a symbol version for pthread_register_cancel. It's >>> too late for that now, so I recommend reverting the faulty commit. >> >> I have finished analyzing this and debugging the root cause myself, >> and I agree with Florian, we need to revert: >> >> commit f81ddabffd76ac9dd600b02adbf3e1dac4bb10ec >> commit cba595c350e52194e10c0006732e1991e3d0803b >> >> At a minimum. I am testing with them reverted locally. >> >> To be honest I'm surprised that this passed review and was checked >> in, because the __pthread_unwind_buf_t has only at most 4-bytes of >> space left before it is an ABI change. In the future please ping >> me if you have any doubts and I'll review. >> >> The addition of __sigset_t saved_mask moves pthread_unwind_buf's >> priv.data.cleanup forward by 124-bytes. The on-stack allocation of >> the pthread_cleanup_push's __pthread_unwind_buf_t is not that big >> and so __pthread_register_cancel writes to other structures which >> are allocated on the stack. >> >> You cannot expand struct pthread_unwind_buf because the on-stack >> allocated __pthread_unwind_buf_t is not large enough in existing >> applications. >> >> You *might* have used feature_1 to change between two different >> layouts of struct pthread_unwind_buf, but that will have to wait >> for 2.28. As Florian suggests though it is cleaner to version >> __pthread_register_cancel for x86 and the older version expects >> the smaller non-CET-enabled structure. >> > > I will try to fix it by next Monday. I will be reverting these changes in the next 24 hours. They have a directly proven negative ABI consequences and will be removed as soon as I finish validation that the reverted patches are functional and pass the expected tests. On Monday when you have a full fix ready we can discuss this with Dmitry Levin and the other developers to see if everyone agrees that the solution is acceptable risk for 2.27.
On Wed, Jan 24, 2018 at 05:09:39PM -0800, H.J. Lu wrote: > On Wed, Jan 24, 2018 at 4:32 PM, Carlos O'Donell <carlos@redhat.com> wrote: > > On 01/24/2018 10:23 AM, Florian Weimer wrote: > >> On 01/24/2018 07:08 PM, H.J. Lu wrote: > >>> We opened a bug: > >>> > >>> https://sourceware.org/bugzilla/show_bug.cgi?id=22743 > >>> > >>> Any help to track down the root cause is appreciated. > >> > >> Doesn't the bug report clearly show the root cause? The offset of > >> priv.data.cleanup changed, and old binaries have an insufficiently > >> large stack allocation for the new offset. > >> > >> (Congratulations for tracking it down, by the way. I know that such > >> bugs are hard.) > >> > >> You need to add a symbol version for pthread_register_cancel. It's > >> too late for that now, so I recommend reverting the faulty commit. > > > > I have finished analyzing this and debugging the root cause myself, > > and I agree with Florian, we need to revert: > > > > commit f81ddabffd76ac9dd600b02adbf3e1dac4bb10ec > > commit cba595c350e52194e10c0006732e1991e3d0803b > > > > At a minimum. I am testing with them reverted locally. > > > > To be honest I'm surprised that this passed review and was checked > > in, because the __pthread_unwind_buf_t has only at most 4-bytes of > > space left before it is an ABI change. In the future please ping > > me if you have any doubts and I'll review. > > > > The addition of __sigset_t saved_mask moves pthread_unwind_buf's > > priv.data.cleanup forward by 124-bytes. The on-stack allocation of > > the pthread_cleanup_push's __pthread_unwind_buf_t is not that big > > and so __pthread_register_cancel writes to other structures which > > are allocated on the stack. > > > > You cannot expand struct pthread_unwind_buf because the on-stack > > allocated __pthread_unwind_buf_t is not large enough in existing > > applications. > > > > You *might* have used feature_1 to change between two different > > layouts of struct pthread_unwind_buf, but that will have to wait > > for 2.28. As Florian suggests though it is cleaner to version > > __pthread_register_cancel for x86 and the older version expects > > the smaller non-CET-enabled structure. > > I will try to fix it by next Monday. I'm afraid by Monday it will be too late for 2.27 as we will get very little testing before the release.
From b6aad5a9958354e236f42877575e9f76f90348d1 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Wed, 6 Dec 2017 15:00:46 -0800 Subject: [PATCH] Linux/x86: Update cancel_jmp_buf to match __jmp_buf_tag [BZ #22563] On x86, padding in struct __jmp_buf_tag is used for shadow stack pointer to support shadow stack in Intel Control-flow Enforcemen Technology. Since the cancel_jmp_buf array is passed to setjmp and longjmp by casting it to pointer to struct __jmp_buf_tag, it should be as large as struct __jmp_buf_tag. This 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. Tested by build-many-glibcs.py. [BZ #22563] * bits/types/__cancel_jmp_buf_tag.h: New file. * sysdeps/unix/sysv/linux/x86/bits/types/__cancel_jmp_buf_tag.h * sysdeps/unix/sysv/linux/x86/pthreaddef.h: Likewise. * sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h: Likewise. * nptl/Makefile (headers): Add bits/types/__cancel_jmp_buf_tag.h. * nptl/descr.h [NEED_SAVED_MASK_IN_CANCEL_JMP_BUF] (pthread_unwind_buf): Add saved_mask to cancel_jmp_buf. * sysdeps/nptl/pthread.h: Include <bits/types/__cancel_jmp_buf_tag.h>. (__pthread_unwind_buf_t): Use struct __cancel_jmp_buf_tag with __cancel_jmp_buf. --- bits/types/__cancel_jmp_buf_tag.h | 28 +++++++++++++++++ nptl/Makefile | 3 +- nptl/descr.h | 3 ++ sysdeps/nptl/pthread.h | 7 ++--- .../linux/x86/bits/types/__cancel_jmp_buf_tag.h | 31 +++++++++++++++++++ sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h | 36 ++++++++++++++++++++++ sysdeps/unix/sysv/linux/x86/pthreaddef.h | 22 +++++++++++++ 7 files changed, 124 insertions(+), 6 deletions(-) create mode 100644 bits/types/__cancel_jmp_buf_tag.h create mode 100644 sysdeps/unix/sysv/linux/x86/bits/types/__cancel_jmp_buf_tag.h create mode 100644 sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h create mode 100644 sysdeps/unix/sysv/linux/x86/pthreaddef.h diff --git a/bits/types/__cancel_jmp_buf_tag.h b/bits/types/__cancel_jmp_buf_tag.h new file mode 100644 index 0000000000..c843f44239 --- /dev/null +++ b/bits/types/__cancel_jmp_buf_tag.h @@ -0,0 +1,28 @@ +/* Define struct __cancel_jmp_buf_tag. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#ifndef ____cancel_jmp_buf_tag_defined +#define ____cancel_jmp_buf_tag_defined 1 + +struct __cancel_jmp_buf_tag + { + __jmp_buf __cancel_jmp_buf; + int __mask_was_saved; + }; + +#endif diff --git a/nptl/Makefile b/nptl/Makefile index 11e6ecd88b..46e110773c 100644 --- a/nptl/Makefile +++ b/nptl/Makefile @@ -22,7 +22,8 @@ subdir := nptl include ../Makeconfig -headers := pthread.h semaphore.h bits/semaphore.h +headers := pthread.h semaphore.h bits/semaphore.h \ + bits/types/__cancel_jmp_buf_tag.h extra-libs := libpthread extra-libs-others := $(extra-libs) diff --git a/nptl/descr.h b/nptl/descr.h index c83b17b674..fdeb397eab 100644 --- a/nptl/descr.h +++ b/nptl/descr.h @@ -65,6 +65,9 @@ struct pthread_unwind_buf { __jmp_buf jmp_buf; int mask_was_saved; +#ifdef NEED_SAVED_MASK_IN_CANCEL_JMP_BUF + __sigset_t saved_mask; +#endif } cancel_jmp_buf[1]; union diff --git a/sysdeps/nptl/pthread.h b/sysdeps/nptl/pthread.h index 2b2b386ab3..787ac6e4cd 100644 --- a/sysdeps/nptl/pthread.h +++ b/sysdeps/nptl/pthread.h @@ -27,6 +27,7 @@ #include <bits/setjmp.h> #include <bits/wordsize.h> #include <bits/types/struct_timespec.h> +#include <bits/types/__cancel_jmp_buf_tag.h> /* Detach state. */ @@ -523,11 +524,7 @@ extern void pthread_testcancel (void); typedef struct { - struct - { - __jmp_buf __cancel_jmp_buf; - int __mask_was_saved; - } __cancel_jmp_buf[1]; + struct __cancel_jmp_buf_tag __cancel_jmp_buf[1]; void *__pad[4]; } __pthread_unwind_buf_t __attribute__ ((__aligned__)); diff --git a/sysdeps/unix/sysv/linux/x86/bits/types/__cancel_jmp_buf_tag.h b/sysdeps/unix/sysv/linux/x86/bits/types/__cancel_jmp_buf_tag.h new file mode 100644 index 0000000000..830a6ec90c --- /dev/null +++ b/sysdeps/unix/sysv/linux/x86/bits/types/__cancel_jmp_buf_tag.h @@ -0,0 +1,31 @@ +/* Define struct __cancel_jmp_buf_tag. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#ifndef ____cancel_jmp_buf_tag_defined +#define ____cancel_jmp_buf_tag_defined 1 + +#include <bits/types/__sigset_t.h> + +struct __cancel_jmp_buf_tag + { + __jmp_buf __cancel_jmp_buf; + int __mask_was_saved; + __sigset_t __saved_mask; + }; + +#endif diff --git a/sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h b/sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h new file mode 100644 index 0000000000..8c36ba3a5d --- /dev/null +++ b/sysdeps/unix/sysv/linux/x86/nptl/pthreadP.h @@ -0,0 +1,36 @@ +/* Internal pthread header. Linux/x86 version. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include_next <nptl/pthreadP.h> + +#ifndef _PTHREADP_H_X86 +#define _PTHREADP_H_X86 1 + +extern struct pthread_unwind_buf ____pthread_unwind_buf_private; + +_Static_assert (sizeof (____pthread_unwind_buf_private.cancel_jmp_buf) + >= sizeof (struct __jmp_buf_tag), + "size of cancel_jmp_buf < sizeof __jmp_buf_tag"); + +extern __pthread_unwind_buf_t ____pthread_unwind_buf; + +_Static_assert (sizeof (____pthread_unwind_buf.__cancel_jmp_buf) + >= sizeof (struct __jmp_buf_tag), + "size of __cancel_jmp_buf < sizeof __jmp_buf_tag"); + +#endif diff --git a/sysdeps/unix/sysv/linux/x86/pthreaddef.h b/sysdeps/unix/sysv/linux/x86/pthreaddef.h new file mode 100644 index 0000000000..89d19d60a1 --- /dev/null +++ b/sysdeps/unix/sysv/linux/x86/pthreaddef.h @@ -0,0 +1,22 @@ +/* Pthread macros. Linux/x86 version. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include_next <pthreaddef.h> + +/* Need saved_mask in cancel_jmp_buf. */ +#define NEED_SAVED_MASK_IN_CANCEL_JMP_BUF 1 -- 2.14.3