public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 07/14] Prevent the rtld mapfile computation from dragging in __stack_chk_fail().
  2016-02-23 23:39 --enable-stack-protector for glibc, v2, now with sparc Nix
  2016-02-23 23:39 ` [PATCH 05/14] Allow overriding of CFLAGS as well as CPPFLAGS for rtld Nix
@ 2016-02-23 23:39 ` Nix
  2016-02-23 23:40 ` [PATCH 14/14] sparc: do not stack-protect the sigreturn handler Nix
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Nix @ 2016-02-23 23:39 UTC (permalink / raw)
  To: libc-alpha; +Cc: carlos

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

The previous commit prevented rtld itself from being built with
-fstack-protector, but this is not quite enough.  We identify which
objects belong in rtld via a test link and analysis of the resulting
mapfile.  That link is necessarily done against objects that are
stack-protected, so drags in __stack_chk_fail() and all the libc
and libio code it uses.

To stop this happening, use --defsym in the test librtld.map-production
link to force the linker to predefine __stack_chk_fail() (to 0, but it
could be to anything).  (In a real link, this would of course be
catastrophic, but these object files are never used for anything else.)

v2: New.
---
 elf/Makefile | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/elf/Makefile b/elf/Makefile
index 0cb03b0..dcf44c8 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -346,9 +346,19 @@ $(objpfx)dl-allobjs.os: $(all-rtld-routines:%=$(objpfx)%.os)
 # are compiled with special flags, and puts these modules into rtld-libc.a
 # for us.  Then we do the real link using rtld-libc.a instead of libc_pic.a.
 
+# If the compiler can do SSP, build the mapfile with a dummy __stack_chk_fail
+# symbol defined, to prevent the real thing being dragged into rtld
+# even though rtld is never built with stack-protection.
+
+ifeq ($(have-ssp),yes)
+dummy-stack-chk-fail := -Wl,--defsym='__stack_chk_fail=0'
+else
+dummy-stack-chk-fail :=
+endif
+
 $(objpfx)librtld.map: $(objpfx)dl-allobjs.os $(common-objpfx)libc_pic.a
 	@-rm -f $@T
-	$(reloc-link) -o $@.o '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
+	$(reloc-link) -o $@.o $(dummy-stack-chk-fail) '-Wl,-(' $^ -lgcc '-Wl,-)' -Wl,-Map,$@T
 	rm -f $@.o
 	mv -f $@T $@
 
-- 
2.7.0.198.g6dd47b6

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 05/14] Allow overriding of CFLAGS as well as CPPFLAGS for rtld.
  2016-02-23 23:39 --enable-stack-protector for glibc, v2, now with sparc Nix
@ 2016-02-23 23:39 ` Nix
  2016-02-23 23:39 ` [PATCH 07/14] Prevent the rtld mapfile computation from dragging in __stack_chk_fail() Nix
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Nix @ 2016-02-23 23:39 UTC (permalink / raw)
  To: libc-alpha; +Cc: carlos

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

We need this to pass -fno-stack-protector to all the pieces of rtld in
non-elf/ directories.
---
 elf/rtld-Rules         | 2 +-
 scripts/sysd-rules.awk | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/elf/rtld-Rules b/elf/rtld-Rules
index 94ca39b..c1bb506 100644
--- a/elf/rtld-Rules
+++ b/elf/rtld-Rules
@@ -90,7 +90,7 @@ else
 
 rtld-compile-command.S = $(compile-command.S) $(rtld-CPPFLAGS)
 rtld-compile-command.s = $(compile-command.s) $(rtld-CPPFLAGS)
-rtld-compile-command.c = $(compile-command.c) $(rtld-CPPFLAGS)
+rtld-compile-command.c = $(compile-command.c) $(rtld-CPPFLAGS) $(rtld-CFLAGS)
 
 # These are the basic compilation rules corresponding to the Makerules ones.
 # The sysd-rules generated makefile already defines pattern rules for rtld-%
diff --git a/scripts/sysd-rules.awk b/scripts/sysd-rules.awk
index cebc9d3..69af400 100644
--- a/scripts/sysd-rules.awk
+++ b/scripts/sysd-rules.awk
@@ -54,7 +54,7 @@ BEGIN {
           command_suffix = "";
         } else {
           prefix = gensub(/%/, "", 1, target_pattern);
-          command_suffix = " $(" prefix  "CPPFLAGS)";
+          command_suffix = " $(" prefix  "CPPFLAGS)" " $(" prefix  "CFLAGS)";
         }
         target = "$(objpfx)" target_pattern o ":";
         if (asm_rules) {
-- 
2.7.0.198.g6dd47b6

^ permalink raw reply	[flat|nested] 20+ messages in thread

* --enable-stack-protector for glibc, v2, now with sparc.
@ 2016-02-23 23:39 Nix
  2016-02-23 23:39 ` [PATCH 05/14] Allow overriding of CFLAGS as well as CPPFLAGS for rtld Nix
                   ` (13 more replies)
  0 siblings, 14 replies; 20+ messages in thread
From: Nix @ 2016-02-23 23:39 UTC (permalink / raw)
  To: libc-alpha; +Cc: carlos

This is version 2 of the stack-protected glibc patch, incorporating all review
comments (unless I missed some) and adding several changes brought to light by a
sparc32/sparc64 test cycle.  (Because it is my second posting of the series,
overconfidence will surely lead me to make some catastrophic error.  Apologies in
advance if merely reading this causes your computer to catch fire.)

Still no changelog citing bug 7065: I'm not confident enough that things won't
change.

It's not rebased and is still against glibc head as of a few days ago,
a5df3210a641c17.

Tested with these flag combinations on {i686,x86_64)-pc-linux-gnu:

--enable-omitfp --enable-stack-protector=all
--enable-stack-protector
--enable-stack-protector=strong
--enable-stack-protector=all
--enable-stackguard-randomization --enable-stack-protector=all
--enable-omitfp --enable-stackguard-randomization --enable-stack-protector
--enable-omitfp --enable-stackguard-randomization --enable-stack-protector=strong
--enable-omitfp --enable-stackguard-randomization --enable-stack-protector=all
--without-stack-protector
--enable-stack-protector=no

Tested with --enable-stack-protector, --enable-stack-protector=strong and
--enable-stack-protector=all on sparc32 and sparc64 (the machines were too
slow to want to do a fuller test cycle like the one above, and --enable-omitfp
does nothing on sparc anyway, IIRC).  No failures are observed that are not
also observed on an unpatched glibc with the same flag combinations, other
than localplt (due to the new __stack_chk_fail PLT, which I'd suggest is
desirable: it seems like something people might reasonably want to interpose,
and interposing it will have no performance implications).


Overview of changes in this posting:

 - Dropped the __stack_chk_fail() changes in their entirety: the bug they were
   working around was actually a problem with the stack protector interfering
   with the rtld mapfile computation, now fixed properly.

 - Ported to SPARC/SPARC64 (because I have access to the hardware and it was
   certain that porting it to more than one architecture would comb out
   problems, which it did, and because every time the tree is broken for davem,
   a fairy dies).

 - A new patch (patch 4) to deal with problems with TLS initialization in
   statically linked binaries.  This one is definitely an efficiency loss for
   such binaries but almost certainly nobody will ever notice or care: the
   alternatives are (to me, anyway), less palatable: see the patch.

 - memcpy() is stack-protected now.

Remaining mysteries:

 - In patch 13, I have no understanding of why __pthread_mutex_unlock_usercnt()
   and __pthread_mutex_cond_lock_adjust() are special: yes, they're called
   directly from assembler, but that shouldn't be a problem: -fstack-protector
   doesn't change the function-call ABI!  Nonetheless, it *is* a problem, and
   this is definitely necessary.  I just don't know what *else* might be
   necessary.  It is almost certain that this sort of thing will be the primary
   problem when making other arches happy with this change (it was the only real
   thing I had to do with sparc, in patch 14, -fno-sparc-protecting the
   sigreturn handler, and just as with the pthread functions I don't really know
   why -- but it is clearly necessary.)

   It *is* clear why we have to -fno-stack-protect setjmp/sigjmp.c: the thing is
   *sibcalled* from assembler, and the assembler has pre-built a stack frame,
   complete with lack of canary, so setjmp/sigjmp.c had better not introduce
   one.  (Even in the absence of local variables, -fstack-protector-all will add
   a canary to the lone function in that file.)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 03/14] Mark all machinery needed in early static-link init as -fno-stack-protector.
  2016-02-23 23:39 --enable-stack-protector for glibc, v2, now with sparc Nix
                   ` (3 preceding siblings ...)
  2016-02-23 23:40 ` [PATCH 04/14] Open-code the memcpy() at static TLS initialization time Nix
@ 2016-02-23 23:40 ` Nix
  2016-02-23 23:40 ` [PATCH 11/14] Work even with compilers hacked to enable -fstack-protector by default Nix
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Nix @ 2016-02-23 23:40 UTC (permalink / raw)
  To: libc-alpha; +Cc: carlos

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. Mark all of these as
-fno-stack-protector.

We also finally introduce @libc_cv_ssp@, substituted by the configury
changes made much 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: 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 they still "work", i.e. fail with the
appropriate message.)

v2: No longer mark memcpy() as -fno-stack-protector.
---
 config.make.in | 1 +
 csu/Makefile   | 7 +++++++
 misc/Makefile  | 6 ++++++
 nptl/Makefile  | 5 +++++
 4 files changed, 19 insertions(+)

diff --git a/config.make.in b/config.make.in
index 05ed6ec..847931f 100644
--- a/config.make.in
+++ b/config.make.in
@@ -55,6 +55,7 @@ with-fp = @with_fp@
 enable-timezone-tools = @enable_timezone_tools@
 unwind-find-fde = @libc_cv_gcc_unwind_find_fde@
 have-fpie = @libc_cv_fpie@
+have-ssp = @libc_cv_ssp@
 stack-protector = @stack_protector@
 have-selinux = @have_selinux@
 have-libaudit = @have_libaudit@
diff --git a/csu/Makefile b/csu/Makefile
index 31e8bb9..8d7cbb5 100644
--- a/csu/Makefile
+++ b/csu/Makefile
@@ -45,6 +45,13 @@ before-compile += $(objpfx)version-info.h
 tests := tst-empty tst-atomic tst-atomic-long
 tests-static := tst-empty
 
+ifeq ($(have-ssp),yes)
+CFLAGS-.o += -fno-stack-protector
+CFLAGS-.og += -fno-stack-protector
+CFLAGS-.op += -fno-stack-protector
+CFLAGS-.os += -fno-stack-protector
+endif
+
 ifeq (yes,$(build-shared))
 extra-objs += S$(start-installed-name) gmon-start.os
 ifneq ($(start-installed-name),$(static-start-installed-name))
diff --git a/misc/Makefile b/misc/Makefile
index d7bbc85..ba5c5d0 100644
--- a/misc/Makefile
+++ b/misc/Makefile
@@ -99,6 +99,12 @@ CFLAGS-getusershell.c = -fexceptions
 CFLAGS-err.c = -fexceptions
 CFLAGS-tst-tsearch.c = $(stack-align-test-flags)
 
+ifeq ($(have-ssp),yes)
+# Called during static library initialization.
+CFLAGS-sbrk.c = -fno-stack-protector
+CFLAGS-brk.c = -fno-stack-protector
+endif
+
 include ../Rules
 
 $(objpfx)libg.a: $(dep-dummy-lib); $(make-dummy-lib)
diff --git a/nptl/Makefile b/nptl/Makefile
index dc3ccab..a1d52a2 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -327,6 +327,11 @@ tests += tst-execstack
 endif
 endif
 
+ifeq ($(have-ssp),yes)
+# Parts of nptl-init.c are called before the stack guard is initialized.
+CFLAGS-nptl-init.c += -fno-stack-protector
+endif
+
 modules-names = tst-atfork2mod tst-tls3mod tst-tls4moda tst-tls4modb \
 		tst-tls5mod tst-tls5moda tst-tls5modb tst-tls5modc \
 		tst-tls5modd tst-tls5mode tst-tls5modf tst-stack4mod \
-- 
2.7.0.198.g6dd47b6

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 08/14] Link libc.so with libc_nonshared.a to pull in __stack_chk_fail.
  2016-02-23 23:39 --enable-stack-protector for glibc, v2, now with sparc Nix
                   ` (5 preceding siblings ...)
  2016-02-23 23:40 ` [PATCH 11/14] Work even with compilers hacked to enable -fstack-protector by default Nix
@ 2016-02-23 23:40 ` Nix
  2016-02-23 23:40 ` [PATCH 12/14] Drop explicit stack-protection of pieces of the system Nix
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Nix @ 2016-02-23 23:40 UTC (permalink / raw)
  To: libc-alpha; +Cc: carlos

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

This prevents a spurious undefined reference from libc (which would be
an ABI break).
---
 Makerules | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makerules b/Makerules
index 53eabfa..d369d78 100644
--- a/Makerules
+++ b/Makerules
@@ -674,6 +674,7 @@ $(common-objpfx)linkobj/libc.so: link-libc-deps = # empty
 # libraries.
 $(common-objpfx)libc.so: $(elf-objpfx)soinit.os \
 			 $(common-objpfx)libc_pic.os$(libc_pic_clean) \
+			 $(common-objpfx)libc_nonshared.a \
 			 $(elf-objpfx)sofini.os \
 			 $(elf-objpfx)interp.os \
 			 $(elf-objpfx)ld.so \
@@ -683,6 +684,7 @@ $(common-objpfx)libc.so: $(elf-objpfx)soinit.os \
 
 $(common-objpfx)linkobj/libc.so: $(elf-objpfx)soinit.os \
 			 $(common-objpfx)linkobj/libc_pic.a \
+			 $(common-objpfx)libc_nonshared.a \
 			 $(elf-objpfx)sofini.os \
 			 $(elf-objpfx)interp.os \
 			 $(elf-objpfx)ld.so \
-- 
2.7.0.198.g6dd47b6

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 13/14] Avoid stack-protecting certain functions called from assembly.
  2016-02-23 23:39 --enable-stack-protector for glibc, v2, now with sparc Nix
                   ` (8 preceding siblings ...)
  2016-02-23 23:40 ` [PATCH 02/14] Initialize the stack guard earlier when linking statically Nix
@ 2016-02-23 23:40 ` Nix
  2016-02-23 23:40 ` [PATCH 09/14] Link various tests with -fno-stack-protector Nix
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Nix @ 2016-02-23 23:40 UTC (permalink / raw)
  To: libc-alpha; +Cc: carlos

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

This is the problematic part.  Without -fno-stack-protector on
__pthread_mutex_cond_lock_adjust() and __pthread_mutex_unlock_usercnt(),
nptl/tst-cond24 and nptl/tst-cond25 receive a NULL mutex at unlock time
and segfault.  However... I don't understand why.  It is the callee's
responsibility both to add the stack canary and to initialize it, just
like any other local variable.  It has to be, or the ABI for stack-
protected code would be incompatible with that for non-protected code.
But the fact remains that
sysdeps/unix/sysv/linux/i386/pthread_cond_timedwait.S both explicitly
mentions the stack frame layout and calls this function, and this call
goes wrong if we stack-protect it.

So this is somewhere where I need someone to tell me what's special about
sysdeps/unix/sysv/linux/i386/pthread_cond_timedwait.S (and in particular
special about priority-inheritance mutexes: everything else works),
before I can be confident that this is even remotely the right thing to
do.

We also de-stack-protect setjmp/sigjmp.c: it receives a sibcall from
sysdeps/x86_64/setjmp.S and lands in rtld, but is *not* rebuilt by
the machinery that rebuilds almost everything else that lands in
rtld with an appropriate MODULE_NAME.

Similar fixups may be required for things called directly from
assembly on other architectures.

v2: de-stack-protect setjmp/sigjmp.c.
---
 nptl/Makefile   | 4 ++++
 setjmp/Makefile | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/nptl/Makefile b/nptl/Makefile
index a1d52a2..95240c7 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -330,6 +330,10 @@ endif
 ifeq ($(have-ssp),yes)
 # Parts of nptl-init.c are called before the stack guard is initialized.
 CFLAGS-nptl-init.c += -fno-stack-protector
+# Parts of these files are called from assembler, with a hand-made stack,
+# sans canary.
+CFLAGS-pthread_mutex_cond_lock.c += -fno-stack-protector
+CFLAGS-pthread_mutex_unlock.c += -fno-stack-protector
 endif
 
 modules-names = tst-atfork2mod tst-tls3mod tst-tls4moda tst-tls4modb \
diff --git a/setjmp/Makefile b/setjmp/Makefile
index 5b677cc..37c5a1b 100644
--- a/setjmp/Makefile
+++ b/setjmp/Makefile
@@ -35,3 +35,9 @@ tests-static	:= tst-setjmp-static
 include ../Rules
 
 $(objpfx)tst-setjmp-fp: $(libm)
+
+ifeq ($(have-ssp),yes)
+# This is sibcalled directly from arch-specific assembly, included in rtld,
+# but never rebuilt, so it must never be built with stack protection.
+CFLAGS-sigjmp.c += -fno-stack-protector
+endif
-- 
2.7.0.198.g6dd47b6

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 14/14] sparc: do not stack-protect the sigreturn handler.
  2016-02-23 23:39 --enable-stack-protector for glibc, v2, now with sparc Nix
  2016-02-23 23:39 ` [PATCH 05/14] Allow overriding of CFLAGS as well as CPPFLAGS for rtld Nix
  2016-02-23 23:39 ` [PATCH 07/14] Prevent the rtld mapfile computation from dragging in __stack_chk_fail() Nix
@ 2016-02-23 23:40 ` Nix
  2016-02-23 23:40 ` [PATCH 04/14] Open-code the memcpy() at static TLS initialization time Nix
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Nix @ 2016-02-23 23:40 UTC (permalink / raw)
  To: libc-alpha; +Cc: carlos

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

This is called from the kernel and must not have a canary.

v2: New.
---
 sysdeps/unix/sysv/linux/sparc/sparc32/Makefile | 7 +++++++
 sysdeps/unix/sysv/linux/sparc/sparc64/Makefile | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/Makefile b/sysdeps/unix/sysv/linux/sparc/sparc32/Makefile
index 21c7dc1..60fb6f6 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc32/Makefile
+++ b/sysdeps/unix/sysv/linux/sparc/sparc32/Makefile
@@ -26,3 +26,10 @@ ifeq ($(subdir),math)
 # Provide these routines here as well.
 libm-routines += multc3 divtc3
 endif   # math
+
+ifneq (,$(filter signal nptl,$(subdir)))
+ifeq ($(have-ssp),yes)
+# We must not stack-protect the sigreturn handler.
+CFLAGS-sigaction.c += -fno-stack-protector
+endif  # have-ssp
+endif  # signal
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/Makefile b/sysdeps/unix/sysv/linux/sparc/sparc64/Makefile
index 7ea433f..dada022 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc64/Makefile
+++ b/sysdeps/unix/sysv/linux/sparc/sparc64/Makefile
@@ -12,3 +12,10 @@ ifeq ($(subdir),nptl)
 CFLAGS-pause.c += -fexceptions
 CFLAGS-sigsuspend.c += -fexceptions
 endif
+
+ifneq (,$(filter signal nptl,$(subdir)))
+ifeq ($(have-ssp),yes)
+# We must not stack-protect the sigreturn handler.
+CFLAGS-sigaction.c += -fno-stack-protector
+endif  # have-ssp
+endif  # signal
-- 
2.7.0.198.g6dd47b6

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 04/14] Open-code the memcpy() at static TLS initialization time.
  2016-02-23 23:39 --enable-stack-protector for glibc, v2, now with sparc Nix
                   ` (2 preceding siblings ...)
  2016-02-23 23:40 ` [PATCH 14/14] sparc: do not stack-protect the sigreturn handler Nix
@ 2016-02-23 23:40 ` Nix
  2016-02-23 23:40 ` [PATCH 03/14] Mark all machinery needed in early static-link init as -fno-stack-protector Nix
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Nix @ 2016-02-23 23:40 UTC (permalink / raw)
  To: libc-alpha; +Cc: carlos

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

This one is a bit nasty.  Now that we are initializing TLS earlier for
the stack canary's sake, existing memcpy() implementations become
problematic.  We cannot use the multiarch implementations, even if we
move the irel initialization up above TLS initialization, because ifunc
resolvers may not work this early (among other things, they might be
compiled with stack-protection!).  We cannot use posix/memcpy.c
unmodified without marking both it and */wordcopy.c as non-stack-protected,
which for memcpy() of all things seems like a seriously bad idea: if
any function in glibc should be stack-protected, it's memcpy() (though
stack-protecting the many optimized assembly versions is not done in this
patch series).

So we have two real options: hack up the guts of posix/memcpy.c and
*/wordcopy.c so that they can be #included (renamed and declared static)
inside libc-tls.c, or simply open-code the memcpy().  For simplicity's
sake, this patch open-codes it, on the grounds that static binaries are
relatively rare and quasi-deprecated anyway, and static binaries with
large TLS sections are yet rarer, and not worth the complexity of hacking
up all the arch-dependent wordcopy files.

(This was not revealed when testing on x86 because on that platform
GCC was open-coding the memcpy() for us.)

v2: New, lets us remove the memcpy() -fno-stack-protection, which wasn't
    enough in any case.
---
 csu/libc-tls.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/csu/libc-tls.c b/csu/libc-tls.c
index 3d67a64..20f478d 100644
--- a/csu/libc-tls.c
+++ b/csu/libc-tls.c
@@ -176,8 +176,17 @@ __libc_setup_tls (size_t tcbsize, size_t tcbalign)
 # error "Either TLS_TCB_AT_TP or TLS_DTV_AT_TP must be defined"
 #endif
   _dl_static_dtv[2].pointer.is_static = true;
-  /* sbrk gives us zero'd memory, so we don't need to clear the remainder.  */
-  memcpy (_dl_static_dtv[2].pointer.val, initimage, filesz);
+
+  /* sbrk gives us zero'd memory, so we don't need to clear the remainder.
+
+     Copy by hand, because memcpy() is stack-protected and is often multiarch too.  */
+
+  char *dst = (char *) _dl_static_dtv[2].pointer.val;
+  char *src = (char *) initimage;
+  size_t i;
+
+  for (i = 0; i < filesz; dst++, src++, i++)
+    *dst = *src;
 
   /* Install the pointer to the dtv.  */
 
-- 
2.7.0.198.g6dd47b6

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 12/14] Drop explicit stack-protection of pieces of the system.
  2016-02-23 23:39 --enable-stack-protector for glibc, v2, now with sparc Nix
                   ` (6 preceding siblings ...)
  2016-02-23 23:40 ` [PATCH 08/14] Link libc.so with libc_nonshared.a to pull in __stack_chk_fail Nix
@ 2016-02-23 23:40 ` Nix
  2016-02-23 23:40 ` [PATCH 02/14] Initialize the stack guard earlier when linking statically Nix
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Nix @ 2016-02-23 23:40 UTC (permalink / raw)
  To: libc-alpha; +Cc: carlos

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

This is probably a bad idea: maybe we want to stack-protect some parts
of the system even when ! --enable-stack-protector. I can easily adjust
the patch to do that (though it'll mean introducing a new variable
analogous to $(stack-protector) but not ocntrolled by the configure
flag.)

But if we wanted to value consistency over security, and use the same
stack-protection configure flag to control everything, this is how we'd
do it!

("Always include at least one patch with something obviously wrong with
it.")
---
 login/Makefile  | 1 -
 nscd/Makefile   | 1 -
 resolv/Makefile | 1 -
 3 files changed, 3 deletions(-)

diff --git a/login/Makefile b/login/Makefile
index 9ff36d6..1a6161c 100644
--- a/login/Makefile
+++ b/login/Makefile
@@ -58,7 +58,6 @@ CFLAGS-getpt.c = -fexceptions
 ifeq (yesyes,$(have-fpie)$(build-shared))
 pt_chown-cflags += $(pie-ccflag)
 endif
-pt_chown-cflags += $(stack-protector)
 ifeq (yes,$(have-libcap))
 libcap = -lcap
 endif
diff --git a/nscd/Makefile b/nscd/Makefile
index 50bad32..bfd72d5 100644
--- a/nscd/Makefile
+++ b/nscd/Makefile
@@ -84,7 +84,6 @@ CPPFLAGS-nscd += -D_FORTIFY_SOURCE=2
 ifeq (yesyes,$(have-fpie)$(build-shared))
 CFLAGS-nscd += $(pie-ccflag)
 endif
-CFLAGS-nscd += $(stack-protector)
 
 ifeq (yesyes,$(have-fpie)$(build-shared))
 LDFLAGS-nscd = -Wl,-z,now
diff --git a/resolv/Makefile b/resolv/Makefile
index 8be41d3..0395b1a 100644
--- a/resolv/Makefile
+++ b/resolv/Makefile
@@ -90,7 +90,6 @@ CPPFLAGS += -Dgethostbyname=res_gethostbyname \
 	    -Dgetnetbyname=res_getnetbyname \
 	    -Dgetnetbyaddr=res_getnetbyaddr
 
-CFLAGS-libresolv += $(stack-protector)
 CFLAGS-res_hconf.c = -fexceptions
 
 # The BIND code elicits some harmless warnings.
-- 
2.7.0.198.g6dd47b6

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 02/14] Initialize the stack guard earlier when linking statically.
  2016-02-23 23:39 --enable-stack-protector for glibc, v2, now with sparc Nix
                   ` (7 preceding siblings ...)
  2016-02-23 23:40 ` [PATCH 12/14] Drop explicit stack-protection of pieces of the system Nix
@ 2016-02-23 23:40 ` Nix
  2016-02-23 23:40 ` [PATCH 13/14] Avoid stack-protecting certain functions called from assembly Nix
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Nix @ 2016-02-23 23:40 UTC (permalink / raw)
  To: libc-alpha; +Cc: carlos

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

The address of the stack canary is stored in a per-thread variable,
which means that we must ensure that the TLS area is intialized before
calling any -fstack-protector'ed functions.  For dynamically linked
applications, we ensure this (in a later patch) by disabling
-fstack-protector for the whole dynamic linker, but for static
applications the AT_ENTRY address is called directly by the kernel, so
we must deal with the problem differently.

So split out the part of pthread initialization that sets up the TCB
(and, more generally, the TLS area) into a separate function (twice --
there is one implementation in libpthread.a, and another outside it for
programs that do not link with libpthread), then call it at
initialization time. Call that, and move the stack guard initialization
above the DL_SYSDEP_OSCHECK hook, which if set will probably call
functions which are stack-protected (it does on Linux and NaCL too).

We do not move apply_irel() up, because applying irels can require
state set up by the DL_SYSDEP_OSCHECK hook on some platforms.  This
means, as a consequence, that __pthread_initialize_tcb_internal()
had better not call anything that requires ifuncs.  (This is dealt
with in a later patch.)

v2: describe why we don't move apply_irel() up, and the consequences.
---
 csu/libc-start.c | 20 ++++++++++++--------
 csu/libc-tls.c   |  8 ++++++++
 nptl/nptl-init.c | 11 +++++++----
 3 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/csu/libc-start.c b/csu/libc-start.c
index f4aa01a..140b079 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -33,6 +33,7 @@ extern int __libc_multiple_libcs;
 #ifndef SHARED
 # include <dl-osinfo.h>
 extern void __pthread_initialize_minimal (void);
+extern void __pthread_initialize_tcb_internal (void);
 # ifndef THREAD_SET_STACK_GUARD
 /* Only exported for architectures that don't store the stack guard canary
    in thread local area.  */
@@ -178,6 +179,17 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
         }
     }
 
+  /* 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
+
 # ifdef DL_SYSDEP_OSCHECK
   if (!__libc_multiple_libcs)
     {
@@ -195,14 +207,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
      we need to setup errno.  */
   __pthread_initialize_minimal ();
 
-  /* 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
-
   /* Set up the pointer guard value.  */
   uintptr_t pointer_chk_guard = _dl_setup_pointer_guard (_dl_random,
 							 stack_chk_guard);
diff --git a/csu/libc-tls.c b/csu/libc-tls.c
index d6425e0..3d67a64 100644
--- a/csu/libc-tls.c
+++ b/csu/libc-tls.c
@@ -241,5 +241,13 @@ void
 __attribute__ ((weak))
 __pthread_initialize_minimal (void)
 {
+}
+
+/* This is the minimal initialization function used when libpthread is
+   not used.  */
+void
+__attribute__ ((weak))
+__pthread_initialize_tcb_internal (void)
+{
   __libc_setup_tls (TLS_INIT_TCB_SIZE, TLS_INIT_TCB_ALIGN);
 }
diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c
index bdbdfed..a4626be 100644
--- a/nptl/nptl-init.c
+++ b/nptl/nptl-init.c
@@ -296,21 +296,24 @@ extern void **__libc_dl_error_tsd (void) __attribute__ ((const));
 /* This can be set by the debugger before initialization is complete.  */
 static bool __nptl_initial_report_events __attribute_used__;
 
+#ifndef SHARED
 void
-__pthread_initialize_minimal_internal (void)
+__pthread_initialize_tcb_internal (void)
 {
-#ifndef SHARED
   /* Unlike in the dynamically linked case the dynamic linker has not
      taken care of initializing the TLS data structures.  */
   __libc_setup_tls (TLS_TCB_SIZE, TLS_TCB_ALIGN);
 
-  /* We must prevent gcc from being clever and move any of the
+  /* We must prevent gcc from being clever after inlining and moving any of the
      following code ahead of the __libc_setup_tls call.  This function
      will initialize the thread register which is subsequently
      used.  */
   __asm __volatile ("");
+}
 #endif
-
+void
+__pthread_initialize_minimal_internal (void)
+{
   /* Minimal initialization of the thread descriptor.  */
   struct pthread *pd = THREAD_SELF;
   __pthread_initialize_pids (pd);
-- 
2.7.0.198.g6dd47b6

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 10/14] Enable -fstack-protector=* when requested by configure.
  2016-02-23 23:39 --enable-stack-protector for glibc, v2, now with sparc Nix
                   ` (11 preceding siblings ...)
  2016-02-23 23:40 ` [PATCH 01/14] Configury support for --enable-stack-protector Nix
@ 2016-02-23 23:40 ` Nix
  2016-02-24  9:46   ` Andreas Schwab
  2016-02-24  0:57 ` [PATCH 06/14] Compile the entire dynamic linker with -fno-stack-protector Nix
  13 siblings, 1 reply; 20+ messages in thread
