gcc: improve checking of stack smashing support with uclibc
RFC PATCH v2 1/1 gcc: improve checking of stack smashing support with uclibc
Fixes:
http://autobuild.buildroot.net/results/123/123a5b3f72ba8c1a4aa1cea5b7b846a04fd4e923/
http://autobuild.buildroot.net/results/38c/38cfa4e7249a8770b06dbd392acba79303d3f9bc/
.. and others.
Improve GCC's checking of stack smashing support, by moving the check for uclibc further up, and splitting the condition there so that stack smashing support is tested in any case where uclibc is detected.
---
v1 -> v2 : improved following comments from Arnout :
- reworked the conditional statement instead of adding a separate case statement. This is a bit less intrusive.
- formatting etc. improvements
- still only fixing 4.7.4 - once we've got it right I'll port the patch to the other versions
Patch V1 - this initial version only fixes 4.7.4 - wanted to check that we are heading the right way before I fix all the other versions.
Note that I manually modified configure, rather than regenerating it from configure.ac.
Original plan was to reverse the order of the existing __GLIBC_MINOR__ and uclibc check. However, the uclibc check falls through if it does not detect uclibc, so I figure it better to add the separate case statement.
--- ...ing-of-stack-smashing-support-with-uclibc.patch | 81 ++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch
package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch new file +81 -0
@@ -0,0 +1,81 @@ +From ff055a237ef91673a031bfe9dab743b01bd93d70 Mon Sep 17 00:00:00 2001 +From: Brendan Heading <[email protected]> +Date: Mon, 28 Sep 2015 16:14:41 +0100 +Subject: [PATCH 1/1] Improve checking of stack-smashing support with uclibc + +Detect if uclibc has stack-smashing enabled, and fall out if it does not. + +A more comprehensive solution is to be proposed for upstream. + +Upstream-status: inappropriate +Signed-off-by: Brendan Heading <[email protected]> +--- + gcc/configure | 15 +++++++++------ + gcc/configure.ac | 15 +++++++++------ + 2 files changed, 18 insertions(+), 12 deletions(-) + +diff --git a/gcc/configure b/gcc/configure +index 63cba0a..658a3e6 100755 +--- a/gcc/configure ++++ b/gcc/configure +@@ -26806,17 +26806,20 @@ else + if $EGREP '^[ ]*#[ ]*define[ ]+__GLIBC__[ ]+([1-9][0-9]|[3-9])' \ + $target_header_dir/features.h > /dev/null; then + gcc_cv_libc_provides_ssp=yes ++ elif $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC__[ ]+1' \ ++ $target_header_dir/features.h > /dev/null && \ ++ test -f $target_header_dir/bits/uClibc_config.h; then ++ if $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC_HAS_SSP__[ ]+1' \ ++ $target_header_dir/bits/uClibc_config.h > /dev/null; then ++ gcc_cv_libc_provides_ssp=yes ++ else ++ gcc_cv_libc_provides_ssp=no ++ fi + elif $EGREP '^[ ]*#[ ]*define[ ]+__GLIBC__[ ]+2' \ + $target_header_dir/features.h > /dev/null \ + && $EGREP '^[ ]*#[ ]*define[ ]+__GLIBC_MINOR__[ ]+([1-9][0-9]|[4-9])' \ + $target_header_dir/features.h > /dev/null; then + gcc_cv_libc_provides_ssp=yes +- elif $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC__[ ]+1' \ +- $target_header_dir/features.h > /dev/null && \ +- test -f $target_header_dir/bits/uClibc_config.h && \ +- $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC_HAS_SSP__[ ]+1' \ +- $target_header_dir/bits/uClibc_config.h > /dev/null; then +- gcc_cv_libc_provides_ssp=yes + fi + # all versions of Bionic support stack protector + elif test -f $target_header_dir/sys/cdefs.h \ +diff --git a/gcc/configure.ac b/gcc/configure.ac +index ea1c147..7282ea4 100644 +--- a/gcc/configure.ac ++++ b/gcc/configure.ac +@@ -4681,17 +4681,20 @@ AC_CACHE_CHECK(__stack_chk_fail in target C library, + if $EGREP '^[ ]*#[ ]*define[ ]+__GLIBC__[ ]+([1-9][0-9]|[3-9])' \ + $target_header_dir/features.h > /dev/null; then + gcc_cv_libc_provides_ssp=yes ++ elif $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC__[ ]+1' \ ++ $target_header_dir/features.h > /dev/null && \ ++ test -f $target_header_dir/bits/uClibc_config.h; then ++ if $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC_HAS_SSP__[ ]+1' \ ++ $target_header_dir/bits/uClibc_config.h > /dev/null; then ++ gcc_cv_libc_provides_ssp=yes ++ else ++ gcc_cv_libc_provides_ssp=no ++ fi + elif $EGREP '^[ ]*#[ ]*define[ ]+__GLIBC__[ ]+2' \ + $target_header_dir/features.h > /dev/null \ + && $EGREP '^[ ]*#[ ]*define[ ]+__GLIBC_MINOR__[ ]+([1-9][0-9]|[4-9])' \ + $target_header_dir/features.h > /dev/null; then + gcc_cv_libc_provides_ssp=yes +- elif $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC__[ ]+1' \ +- $target_header_dir/features.h > /dev/null && \ +- test -f $target_header_dir/bits/uClibc_config.h && \ +- $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC_HAS_SSP__[ ]+1' \ +- $target_header_dir/bits/uClibc_config.h > /dev/null; then +- gcc_cv_libc_provides_ssp=yes + fi + # all versions of Bionic support stack protector + elif test -f $target_header_dir/sys/cdefs.h \ +-- +2.4.3 +
Signature · 1 line
2.4.3
RFC PATCH v2 1/1 gcc: improve checking of stack smashing support with uclibc
Quoted 62 lines · Brendan Heading
Fixes:
http://autobuild.buildroot.net/results/123/123a5b3f72ba8c1a4aa1cea5b7b846a04fd4e923/
http://autobuild.buildroot.net/results/38c/38cfa4e7249a8770b06dbd392acba79303d3f9bc/.. and others.
Improve GCC's checking of stack smashing support, by moving the check for uclibc further up, and splitting the condition there so that stack smashing support is tested in any case where uclibc is detected.
Signed-off-by Brendan Heading <[email protected]>---
v1 -> v2 : improved following comments from Arnout :
- reworked the conditional statement instead of adding a separate case statement. This is a bit less intrusive.
- formatting etc. improvements
- still only fixing 4.7.4 - once we've got it right I'll port the patch to the other versionsPatch V1 - this initial version only fixes 4.7.4 - wanted to check that we are heading the right way before I fix all the other versions.
Note that I manually modified configure, rather than regenerating it from configure.ac.
Original plan was to reverse the order of the existing
__GLIBC_MINOR__and uclibc check. However, the uclibc check falls through if it does not detect uclibc, so I figure it better to add the separate case statement.--- ...ing-of-stack-smashing-support-with-uclibc.patch | 81 ++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patchpackage/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch new file +23
-0@@ -0,0 +1,81 @@ +From ff055a237ef91673a031bfe9dab743b01bd93d70 Mon Sep 17 00:00:00 2001 +From: Brendan Heading <[email protected]> +Date: Mon, 28 Sep 2015 16:14:41 +0100 +Subject: [PATCH 1/1] Improve checking of stack-smashing support with uclibc + +Detect if uclibc has stack-smashing enabled, and fall out if it does not. + +A more comprehensive solution is to be proposed for upstream. + +Upstream-status: inappropriate +Signed-off-by: Brendan Heading <[email protected]> +--- + gcc/configure | 15 +++++++++------ + gcc/configure.ac | 15 +++++++++------ + 2 files changed, 18 insertions(+), 12 deletions(-) + +diff --git a/gcc/configure b/gcc/configure +index 63cba0a..658a3e6 100755 +--- a/gcc/configure ++++ b/gcc/configure +@@ -26806,17 +26806,20 @@ else + if $EGREP '^[ ]*#[ ]*define[ ]+__GLIBC__[ ]+([1-9][0-9]|[3-9])' \ + $target_header_dir/features.h > /dev/null; then
package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch line 33
Quoted 10 lines
package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch new file +10
-0+ gcc_cv_libc_provides_ssp=yes ++ elif $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC__[ ]+1' \ ++ $target_header_dir/features.h > /dev/null && \ ++ test -f $target_header_dir/bits/uClibc_config.h; then ++ if $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC_HAS_SSP__[ ]+1' \ ++ $target_header_dir/bits/uClibc_config.h > /dev/null; then ++ gcc_cv_libc_provides_ssp=yes ++ else ++ gcc_cv_libc_provides_ssp=no ++ fi
This looks OK to me. However, I would wait a bit to see what upstream says, if
they have a better solution we can use it for the old versions as well.
OTOH, this fixes an autobuilder failure so perhaps it's better to commit
quickly and fix up later if needed.
Regards,
Arnout
Quoted 49 lines
package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch new file +48
-0+ elif $EGREP '^[ ]*#[ ]*define[ ]+__GLIBC__[ ]+2' \ + $target_header_dir/features.h > /dev/null \ + && $EGREP '^[ ]*#[ ]*define[ ]+__GLIBC_MINOR__[ ]+([1-9][0-9]|[4-9])' \ + $target_header_dir/features.h > /dev/null; then + gcc_cv_libc_provides_ssp=yes +- elif $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC__[ ]+1' \ +- $target_header_dir/features.h > /dev/null && \ +- test -f $target_header_dir/bits/uClibc_config.h && \ +- $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC_HAS_SSP__[ ]+1' \ +- $target_header_dir/bits/uClibc_config.h > /dev/null; then +- gcc_cv_libc_provides_ssp=yes + fi + # all versions of Bionic support stack protector + elif test -f $target_header_dir/sys/cdefs.h \ +diff --git a/gcc/configure.ac b/gcc/configure.ac +index ea1c147..7282ea4 100644 +--- a/gcc/configure.ac ++++ b/gcc/configure.ac +@@ -4681,17 +4681,20 @@ AC_CACHE_CHECK(__stack_chk_fail in target C library, + if $EGREP '^[ ]*#[ ]*define[ ]+__GLIBC__[ ]+([1-9][0-9]|[3-9])' \ + $target_header_dir/features.h > /dev/null; then + gcc_cv_libc_provides_ssp=yes ++ elif $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC__[ ]+1' \ ++ $target_header_dir/features.h > /dev/null && \ ++ test -f $target_header_dir/bits/uClibc_config.h; then ++ if $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC_HAS_SSP__[ ]+1' \ ++ $target_header_dir/bits/uClibc_config.h > /dev/null; then ++ gcc_cv_libc_provides_ssp=yes ++ else ++ gcc_cv_libc_provides_ssp=no ++ fi + elif $EGREP '^[ ]*#[ ]*define[ ]+__GLIBC__[ ]+2' \ + $target_header_dir/features.h > /dev/null \ + && $EGREP '^[ ]*#[ ]*define[ ]+__GLIBC_MINOR__[ ]+([1-9][0-9]|[4-9])' \ + $target_header_dir/features.h > /dev/null; then + gcc_cv_libc_provides_ssp=yes +- elif $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC__[ ]+1' \ +- $target_header_dir/features.h > /dev/null && \ +- test -f $target_header_dir/bits/uClibc_config.h && \ +- $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC_HAS_SSP__[ ]+1' \ +- $target_header_dir/bits/uClibc_config.h > /dev/null; then +- gcc_cv_libc_provides_ssp=yes + fi + # all versions of Bionic support stack protector + elif test -f $target_header_dir/sys/cdefs.h \ +-- +2.4.3 +
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 Essensium/Mind http://www.mind.be G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
RFC PATCH v2 1/1 gcc: improve checking of stack smashing support with uclibc
In reply to Brendan HeadingBrendan,
Quoted 6 lines · Brendan Heading
package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch new file +0
-0
package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch line 10
Quoted 10 lines
package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch new file +10
-0@@ -0,0 +1,81 @@ +From ff055a237ef91673a031bfe9dab743b01bd93d70 Mon Sep 17 00:00:00 2001 +From: Brendan Heading <[email protected]> +Date: Mon, 28 Sep 2015 16:14:41 +0100 +Subject: [PATCH 1/1] Improve checking of stack-smashing support with uclibc + +Detect if uclibc has stack-smashing enabled, and fall out if it does not. + +A more comprehensive solution is to be proposed for upstream. + +Upstream-status: inappropriate
Why ?
Quoted 4 lines
package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch new file +4
-0+Signed-off-by: Brendan Heading <[email protected]> +--- + gcc/configure | 15 +++++++++------ + gcc/configure.ac | 15 +++++++++------
package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch line 24
Quoted 10 lines
package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch new file +10
-0+ 2 files changed, 18 insertions(+), 12 deletions(-) + +diff --git a/gcc/configure b/gcc/configure +index 63cba0a..658a3e6 100755 +--- a/gcc/configure ++++ b/gcc/configure +@@ -26806,17 +26806,20 @@ else + if $EGREP '^[ ]*#[ ]*define[ ]+__GLIBC__[ ]+([1-9][0-9]|[3-9])' \ + $target_header_dir/features.h > /dev/null; then + gcc_cv_libc_provides_ssp=yes
What prevents uClibc from ever matching this case? Shouldn't the __UCLIBC__ case be *before* any __GLIBC__ case ?
package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch line 32
Quoted 8 lines
package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch new file +8
-0++ elif $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC__[ ]+1' \ ++ $target_header_dir/features.h > /dev/null && \ ++ test -f $target_header_dir/bits/uClibc_config.h; then ++ if $EGREP '^[ ]*#[ ]*define[ ]+__UCLIBC_HAS_SSP__[ ]+1' \ ++ $target_header_dir/bits/uClibc_config.h > /dev/null; then ++ gcc_cv_libc_provides_ssp=yes ++ else ++ gcc_cv_libc_provides_ssp=no
This else close is useless, gcc_cv_libc_provides_ssp is initialized to "no" at the beginning of this piece of code.
If you look further down below in the code, you can see:
*-*-darwin* | *-*-freebsd*)
AC_CHECK_FUNC(__stack_chk_fail,[gcc_cv_libc_provides_ssp=yes],
[echo "no __stack_chk_fail on this target"])Do you know why a similar check isn't used for Linux ? It really seems to be a lot easier than to poke into the C library details, no? But maybe it's a too drastic change.
But now that I think of it, there is probably a much, much simpler
change: use the gcc_cv_libc_provides_ssp cache variable, which we already use during gcc-initial.
My proposal would be something like:
http://git.free-electrons.com/users/thomas-petazzoni/buildroot/log/?h=fix-ssp
(see the last three commits)
I am currently doing a test build of glibc, uClibc w/ SSP, uClibc w/o SSP and musl to see how it goes. But I believe it's actually simpler than patching gcc, no?
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
RFC PATCH v2 1/1 gcc: improve checking of stack smashing support with uclibc
Quoted 3 lines
Quoted 1 line
+Upstream-status: inappropriate
Why ?
There was a bit of debate a few weeks ago on whether or not to submit the proposed changes upstream, but either way, it was never going to be likely that we'd get them to accept a patch against an old version of GCC that is in maintenance-only mode.
Quoted 4 lines
Quoted 1 line
+ gcc_cv_libc_provides_ssp=yes
What prevents uClibc from ever matching this case? Shouldn't the
__UCLIBC__case be *before* any__GLIBC__case ?
Don't recall, but now it's academic now, as your proposed solution is better.
Quoted 9 lines
My proposal would be something like:
http://git.free-electrons.com/users/thomas-petazzoni/buildroot/log/?h=fix-ssp
(see the last three commits)
I am currently doing a test build of glibc, uClibc w/ SSP, uClibc w/o SSP and musl to see how it goes. But I believe it's actually simpler than patching gcc, no?
Definitely, I always thought that trying to get patches upstream and/or maintaining the patches locally would be a pain. I'd assumed that someone in the past had a good reason for adding the SSP checks in a patch for these cases, rather than using the cache variable, so I did not consider that option.
I've mark my patch proposal as superseded.
At least there is now a fix though, this should kill off a sizeable bunch of autobuild problems.
Brendan