RFA: Need to extend x86 psABI to support thread cancellation and alternate signal stack

RFA: Need to extend x86 psABI to support thread cancellation and alternate signal stack

[H.J. Lu]

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.

Re: RFA: Need to extend x86 psABI to support thread cancellation and alternate signal stack

[Florian Weimer]

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

Re: RFA: Need to extend x86 psABI to support thread cancellation and alternate signal stack

[H.J. Lu]

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.

Re: RFA: Need to extend x86 psABI to support thread cancellation and alternate signal stack

[Florian Weimer]

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

Re: RFA: Need to extend x86 psABI to support thread cancellation and alternate signal stack

[H.J. Lu]

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.

Re: RFA: Need to extend x86 psABI to support thread cancellation and alternate signal stack

[H.J. Lu]

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.

Re: GCC contribution

[Katsunori Kumatani]

Please don't change the lang_hooks into virtual methods. Speaking from a 
plugin development viewpoint, C hooks are very easy to override 
individually, without having to change the entire object or create a new 
one.

I realize that it's not the intended use for the hooks, and not why they 
were created. But they enable plugins to do a whole lot more, despite 
the limited plugin event callbacks already present (*especially* the 
targetm hooks!).

For example, I wanted to add some plugin processing (actually a 
top-level asm statement) at the end of a translation unit (with gathered 
data) to add some special symbols / GAS directives (not instructions). 
But there's no "normal" plugin event for this.

So instead I simply saved the original lang_hooks.parse_file hook 
(pointer), overrode the hook with the plugin's, which calls the original 
first and then does whatever plugin does. And it works fine. This kind 
of thing wouldn't be easily changed with virtual methods I believe.

Once again, I realize that was not the intended use for hooks, but it's 
a nice side effect and IMO should be kept.

Plugins suffer a lot from the "chicken and egg" problem (nobody wants to 
add events that would not be needed, but nobody codes plugins without 
enough events) and hooks kind of solve a lot of it, so that's a good 
thing (especially targetm, more so than lang_hooks).

I mean, if you had to first suggest an event and *hope* it gets added 
before even starting your plugin (otherwise it would be a waste of 
time), you'd just give up. Or at least, the majority would. At least 
with hooks, nobody needs to add anything for plugins, since you can 
override the C hooks directly from the plugin (while maintaining a chain 
for the old one).

Re: GCC contribution

[Andre Groenewald]

The heart of GCC should remain pure C as far as possible, for the very
same reason the Linux kernel is only in C. Deviate from this, and in a
few years we will end up with Java as the programming language of GCC.

It is the duty of the steering community and our leaders in GCC to be
very conservative in this regard.

Please I don't want to oppose someone’s proposal, I want the people to
understand reason of being conservative in this regard, because they
will end up being the leaders of GCC in the following generations.

GCC steering community I count on you and speaking behalf other
developers to keep GCC as close to C as possible for at least the next
1000 years.

On Thu, Mar 29, 2018 at 12:15 AM, Katsunori Kumatani
<katsunori.kumatani@gmail.com> wrote:
> Please don't change the lang_hooks into virtual methods. Speaking from a
> plugin development viewpoint, C hooks are very easy to override
> individually, without having to change the entire object or create a new
> one.
>
> I realize that it's not the intended use for the hooks, and not why they
> were created. But they enable plugins to do a whole lot more, despite the
> limited plugin event callbacks already present (*especially* the targetm
> hooks!).
>
> For example, I wanted to add some plugin processing (actually a top-level
> asm statement) at the end of a translation unit (with gathered data) to add
> some special symbols / GAS directives (not instructions). But there's no
> "normal" plugin event for this.
>
> So instead I simply saved the original lang_hooks.parse_file hook (pointer),
> overrode the hook with the plugin's, which calls the original first and then
> does whatever plugin does. And it works fine. This kind of thing wouldn't be
> easily changed with virtual methods I believe.
>
> Once again, I realize that was not the intended use for hooks, but it's a
> nice side effect and IMO should be kept.
>
> Plugins suffer a lot from the "chicken and egg" problem (nobody wants to add
> events that would not be needed, but nobody codes plugins without enough
> events) and hooks kind of solve a lot of it, so that's a good thing
> (especially targetm, more so than lang_hooks).
>
> I mean, if you had to first suggest an event and *hope* it gets added before
> even starting your plugin (otherwise it would be a waste of time), you'd
> just give up. Or at least, the majority would. At least with hooks, nobody
> needs to add anything for plugins, since you can override the C hooks
> directly from the plugin (while maintaining a chain for the old one).

Re: GCC contribution

[Jeff Law]

On 03/29/2018 12:56 AM, Andre Groenewald wrote:
> The heart of GCC should remain pure C as far as possible, for the very
> same reason the Linux kernel is only in C. Deviate from this, and in a
> few years we will end up with Java as the programming language of GCC.
> 
> It is the duty of the steering community and our leaders in GCC to be
> very conservative in this regard.
> 
> Please I don't want to oppose someone’s proposal, I want the people to
> understand reason of being conservative in this regard, because they
> will end up being the leaders of GCC in the following generations.
> 
> GCC steering community I count on you and speaking behalf other
> developers to keep GCC as close to C as possible for at least the next
> 1000 years.
We've already made a decision to use C++ when it makes sense.  That ship
sailed years ago.

