New configure option --with-elided-stack-protector-cflags

Message ID 20210203093532.2225969-1-siddhesh@sourceware.org
State Dropped
Headers
Series New configure option --with-elided-stack-protector-cflags |

Commit Message

Siddhesh Poyarekar Feb. 3, 2021, 9:35 a.m. UTC
  Add a new configure option --with-elided-stack-protector-cflags that
allows customization of flags added to routines for whom the stack
protector is elided due to them being called too early in program
startup.  The default remains "-fno-stack-protector
-DSTACK_PROTECTOR_LEVEL=0"
---
 INSTALL             |  7 +++++++
 configure           | 15 ++++++++++++++-
 configure.ac        |  8 +++++++-
 manual/install.texi |  7 +++++++
 4 files changed, 35 insertions(+), 2 deletions(-)
  

Comments

Adhemerval Zanella Feb. 3, 2021, 5:41 p.m. UTC | #1
On 03/02/2021 06:35, Siddhesh Poyarekar via Libc-alpha wrote:
> Add a new configure option --with-elided-stack-protector-cflags that
> allows customization of flags added to routines for whom the stack
> protector is elided due to them being called too early in program
> startup.  The default remains "-fno-stack-protector
> -DSTACK_PROTECTOR_LEVEL=0"

Why exactly do you need to override the no_stack_protector default
value? I don't see why we can't just infer the required flags based
on the one used for stack protection.