From: Nix @ 2016-02-23 23:40 UTC (permalink / raw)
  To: libc-alpha; +Cc: carlos

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

This finally turns on all the machinery added in previous commits.
---
 Makeconfig | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Makeconfig b/Makeconfig
index 87a22e8..cdffdc7 100644
--- a/Makeconfig
+++ b/Makeconfig
@@ -762,6 +762,11 @@ endif
 # disable any optimization that assume default rounding mode.
 +math-flags = -frounding-math
 
+# We might want to compile with some stack-protection flag.
+ifneq ($(stack-protector),)
++stack-protector=$(stack-protector)
+endif
+
 # This is the program that generates makefile dependencies from C source files.
 # The -MP flag tells GCC >= 3.2 (which we now require) to produce dummy
 # targets for headers so that removed headers don't break the build.
@@ -821,7 +826,7 @@ ifeq	"$(strip $(+cflags))" ""
 +cflags	:= $(default_cflags)
 endif	# $(+cflags) == ""
 
-+cflags += $(cflags-cpu) $(+gccwarn) $(+merge-constants) $(+math-flags)
++cflags += $(cflags-cpu) $(+gccwarn) $(+merge-constants) $(+math-flags) $(+stack-protector)
 +gcc-nowarn := -w
 
 # Don't duplicate options if we inherited variables from the parent.
