All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/1] package/sudo: disable use of stack protector when not available
@ 2015-09-15 18:49 Brendan Heading
  2015-09-15 21:30 ` Thomas Petazzoni
  0 siblings, 1 reply; 7+ messages in thread
From: Brendan Heading @ 2015-09-15 18:49 UTC (permalink / raw
  To: buildroot

Fixes:
http://autobuild.buildroot.net/results/d93/d9390b929328e6253b883f000f6f09972df90f47/

sudo, 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 pass --disable-hardening if it is not - similar to
psmisc and sox.

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

diff --git a/package/sudo/sudo.mk b/package/sudo/sudo.mk
index 4327c8a..b839ee4 100644
--- a/package/sudo/sudo.mk
+++ b/package/sudo/sudo.mk
@@ -30,6 +30,11 @@ else
 SUDO_CONF_OPTS += --without-pam
 endif
 
+ifeq ($(BR2_TOOLCHAIN_HAS_SSP),)
+# Don't force -fstack-protector when SSP is not available in toolchain
+SUDO_CONF_OPTS += --disable-hardening
+endif
+
 # mksigname/mksiglist needs to run on build host to generate source files
 define SUDO_BUILD_MKSIGNAME_MKSIGLIST_HOST
 	$(MAKE) $(HOST_CONFIGURE_OPTS) \
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH 1/1] package/sudo: disable use of stack protector when not available
  2015-09-15 18:49 [Buildroot] [PATCH 1/1] package/sudo: disable use of stack protector when not available Brendan Heading
@ 2015-09-15 21:30 ` Thomas Petazzoni
  2015-09-15 21:41   ` Brendan Heading
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Petazzoni @ 2015-09-15 21:30 UTC (permalink / raw
  To: buildroot

Brendan,

On Tue, 15 Sep 2015 19:49:13 +0100, Brendan Heading wrote:
> Fixes:
> http://autobuild.buildroot.net/results/d93/d9390b929328e6253b883f000f6f09972df90f47/
> 
> sudo, 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 pass --disable-hardening if it is not - similar to
> psmisc and sox.
> 
> Signed-off-by: Brendan Heading <brendanheading@gmail.com>

I'm not sure to understand here. I tested with the pre-built toolchain
at
http://autobuild.buildroot.org/toolchains/configs/br-arm-full.config,
and it does properly detect that SSP support is not available:

checking whether C compiler accepts -fstack-protector-strong... no
checking whether C compiler accepts -fstack-protector-all... yes
checking whether the linker accepts -fstack-protector-all... no
checking whether C compiler accepts -fstack-protector... yes
checking whether the linker accepts -fstack-protector... no

And therefore, it doesn't use it, and sudo builds successfully.

In the autobuilder failure you pointed, it however thinks that SSP is
available. According to config.log:

configure:24179: checking whether C compiler accepts -fstack-protector-strong
configure:24198: /home/test/autobuild/instance-2/output/host/usr/bin/powerpc-buildroot-linux-uclibc-gcc -std=gnu99 -c -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64   -Os  -fvisibility=hidden  -fstack-protector-strong -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 conftest.c >&5
configure:24198: $? = 0
configure:24206: result: yes
configure:24210: checking whether the linker accepts -fstack-protector-strong
configure:24229: /home/test/autobuild/instance-2/output/host/usr/bin/powerpc-buildroot-linux-uclibc-gcc -std=gnu99 -o conftest -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64   -Os  -fvisibility=hidden -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64   -fstack-protector-strong conftest.c  >&5
configure:24229: $? = 0
configure:24238: result: yes

I don't understand why the second test works. Without SSP support in
the toolchain, it should fail.

Do you understand why this is failing with an internal toolchain, and
not with an external toolchain (which was built by Buildroot) ?

Thanks,

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH 1/1] package/sudo: disable use of stack protector when not available
  2015-09-15 21:30 ` Thomas Petazzoni
@ 2015-09-15 21:41   ` Brendan Heading
  2015-09-15 21:44     ` Thomas Petazzoni
  0 siblings, 1 reply; 7+ messages in thread
