[Buildroot] [PATCH 1/1] package/ruby: disable use of stack protector when not available

[Buildroot] [PATCH 1/1] package/ruby: disable use of stack protector when not available

[Brendan Heading]

Fixes:
http://autobuild.buildroot.net/results/22e/22eced2dc9ca1bc90ef193b4dc40891c47157e89/

ruby, by default, attempts to use the stack protector if configure detects
that it exists. The stack protector detection does not attempt to link
libssp, which can cause a false positive.

Instead, check if the stack protector is enabled in the buildroot
toolchain config, and set the stack_protector=no environment variable to
force the stack protector off.

Signed-off-by: Brendan Heading <brendanheading@gmail.com>
---
 package/ruby/ruby.mk | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/package/ruby/ruby.mk b/package/ruby/ruby.mk
index 243cd0b..9f78c33 100644
--- a/package/ruby/ruby.mk
+++ b/package/ruby/ruby.mk
@@ -36,6 +36,11 @@ RUBY_CONF_ENV = ac_cv_func_dl_iterate_phdr=no
 RUBY_CONF_OPTS += --with-out-ext=fiddle
 endif
 
+ifeq ($(BR2_TOOLCHAIN_HAS_SSP),)
+# Don't force -fstack-protector when SSP is not available in toolchain
+RUBY_CONF_ENV += stack_protector=no
+endif
+
 # Force optionals to build before we do
 ifeq ($(BR2_PACKAGE_BERKELEYDB),y)
 RUBY_DEPENDENCIES += berkeleydb
-- 
2.4.3

[Buildroot] [PATCH 1/1] package/ruby: disable use of stack protector when not available

[Thomas Petazzoni]

Brendan,

On Tue, 15 Sep 2015 20:56:59 +0100, Brendan Heading wrote:
> Fixes:
> http://autobuild.buildroot.net/results/22e/22eced2dc9ca1bc90ef193b4dc40891c47157e89/
> 
> ruby, by default, attempts to use the stack protector if configure detects
> that it exists. The stack protector detection does not attempt to link
> libssp, which can cause a false positive.
> 
> Instead, check if the stack protector is enabled in the buildroot
> toolchain config, and set the stack_protector=no environment variable to
> force the stack protector off.
> 
> Signed-off-by: Brendan Heading <brendanheading@gmail.com>

I have the same concern/question for this patch as the one for the sudo
patch. With an ARM uClibc toolchain that is pre-built
(http://autobuild.buildroot.org/toolchains/configs/br-arm-full.config),
Ruby builds just fine because:

checking whether -fstack-protector is accepted as CFLAGS... yes
checking whether -fstack-protector is accepted as LDFLAGS... no

So it knows that SSP support is not available.

However, with the PowerPC toolchain of the autobuilder failure you're
pointing to:

checking whether -fstack-protector is accepted as CFLAGS... yes
checking whether -fstack-protector is accepted as LDFLAGS... yes

So same as for the sudo patch: I'm fine with applying your patches, but
I'd prefer first to understand why we see this difference in behavior.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] package/ruby: disable use of stack protector when not available

[Brendan Heading]

> So same as for the sudo patch: I'm fine with applying your patches, but
> I'd prefer first to understand why we see this difference in behavior.

Thomas,

Yes I agree. This is a toolchain compilation issue and the right place
to solve it is there.

By way of an update, during the host-gcc-final build there is a check
for the symbol __stack_chk_fail. Under glibc and uclibc this test
fails, as expected. But under uclibc-ng, this test passes (ie returns
"yes") when it should actually fail, even though the symbol is
definitely not present. Inside the configure script there is some
logic that can cause it to return "yes" (for example, buildroot added
a patch such that when musl is detected it's hardcoded to always
return yes) so I'm trying to work out what code path in there is doing
this.

The presence of this symbol leads, ultimately, to slightly different
GCC specs. If it thinks that the target C library does not provide
SSP, GCC tries to link in its own SSP support libraries. This is the
path that is supposed to be executed. However, in the uclibc-ng case
we are wrongly detecting that the C library *does* have SSP even
though it doesn't. This causes the specs *not* to link the SSP support
libraries.

The reason why configure doesn't flag up a linker error in these
circumstances is because the conftest.c program is too simple. A
"hello world" type program does no stack operations and hence GCC
never emits any of the stack checking calls. So the problem slips
through the configure script.

So, once I've found out why uclibc-ng is triggering the false positive
in the GCC build I'll send in a patch.

Brendan

[Buildroot] [PATCH 1/1] package/ruby: disable use of stack protector when not available

[Thomas Petazzoni]

Brendan,

On Wed, 16 Sep 2015 22:29:01 +0100, Brendan Heading wrote:

> Yes I agree. This is a toolchain compilation issue and the right place
> to solve it is there.
> 
> By way of an update, during the host-gcc-final build there is a check
> for the symbol __stack_chk_fail. Under glibc and uclibc this test
> fails, as expected. But under uclibc-ng, this test passes (ie returns
> "yes") when it should actually fail, even though the symbol is
> definitely not present. Inside the configure script there is some
> logic that can cause it to return "yes" (for example, buildroot added
> a patch such that when musl is detected it's hardcoded to always
> return yes) so I'm trying to work out what code path in there is doing
> this.
> 
> The presence of this symbol leads, ultimately, to slightly different
> GCC specs. If it thinks that the target C library does not provide
> SSP, GCC tries to link in its own SSP support libraries. This is the
> path that is supposed to be executed. However, in the uclibc-ng case
> we are wrongly detecting that the C library *does* have SSP even
> though it doesn't. This causes the specs *not* to link the SSP support
> libraries.
> 
> The reason why configure doesn't flag up a linker error in these
> circumstances is because the conftest.c program is too simple. A
> "hello world" type program does no stack operations and hence GCC
> never emits any of the stack checking calls. So the problem slips
> through the configure script.
> 
> So, once I've found out why uclibc-ng is triggering the false positive
> in the GCC build I'll send in a patch.

Thanks for the update. However, notice that the toolchain at
http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-2015.08-rc1-38-gad0f85e.tar.bz2
is capable of building sudo and ruby, even if:

 1/ It is using uClibc-ng 1.0.5
 2/ It has SSP disabled: #undef __UCLIBC_HAS_SSP__

So it's not simply a matter of uClibc vs. uClibc-ng, since one
uClibc-ng toolchains works fine.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] package/ruby: disable use of stack protector when not available