-- 
2.7.0.198.g6dd47b6

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 11/14] Work even with compilers hacked to enable -fstack-protector by default.
  2016-02-23 23:39 --enable-stack-protector for glibc, v2, now with sparc Nix
                   ` (4 preceding siblings ...)
  2016-02-23 23:40 ` [PATCH 03/14] Mark all machinery needed in early static-link init as -fno-stack-protector Nix
@ 2016-02-23 23:40 ` Nix
  2016-02-23 23:40 ` [PATCH 08/14] Link libc.so with libc_nonshared.a to pull in __stack_chk_fail Nix
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Nix @ 2016-02-23 23:40 UTC (permalink / raw)
  To: libc-alpha; +Cc: carlos

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

With all the machinery we just added, we can easily arrange to work even
when the compiler passes in -fstack-protector automatically: all the
necessary bits of glibc are always compiled with -fno-stack-protector
now.

So tear out the check in configure, and add appropriate calls to
-fno-stack-protector in tests that need them (largely those that use
-nostdlib), since we don't yet have a __stack_chk_fail() that those
tests can rely upon.  (GCC often provides one, but we cannot rely on
this, especially not when bootstrapping.)

v2: No longer pass in -lssp to anything.
---
 aclocal.m4   |  6 ++---
 configure.ac | 74 +++++++++++++++++++-----------------------------------------
 2 files changed, 26 insertions(+), 54 deletions(-)

