diff mbox series

[v2,2/4] configure: Default --enable-stack-protector to strong

Message ID 20230630014248.2819836-3-siddhesh@sourceware.org
State New
Headers show
Series Update default build configuration | expand

Commit Message

Siddhesh Poyarekar June 30, 2023, 1:42 a.m. UTC
All major distributions use this level of stack protector, so make it
the default.

Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
---
 INSTALL             | 3 ++-
 NEWS                | 4 ++++
 configure           | 2 +-
 configure.ac        | 2 +-
 manual/install.texi | 3 ++-
 5 files changed, 10 insertions(+), 4 deletions(-)

Comments

Florian Weimer July 13, 2023, 9:51 a.m. UTC | #1
* Siddhesh Poyarekar via Libc-alpha:

> All major distributions use this level of stack protector, so make it
> the default.
>
> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

I think if strong is the default (correct IMHO), it should be the
default for --enable-stack-protector, too.  I'm not sure if we need to
support legacy -fstack-protector at all, but we do, we'd need a separate
option argument.

I would expect some build-many-glibcs.py updates for this one, too.

But I think for this one, --disable-stack-protector actually works.

Thanks,
Florian
Siddhesh Poyarekar July 17, 2023, 2:55 p.m. UTC | #2
On 2023-07-13 05:51, Florian Weimer wrote:
> * Siddhesh Poyarekar via Libc-alpha:
> 
>> All major distributions use this level of stack protector, so make it
>> the default.
>>
>> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
> 
> I think if strong is the default (correct IMHO), it should be the
> default for --enable-stack-protector, too.  I'm not sure if we need to
> support legacy -fstack-protector at all, but we do, we'd need a separate
> option argument.
> 
> I would expect some build-many-glibcs.py updates for this one, too.
> 
> But I think for this one, --disable-stack-protector actually works.

There's also --enable-stack-protector=full, which enables 
-fstack-protector-full, which is why I kept it as --enable-* rather than 
flipping it to --disable.

If we want to drop everything except strong, then IMO it doesn't make 
sense to add an addition configure option to enable the options that we 
dropped; it kinda defeats the whole point of the exercise.  If we want 
to retain them, then how about 
--enable-stack-protector={strong,full,legacy,no} with the default as strong?

I'm not going to push to resolve this during the freeze but we could 
work towards consensus.

Thanks,
Sid
Florian Weimer July 17, 2023, 3:45 p.m. UTC | #3
* Siddhesh Poyarekar via Libc-alpha:

> On 2023-07-13 05:51, Florian Weimer wrote:
>> * Siddhesh Poyarekar via Libc-alpha:
>> 
>>> All major distributions use this level of stack protector, so make it
>>> the default.
>>>
>>> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
>> I think if strong is the default (correct IMHO), it should be the
>> default for --enable-stack-protector, too.  I'm not sure if we need to
>> support legacy -fstack-protector at all, but we do, we'd need a separate
>> option argument.
>> I would expect some build-many-glibcs.py updates for this one, too.
>> But I think for this one, --disable-stack-protector actually works.
>
> There's also --enable-stack-protector=full, which enables
> -fstack-protector-full, which is why I kept it as --enable-* rather
> than flipping it to --disable.

Do you mean =all?

> If we want to drop everything except strong, then IMO it doesn't make
> sense to add an addition configure option to enable the options that
> we dropped; it kinda defeats the whole point of the exercise.  If we
> want to retain them, then how about
> --enable-stack-protector={strong,full,legacy,no} with the default as
> strong?

I don't think we really need anything but strong; -all is occasionally
interesting for correctness checking, though.  With -strong we might
miss cases where we did not systematically exclude some code because of
ordering dependencies because the compilers we tested with removed the
instrumentation as part of optimization.

My main point was that --enable-stack-protector should enable the
default (-strong), otherwise it's confusing.

Thanks,
Florian
Siddhesh Poyarekar July 17, 2023, 3:52 p.m. UTC | #4
On 2023-07-17 11:45, Florian Weimer via Libc-alpha wrote:
> * Siddhesh Poyarekar via Libc-alpha:
> 
>> On 2023-07-13 05:51, Florian Weimer wrote:
>>> * Siddhesh Poyarekar via Libc-alpha:
>>>
>>>> All major distributions use this level of stack protector, so make it
>>>> the default.
>>>>
>>>> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
>>> I think if strong is the default (correct IMHO), it should be the
>>> default for --enable-stack-protector, too.  I'm not sure if we need to
>>> support legacy -fstack-protector at all, but we do, we'd need a separate
>>> option argument.
>>> I would expect some build-many-glibcs.py updates for this one, too.
>>> But I think for this one, --disable-stack-protector actually works.
>>
>> There's also --enable-stack-protector=full, which enables
>> -fstack-protector-full, which is why I kept it as --enable-* rather
>> than flipping it to --disable.
> 
> Do you mean =all?

Sorry, yes, I meant =all.

