Skip to contentSkip to conversation

trousers: disable pie option on ARC

PATCH Superseded Lada Trimasova 3 participants 4 messages Jan 15, 2016

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 trousers: disable pie option on ARC

ARC gcc understands "-pie" option and attempts to generate PIE binaries as of today PIE is not really supported for user-space applications. So we provide option which can make relro and pie usage optional and disable PIE detection if building for ARC. Also AUTORECONF option should be added because of modified configure.in and Makefile.am files.

Cc Alexey Brodkin <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Peter Korsgaard <[email protected]>
---
 package/trousers/0002-add-pie-option.patch | 69 ++++++++++++++++++++++++++++++
 package/trousers/trousers.mk               |  8 ++++
 2 files changed, 77 insertions(+)
 create mode 100644 package/trousers/0002-add-pie-option.patch
package/trousers/0002-add-pie-option.patch new file +69 -0
new file mode 100644
index 0000000..95a2f01
@@ -0,0 +1,69 @@
+Even though ARC gcc understands "-pie" option and attempts
+to generate PIE binaries as of today PIE is not really supported
+for user-space applications. Trousers configure scripts don't provide
+any options which can disable "-pie" usage. This patch provides an 
+option to make PIE usage optional.
+
+Signed-off-by: Lada Trimasova <[email protected]>
+
+
+diff -Naur trousers.orig/configure.in trousers/configure.in
+--- trousers.orig/configure.in	2014-04-24 22:05:43.000000000 +0400
++++ trousers/configure.in	2016-01-15 13:15:44.486371425 +0300
+@@ -144,6 +144,18 @@
+ 	AC_MSG_ERROR(["gtk", "openssl" and "none" are the only supported gui options for trousers])
+ fi
+ 
++#"enable" options
++ AC_ARG_ENABLE(pie,
++	[AC_HELP_STRING([--enable-pie],
++			[Produce position independent executables @<:@default=yes@:>@])],
++	enable_pie=$enableval,
++	enable_pie="maybe")
++
++AC_ARG_ENABLE(relro,
++	[AC_HELP_STRING([--enable-relro],
++			[Enable relocations read-only support @<:@default=yes@:>@])],
++	enable_relro=$enableval,
++	enable_relro="maybe")
+ #
+ # The default port that the TCS daemon listens on
+ #
+@@ -383,6 +395,21 @@
+ 	localstatedir="/usr/local/var"
+ fi
+ 
++if test $enable_pie != "no"; then
++	PIE_CFLAGS="-fpie -pie"
++else
++	PIE_CFLAGS=""
++fi
++AC_SUBST([PIE_CFLAGS])
++
++if test $enable_relro != "no"; then
++	RELRO_CFLAGS="-Wl,-z,relro"
++else
++	RELRO_CFLAGS=""
++fi
++AC_SUBST([RELRO_CFLAGS])
++
++
+ AC_OUTPUT(dist/tcsd.conf \
+ 	  dist/fedora/trousers.spec \
+ 	  dist/trousers.spec \
+
+diff -Naur trousers.orig/src/tcsd/Makefile.am trousers/src/tcsd/Makefile.am
+--- trousers.orig/src/tcsd/Makefile.am	2014-04-24 22:05:44.000000000 +0400
++++ trousers/src/tcsd/Makefile.am	2016-01-15 13:15:44.486371425 +0300
+@@ -2,8 +2,7 @@
+ 
+ tcsd_CFLAGS=-DAPPID=\"TCSD\" -DVAR_PREFIX=\"@localstatedir@\" -DETC_PREFIX=\"@sysconfdir@\" -I${top_srcdir}/src/include -fPIE -DPIE
+ tcsd_LDADD=${top_builddir}/src/tcs/libtcs.a ${top_builddir}/src/tddl/libtddl.a -lpthread @CRYPTOLIB@
+-tcsd_LDFLAGS=-pie -Wl,-z,relro -Wl,-z,now
+-
++tcsd_LDFLAGS=$(PIE_CFLAGS) $(RELRO_CFLAGS)
+ tcsd_SOURCES=svrside.c tcsd_conf.c tcsd_threads.c platform.c
+ 
+ if TSS_BUILD_PS
+
+
package/trousers/trousers.mk +8 -0
index 5ecab70..1397ac4 100644
@@ -10,6 +10,8 @@ TROUSERS_SITE = http://downloads.sourceforge.net/project/trousers/trousers/$(TRO
 TROUSERS_LICENSE = BSD-3c
 TROUSERS_LICENSE_FILES = LICENSE
 TROUSERS_INSTALL_STAGING = YES
+# Need autoreconf because of a patch touching configure.in and Makefile.am
+TROUSERS_AUTORECONF = YES
 TROUSERS_DEPENDENCIES = openssl
 
 ifeq ($(BR2_PACKAGE_LIBICONV),y)
@@ -21,4 +23,10 @@ endif
 # workaround.
 TROUSERS_CONF_OPTS += --disable-usercheck
 
+# uClibc toolchain for ARC doesn't support PIE at the moment
+ifeq ($(BR2_arc),y)
+TROUSERS_CONF_OPTS += --disable-pie
+endif
+
+
 $(eval $(autotools-package))
Signature · 1 line
2.5.0
#1 lore raw

PATCH 1/1 trousers: disable pie option on ARC

Dear Lada Trimasova,

Quoted 11 lines · Lada Trimasova

ARC gcc understands "-pie" option and attempts to generate PIE binaries as of today PIE is not really supported for user-space applications. So we provide option which can make relro and pie usage optional and disable PIE detection if building for ARC. Also AUTORECONF option should be added because of modified configure.in and Makefile.am files.

Cc Alexey Brodkin <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Peter Korsgaard <[email protected]>

Thanks for the new patch. See some comments below.

Quoted 12 lines
---
 package/trousers/0002-add-pie-option.patch | 69 ++++++++++++++++++++++++++++++
 package/trousers/trousers.mk               |  8 ++++
 2 files changed, 77 insertions(+)
 create mode 100644 package/trousers/0002-add-pie-option.patch
package/trousers/0002-add-pie-option.patch new file +0 -0
new file mode 100644
index 0000000..95a2f01

package/trousers/0002-add-pie-option.patch line 7

Quoted 7 lines
package/trousers/0002-add-pie-option.patch new file +7 -0
@@ -0,0 +1,69 @@
+Even though ARC gcc understands "-pie" option and attempts
+to generate PIE binaries as of today PIE is not really supported
+for user-space applications. Trousers configure scripts don't provide
+any options which can disable "-pie" usage. This patch provides an 
+option to make PIE usage optional.
+
+Signed-off-by: Lada Trimasova <[email protected]>

Could you use Git to create this patch? The existing patch on the trousers package is a Git formatted patch, so it would be good if additional patches were git formatted as well. The upstream project uses Git, so it is easy to do.

package/trousers/0002-add-pie-option.patch line 22

Quoted 6 lines
package/trousers/0002-add-pie-option.patch new file +6 -0
++#"enable" options
++ AC_ARG_ENABLE(pie,
++	[AC_HELP_STRING([--enable-pie],
++			[Produce position independent executables @<:@default=yes@:>@])],
++	enable_pie=$enableval,
++	enable_pie="maybe")

This last line should probably be:

enable_pie="yes"

Because we're actually enabling pie by default.

package/trousers/0002-add-pie-option.patch line 28

Quoted 6 lines
package/trousers/0002-add-pie-option.patch new file +6 -0
++
++AC_ARG_ENABLE(relro,
++	[AC_HELP_STRING([--enable-relro],
++			[Enable relocations read-only support @<:@default=yes@:>@])],
++	enable_relro=$enableval,
++	enable_relro="maybe")

Ditto.

Quoted 22 lines
package/trousers/0002-add-pie-option.patch new file +22 -0
+ #
+ # The default port that the TCS daemon listens on
+ #
+@@ -383,6 +395,21 @@
+ 	localstatedir="/usr/local/var"
+ fi
+ 
++if test $enable_pie != "no"; then
++	PIE_CFLAGS="-fpie -pie"
++else
++	PIE_CFLAGS=""
++fi
++AC_SUBST([PIE_CFLAGS])
++
++if test $enable_relro != "no"; then
++	RELRO_CFLAGS="-Wl,-z,relro"
++else
++	RELRO_CFLAGS=""
++fi
++AC_SUBST([RELRO_CFLAGS])
++
++

package/trousers/0002-add-pie-option.patch line 60

Quoted 10 lines
package/trousers/0002-add-pie-option.patch new file +10 -0
+ AC_OUTPUT(dist/tcsd.conf \
+ 	  dist/fedora/trousers.spec \
+ 	  dist/trousers.spec \
+
+diff -Naur trousers.orig/src/tcsd/Makefile.am trousers/src/tcsd/Makefile.am
+--- trousers.orig/src/tcsd/Makefile.am	2014-04-24 22:05:44.000000000 +0400
++++ trousers/src/tcsd/Makefile.am	2016-01-15 13:15:44.486371425 +0300
+@@ -2,8 +2,7 @@
+ 
+ tcsd_CFLAGS=-DAPPID=\"TCSD\" -DVAR_PREFIX=\"@localstatedir@\" -DETC_PREFIX=\"@sysconfdir@\" -I${top_srcdir}/src/include -fPIE -DPIE

We probably want to remove -fPIE -DPIE when PIE support is disabled.

package/trousers/0002-add-pie-option.patch line 64

Quoted 4 lines
package/trousers/0002-add-pie-option.patch new file +4 -0
+ tcsd_LDADD=${top_builddir}/src/tcs/libtcs.a ${top_builddir}/src/tddl/libtddl.a -lpthread @CRYPTOLIB@
+-tcsd_LDFLAGS=-pie -Wl,-z,relro -Wl,-z,now
+-
++tcsd_LDFLAGS=$(PIE_CFLAGS) $(RELRO_CFLAGS)

This should be:

tcsd_LDFLAGS=@PIE_CFLAGS@ @RELRO_CFLAGS@

*But* as you can see, those are CFLAGS, but LDFLAGS, so the variables should really be named PIE_LDFLAGS and RELRO_LDFLAGS. However, -fpie is a CFLAGS, so it shouldn't be in PIE_LDFLAGS.

package/trousers/0002-add-pie-option.patch line 61

Quoted 6 lines
package/trousers/0002-add-pie-option.patch new file +6 -0
+# uClibc toolchain for ARC doesn't support PIE at the moment
+ifeq ($(BR2_arc),y)
+TROUSERS_CONF_OPTS += --disable-pie
+endif
+
+

One empty new line is enough.

Also, could you submit this patch upstream ?

Now that I think of it, what would be even better than adding a new config option is to simply test if the compiler is capable of linking a simple PIE binary. If the compiler is PIE capable, then trousers would use PIE, otherwise not.

Best regards,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#2 lore raw

PATCH 1/1 trousers: disable pie option on ARC

Hi Thomas, Lada,

Quoted 14 lines · Thomas Petazzoni

Dear Lada Trimasova,

Quoted 11 lines · Lada Trimasova

ARC gcc understands "-pie" option and attempts to generate PIE binaries as of today PIE is not really supported for user-space applications. So we provide option which can make relro and pie usage optional and disable PIE detection if building for ARC. Also AUTORECONF option should be added because of modified configure.in and Makefile.am files.

Cc Alexey Brodkin <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Peter Korsgaard <[email protected]>

[snip]

Quoted 4 lines

Now that I think of it, what would be even better than adding a new config option is to simply test if the compiler is capable of linking a simple PIE binary. If the compiler is PIE capable, then trousers would use PIE, otherwise not.

I'm afraid that won't work for ARC. That's because simple apps could be built but as we've seen already more complicated apps will fail to build. I.e. we need to have an ability to force disable PIE on ARC.

Still check for PIE makes perfect sense but in addition to it we'll need a way to disable that check and force disable PIE.

-Alexey

#3 lore raw

PATCH 1/1 trousers: disable pie option on ARC

Alexey,

Quoted 3 lines · Alexey Brodkin

I'm afraid that won't work for ARC. That's because simple apps could be built but as we've seen already more complicated apps will fail to build. I.e. we need to have an ability to force disable PIE on ARC.

Aah, okay.

Quoted 2 lines

Still check for PIE makes perfect sense but in addition to it we'll need a way to disable that check and force disable PIE.

Indeed. Then a compiler check could be done by the configure script, and the Buildroot packages could specify the appropriate ac_cv_<foo>=no variable.

Best regards,

Thomas

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