diff --git a/aclocal.m4 b/aclocal.m4
index 3d64f77..6902155 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -141,7 +141,7 @@ int _start (void) { return 0; }
 int __start (void) { return 0; }
 $1
 EOF
-AS_IF([AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -o conftest
+AS_IF([AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp -o conftest
 		       conftest.c -static -nostartfiles -nostdlib
 		       1>&AS_MESSAGE_LOG_FD])],
       [$2], [$3])
@@ -226,7 +226,7 @@ if test x"$gnu_ld" = x"yes"; then
     cat > conftest.c <<EOF
 int _start (void) { return 42; }
 EOF
-    if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS
+    if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp
 				$2 -nostdlib -nostartfiles
 				-fPIC -shared -o conftest.so conftest.c
 				1>&AS_MESSAGE_LOG_FD])
@@ -268,7 +268,7 @@ libc_compiler_builtin_inlined=no
 cat > conftest.c <<EOF
 int _start (void) { $2 return 0; }
 EOF
-if ! AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS
+if ! AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp
 		     $3 -nostdlib -nostartfiles
 		     -S conftest.c -o - | fgrep "$1"
 		     1>&AS_MESSAGE_LOG_FD])
diff --git a/configure.ac b/configure.ac
index 61bf882..4884479 100644
--- a/configure.ac
+++ b/configure.ac
@@ -643,6 +643,18 @@ fi
 AC_SUBST(libc_cv_ssp)
 AC_SUBST(stack_protector)
 