[Brendan Heading]

> Thanks for the update. However, notice that the toolchain at
> http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-2015.08-rc1-38-gad0f85e.tar.bz2
> is capable of building sudo and ruby, even if:
>
>  1/ It is using uClibc-ng 1.0.5
>  2/ It has SSP disabled: #undef __UCLIBC_HAS_SSP__
>
> So it's not simply a matter of uClibc vs. uClibc-ng, since one
> uClibc-ng toolchains works fine.

Thomas,

thanks for the tip - I will find out why that scenario seems to work,
after I've identified the failure mode of the one I'm looking at now.

Brendan

[Buildroot] [PATCH 1/1] package/ruby: disable use of stack protector when not available

[Gustavo Zacarias]

On 16/09/15 18:46, Brendan Heading wrote:

>> Thanks for the update. However, notice that the toolchain at
>> http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-2015.08-rc1-38-gad0f85e.tar.bz2
>> is capable of building sudo and ruby, even if:
>>
>>   1/ It is using uClibc-ng 1.0.5
>>   2/ It has SSP disabled: #undef __UCLIBC_HAS_SSP__
>>
>> So it's not simply a matter of uClibc vs. uClibc-ng, since one
>> uClibc-ng toolchains works fine.
>
> Thomas,
>
> thanks for the tip - I will find out why that scenario seems to work,
> after I've identified the failure mode of the one I'm looking at now.

Hi.
See 
http://git.buildroot.net/buildroot/tree/package/samba4/0002-build-improve-stack-protector-check.patch
I've pointed Vicente to that problem when bumping to samba 4.3.0, and 
from the test in Ruby's configure.in i'm 99% certain we're seeing the 
same issue - the compiler optimizes away any stack usage when the test 
program does nothing, hence building/linking inserts no guards, thus "it 
works" (though not quite in reality).
The solution is having a foolproof test that can't be optimized away, 
unfortunately many packagers/developers use this simplified test that is 
flawed, it will require a lot of education to properly fix :)
(test program formulated together with Vicente and Arnout).
Regards.

[Buildroot] [PATCH 1/1] package/ruby: disable use of stack protector when not available

[Brendan Heading]

>> thanks for the tip - I will find out why that scenario seems to work,
>> after I've identified the failure mode of the one I'm looking at now.
>
> Hi.
> See
> http://git.buildroot.net/buildroot/tree/package/samba4/0002-build-improve-stack-protector-check.patch
> I've pointed Vicente to that problem when bumping to samba 4.3.0, and from
> the test in Ruby's configure.in i'm 99% certain we're seeing the same issue
> - the compiler optimizes away any stack usage when the test program does
> nothing, hence building/linking inserts no guards, thus "it works" (though
> not quite in reality).

Gustavo,

Yes, that's exactly what I'm seeing.

A regular main() which does nothing will not show the problem. I found
that if the test is extended so that a simple function is called,
which takes a pointer to an array and updates it, will properly test
what the compiler does, but some more testing would be needed to make
sure that it holds up under more aggressive optimisation flags (such
as using LTO).

> The solution is having a foolproof test that can't be optimized away,
> unfortunately many packagers/developers use this simplified test that is
> flawed, it will require a lot of education to properly fix :)
> (test program formulated together with Vicente and Arnout).

There are probably quite a few packages that have this problem.

However, there's definitely a problem with GCC. I posted a lengthy
explanation last night, but basically GCC assumes that if glibc
version 2.4 or greater is in use (as determined by __GLIBC__ and
__GLIBC_MINOR__) then the C library includes stack protection support,
which is a false assumption in the case of uclibc-ng.

I'm about to submit an initial patch which works, but it's not a very
satisfactory solution - I will CC you for comments.

regards

Brendan