>> If we want to drop everything except strong, then IMO it doesn't make
>> sense to add an addition configure option to enable the options that
>> we dropped; it kinda defeats the whole point of the exercise.  If we
>> want to retain them, then how about
>> --enable-stack-protector={strong,full,legacy,no} with the default as
>> strong?
> 
> I don't think we really need anything but strong; -all is occasionally
> interesting for correctness checking, though.  With -strong we might
> miss cases where we did not systematically exclude some code because of
> ordering dependencies because the compilers we tested with removed the
> instrumentation as part of optimization.
> 
> My main point was that --enable-stack-protector should enable the
> default (-strong), otherwise it's confusing.

OK, so  --enable-stack-protector={strong,all,no} with the default as 
strong, thus dropping -fstack-protector?

Thanks,
Sid
Florian Weimer Aug. 3, 2023, 10:06 a.m. UTC | #5
* Siddhesh Poyarekar:

> On 2023-07-17 11:45, Florian Weimer via Libc-alpha wrote:
>> * Siddhesh Poyarekar via Libc-alpha:
>> 
>>> On 2023-07-13 05:51, Florian Weimer wrote:
>>>> * Siddhesh Poyarekar via Libc-alpha:
>>>>
>>>>> All major distributions use this level of stack protector, so make it
>>>>> the default.
>>>>>
>>>>> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
>>>> I think if strong is the default (correct IMHO), it should be the
>>>> default for --enable-stack-protector, too.  I'm not sure if we need to
>>>> support legacy -fstack-protector at all, but we do, we'd need a separate
>>>> option argument.
>>>> I would expect some build-many-glibcs.py updates for this one, too.
>>>> But I think for this one, --disable-stack-protector actually works.
>>>
>>> There's also --enable-stack-protector=full, which enables
>>> -fstack-protector-full, which is why I kept it as --enable-* rather
>>> than flipping it to --disable.
>> Do you mean =all?
>
> Sorry, yes, I meant =all.
>
>>> If we want to drop everything except strong, then IMO it doesn't make
>>> sense to add an addition configure option to enable the options that
>>> we dropped; it kinda defeats the whole point of the exercise.  If we
>>> want to retain them, then how about
>>> --enable-stack-protector={strong,full,legacy,no} with the default as
>>> strong?
>> I don't think we really need anything but strong; -all is
>> occasionally
>> interesting for correctness checking, though.  With -strong we might
>> miss cases where we did not systematically exclude some code because of
>> ordering dependencies because the compilers we tested with removed the
>> instrumentation as part of optimization.
>> My main point was that --enable-stack-protector should enable the
>> default (-strong), otherwise it's confusing.
>
> OK, so  --enable-stack-protector={strong,all,no} with the default as
> strong, thus dropping -fstack-protector?

Or perhaps --enable-stack-protector=plain for the legacy
-fstack-protector setting.

Thanks,
Florian
diff mbox series

Patch

diff --git a/INSTALL b/INSTALL
index a1e189eb9f..f02358e933 100644
--- a/INSTALL
+++ b/INSTALL
@@ -196,13 +196,14 @@  if ‘CFLAGS’ is specified it must enable optimization.  For example:
 ‘--enable-stack-protector’
 ‘--enable-stack-protector=strong’
 ‘--enable-stack-protector=all’
+‘--enable-stack-protector=no’
      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.
+     protection.  This option is enabled by default and set to ‘strong’.
 
 ‘--enable-bind-now’
      Disable lazy binding for installed shared objects and programs.
diff --git a/NEWS b/NEWS
index 709ee40e50..47ec0b741c 100644
--- a/NEWS
+++ b/NEWS
@@ -48,6 +48,10 @@  Major new features:
 * The strlcpy and strlcat functions have been added.  They are derived
   from OpenBSD, and are expected to be added to a future POSIX version.
 
+* The GNU C Library is now built with -fstack-protector-strong by
+  default.  This may be overridden by using the --enable-stack-protector
+  configure option.
+
 Deprecated and removed features, and other changes affecting compatibility:
 
 * In the Linux kernel for the hppa/parisc architecture some of the
diff --git a/configure b/configure
index 11538ee1b3..863621cabf 100755
--- a/configure
+++ b/configure
@@ -4462,7 +4462,7 @@  if test ${enable_stack_protector+y}
 then :
   enableval=$enable_stack_protector; enable_stack_protector=$enableval
 else $as_nop
-  enable_stack_protector=no
+  enable_stack_protector=strong
 fi
 
 case "$enable_stack_protector" in
diff --git a/configure.ac b/configure.ac
index 18bb989ade..d85452b3b3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -228,7 +228,7 @@  AC_ARG_ENABLE([stack-protector],
 	      AS_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])
+	      [enable_stack_protector=strong])
 case "$enable_stack_protector" in
 all|yes|no|strong) ;;
 *) AC_MSG_ERROR([Not a valid argument for --enable-stack-protector: "$enable_stack_protector"]);;
diff --git a/manual/install.texi b/manual/install.texi
index 52eb2d8a23..b1aa5eb60c 100644
--- a/manual/install.texi
+++ b/manual/install.texi
@@ -222,13 +222,14 @@  time.  Consult the @file{timezone} subdirectory for more details.
 @item --enable-stack-protector
 @itemx --enable-stack-protector=strong
 @itemx --enable-stack-protector=all
+@itemx --enable-stack-protector=no
 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.
+protection.  This option is enabled by default and set to @option{strong}.
 
 @item --enable-bind-now
 Disable lazy binding for installed shared objects and programs.  This