+if test -n "$stack_protector"; then
+  dnl Don't run configure tests with stack-protection on, to avoid problems with
+  dnl bootstrapping.
+  no_ssp=-fno-stack-protector
+else
+  no_ssp=
+
+  if test x"$enable_stack_protector" != xno; then
+    AC_MSG_ERROR([--enable-stack-protector=$enable_stack_protector specified, but specified level of stack protection is not supported by the compiler.])
+  fi
+fi
+
 # For the multi-arch option we need support in the assembler & linker.
 AC_CACHE_CHECK([for assembler and linker STT_GNU_IFUNC support],
 	       libc_cv_ld_gnu_indirect_function, [dnl
@@ -662,7 +674,7 @@ __start:
 EOF
 libc_cv_ld_gnu_indirect_function=no
 if ${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS \
-	    -nostartfiles -nostdlib \
+	    -nostartfiles -nostdlib $no_ssp \
 	    -o conftest conftest.S 1>&AS_MESSAGE_LOG_FD 2>&AS_MESSAGE_LOG_FD; then
   # Do a link to see if the backend supports IFUNC relocs.
   $READELF -r conftest 1>&AS_MESSAGE_LOG_FD
@@ -1131,8 +1143,8 @@ extern int glibc_conftest_frobozz;
 void _start() { glibc_conftest_frobozz = 1; }
 EOF
 if ${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS \
-	    -nostartfiles -nostdlib \
-	    -o conftest conftest.s conftest1.c 1>&AS_MESSAGE_LOG_FD 2>&AS_MESSAGE_LOG_FD; then
+	    -nostartfiles -nostdlib $no_ssp \
+	    -o conftest conftest.s conftest1.c $no_ssp 1>&AS_MESSAGE_LOG_FD 2>&AS_MESSAGE_LOG_FD; then
   libc_cv_asm_set_directive=yes
 else
   libc_cv_asm_set_directive=no
@@ -1148,12 +1160,12 @@ AC_CACHE_CHECK(linker support for protected data symbol,
 		int bar __attribute__ ((visibility ("protected"))) = 1;
 EOF
 		libc_cv_protected_data=no
-		if AC_TRY_COMMAND(${CC-cc} -nostdlib -nostartfiles -fPIC -shared conftest.c -o conftest.so); then
+		if AC_TRY_COMMAND(${CC-cc} -nostdlib -nostartfiles $no_ssp -fPIC -shared conftest.c -o conftest.so); then
 		  cat > conftest.c <<EOF
 		  extern int bar;
 		  int main (void) { return bar; }
 EOF
-		  if AC_TRY_COMMAND(${CC-cc} -nostdlib -nostartfiles conftest.c -o conftest conftest.so); then
+		  if AC_TRY_COMMAND(${CC-cc} -nostdlib -nostartfiles $no_ssp conftest.c -o conftest conftest.so); then
 		    libc_cv_protected_data=yes
 		  fi
 		fi
@@ -1275,7 +1287,7 @@ extern int mumble;
 int foo (void) { return bar (mumble); }
 EOF
 if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS
-			-fPIC -shared -o conftest.so conftest.c
+			-fPIC -shared $no_ssp -o conftest.so conftest.c
 			-nostdlib -nostartfiles
 			-Wl,-z,combreloc 1>&AS_MESSAGE_LOG_FD])
 then
@@ -1313,9 +1325,9 @@ AC_CACHE_CHECK(for --hash-style option,
 cat > conftest.c <<EOF
 int _start (void) { return 42; }
 EOF
-if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS
+if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS $no_ssp
 			    -fPIC -shared -o conftest.so conftest.c
-			    -Wl,--hash-style=both -nostdlib 1>&AS_MESSAGE_LOG_FD])
+			    -Wl,--hash-style=both -nostdlib $no_ssp 1>&AS_MESSAGE_LOG_FD])
 then
   libc_cv_hashstyle=yes
 else
