Re: [PATCH 0/2] nptl: Update struct pthread_unwind_buf

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

On Wed, Mar 7, 2018 at 12:14 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Mar 7, 2018 at 11:47 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Mar 7, 2018 at 9:34 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>>> On 03/07/2018 03:56 AM, H.J. Lu wrote:
>>>>>> 1. I have to add  __setjmp_cancel and __sigsetjmp_cancel which won't
>>>>>> save and restore shadow stack register.
>>>>
>>>> I have been testing this.  I ran into one issue.  GCC knows that setjmp will
>>>> return via longjmp and inserts ENDBR after it.  But it doesn't know
>>>>   __setjmp_cancel and __sigsetjmp_cancel.   We can either add them to GCC
>>>> or add NOTRACK prefix to the corresponding longjmps.
>>>
>>> I would rather GCC did not know about these implementation details.
>>>
>>> I have no objection to the NOTRACK prefix in the corresponding longjmps.
>>>
>>> What would be a downside to this choice?
>>>
>>
>> NOTRACK prefix is typically generated by compiler for switch table.  Compiler
>> knows each indirect jump target is valid and pointer load for indirect jump is
>> generated by compiler in read-only section.  This is pretty safe since there is
>> very little chance for malicious codes to temper the pointer value.  However,
>> in case of longjmp, the indirect jump target is in jmpbuf.   There is
>> a possilibty
>> for malicious codes to change the indirect jump target such that longjmp wil
>> jump to the wrong place.  Use NOTRACK prefix here defeats the purpose of
>> indirect branch tracking in CET.
>>
>
> Also GCC needs to know that __setjmp_cancel and __sigsetjmp_cancel may
> return twice, similar to setjmp.
>

Here is the GCC patch:


diff --git a/gcc/calls.c b/gcc/calls.c
index 19c95b8455b..d1f436dfa91 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -604,7 +604,7 @@ special_function_p (const_tree fndecl, int flags)
     name_decl = DECL_NAME (cgraph_node::get (fndecl)->orig_decl);

   if (fndecl && name_decl
-      && IDENTIFIER_LENGTH (name_decl) <= 11
+      && IDENTIFIER_LENGTH (name_decl) <= 18
       /* Exclude functions not at the file scope, or not `extern',
    since they are not the magic functions we would otherwise
    think they are.
@@ -637,8 +637,8 @@ special_function_p (const_tree fndecl, int flags)
   }

       /* ECF_RETURNS_TWICE is safe even for -ffreestanding.  */
-      if (! strcmp (tname, "setjmp")
-    || ! strcmp (tname, "sigsetjmp")
+      if (! strncmp (tname, "setjmp", 6)
+    || ! strncmp (tname, "sigsetjmp", 9)
     || ! strcmp (name, "savectx")
     || ! strcmp (name, "vfork")
     || ! strcmp (name, "getcontext"))


-- 
H.J.