[03/12] Mark all machinery needed in early static-link init as -fno-stack-protector.

  From: Nick Alcock <nick.alcock@oracle.com>

The startup code in csu/, brk() and sbrk(), and the
__pthread_initialize_tcb_internal() function we just introduced are
needed very early in initialization of a statically-linked program,
before the stack guard is initialized; TLS initialization also uses
memcpy(), which cannot overrun its own stack.  Mark all of these as
-fno-stack-protector.

We also finally introduce @libc_cv_ssp@ and @no_stack_protector@, both
substituted by the configury changes made earlier, to detect the case
when -fno-stack-protector is supported by the compiler, and
unconditionally pass it in when this is the case, whether or not
--enable-stack-protector is passed to configure.  (This means that
it'll even work when the compiler's been hacked to pass
-fstack-protector by default, unless the hackage is so broken that
it does so in a way that is impossible to override.)

(At one point we marked __libc_fatal() as non-stack-protected too,
but this was pointless: all it did was call other routines which *are*
stack-protected.  The earliest __libc_fatal() call is in the
DL_SYSDEP_OSCHECK hook on some platforms, when statically linking:
this is fine, since it is after TLS and stack-canary initialization.
I have tested invocation of programs statically and dynamically
linked against this glibc on older kernels on x86 and ARM, and they
still "work", i.e. fail with the appropriate message.)

v2: No longer mark memcpy() as -fno-stack-protector.
v3: Use $(no-stack-protector).
v4: Use inhibit_stack_protector rather than de-protecting all of nptl-init.c.
v5: Don't stack-protect brk() and sbrk() in the shared library.
v7: Add comment in misc/Makefile.  Commit message tweak.
v8: Mark memcpy() as -fstack-protector again, along with wordcpy.

	* config.make.in (have-ssp): New.
	(no-stack-protector): New.
	* csu/Makefile (CFLAGS-.o): Use it.
	(CFLAGS-.og): Likewise.
	(CFLAGS-.op): Likewise.
	(CFLAGS-.os): Likewise.
	* misc/Makefile (CFLAGS-sbrk.o): Likewise.
	(CFLAGS-sbrk.op): Likewise.
	(CFLAGS-sbrk.og): Likewise.
	(CFLAGS-brk.o): Likewise.
	(CFLAGS-brk.op): Likewise.
	(CFLAGS-brk.og): Likewise.
	* string/Makefile (CFLAGS-memcpy.c): Likewise.
	(CFLAGS-wordcopy.c): Likewise.
	* nptl/nptl-init.c [!SHARED] (__pthread_initialize_tcb_internal):
	Likewise.
---
 config.make.in   | 2 ++
 csu/Makefile     | 5 +++++
 misc/Makefile    | 9 +++++++++
 nptl/nptl-init.c | 1 +
 string/Makefile  | 4 ++++
 5 files changed, 21 insertions(+)
  

On 11/28/2016 01:32 PM, Nix wrote:
> diff --git a/csu/Makefile b/csu/Makefile
> index 31e8bb9..22afe67 100644
> --- a/csu/Makefile
> +++ b/csu/Makefile
> @@ -45,6 +45,11 @@ before-compile += $(objpfx)version-info.h
>  tests := tst-empty tst-atomic tst-atomic-long
>  tests-static := tst-empty
>
> +CFLAGS-.o += $(no-stack-protector)

This also applies to the tests in the csu subdirectory, which is 
probably not what we want.

We have two options here: List the .c source files explicitly (like you 
do in string/), or move the test to a different subdirectory (perhaps 
misc/ or stdlib/).  In the second case, there should be an empty “tests” 
variable assignment in csu/Makefile which documents why there can't be 
any tests in this subdirectory.

Thanks,
Florian
  

On 15 Dec 2016, Florian Weimer outgrape:

> On 11/28/2016 01:32 PM, Nix wrote:
>> diff --git a/csu/Makefile b/csu/Makefile
>> index 31e8bb9..22afe67 100644
>> --- a/csu/Makefile
>> +++ b/csu/Makefile
>> @@ -45,6 +45,11 @@ before-compile += $(objpfx)version-info.h
>>  tests := tst-empty tst-atomic tst-atomic-long
>>  tests-static := tst-empty
>>
>> +CFLAGS-.o += $(no-stack-protector)
>
> This also applies to the tests in the csu subdirectory, which is probably not what we want.