> ---
>  INSTALL             |  7 +++++++
>  configure           | 15 ++++++++++++++-
>  configure.ac        |  8 +++++++-
>  manual/install.texi |  7 +++++++
>  4 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/INSTALL b/INSTALL
> index 9a50e3ee06..26509da5b9 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -106,6 +106,13 @@ if 'CFLAGS' is specified it must enable optimization.  For example:
>       particular case and potentially change debugging information and
>       metadata only).
>  
> +'--with-elided-stack-protector-cflags=CFLAGS'
> +     When the '--enable-stack-protector' configure option is set, use
> +     compiler flags CFLAGS to build the parts of the library that cannot
> +     be built with stack protector enabled.  The default value is
> +     '-fno-stack-protector -DSTACK_PROTECTOR_LEVEL=0'.  This option has
> +     no effect when used without '--enable-stack-protector'.
> +
>  '--disable-shared'
>       Don't build shared libraries even if it is possible.  Not all
>       systems support shared libraries; you need ELF support and
> diff --git a/configure b/configure
> index 1dc3af60b4..3317170ed2 100755
> --- a/configure
> +++ b/configure
> @@ -763,6 +763,7 @@ with_selinux
>  with_headers
>  with_default_link
>  with_nonshared_cflags
> +with_elided_stack_protector_cflags
>  enable_sanity_checks
>  enable_shared
>  enable_profile
> @@ -1482,6 +1483,9 @@ Optional Packages:
>    --with-default-link     do not use explicit linker scripts
>    --with-nonshared-cflags=CFLAGS
>                            build nonshared libraries with additional CFLAGS
> +  --with-elided-stack-protector-cflags=CFLAGS
> +                          additional CFLAGS to build routines that cannot be
> +                          built with stack protector
>    --with-cpu=CPU          select code for CPU variant
>  
>  Some influential environment variables:
> @@ -3349,6 +3353,15 @@ fi
>  
>  
>  
> +
> +# Check whether --with-elided-stack-protector-cflags was given.
> +if test "${with_elided_stack_protector_cflags+set}" = set; then :
> +  withval=$with_elided_stack_protector_cflags; elided_ssp_cflags=$withval
> +else
> +  elided_ssp_cflags="-fno-stack-protector -DSTACK_PROTECTOR_LEVEL=0"
> +fi
> +
> +
>  # Check whether --enable-sanity-checks was given.
>  if test "${enable_sanity_checks+set}" = set; then :
>    enableval=$enable_sanity_checks; enable_sanity=$enableval
> @@ -3958,7 +3971,7 @@ $as_echo "$libc_cv_ssp_all" >&6; }
>  stack_protector=
>  no_stack_protector=
>  if test "$libc_cv_ssp" = yes; then
> -  no_stack_protector="-fno-stack-protector -DSTACK_PROTECTOR_LEVEL=0"
> +  no_stack_protector="$elided_ssp_cflags"
>    $as_echo "#define HAVE_CC_NO_STACK_PROTECTOR 1" >>confdefs.h
>  
>  fi
> diff --git a/configure.ac b/configure.ac
> index dfebb8a7cc..54368625d2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -163,6 +163,12 @@ AC_ARG_WITH([nonshared-cflags],
>  	    [extra_nonshared_cflags=])
>  AC_SUBST(extra_nonshared_cflags)
>  
> +AC_ARG_WITH([elided-stack-protector-cflags],
> +	    AC_HELP_STRING([--with-elided-stack-protector-cflags=CFLAGS],
> +			   [additional CFLAGS to build routines that cannot be built with stack protector]),
> +	    [elided_ssp_cflags=$withval],
> +	    [elided_ssp_cflags="-fno-stack-protector -DSTACK_PROTECTOR_LEVEL=0"])
> +
>  AC_ARG_ENABLE([sanity-checks],
>  	      AC_HELP_STRING([--disable-sanity-checks],
>  			     [really do not use threads (should not be used except in special situations) @<:@default=yes@:>@]),
> @@ -586,7 +592,7 @@ LIBC_TRY_CC_OPTION([$CFLAGS $CPPFLAGS -Werror -fstack-protector-all],
>  stack_protector=
>  no_stack_protector=
>  if test "$libc_cv_ssp" = yes; then
> -  no_stack_protector="-fno-stack-protector -DSTACK_PROTECTOR_LEVEL=0"
> +  no_stack_protector="$elided_ssp_cflags"
>    AC_DEFINE(HAVE_CC_NO_STACK_PROTECTOR)
>  fi
>  
> diff --git a/manual/install.texi b/manual/install.texi
> index 419576f49c..60a8932fb5 100644
> --- a/manual/install.texi
> +++ b/manual/install.texi
> @@ -131,6 +131,13 @@ that the objects in @file{libc_nonshared.a} are compiled with this flag
>  (although this will not affect the generated code in this particular
>  case and potentially change debugging information and metadata only).
>  
> +@item --with-elided-stack-protector-cflags=@var{cflags}
> +When the @option{--enable-stack-protector} configure option is set, use
> +compiler flags @var{cflags} to build the parts of the library that cannot be
> +built with stack protector enabled.  The default value is
> +@samp{-fno-stack-protector -DSTACK_PROTECTOR_LEVEL=0}.  This option has no
> +effect when used without @option{--enable-stack-protector}.
> +
>  @c disable static doesn't work currently
>  @c @item --disable-static
>  @c Don't build static libraries.  Static libraries aren't that useful these
>
  
Siddhesh Poyarekar Feb. 3, 2021, 5:45 p.m. UTC | #2
On 2/3/21 11:11 PM, Adhemerval Zanella via Libc-alpha wrote:
> 
> 
> On 03/02/2021 06:35, Siddhesh Poyarekar via Libc-alpha wrote:
>> Add a new configure option --with-elided-stack-protector-cflags that
>> allows customization of flags added to routines for whom the stack
>> protector is elided due to them being called too early in program
>> startup.  The default remains "-fno-stack-protector
>> -DSTACK_PROTECTOR_LEVEL=0"
> 
> Why exactly do you need to override the no_stack_protector default
> value? I don't see why we can't just infer the required flags based
> on the one used for stack protection.

In Fedora we use annobin to annotate objects with the flags they are 
built with.  For objects that are specifically built without stack 
protector, we would like to pass additional annobin related flags to 
make sure that the annotations are consistent.

Siddhesh
  
Adhemerval Zanella Feb. 3, 2021, 6:09 p.m. UTC | #3
On 03/02/2021 14:45, Siddhesh Poyarekar wrote:
> On 2/3/21 11:11 PM, Adhemerval Zanella via Libc-alpha wrote:
>>
>>
>> On 03/02/2021 06:35, Siddhesh Poyarekar via Libc-alpha wrote:
>>> Add a new configure option --with-elided-stack-protector-cflags that
>>> allows customization of flags added to routines for whom the stack
>>> protector is elided due to them being called too early in program
>>> startup.  The default remains "-fno-stack-protector
>>> -DSTACK_PROTECTOR_LEVEL=0"
>>
>> Why exactly do you need to override the no_stack_protector default
>> value? I don't see why we can't just infer the required flags based
>> on the one used for stack protection.
> 
> In Fedora we use annobin to annotate objects with the flags they are built with.  For objects that are specifically built without stack protector, we would like to pass additional annobin related flags to make sure that the annotations are consistent.

Can't you deal with this specific requirement on annobin itself? I am not
seeing why the compiler plugin (if I understood correctly how the tool
works) can't add itself the required additional notes.

These kind of configuration should not really be required to be set and
exposing it is bleeding out an implementation details (that some objects
within glibc requires stack protection disabled).
  
Siddhesh Poyarekar Feb. 3, 2021, 6:15 p.m. UTC | #4
On 2/3/21 11:39 PM, Adhemerval Zanella wrote:
> Can't you deal with this specific requirement on annobin itself? I am not
> seeing why the compiler plugin (if I understood correctly how the tool
> works) can't add itself the required additional notes.
> 
> These kind of configuration should not really be required to be set and
> exposing it is bleeding out an implementation details (that some objects
> within glibc requires stack protection disabled).

I don't disagree, I'm a bit reluctant about this too.  Let me see if I 
can figure out a way to make this a bit less clunky.  Telling annobin 
about it seems like a nonstarter since it would mean bleeding 
implementation details (i.e. _which_ objects within glibc require stack 
protection disabled) to annobin, which seems worse.

Siddhesh
  

Patch

diff --git a/INSTALL b/INSTALL
index 9a50e3ee06..26509da5b9 100644
--- a/INSTALL
+++ b/INSTALL
@@ -106,6 +106,13 @@  if 'CFLAGS' is specified it must enable optimization.  For example:
      particular case and potentially change debugging information and
      metadata only).
 
+'--with-elided-stack-protector-cflags=CFLAGS'
+     When the '--enable-stack-protector' configure option is set, use
+     compiler flags CFLAGS to build the parts of the library that cannot
+     be built with stack protector enabled.  The default value is
+     '-fno-stack-protector -DSTACK_PROTECTOR_LEVEL=0'.  This option has
+     no effect when used without '--enable-stack-protector'.
+
 '--disable-shared'
      Don't build shared libraries even if it is possible.  Not all
      systems support shared libraries; you need ELF support and
diff --git a/configure b/configure
index 1dc3af60b4..3317170ed2 100755
--- a/configure
+++ b/configure
@@ -763,6 +763,7 @@  with_selinux
 with_headers
 with_default_link
 with_nonshared_cflags
+with_elided_stack_protector_cflags
 enable_sanity_checks
 enable_shared
 enable_profile
@@ -1482,6 +1483,9 @@  Optional Packages:
   --with-default-link     do not use explicit linker scripts
   --with-nonshared-cflags=CFLAGS
                           build nonshared libraries with additional CFLAGS
+  --with-elided-stack-protector-cflags=CFLAGS
+                          additional CFLAGS to build routines that cannot be
+                          built with stack protector
   --with-cpu=CPU          select code for CPU variant
 
 Some influential environment variables:
@@ -3349,6 +3353,15 @@  fi
 
 
 
+
+# Check whether --with-elided-stack-protector-cflags was given.
+if test "${with_elided_stack_protector_cflags+set}" = set; then :
+  withval=$with_elided_stack_protector_cflags; elided_ssp_cflags=$withval
+else
+  elided_ssp_cflags="-fno-stack-protector -DSTACK_PROTECTOR_LEVEL=0"
+fi
+
+
 # Check whether --enable-sanity-checks was given.
 if test "${enable_sanity_checks+set}" = set; then :
   enableval=$enable_sanity_checks; enable_sanity=$enableval
@@ -3958,7 +3971,7 @@  $as_echo "$libc_cv_ssp_all" >&6; }
 stack_protector=
 no_stack_protector=
 if test "$libc_cv_ssp" = yes; then