From: Brendan Heading @ 2015-09-15 21:41 UTC (permalink / raw
  To: buildroot

[cc list]

>> Signed-off-by: Brendan Heading <brendanheading@gmail.com>
>
> I'm not sure to understand here. I tested with the pre-built toolchain
> at
> http://autobuild.buildroot.org/toolchains/configs/br-arm-full.config,
> and it does properly detect that SSP support is not available:

Yup, and I think x86 works without my patch too.

My guess is that this is down to differences between how SSP is
implemented on certain arches. powerpc has a few stack protector
related failures in similar circumstances (I submitted a patch for
ruby with a similar fix). It seems that the compiler and linker
sometimes accept -fstack-protector even when SSP is unsupported.

Do you want me to dive a bit deeper ?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH 1/1] package/sudo: disable use of stack protector when not available
  2015-09-15 21:41   ` Brendan Heading
@ 2015-09-15 21:44     ` Thomas Petazzoni
  2015-09-15 21:54       ` Brendan Heading
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Petazzoni @ 2015-09-15 21:44 UTC (permalink / raw
  To: buildroot

Dear Brendan Heading,

On Tue, 15 Sep 2015 22:41:11 +0100, Brendan Heading wrote:

> My guess is that this is down to differences between how SSP is
> implemented on certain arches. powerpc has a few stack protector
> related failures in similar circumstances (I submitted a patch for
> ruby with a similar fix). It seems that the compiler and linker
> sometimes accept -fstack-protector even when SSP is unsupported.

I also thought of an architecture specific issue, but I tried with
http://autobuild.buildroot.org/toolchains/configs/br-powerpc-e500mc-full.config
and it also built just fine.

I have nothing against your patch in principle, it can only be good to
explicitly disable stack protector usage when the necessary support is
not there. I'm just curious to understand why the detection does work
in certain cases and not in others.

Best regards,

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH 1/1] package/sudo: disable use of stack protector when not available
  2015-09-15 21:44     ` Thomas Petazzoni
@ 2015-09-15 21:54       ` Brendan Heading
  2015-09-15 23:52         ` Brendan Heading
  0 siblings, 1 reply; 7+ messages in thread
From: Brendan Heading @ 2015-09-15 21:54 UTC (permalink / raw
  To: buildroot

> I have nothing against your patch in principle, it can only be good to
> explicitly disable stack protector usage when the necessary support is
> not there. I'm just curious to understand why the detection does work
> in certain cases and not in others.

Understood Thomas. I'll have a closer look and see what's going on.

Brendan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH 1/1] package/sudo: disable use of stack protector when not available
  2015-09-15 21:54       ` Brendan Heading
@ 2015-09-15 23:52         ` Brendan Heading
  2015-09-16 13:58           ` Brendan Heading
  0 siblings, 1 reply; 7+ messages in thread
From: Brendan Heading @ 2015-09-15 23:52 UTC (permalink / raw
  To: buildroot

Thomas,

Yes you were right, there's something fishy going on with internal
toolchains and it's probably not arch-specific.

I checked it out in the x86 case and tried to set up a buildroot
toolchain closely matching the non-buildroot toolchain.

This defconfig fails (ie doesn't correctly detect stack protection):

BR2_x86_pentium4=y
BR2_COMPILER_PARANOID_UNSAFE_PATH=y
BR2_TOOLCHAIN_BUILDROOT_INET_RPC=y
BR2_TOOLCHAIN_BUILDROOT_CXX=y
BR2_PACKAGE_SUDO=y

This defconfig works :

BR2_x86_pentium4=y
BR2_COMPILER_PARANOID_UNSAFE_PATH=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-i386-pentium4-full-2015.08-rc1-38-gad0f85e.tar.bz2"
BR2_TOOLCHAIN_EXTERNAL_GCC_4_9=y
BR2_TOOLCHAIN_EXTERNAL_HEADERS_3_2=y
BR2_TOOLCHAIN_EXTERNAL_LOCALE=y
# BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set
BR2_TOOLCHAIN_EXTERNAL_INET_RPC=y
BR2_TOOLCHAIN_EXTERNAL_CXX=y
BR2_PACKAGE_SUDO=y

I'll try to find out what's going on tomorrow.

regards

Brendan

On 15 September 2015 at 22:54, Brendan Heading <brendanheading@gmail.com> wrote:
>> I have nothing against your patch in principle, it can only be good to
>> explicitly disable stack protector usage when the necessary support is
>> not there. I'm just curious to understand why the detection does work
>> in certain cases and not in others.
>
> Understood Thomas. I'll have a closer look and see what's going on.
>
> Brendan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH 1/1] package/sudo: disable use of stack protector when not available
  2015-09-15 23:52         ` Brendan Heading
@ 2015-09-16 13:58           ` Brendan Heading
  0 siblings, 0 replies; 7+ messages in thread
From: Brendan Heading @ 2015-09-16 13:58 UTC (permalink / raw
  To: buildroot

> I'll try to find out what's going on tomorrow.

Thomas,

I've found that the SSP detection seems to go wrong with a buildroot
toolchain and when uclibc-ng is in use. regular uclibc and glibc
appear work fine.

I'm looking in and around the uclibc feature checking. It smells as if
there's probably a difference with uclibc-ng which is not being
accounted for, but I've not spotted it just yet.

Brendan

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-09-16 13:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-15 18:49 [Buildroot] [PATCH 1/1] package/sudo: disable use of stack protector when not available Brendan Heading
2015-09-15 21:30 ` Thomas Petazzoni
2015-09-15 21:41   ` Brendan Heading
2015-09-15 21:44     ` Thomas Petazzoni
2015-09-15 21:54       ` Brendan Heading
2015-09-15 23:52         ` Brendan Heading
2015-09-16 13:58           ` Brendan Heading

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.