@@ -1385,7 +1397,7 @@ int foo (void) { return mumble; }
 EOF
 if AC_TRY_COMMAND([${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS
 			-fPIC -shared -o conftest.so conftest.c
-			-nostdlib -nostartfiles
+			-nostdlib -nostartfiles $no_ssp
 			1>&AS_MESSAGE_LOG_FD])
 then
 dnl look for GLOB_DAT relocation.
@@ -1402,7 +1414,7 @@ AC_SUBST(libc_cv_has_glob_dat)
 
 AC_CACHE_CHECK(linker output format, libc_cv_output_format, [dnl
 if libc_cv_output_format=`
-${CC-cc} -nostartfiles -nostdlib -Wl,--print-output-format 2>&AS_MESSAGE_LOG_FD`
+${CC-cc} -nostartfiles -nostdlib $no_ssp -Wl,--print-output-format 2>&AS_MESSAGE_LOG_FD`
 then
   :
 else
@@ -1601,46 +1613,6 @@ if test $libc_cv_predef_fortify_source = yes; then
 fi
 AC_SUBST(CPPUNDEFS)
 
-dnl Check for silly hacked compilers inserting -fstack-protector.
-dnl This breaks badly for the early startup code we compile, since
-dnl the compiled code can refer to a magic machine-dependent location
-dnl for the canary value before we have sufficient setup for that to
-dnl work.  It's also questionable to build all of libc with this flag
-dnl even when you're doing that for most applications you build, since
-dnl libc's code is so heavily-used and performance-sensitive.  If we
-dnl ever really want to make that work, it should be enabled explicitly
-dnl in the libc build, not inherited from implicit compiler settings.
-AC_CACHE_CHECK([whether $CC implicitly enables -fstack-protector],
-	       libc_cv_predef_stack_protector, [
-AC_TRY_COMPILE([extern void foobar (char *);],
-	       [char large_array[2048]; foobar (large_array);], [
-libc_undefs=`$NM -u conftest.o |
-  LC_ALL=C $AWK '$1 == "U" { print $2 | "sort -u"; next } { exit(1) }' \
-    2>&AS_MESSAGE_LOG_FD` || {
-  AC_MSG_ERROR([confusing output from $NM -u])
-}
-echo >&AS_MESSAGE_LOG_FD "libc_undefs='$libc_undefs'"
-# On some architectures, there are architecture-specific undefined
-# symbols (resolved by the linker), so filter out unknown symbols.
-# This will fail to produce the correct result if the compiler
-# defaults to -fstack-protector but this produces an undefined symbol
-# other than __stack_chk_fail.  However, compilers like that have not
-# been encountered in practice.
-libc_undefs=`echo "$libc_undefs" | egrep '^(foobar|__stack_chk_fail)$'`
-case "$libc_undefs" in
-foobar) libc_cv_predef_stack_protector=no ;;
-'__stack_chk_fail
-foobar') libc_cv_predef_stack_protector=yes ;;
-*) AC_MSG_ERROR([unexpected symbols in test: $libc_undefs]) ;;
-esac],
-	       [AC_MSG_ERROR([test compilation failed])])
-])
-libc_extra_cflags=
-if test $libc_cv_predef_stack_protector = yes; then
-  libc_extra_cflags="$libc_extra_cflags -fno-stack-protector"
-fi
-libc_extra_cppflags=
-
 # Some linkers on some architectures support __ehdr_start but with
 # bugs.  Make sure usage of it does not create relocations in the
 # output (as the linker should resolve them all for us).
@@ -1650,7 +1622,7 @@ old_CFLAGS="$CFLAGS"
 old_LDFLAGS="$LDFLAGS"
 old_LIBS="$LIBS"
 CFLAGS="$CFLAGS -fPIC"
-LDFLAGS="$LDFLAGS -nostdlib -nostartfiles -shared"
+LDFLAGS="$LDFLAGS -nostdlib -nostartfiles -shared $no_ssp"
 LIBS=
 AC_LINK_IFELSE([AC_LANG_SOURCE([
 typedef struct {
-- 
2.7.0.198.g6dd47b6

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 01/14] Configury support for --enable-stack-protector.
  2016-02-23 23:39 --enable-stack-protector for glibc, v2, now with sparc Nix
                   ` (10 preceding siblings ...)
  2016-02-23 23:40 ` [PATCH 09/14] Link various tests with -fno-stack-protector Nix
@ 2016-02-23 23:40 ` Nix
  2016-02-23 23:40 ` [PATCH 10/14] Enable -fstack-protector=* when requested by configure Nix
  2016-02-24  0:57 ` [PATCH 06/14] Compile the entire dynamic linker with -fno-stack-protector Nix
  13 siblings, 0 replies; 20+ messages in thread
From: Nix @ 2016-02-23 23:40 UTC (permalink / raw)
  To: libc-alpha; +Cc: carlos

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

This adds =all and =strong, with obvious semantics, and with a rather
arbitrarily-chosen default off, which we might well want to change to
something stronger once this patch has been tested by people other than
me.

We don't validate the value of the option yet: that's in a later patch.
Nor do we use it for anything at this stage.

We differentiate between 'the compiler understands -fstack-protector'
and 'the user wanted -fstack-protector' so that we can pass
-fno-stack-protector in appropriate places even if the user didn't want
to turn on -fstack-protector for other parts.  (This helps us overcome
another existing limitation, that glibc doesn't work with GCCs hacked
to pass in -fstack-protector by default.)

We might want to add another configuration option to turn on
-fstack-protector for nscd and other network-facing operations by
default, but for now I've stuck with one option to control everything.

v2: documentation in install.texi; better description of the option.
    INSTALL regenerated.
---
 INSTALL             | 39 ++++++++++++++++++++++------------
 configure.ac        | 61 +++++++++++++++++++++++++++++++++++------------------
 manual/install.texi | 12 +++++++++++
 3 files changed, 78 insertions(+), 34 deletions(-)

diff --git a/INSTALL b/INSTALL
index a0d0c05..3d55585 100644
--- a/INSTALL
+++ b/INSTALL
@@ -141,20 +141,31 @@ will be used, and CFLAGS sets optimization options for the compiler.
 '--enable-lock-elision=yes'
      Enable lock elision for pthread mutexes by default.
 
-'--enable-pt_chown'
-     The file 'pt_chown' is a helper binary for 'grantpt' (*note
-     Pseudo-Terminals: Allocation.) that is installed setuid root to fix
-     up pseudo-terminal ownership.  It is not built by default because
-     systems using the Linux kernel are commonly built with the 'devpts'
-     filesystem enabled and mounted at '/dev/pts', which manages
-     pseudo-terminal ownership automatically.  By using
-     '--enable-pt_chown', you may build 'pt_chown' and install it setuid
-     and owned by 'root'.  The use of 'pt_chown' introduces additional
-     security risks to the system and you should enable it only if you
-     understand and accept those risks.
-
-'--disable-werror'
-     By default, the GNU C Library is built with '-Werror'.  If you wish
+`--enable-stack-protector'
+`--enable-stack-protector=strong'
+`--enable-stack-protector=all'
+     Compile the C library and all other parts of the glibc package
+     (including the threading and math libraries, NSS modules, and
+     transliteration modules) using the GCC `-fstack-protector',
+     `-fstack-protector-strong' or `-fstack-protector-all' options to
+     detect stack overruns.  Only the dynamic linker and a small number
+     of routines called directly from assembler are excluded from this
+     protection.
+
+`--enable-pt_chown'
+     The file `pt_chown' is a helper binary for `grantpt' (*note
+     Pseudo-Terminals: Allocation.) that is installed setuid root to
+     fix up pseudo-terminal ownership.  It is not built by default
+     because systems using the Linux kernel are commonly built with the
+     `devpts' filesystem enabled and mounted at `/dev/pts', which
+     manages pseudo-terminal ownership automatically.  By using
+     `--enable-pt_chown', you may build `pt_chown' and install it
+     setuid and owned by `root'.  The use of `pt_chown' introduces
+     additional security risks to the system and you should enable it
+     only if you understand and accept those risks.
+
+`--disable-werror'
+     By default, the GNU C Library is built with `-Werror'.  If you wish
      to build without this option (for example, if building with a newer
      version of GCC than this version of the GNU C Library was tested
      with, so new warnings cause the build with '-Werror' to fail), you
diff --git a/configure.ac b/configure.ac
index 3c766b7..61bf882 100644
--- a/configure.ac
+++ b/configure.ac
@@ -232,6 +232,18 @@ AC_ARG_ENABLE([bind-now],
 	      [bindnow=no])
 AC_SUBST(bindnow)
 
+dnl Build glibc with -fstack-protector, -fstack-protector-all, or
+dnl -fstack-protector-strong.
+AC_ARG_ENABLE([stack-protector],
+            AC_HELP_STRING([--enable-stack-protector=@<:@yes|no|all|strong@:>@],
+                           [Detect stack overflows in glibc functions, either with local buffers (yes), or with those plus arrays (strong), or all functions (all)]),
+            [enable_stack_protector=$enableval],
+            [enable_stack_protector=no])
+case x"$enable_stack_protector" in
+    xall|xyes|xno|xstrong) ;;
+    *) AC_MSG_ERROR([Not a valid argument for --enable-stack-protector]);;
+esac
+
 dnl On some platforms we cannot use dynamic loading.  We must provide
 dnl static NSS modules.
 AC_ARG_ENABLE([static-nss],
@@ -602,6 +614,35 @@ fi
 test -n "$base_machine" || base_machine=$machine
 AC_SUBST(base_machine)
 
+AC_CACHE_CHECK(for -fstack-protector, libc_cv_ssp, [dnl
+LIBC_TRY_CC_OPTION([$CFLAGS $CPPFLAGS -Werror -fstack-protector],
+		   [libc_cv_ssp=yes],
+		   [libc_cv_ssp=no])
+])
+
+AC_CACHE_CHECK(for -fstack-protector-strong, libc_cv_ssp_strong, [dnl
+LIBC_TRY_CC_OPTION([$CFLAGS $CPPFLAGS -Werror -fstack-protector-strong],
+		   [libc_cv_ssp_strong=yes],
+		   [libc_cv_ssp_strong=no])
+])
+
+AC_CACHE_CHECK(for -fstack-protector-all, libc_cv_ssp_all, [dnl
+LIBC_TRY_CC_OPTION([$CFLAGS $CPPFLAGS -Werror -fstack-protector-all],
+		   [libc_cv_ssp_all=yes],
+		   [libc_cv_ssp_all=no])
+])
+
+stack_protector=
+if test x$enable_stack_protector = xyes && test $libc_cv_ssp = yes; then
+  stack_protector=-fstack-protector
+elif test x$enable_stack_protector = xall && test $libc_cv_ssp_all = yes; then
+  stack_protector=-fstack-protector-all
+elif test x$enable_stack_protector = xstrong && test $libc_cv_ssp_strong = yes; then
+  stack_protector=-fstack-protector-strong
+fi
+AC_SUBST(libc_cv_ssp)
+AC_SUBST(stack_protector)
+
 # For the multi-arch option we need support in the assembler & linker.
 AC_CACHE_CHECK([for assembler and linker STT_GNU_IFUNC support],
 	       libc_cv_ld_gnu_indirect_function, [dnl
@@ -1389,26 +1430,6 @@ else
 fi
 AC_SUBST(fno_unit_at_a_time)
 
-AC_CACHE_CHECK(for -fstack-protector, libc_cv_ssp, [dnl
-LIBC_TRY_CC_OPTION([$CFLAGS $CPPFLAGS -Werror -fstack-protector],
-		   [libc_cv_ssp=yes],
-		   [libc_cv_ssp=no])
-])
-
-AC_CACHE_CHECK(for -fstack-protector-strong, libc_cv_ssp_strong, [dnl
-LIBC_TRY_CC_OPTION([$CFLAGS $CPPFLAGS -Werror -fstack-protector-strong],
-		   [libc_cv_ssp_strong=yes],
-		   [libc_cv_ssp_strong=no])
-])
-
-stack_protector=
-if test "$libc_cv_ssp_strong" = "yes"; then
-  stack_protector="-fstack-protector-strong"
-elif test "$libc_cv_ssp" = "yes"; then
-  stack_protector="-fstack-protector"
-fi
-AC_SUBST(stack_protector)
-
 AC_CACHE_CHECK(whether cc puts quotes around section names,
 	       libc_cv_have_section_quotes,
 	       [cat > conftest.c <<EOF
diff --git a/manual/install.texi b/manual/install.texi
index b329950..e05424a 100644
--- a/manual/install.texi
+++ b/manual/install.texi
@@ -170,6 +170,18 @@ time.  Consult the @file{timezone} subdirectory for more details.
 @item --enable-lock-elision=yes
 Enable lock elision for pthread mutexes by default.
 
+@item --enable-stack-protector
+@itemx --enable-stack-protector=strong
+@itemx --enable-stack-protector=all
+Compile the C library and all other parts of the glibc package
+(including the threading and math libraries, NSS modules, and
+transliteration modules) using the GCC @option{-fstack-protector},
+@option{-fstack-protector-strong} or @option{-fstack-protector-all}
+options to detect stack overruns.  Only the dynamic linker and a small
+number of routines called directly from assembler are excluded from this
+protection.
+
+
 @pindex pt_chown
 @findex grantpt
 @item --enable-pt_chown
-- 
2.7.0.198.g6dd47b6

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 09/14] Link various tests with -fno-stack-protector.
  2016-02-23 23:39 --enable-stack-protector for glibc, v2, now with sparc Nix
                   ` (9 preceding siblings ...)
  2016-02-23 23:40 ` [PATCH 13/14] Avoid stack-protecting certain functions called from assembly Nix
@ 2016-02-23 23:40 ` Nix
  2016-02-24 13:07   ` Andreas Schwab
  2016-02-23 23:40 ` [PATCH 01/14] Configury support for --enable-stack-protector Nix
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 20+ messages in thread
From: Nix @ 2016-02-23 23:40 UTC (permalink / raw)
  To: libc-alpha; +Cc: carlos

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

These tests do not link with libc, so cannot see __stack_chk_fail().
---
 elf/Makefile            | 6 ++++++
 stdlib/Makefile         | 4 ++++
 sysdeps/x86_64/Makefile | 3 +++
 3 files changed, 13 insertions(+)

diff --git a/elf/Makefile b/elf/Makefile
index dcf44c8..22baac6 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -751,6 +751,12 @@ $(objpfx)filtmod1.so: $(objpfx)filtmod1.os $(objpfx)filtmod2.so
 		  $< -Wl,-F,$(objpfx)filtmod2.so
 $(objpfx)filter: $(objpfx)filtmod1.so
 
+ifeq ($(have-ssp),yes)
+# These do not link against libc.
+CFLAGS-filtmod1.c = -fno-stack-protector
+CFLAGS-filtmod2.c = -fno-stack-protector
+endif
+
 $(objpfx)unload: $(libdl)
 $(objpfx)unload.out: $(objpfx)unloadmod.so
 
diff --git a/stdlib/Makefile b/stdlib/Makefile
index 26fe67a..bb9d4b1 100644
--- a/stdlib/Makefile
+++ b/stdlib/Makefile
@@ -164,6 +164,10 @@ LDFLAGS-tst-putenv = $(no-as-needed)
 
 $(objpfx)tst-putenvmod.so: $(objpfx)tst-putenvmod.os $(link-libc-deps)
 	$(build-module)
+# This is not only not in libc, it's not even linked with it.
+ifeq ($(have-ssp),yes)
+CFLAGS-tst-putenvmod.c += -fno-stack-protector
+endif
 libof-tst-putenvmod = extramodules
 
 $(objpfx)bug-getcontext: $(libm)
diff --git a/sysdeps/x86_64/Makefile b/sysdeps/x86_64/Makefile
index 67ed5ba..3183cb3 100644
--- a/sysdeps/x86_64/Makefile
+++ b/sysdeps/x86_64/Makefile
@@ -40,6 +40,9 @@ quad-pie-test += tst-quad1pie tst-quad2pie
 tests += $(quad-pie-test)
 tests-pie += $(quad-pie-test)
 
+CFLAGS-tst-quad1pie.c = -fno-stack-protector
+CFLAGS-tst-quad2pie.c = -fno-stack-protector
+
 $(objpfx)tst-quad1pie: $(objpfx)tst-quadmod1pie.o
 $(objpfx)tst-quad2pie: $(objpfx)tst-quadmod2pie.o
 
-- 
2.7.0.198.g6dd47b6

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 06/14] Compile the entire dynamic linker with -fno-stack-protector.
  2016-02-23 23:39 --enable-stack-protector for glibc, v2, now with sparc Nix
                   ` (12 preceding siblings ...)
  2016-02-23 23:40 ` [PATCH 10/14] Enable -fstack-protector=* when requested by configure Nix
@ 2016-02-24  0:57 ` Nix
  13 siblings, 0 replies; 20+ messages in thread
From: Nix @ 2016-02-24  0:57 UTC (permalink / raw)
  To: libc-alpha; +Cc: carlos

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

Also compile corresponding routines in the static libc.a with the same
flag.
---
 elf/Makefile   | 13 +++++++++++++
 elf/rtld-Rules |  4 ++++
 2 files changed, 17 insertions(+)

diff --git a/elf/Makefile b/elf/Makefile
index 63a5355..0cb03b0 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -50,6 +50,16 @@ CFLAGS-dl-runtime.c = -fexceptions -fasynchronous-unwind-tables
 CFLAGS-dl-lookup.c = -fexceptions -fasynchronous-unwind-tables
 CFLAGS-dl-iterate-phdr.c = $(uses-callbacks)
 
+ifeq ($(have-ssp),yes)
+# In the dynamic loader, some routines (which include routines called before
+# the stack guard is initialized) are compiled without stack protection.
+# Do the same when those routines are found in the static library.
+
+CFLAGS-.o += $(if $(filter $(@F),$(patsubst %,%.o,$(elide-routines.os))),-fno-stack-protector)
+CFLAGS-.op += $(if $(filter $(@F),$(patsubst %,%.op,$(elide-routines.os))),-fno-stack-protector)
+CFLAGS-.og += $(if $(filter $(@F),$(patsubst %,%.og,$(elide-routines.os))),-fno-stack-protector)
+endif
+
 ifeq ($(unwind-find-fde),yes)
 routines += unwind-dw2-fde-glibc
 shared-only-routines += unwind-dw2-fde-glibc
@@ -468,6 +478,9 @@ libof-ldconfig = ldconfig
 CFLAGS-dl-cache.c = $(SYSCONF-FLAGS)
 CFLAGS-cache.c = $(SYSCONF-FLAGS)
 CFLAGS-rtld.c = $(SYSCONF-FLAGS)
+ifeq ($(have-ssp),yes)
+CFLAGS-.os += $(if $(filter $(@F),$(patsubst %,%.os,$(all-rtld-routines))),-fno-stack-protector)
+endif
 
 cpp-srcs-left := $(all-rtld-routines:=.os)
 lib := rtld
diff --git a/elf/rtld-Rules b/elf/rtld-Rules
index c1bb506..1d571d3 100644
--- a/elf/rtld-Rules
+++ b/elf/rtld-Rules
@@ -144,4 +144,8 @@ cpp-srcs-left := $(rtld-modules:%.os=%)
 lib := rtld
 include $(patsubst %,$(..)cppflags-iterator.mk,$(cpp-srcs-left))
 
+ifeq ($(have-ssp),yes)
+rtld-CFLAGS := -fno-stack-protector
+endif
+
 endif
-- 
2.7.0.198.g6dd47b6

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 10/14] Enable -fstack-protector=* when requested by configure.
  2016-02-23 23:40 ` [PATCH 10/14] Enable -fstack-protector=* when requested by configure Nix
@ 2016-02-24  9:46   ` Andreas Schwab
  2016-02-24 17:16     ` Nix
  0 siblings, 1 reply; 20+ messages in thread
From: Andreas Schwab @ 2016-02-24  9:46 UTC (permalink / raw)
  To: Nix; +Cc: libc-alpha, carlos

Nix <nix@esperi.org.uk> writes:

> -+cflags += $(cflags-cpu) $(+gccwarn) $(+merge-constants) $(+math-flags)
> ++cflags += $(cflags-cpu) $(+gccwarn) $(+merge-constants) $(+math-flags) $(+stack-protector)

Please fold long lines.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 09/14] Link various tests with -fno-stack-protector.
  2016-02-23 23:40 ` [PATCH 09/14] Link various tests with -fno-stack-protector Nix
@ 2016-02-24 13:07   ` Andreas Schwab
  2016-02-24 13:09     ` Nix
  0 siblings, 1 reply; 20+ messages in thread
From: Andreas Schwab @ 2016-02-24 13:07 UTC (permalink / raw)
  To: Nix; +Cc: libc-alpha, carlos

Nix <nix@esperi.org.uk> writes:

> +ifeq ($(have-ssp),yes)
> +# These do not link against libc.
> +CFLAGS-filtmod1.c = -fno-stack-protector
> +CFLAGS-filtmod2.c = -fno-stack-protector
> +endif

Please add a variable no-stack-protector to config.make, defined to
-fno-stack-protector depending on have-ssp.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 09/14] Link various tests with -fno-stack-protector.
  2016-02-24 13:07   ` Andreas Schwab
@ 2016-02-24 13:09     ` Nix
  0 siblings, 0 replies; 20+ messages in thread
From: Nix @ 2016-02-24 13:09 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: libc-alpha, carlos

On 24 Feb 2016, Andreas Schwab uttered the following:

> Nix <nix@esperi.org.uk> writes:
>
>> +ifeq ($(have-ssp),yes)
>> +# These do not link against libc.
>> +CFLAGS-filtmod1.c = -fno-stack-protector
>> +CFLAGS-filtmod2.c = -fno-stack-protector
>> +endif
>
> Please add a variable no-stack-protector to config.make, defined to
> -fno-stack-protector depending on have-ssp.

... and then drop all the $(have-ssp) ifeqs?

Will do! (I thought it would increase the amount of overhead, but given
the number of one-line additions, this change makes a lot of sense.)

(This changes numerous patches, so I'll post this change when I post
v3 as a whole.)

-- 
NULL && (void)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 10/14] Enable -fstack-protector=* when requested by configure.
  2016-02-24  9:46   ` Andreas Schwab
@ 2016-02-24 17:16     ` Nix
  0 siblings, 0 replies; 20+ messages in thread
From: Nix @ 2016-02-24 17:16 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: libc-alpha, carlos

On 24 Feb 2016, Andreas Schwab stated:

> Nix <nix@esperi.org.uk> writes:
>
>> -+cflags += $(cflags-cpu) $(+gccwarn) $(+merge-constants) $(+math-flags)
>> ++cflags += $(cflags-cpu) $(+gccwarn) $(+merge-constants) $(+math-flags) $(+stack-protector)
>
> Please fold long lines.

I wasn't sure it was syntactically valid here (as in the bit of
configure.ac with a long line). Clearly I was missing the nose on my
face: glibc folds += lines *all the time*. Will fix.

-- 
NULL && (void)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 08/14] Link libc.so with libc_nonshared.a to pull in __stack_chk_fail.
  2016-02-24 23:28 --enable-stack-protector for glibc, v3 Nix
