Re: [PATCH 02/12] Do not stack-protect ifunc resolvers.

[Nick Alcock <nick dot alcock at oracle dot com>]

On 15 Dec 2016, Florian Weimer verbalised:

> On 11/28/2016 01:32 PM, Nix wrote:
>> diff --git a/config.h.in b/config.h.in
>> index 1b58612..b42c4d8 100644
>> --- a/config.h.in
>> +++ b/config.h.in
>> @@ -48,6 +48,10 @@
>>  /* Define if compiler accepts -ftree-loop-distribute-patterns.  */
>>  #undef  HAVE_CC_INHIBIT_LOOP_TO_LIBCALL
>>
>> +/* Define if compiler accepts -fno-stack-protector in an
>> +   __attribute__((__optimize__)).  */
>
> Space after __attribute__; at least that's the current style.

OK. (But note that there is another instance of this problem elsewhere
in the same file, only a few lines above, in HAVE_SECTION_QUOTES: I
was copying it, for consistency.)

>> +#undef	HAVE_CC_NO_STACK_PROTECTOR
>> +
>>  /* The level of stack protection in use for glibc as a whole.  */
>>  #undef	STACK_PROTECTOR_LEVEL
>
> I don't have STACK_PROTECTOR_LEVEL in my sources, so this patch does not apply.  Did you post the sequence in the right order?

Ooof, patch 1 is missing due to a git commit ID foulup. This, of course,
renumbers all the later patches -- but here it is anyway, so you can
apply the series as a whole:

>From c612d071f28b18418873379598ccef5b41bedb34 Mon Sep 17 00:00:00 2001
From: Nick Alcock <nick.alcock@oracle.com>
Date: Fri, 19 Feb 2016 17:25:05 +0000
Subject: [PATCH] Configury support for --enable-stack-protector.

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.
v3: Substitute in no_stack_protector.
v6: Small quoting/spacing revisions following Mike Frysinger's review.
    Add STACK_PROTECTOR_LEVEL.
v7: Quoting changes. Report --enable-stack-protector argument values
    on error.

	[BZ #7065]
	* configure.ac (libc_cv_ssp): Move up.
	(libc_cv_ssp_strong): Likewise.
	(libc_cv_ssp_all): New.
	(stack_protector): Augment, adding -fstack-protector-all.
	(no_stack_protector): New.
	(STACK_PROTECTOR_LEVEL): New.
	(AC_ARG_ENABLE(stack-protector)): New configure flag.
	* manual/install.texi (--enable-stack-protector): Document it.
	* config.h.in (STACK_PROTECTOR_LEVEL): New macro.
	* INSTALL: Regenerate.
---
 INSTALL             | 39 ++++++++++++++++++-----------
 config.h.in         |  3 +++
 configure.ac        | 70 ++++++++++++++++++++++++++++++++++++++---------------
 manual/install.texi | 12 +++++++++
 4 files changed, 90 insertions(+), 34 deletions(-)

diff --git a/INSTALL b/INSTALL
index b5acedc..2b0abf9 100644
--- a/INSTALL
+++ b/INSTALL
@@ -135,20 +135,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/config.h.in b/config.h.in
index 33757bd..1b58612 100644
--- a/config.h.in
+++ b/config.h.in
@@ -48,6 +48,9 @@
 /* Define if compiler accepts -ftree-loop-distribute-patterns.  */
 #undef  HAVE_CC_INHIBIT_LOOP_TO_LIBCALL
 
+/* The level of stack protection in use for glibc as a whole.  */
+#undef	STACK_PROTECTOR_LEVEL
+
 /* Define if the regparm attribute shall be used for local functions
    (gcc on ix86 only).  */
 #undef	USE_REGPARMS
diff --git a/configure.ac b/configure.ac
index de0a40f..859f90b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -235,6 +235,18 @@ if test "x$bindnow" = xyes; then
   AC_DEFINE(BIND_NOW)
 fi
 
+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@:>@],
+			     [Use -fstack-protector[-all|-strong] to detect glibc buffer overflows]),
+	      [enable_stack_protector=$enableval],
+	      [enable_stack_protector=no])
+case "$enable_stack_protector" in
+all|yes|no|strong) ;;
+*) AC_MSG_ERROR([Not a valid argument for --enable-stack-protector: \"$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],
@@ -605,6 +617,44 @@ 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=
+no_stack_protector=
+if test "$libc_cv_ssp" = yes; then
+  no_stack_protector="-fno-stack-protector"
+fi
+
+if test "$enable_stack_protector" = yes && test "$libc_cv_ssp" = yes; then
+  stack_protector="-fstack-protector"
+  AC_DEFINE(STACK_PROTECTOR_LEVEL, 1)
+elif test "$enable_stack_protector" = all && test "$libc_cv_ssp_all" = yes; then
+  stack_protector="-fstack-protector-all"
+  AC_DEFINE(STACK_PROTECTOR_LEVEL, 2)
+elif test "$enable_stack_protector" = strong && test "$libc_cv_ssp_strong" = yes; then
+  stack_protector="-fstack-protector-strong"
+  AC_DEFINE(STACK_PROTECTOR_LEVEL, 3)
+fi
+AC_SUBST(libc_cv_ssp)
+AC_SUBST(stack_protector)
+AC_SUBST(no_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
@@ -1433,26 +1483,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([for -mtls-dialect=gnu2], libc_cv_mtls_dialect_gnu2,
 [dnl
 cat > conftest.c <<EOF
diff --git a/manual/install.texi b/manual/install.texi
index de1c203..b5be87d 100644
--- a/manual/install.texi
+++ b/manual/install.texi
@@ -164,6 +164,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.10.1.208.gbec66bc