* RFA: Need to extend x86 psABI to support thread cancellation and alternate signal stack @ 2018-03-26 22:43 H.J. Lu 2018-03-27 6:31 ` Florian Weimer 0 siblings, 1 reply; 6+ messages in thread From: H.J. Lu @ 2018-03-26 22:43 UTC (permalink / raw) To: GNU C Library, GCC Development, Carlos O'Donell On Linux, when alternate signal stack is used with thread cancellation, _Unwind_Resume fails when it tries to unwind shadow stack from signal handler on alternate signal stack. The issue is that signal handler on alternate signal stack uses a separate shadow stack and we must switch to the original shadow stack to unwind it. But frame count will be wrong in this case. For thread cancellation, there is no need to unwind shadow stack since it will long jump back and exit. One possibility is 1. Add _URC_NO_REASON_CANCEL. 2. unwind_stop in libpthread returns _URC_NO_REASON_CANCEL. 3. _Unwind_ForcedUnwind_Phase2 sets frames to 1 for _URC_NO_REASON_CANCEL BTW, I opened: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85086 -- H.J. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFA: Need to extend x86 psABI to support thread cancellation and alternate signal stack 2018-03-26 22:43 RFA: Need to extend x86 psABI to support thread cancellation and alternate signal stack H.J. Lu @ 2018-03-27 6:31 ` Florian Weimer 2018-03-27 11:26 ` H.J. Lu 0 siblings, 1 reply; 6+ messages in thread From: Florian Weimer @ 2018-03-27 6:31 UTC (permalink / raw) To: H.J. Lu, GNU C Library, GCC Development, Carlos O'Donell On 03/27/2018 12:43 AM, H.J. Lu wrote: > On Linux, when alternate signal stack is used with thread cancellation, > _Unwind_Resume fails when it tries to unwind shadow stack from signal > handler on alternate signal stack. The issue is that signal handler on > alternate signal stack uses a separate shadow stack and we must switch > to the original shadow stack to unwind it. But frame count will be wrong > in this case. For thread cancellation, there is no need to unwind shadow > stack since it will long jump back and exit. > > One possibility is > > 1. Add _URC_NO_REASON_CANCEL. > 2. unwind_stop in libpthread returns _URC_NO_REASON_CANCEL. > 3. _Unwind_ForcedUnwind_Phase2 sets frames to 1 for > _URC_NO_REASON_CANCEL I assume the sequence of events goes like this: 1. The program receives a signal with a SA_ONSTACK handler. 2. The program switches to the alternate signal stack (including an alternate shadow stack) and runs the handler. 3. The handler reaches a cancellation point. 4. Cancellation is acted upon. During unwinding, INCSSP is executed as needed. The switch from the alternate signal stack is implicit in the SP register restore. But there is no corresponding stack switch back to the original shadow stack. This means that INCSSP faults once the alternate stack is empty. Is this description accurate? I think this has to be fixed entirely within the libgcc unwinder. Otherwise, any application which throws from a (synchronous) signal handler will have the same issue, and I think this is something we need to support. It may be possible to implement this without kernel changes: Patch the interrupted context to continue unwinding, and then call sigreturn to switch both stacks at the same time. Thanks, Florian ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFA: Need to extend x86 psABI to support thread cancellation and alternate signal stack 2018-03-27 6:31 ` Florian Weimer @ 2018-03-27 11:26 ` H.J. Lu 2018-03-27 15:43 ` Florian Weimer 0 siblings, 1 reply; 6+ messages in thread From: H.J. Lu @ 2018-03-27 11:26 UTC (permalink / raw) To: Florian Weimer; +Cc: GNU C Library, GCC Development, Carlos O'Donell On Mon, Mar 26, 2018 at 11:31 PM, Florian Weimer <fweimer@redhat.com> wrote: > On 03/27/2018 12:43 AM, H.J. Lu wrote: >> >> On Linux, when alternate signal stack is used with thread cancellation, >> _Unwind_Resume fails when it tries to unwind shadow stack from signal >> handler on alternate signal stack. The issue is that signal handler on >> alternate signal stack uses a separate shadow stack and we must switch >> to the original shadow stack to unwind it. But frame count will be wrong >> in this case. For thread cancellation, there is no need to unwind shadow >> stack since it will long jump back and exit. >> >> One possibility is >> >> 1. Add _URC_NO_REASON_CANCEL. >> 2. unwind_stop in libpthread returns _URC_NO_REASON_CANCEL. >> 3. _Unwind_ForcedUnwind_Phase2 sets frames to 1 for >> _URC_NO_REASON_CANCEL > > > I assume the sequence of events goes like this: > > 1. The program receives a signal with a SA_ONSTACK handler. > 2. The program switches to the alternate signal stack (including an > alternate shadow stack) and runs the handler. > 3. The handler reaches a cancellation point. > 4. Cancellation is acted upon. > > During unwinding, INCSSP is executed as needed. The switch from the > alternate signal stack is implicit in the SP register restore. But there is > no corresponding stack switch back to the original shadow stack. This means > that INCSSP faults once the alternate stack is empty. > > Is this description accurate? That is correct. > I think this has to be fixed entirely within the libgcc unwinder. Otherwise, > any application which throws from a (synchronous) signal handler will have > the same issue, and I think this is something we need to support. There are 2 ways to unwind shadow stack: 1. setjmp saves shadow stack register and longjmp pops shadow stack until shadow stack register matches the saved value. To support longjmp from signal handler, we make a syscall to restore the original shadow stack. 2. Since shadow stack is never saved and restored by compiler, unwinder in libgcc counts how many stack frame it has to unwind and uses INCSSP to pop shadow stack. This can't unwind the original shadow stack when the alternate shadow stack is used. _URC_NO_REASON_CANCEL works only if longjmp will be used to finish stack unwinding, which is the case for thread cancellation in glibc. Here are patches for GCC: https://github.com/hjl-tools/gcc/commit/e9ff815941406e38fa629947af4d809b9129e860 and glibc: https://github.com/hjl-tools/glibc/commit/1aec81528ab26aa8a8a7965317b6e1a8ba4526aa They fixed the issue. > It may be possible to implement this without kernel changes: Patch the > interrupted context to continue unwinding, and then call sigreturn to switch > both stacks at the same time. > We passed almost all 5000+ tests in glibc with shadow stack and indirect branch tracking enabled. The only remaining failures are thread cancellation with alternate signal stack and -fasynchronous-unwind-tables. I couldn't find a way to unwind shadow stack by counting stack frame when exception happens in alternate signal stack. -- H.J. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFA: Need to extend x86 psABI to support thread cancellation and alternate signal stack 2018-03-27 11:26 ` H.J. Lu @ 2018-03-27 15:43 ` Florian Weimer 2018-03-27 15:55 ` H.J. Lu 0 siblings, 1 reply; 6+ messages in thread From: Florian Weimer @ 2018-03-27 15:43 UTC (permalink / raw) To: H.J. Lu; +Cc: GNU C Library, GCC Development, Carlos O'Donell On 03/27/2018 01:26 PM, H.J. Lu wrote: > 2. Since shadow stack is never saved and restored by compiler, unwinder > in libgcc counts how many stack frame it has to unwind and uses INCSSP > to pop shadow stack. This can't unwind the original shadow stack when > the alternate shadow stack is used. _URC_NO_REASON_CANCEL > works only if longjmp will be used to finish stack unwinding, which is > the case for thread cancellation in glibc. > > Here are patches for GCC: > > https://github.com/hjl-tools/gcc/commit/e9ff815941406e38fa629947af4d809b9129e860 > > and glibc: > > https://github.com/hjl-tools/glibc/commit/1aec81528ab26aa8a8a7965317b6e1a8ba4526aa > > They fixed the issue. The patches are nice and short, but: Do they really fix the issue? They make cancellation work again, but they do not fix the general unwinding issue with alternate signal handler stacks AFAICS. >> It may be possible to implement this without kernel changes: Patch the >> interrupted context to continue unwinding, and then call sigreturn to switch >> both stacks at the same time. >> > > We passed almost all 5000+ tests in glibc with shadow stack and indirect > branch tracking enabled. The only remaining failures are thread cancellation > with alternate signal stack and -fasynchronous-unwind-tables. I couldn't > find a way to unwind shadow stack by counting stack frame when exception > happens in alternate signal stack. I'm not sure how comprehensive these tests are, considering that no one expected something like shadow stacks (maybe those dual ia64 stacks are somewhat similar, but I don't know anything about them). Thanks, Florian ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFA: Need to extend x86 psABI to support thread cancellation and alternate signal stack 2018-03-27 15:43 ` Florian Weimer @ 2018-03-27 15:55 ` H.J. Lu 2018-03-28 13:04 ` H.J. Lu 0 siblings, 1 reply; 6+ messages in thread From: H.J. Lu @ 2018-03-27 15:55 UTC (permalink / raw) To: Florian Weimer; +Cc: GNU C Library, GCC Development, Carlos O'Donell On Tue, Mar 27, 2018 at 8:43 AM, Florian Weimer <fweimer@redhat.com> wrote: > On 03/27/2018 01:26 PM, H.J. Lu wrote: > >> 2. Since shadow stack is never saved and restored by compiler, unwinder >> in libgcc counts how many stack frame it has to unwind and uses INCSSP >> to pop shadow stack. This can't unwind the original shadow stack when >> the alternate shadow stack is used. _URC_NO_REASON_CANCEL >> works only if longjmp will be used to finish stack unwinding, which is >> the case for thread cancellation in glibc. >> >> Here are patches for GCC: >> >> >> https://github.com/hjl-tools/gcc/commit/e9ff815941406e38fa629947af4d809b9129e860 >> >> and glibc: >> >> >> https://github.com/hjl-tools/glibc/commit/1aec81528ab26aa8a8a7965317b6e1a8ba4526aa >> >> They fixed the issue. > > > The patches are nice and short, but: Do they really fix the issue? They > make cancellation work again, but they do not fix the general unwinding > issue with alternate signal handler stacks AFAICS. That is true. We do support unwinding with alternate signal handler stack using longjmp. If there is another use case of unwinding with alternate signal handler stack, we can investigate. If this isn't a valid use case, we don't want to create a very complex scheme to support it. >>> It may be possible to implement this without kernel changes: Patch the >>> interrupted context to continue unwinding, and then call sigreturn to >>> switch >>> both stacks at the same time. >>> >> >> We passed almost all 5000+ tests in glibc with shadow stack and indirect >> branch tracking enabled. The only remaining failures are thread >> cancellation >> with alternate signal stack and -fasynchronous-unwind-tables. I couldn't >> find a way to unwind shadow stack by counting stack frame when exception >> happens in alternate signal stack. > > > I'm not sure how comprehensive these tests are, considering that no one > expected something like shadow stacks (maybe those dual ia64 stacks are > somewhat similar, but I don't know anything about them). Glibc tests are invaluable to validate CET implementation. I am planning to backport CET support to glibc 2.27 so that I can enable CET in applications under Fedora 28. -- H.J. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: RFA: Need to extend x86 psABI to support thread cancellation and alternate signal stack 2018-03-27 15:55 ` H.J. Lu @ 2018-03-28 13:04 ` H.J. Lu 0 siblings, 0 replies; 6+ messages in thread From: H.J. Lu @ 2018-03-28 13:04 UTC (permalink / raw) To: Florian Weimer; +Cc: GNU C Library, GCC Development, Carlos O'Donell On Tue, Mar 27, 2018 at 8:55 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Tue, Mar 27, 2018 at 8:43 AM, Florian Weimer <fweimer@redhat.com> wrote: >> On 03/27/2018 01:26 PM, H.J. Lu wrote: >> >>> 2. Since shadow stack is never saved and restored by compiler, unwinder >>> in libgcc counts how many stack frame it has to unwind and uses INCSSP >>> to pop shadow stack. This can't unwind the original shadow stack when >>> the alternate shadow stack is used. _URC_NO_REASON_CANCEL >>> works only if longjmp will be used to finish stack unwinding, which is >>> the case for thread cancellation in glibc. >>> >>> Here are patches for GCC: >>> >>> >>> https://github.com/hjl-tools/gcc/commit/e9ff815941406e38fa629947af4d809b9129e860 >>> >>> and glibc: >>> >>> >>> https://github.com/hjl-tools/glibc/commit/1aec81528ab26aa8a8a7965317b6e1a8ba4526aa >>> >>> They fixed the issue. >> >> >> The patches are nice and short, but: Do they really fix the issue? They >> make cancellation work again, but they do not fix the general unwinding >> issue with alternate signal handler stacks AFAICS. > > That is true. We do support unwinding with alternate signal handler stack > using longjmp. If there is another use case of unwinding with alternate signal > handler stack, we can investigate. If this isn't a valid use case, we > don't want > to create a very complex scheme to support it. Here is a GCC only patch without changing glibc. If we are in alternate signal stack, don't try to unwind shadow stack. diff --git a/libgcc/config/i386/linux-unwind.h b/libgcc/config/i386/linux-unwind.h index f1f52334d8d..0e082873a5f 100644 --- a/libgcc/config/i386/linux-unwind.h +++ b/libgcc/config/i386/linux-unwind.h @@ -24,6 +24,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see /* Unwind shadow stack for -fcf-protection -mshstk. */ #if defined __SHSTK__ && defined __CET__ +# define _Unwind_Frames_Extra_Skip_Alternate_Signal_Stack # include "config/i386/shadow-stack-unwind.h" #endif diff --git a/libgcc/unwind.inc b/libgcc/unwind.inc index 68c08964d30..9fbde024135 100644 --- a/libgcc/unwind.inc +++ b/libgcc/unwind.inc @@ -152,6 +152,19 @@ _Unwind_ForcedUnwind_Phase2 (struct _Unwind_Exception *exc, void *stop_argument = (void *) (_Unwind_Ptr) exc->private_2; _Unwind_Reason_Code code, stop_code; unsigned long frames = 1; + int in_signal_stack = 0; +#ifdef _Unwind_Frames_Extra_Skip_Alternate_Signal_Stack + stack_t signal_stack; + + if (sigaltstack (NULL, &signal_stack) == 0 + && signal_stack.ss_flags == SS_ONSTACK) + { + void *stack = __builtin_frame_address (0); + if (stack >= signal_stack.ss_sp + && stack < (signal_stack.ss_sp + signal_stack.ss_size)) + in_signal_stack = 1; + } +#endif while (1) { @@ -193,6 +206,9 @@ _Unwind_ForcedUnwind_Phase2 (struct _Unwind_Exception *exc, frames++; } + if (in_signal_stack) + frames = 0; + *frames_p = frames; return code; } -- H.J. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-28 13:04 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-26 22:43 RFA: Need to extend x86 psABI to support thread cancellation and alternate signal stack H.J. Lu 2018-03-27 6:31 ` Florian Weimer 2018-03-27 11:26 ` H.J. Lu 2018-03-27 15:43 ` Florian Weimer 2018-03-27 15:55 ` H.J. Lu 2018-03-28 13:04 ` H.J. Lu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).