@ 2016-02-24 23:29 ` Nix
  0 siblings, 0 replies; 20+ messages in thread
From: Nix @ 2016-02-24 23:29 UTC (permalink / raw)
  To: libc-alpha

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

This prevents a spurious undefined reference from libc (which would be
an ABI break).
---
 Makerules | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makerules b/Makerules
index 53eabfa..d369d78 100644
--- a/Makerules
+++ b/Makerules
@@ -674,6 +674,7 @@ $(common-objpfx)linkobj/libc.so: link-libc-deps = # empty
 # libraries.
 $(common-objpfx)libc.so: $(elf-objpfx)soinit.os \
 			 $(common-objpfx)libc_pic.os$(libc_pic_clean) \
+			 $(common-objpfx)libc_nonshared.a \
 			 $(elf-objpfx)sofini.os \
 			 $(elf-objpfx)interp.os \
 			 $(elf-objpfx)ld.so \
@@ -683,6 +684,7 @@ $(common-objpfx)libc.so: $(elf-objpfx)soinit.os \
 
 $(common-objpfx)linkobj/libc.so: $(elf-objpfx)soinit.os \
 			 $(common-objpfx)linkobj/libc_pic.a \
+			 $(common-objpfx)libc_nonshared.a \
 			 $(elf-objpfx)sofini.os \
 			 $(elf-objpfx)interp.os \
 			 $(elf-objpfx)ld.so \
-- 
2.7.0.198.g6dd47b6

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2016-02-24 23:28 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-23 23:39 --enable-stack-protector for glibc, v2, now with sparc Nix
2016-02-23 23:39 ` [PATCH 05/14] Allow overriding of CFLAGS as well as CPPFLAGS for rtld Nix
2016-02-23 23:39 ` [PATCH 07/14] Prevent the rtld mapfile computation from dragging in __stack_chk_fail() Nix
2016-02-23 23:40 ` [PATCH 14/14] sparc: do not stack-protect the sigreturn handler Nix
2016-02-23 23:40 ` [PATCH 04/14] Open-code the memcpy() at static TLS initialization time Nix
2016-02-23 23:40 ` [PATCH 03/14] Mark all machinery needed in early static-link init as -fno-stack-protector Nix
2016-02-23 23:40 ` [PATCH 11/14] Work even with compilers hacked to enable -fstack-protector by default Nix
2016-02-23 23:40 ` [PATCH 08/14] Link libc.so with libc_nonshared.a to pull in __stack_chk_fail Nix
2016-02-23 23:40 ` [PATCH 12/14] Drop explicit stack-protection of pieces of the system Nix
2016-02-23 23:40 ` [PATCH 02/14] Initialize the stack guard earlier when linking statically Nix
2016-02-23 23:40 ` [PATCH 13/14] Avoid stack-protecting certain functions called from assembly Nix
2016-02-23 23:40 ` [PATCH 09/14] Link various tests with -fno-stack-protector Nix
2016-02-24 13:07   ` Andreas Schwab
2016-02-24 13:09     ` Nix
2016-02-23 23:40 ` [PATCH 01/14] Configury support for --enable-stack-protector Nix
2016-02-23 23:40 ` [PATCH 10/14] Enable -fstack-protector=* when requested by configure Nix
2016-02-24  9:46   ` Andreas Schwab
2016-02-24 17:16     ` Nix
2016-02-24  0:57 ` [PATCH 06/14] Compile the entire dynamic linker with -fno-stack-protector Nix
2016-02-24 23:28 --enable-stack-protector for glibc, v3 Nix
2016-02-24 23:29 ` [PATCH 08/14] Link libc.so with libc_nonshared.a to pull in __stack_chk_fail Nix

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