* 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).