jeff

Re: GCC contribution

[Richard Kenner]

> > GCC steering community I count on you and speaking behalf other
> > developers to keep GCC as close to C as possible for at least the next
> > 1000 years.
>
> We've already made a decision to use C++ when it makes sense.  That ship
> sailed years ago.

I don't see "keeping GCC as close to C as possible" and "using C++ when
it makes sense" as contradictory.  In fact, they sound pretty close
to the same thing to me.

Re: GCC contribution

[Mikhail Maltsev]

On Wed, Mar 28, 2018 at 11:15 PM, Katsunori Kumatani <
katsunori.kumatani@gmail.com> wrote:

> For example, I wanted to add some plugin processing (actually a top-level
> asm statement) at the end of a translation unit (with gathered data) to add
> some special symbols / GAS directives (not instructions). But there's no
> "normal" plugin event for this.
>
> So instead I simply saved the original lang_hooks.parse_file hook
> (pointer), overrode the hook with the plugin's, which calls the original
> first and then does whatever plugin does. And it works fine. This kind of
> thing wouldn't be easily changed with virtual methods I believe.
>
I think this would be pretty straightforward: you will need a single plugin
event, which would allow you to provide your own instance of a class
derived from lang_hooks. You could then inherit a new class from the
lang_hooks class in the frontend and override any of its virtual methods.

-- 
Regards,
   Mikhail Maltsev

Re: GCC contribution

[Katsunori Kumatani]

On Thu, Mar 29, 2018 at 8:01 PM, Mikhail Maltsev <maltsevm@gmail.com> wrote:
> On Wed, Mar 28, 2018 at 11:15 PM, Katsunori Kumatani
> <katsunori.kumatani@gmail.com> wrote:
>>
>> For example, I wanted to add some plugin processing (actually a top-level
>> asm statement) at the end of a translation unit (with gathered data) to add
>> some special symbols / GAS directives (not instructions). But there's no
>> "normal" plugin event for this.
>>
>> So instead I simply saved the original lang_hooks.parse_file hook
>> (pointer), overrode the hook with the plugin's, which calls the original
>> first and then does whatever plugin does. And it works fine. This kind of
>> thing wouldn't be easily changed with virtual methods I believe.
>
> I think this would be pretty straightforward: you will need a single plugin
> event, which would allow you to provide your own instance of a class derived
> from lang_hooks. You could then inherit a new class from the lang_hooks
> class in the frontend and override any of its virtual methods.
>
> --
> Regards,
>    Mikhail Maltsev

But the point is that, when I first started using this "trick" with
overriding hooks, there was no such functionality.

I'm not saying that adding it is a bad thing! I'm only saying that, in
this context, C hooks are easier to change without having to account
for special plugin code (which did not exist). For example what if
someone else comes up with another idea like that (but which has no
workaround yet), but he can't do it anymore and has to request a
change? Most people would probably give up on their plugin ideas at
this point.

I mean, most hooks don't even have a "this" pointer or object, I don't
see why making them virtual would be better anyway, especially since
it's *only one* object we're talking about? (one global variable).
Function pointers obviously bloat things up if there's a lot of
instances of the class/struct, but this is not the case, so they make
more sense in this case than virtual methods IMO.

Re: GCC contribution

[Nathan Sidwell]

On 03/29/2018 09:48 AM, Jeff Law wrote:
> On 03/29/2018 12:56 AM, Andre Groenewald wrote:

>> GCC steering community I count on you and speaking behalf other
>> developers to keep GCC as close to C as possible for at least the next
>> 1000 years.
> We've already made a decision to use C++ when it makes sense.  That ship
> sailed years ago.

Yeah, when can I write something as anachronistically futuristic as:

   v.qsort ([](auto a, auto b)
              { return int (**(Obj const *const *)a
                            <=> **(Obj const *const*)b); });

huh?

nathan
-- 
Nathan Sidwell

Re: GCC contribution

[Andre Groenewald]

Your prochronistic expression is why front ends is developed.

The purpose of the GCC engine is to make anachronistic futuristic code
executable by means of a front-end, not to implement every star-trek
expression.

Why having this pointless conversation, the Titanic has left the port.


On Fri, Mar 30, 2018 at 2:16 PM, Nathan Sidwell <nathan@acm.org> wrote:
> On 03/29/2018 09:48 AM, Jeff Law wrote:
>>
>> On 03/29/2018 12:56 AM, Andre Groenewald wrote:
>
>
>>> GCC steering community I count on you and speaking behalf other
>>> developers to keep GCC as close to C as possible for at least the next
>>> 1000 years.
>>
>> We've already made a decision to use C++ when it makes sense.  That ship
>> sailed years ago.
>
>
> Yeah, when can I write something as anachronistically futuristic as:
>
>   v.qsort ([](auto a, auto b)
>              { return int (**(Obj const *const *)a
>                            <=> **(Obj const *const*)b); });
>
> huh?
>
> nathan
> --
> Nathan Sidwell