-  no_stack_protector="-fno-stack-protector -DSTACK_PROTECTOR_LEVEL=0"
+  no_stack_protector="$elided_ssp_cflags"
   $as_echo "#define HAVE_CC_NO_STACK_PROTECTOR 1" >>confdefs.h
 
 fi
diff --git a/configure.ac b/configure.ac
index dfebb8a7cc..54368625d2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -163,6 +163,12 @@  AC_ARG_WITH([nonshared-cflags],
 	    [extra_nonshared_cflags=])
 AC_SUBST(extra_nonshared_cflags)
 
+AC_ARG_WITH([elided-stack-protector-cflags],
+	    AC_HELP_STRING([--with-elided-stack-protector-cflags=CFLAGS],
+			   [additional CFLAGS to build routines that cannot be built with stack protector]),
+	    [elided_ssp_cflags=$withval],
+	    [elided_ssp_cflags="-fno-stack-protector -DSTACK_PROTECTOR_LEVEL=0"])
+
 AC_ARG_ENABLE([sanity-checks],
 	      AC_HELP_STRING([--disable-sanity-checks],
 			     [really do not use threads (should not be used except in special situations) @<:@default=yes@:>@]),
@@ -586,7 +592,7 @@  LIBC_TRY_CC_OPTION([$CFLAGS $CPPFLAGS -Werror -fstack-protector-all],
 stack_protector=
 no_stack_protector=
 if test "$libc_cv_ssp" = yes; then
-  no_stack_protector="-fno-stack-protector -DSTACK_PROTECTOR_LEVEL=0"
+  no_stack_protector="$elided_ssp_cflags"
   AC_DEFINE(HAVE_CC_NO_STACK_PROTECTOR)
 fi
 
diff --git a/manual/install.texi b/manual/install.texi
index 419576f49c..60a8932fb5 100644
--- a/manual/install.texi
+++ b/manual/install.texi
@@ -131,6 +131,13 @@  that the objects in @file{libc_nonshared.a} are compiled with this flag
 (although this will not affect the generated code in this particular
 case and potentially change debugging information and metadata only).
 
+@item --with-elided-stack-protector-cflags=@var{cflags}
+When the @option{--enable-stack-protector} configure option is set, use
+compiler flags @var{cflags} to build the parts of the library that cannot be
+built with stack protector enabled.  The default value is
+@samp{-fno-stack-protector -DSTACK_PROTECTOR_LEVEL=0}.  This option has no
+effect when used without @option{--enable-stack-protector}.
+
 @c disable static doesn't work currently
 @c @item --disable-static
 @c Don't build static libraries.  Static libraries aren't that useful these