Definitely not!

> We have two options here: List the .c source files explicitly (like you do in string/), or move the test to a different subdirectory
> (perhaps misc/ or stdlib/).  In the second case, there should be an empty “tests” variable assignment in csu/Makefile which
> documents why there can't be any tests in this subdirectory.

Listing explicitly seems likely to be horrifically hard to maintain. I'm
not even sure how many .o's there *are* that this would apply to.

Is there really no way to say 'add these flags to all the tests' so we
can add a $(stack-protector) to that?
  

On 12/15/2016 03:21 PM, Nick Alcock wrote:
> On 15 Dec 2016, Florian Weimer outgrape:
>
>> On 11/28/2016 01:32 PM, Nix wrote:
>>> diff --git a/csu/Makefile b/csu/Makefile
>>> index 31e8bb9..22afe67 100644
>>> --- a/csu/Makefile
>>> +++ b/csu/Makefile
>>> @@ -45,6 +45,11 @@ before-compile += $(objpfx)version-info.h
>>>  tests := tst-empty tst-atomic tst-atomic-long
>>>  tests-static := tst-empty
>>>
>>> +CFLAGS-.o += $(no-stack-protector)
>>
>> This also applies to the tests in the csu subdirectory, which is probably not what we want.
>
> Definitely not!
>
>> We have two options here: List the .c source files explicitly (like you do in string/), or move the test to a different subdirectory
>> (perhaps misc/ or stdlib/).  In the second case, there should be an empty “tests” variable assignment in csu/Makefile which
>> documents why there can't be any tests in this subdirectory.
>
> Listing explicitly seems likely to be horrifically hard to maintain. I'm
> not even sure how many .o's there *are* that this would apply to.

I think you added some magic to rtld CFLAGS to get this working without 
eval.

> Is there really no way to say 'add these flags to all the tests' so we
> can add a $(stack-protector) to that?

Even if we have that, it's very subtle because it would depend on the 
relative order of the expansion points (last flag wins).

So maybe moving the tests is the best option here.

Florian
  

On 15 Dec 2016, Florian Weimer outgrape:

> On 12/15/2016 03:21 PM, Nick Alcock wrote:
>> On 15 Dec 2016, Florian Weimer outgrape:
>>
>>> On 11/28/2016 01:32 PM, Nix wrote:
>>>> diff --git a/csu/Makefile b/csu/Makefile
>>>> index 31e8bb9..22afe67 100644
>>>> --- a/csu/Makefile
>>>> +++ b/csu/Makefile
>>>> @@ -45,6 +45,11 @@ before-compile += $(objpfx)version-info.h
>>>>  tests := tst-empty tst-atomic tst-atomic-long
>>>>  tests-static := tst-empty
>>>>
>>>> +CFLAGS-.o += $(no-stack-protector)
>>>
>>> This also applies to the tests in the csu subdirectory, which is probably not what we want.
>>
>> Definitely not!
>>
>>> We have two options here: List the .c source files explicitly (like you do in string/), or move the test to a different subdirectory
>>> (perhaps misc/ or stdlib/).  In the second case, there should be an empty “tests” variable assignment in csu/Makefile which
>>> documents why there can't be any tests in this subdirectory.
>>
>> Listing explicitly seems likely to be horrifically hard to maintain. I'm
>> not even sure how many .o's there *are* that this would apply to.
>
> I think you added some magic to rtld CFLAGS to get this working without eval.

My argument just collapsed because I was looking at elf/, not csu/. csu/
is comparatively tiny and much easier to fix things up for. :)

> So maybe moving the tests is the best option here.

Yeah. None of these tests actually look like they belong in csu/, except
maybe tst-empty.  misc/ seems like a better place for all of them.

I'll whip up a moving patch (it'll be a bit longer because I think I
should actually do at least one quick test run before sending it).