* [Buildroot] [PATCH RFC v2] gcc: improve checking of stack smashing support with uclibc
@ 2015-09-28 16:34 Brendan Heading
2015-10-03 18:05 ` Arnout Vandecappelle
2015-10-15 21:40 ` Thomas Petazzoni
0 siblings, 2 replies; 4+ messages in thread
From: Brendan Heading @ 2015-09-28 16:34 UTC (permalink / raw)
To: buildroot
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 <brendanheading@gmail.com>
---
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
diff --git a/package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch b/package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch
new file mode 100644
index 0000000..31947a0
--- /dev/null
+++ b/package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch
@@ -0,0 +1,81 @@
+From ff055a237ef91673a031bfe9dab743b01bd93d70 Mon Sep 17 00:00:00 2001
+From: Brendan Heading <brendanheading@gmail.com>
+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 <brendanheading@gmail.com>
+---
+ 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
+
--
2.4.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Buildroot] [PATCH RFC v2] gcc: improve checking of stack smashing support with uclibc
2015-09-28 16:34 [Buildroot] [PATCH RFC v2] gcc: improve checking of stack smashing support with uclibc Brendan Heading
@ 2015-10-03 18:05 ` Arnout Vandecappelle
2015-10-15 21:40 ` Thomas Petazzoni
1 sibling, 0 replies; 4+ messages in thread
From: Arnout Vandecappelle @ 2015-10-03 18:05 UTC (permalink / raw)
To: buildroot
On 28-09-15 17:34, Brendan Heading wrote:
> 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 <brendanheading@gmail.com>
> ---
> 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
>
> diff --git a/package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch b/package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch
> new file mode 100644
> index 0000000..31947a0
> --- /dev/null
> +++ b/package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch
> @@ -0,0 +1,81 @@
> +From ff055a237ef91673a031bfe9dab743b01bd93d70 Mon Sep 17 00:00:00 2001
> +From: Brendan Heading <brendanheading@gmail.com>
> +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 <brendanheading@gmail.com>
> +---
> + 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
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
> + 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
> +
>
--
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Buildroot] [PATCH RFC v2] gcc: improve checking of stack smashing support with uclibc
2015-09-28 16:34 [Buildroot] [PATCH RFC v2] gcc: improve checking of stack smashing support with uclibc Brendan Heading
2015-10-03 18:05 ` Arnout Vandecappelle
@ 2015-10-15 21:40 ` Thomas Petazzoni
2015-10-18 14:13 ` Brendan Heading
1 sibling, 1 reply; 4+ messages in thread
From: Thomas Petazzoni @ 2015-10-15 21:40 UTC (permalink / raw)
To: buildroot
Brendan,
On Mon, 28 Sep 2015 17:34:37 +0100, Brendan Heading wrote:
> diff --git a/package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch b/package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch
> new file mode 100644
> index 0000000..31947a0
> --- /dev/null
> +++ b/package/gcc/4.7.4/920-gcc-improve-checking-of-stack-smashing-support-with-uclibc.patch
> @@ -0,0 +1,81 @@
> +From ff055a237ef91673a031bfe9dab743b01bd93d70 Mon Sep 17 00:00:00 2001
> +From: Brendan Heading <brendanheading@gmail.com>
> +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 ?
> +Signed-off-by: Brendan Heading <brendanheading@gmail.com>
> +---
> + 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
What prevents uClibc from ever matching this case? Shouldn't the
__UCLIBC__ case be *before* any __GLIBC__ case ?
> ++ 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
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 4+ messages in thread* [Buildroot] [PATCH RFC v2] gcc: improve checking of stack smashing support with uclibc
2015-10-15 21:40 ` Thomas Petazzoni
@ 2015-10-18 14:13 ` Brendan Heading
0 siblings, 0 replies; 4+ messages in thread
From: Brendan Heading @ 2015-10-18 14:13 UTC (permalink / raw)
To: buildroot
>> +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.
>> + 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.
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-10-18 14:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-28 16:34 [Buildroot] [PATCH RFC v2] gcc: improve checking of stack smashing support with uclibc Brendan Heading
2015-10-03 18:05 ` Arnout Vandecappelle
2015-10-15 21:40 ` Thomas Petazzoni
2015-10-18 14:13 ` Brendan Heading
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox