Re: --enable-stack-protector for glibc, v7

[Nix <nix at esperi dot org dot uk>]

On 24 Jun 2016, Florian Weimer stated:

> On 06/07/2016 01:06 PM, Nix wrote:
>> This is version 7 of the stack-protected glibc patch, incorporating all review
>> comments to date (unless I missed some).
>
> I went through this patch series.
>
> Taking stock, I think the only remaining problem is the order of IFUNC
> resolvers and TCB initializers in the static linking case. The patch
> series does IFUNCs first, then TCBs. I think we should keep the
> existing order, TCB first, then IFUNCs. It seems the more conservative
> choice, but I do not have much experience with static linking.

I didn't either before, I did this work. You get much more experienced
with it when it is your primary source of mysterious crashes for ages :)


As far as I can see, the order has not changed. We used to do:

-  /* Perform IREL{,A} relocations.  */
-  apply_irel ();
-
   /* Initialize the thread library at least a bit since the libgcc
      functions are using thread functions if these are available and
      we need to setup errno.  */
   __pthread_initialize_minimal ();

... and then set up the canary.  pthread_initialize_minimal() sets up
the TCB, so it's IREL, then TCB.

We now do

+  /* Perform IREL{,A} relocations.  */
+  apply_irel ();
+
+  /* The stack guard goes into the TCB.  */
+  __pthread_initialize_tcb_internal ();
+
+  /* Set up the stack checker's canary.  */
+  uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random);
+# ifdef THREAD_SET_STACK_GUARD
+  THREAD_SET_STACK_GUARD (stack_chk_guard);
+# else
+  __stack_chk_guard = stack_chk_guard;
+# endif
+

So it's the same thing, only more explicit.

The real difference is that we now do the TCB init longer before the
rest of pthread initialization, and do all of that before the
sysdep-oscheck stuff, so that we don't need to figure out what the
sysdep-oscheck stuff might call and de-stack-protect it (under the
general rule that the less we stack-protect, the better).

> (It might also help to avoid dropping stack protection from IFUNCs,
> but since IFUNCs can do so little, that should not matter.)

I don't think we can avoid that.  In the dynamic-linking case, the
canary is initialized a long, long time after ifunc resolvers get
applied and ifuncs start getting called, and I don't want to start
messing with ld.so in this patch series :)

(Testing the other changes you suggested now.)

-- 
NULL && (void)