[PATCH] add a configure option for using RELRO by default

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] add a configure option for using RELRO by default

Romain Geissler
Hi,

Daniel Micay originally submitted a patch here
https://sourceware.org/ml/binutils/2015-01/msg00165.html to allow distro
maintainers to enable relro by default when building binutils. However
that patch never made it into the repo, since gold patch was missing. I
have just finished the work made by him by changing gold as well.

Tested without regression both with and without the --enable-default-relro
flag on a SLES 11 SP1 x64, for both ld and gold.

Ok for the trunk ?

Cheers,
Romain

2015-09-18  Romain Geissler  <[hidden email]>

        * ld/configure.ac: Add --enable-default-relro switch.
        * ld/emultempl/elf32.em: Handle ENABLE_DEFAULT_RELRO.
        * ld/testsuite/config/default.exp: Disable RELRO.
        * ld/testsuite/ld-bootstrap/bootstrap.exp: Disable RELRO.
        * ld/config.in: Regenerate.
        * ld/configure: Regenerate.
        * gold/configure.ac: Add --enable-default-relro switch.
        * gold/options.cc (General_options::finalize): Disable relro if not set
        explicitly when linking incrementally.
        * gold/options.h (General_options): Handle ENABLE_DEFAULT_RELRO.
        * gold/config.in: Regenerate.
        * gold/configure: Regenerate.
        * gold/Makefile.in: Regenerate.

---
 gold/ChangeLog                          | 10 ++++++++++
 gold/Makefile.in                        |  4 ++--
 gold/config.in                          |  3 +++
 gold/configure                          | 12 ++++++++++++
 gold/configure.ac                       |  7 +++++++
 gold/options.cc                         |  7 ++++++-
 gold/options.h                          |  7 ++++++-
 ld/ChangeLog                            |  9 +++++++++
 ld/config.in                            |  3 +++
 ld/configure                            | 16 ++++++++++++++--
 ld/configure.ac                         |  7 +++++++
 ld/emultempl/elf32.em                   |  3 +++
 ld/testsuite/config/default.exp         |  6 +++---
 ld/testsuite/ld-bootstrap/bootstrap.exp |  8 +++++++-
 14 files changed, 92 insertions(+), 10 deletions(-)

diff --git a/gold/ChangeLog b/gold/ChangeLog
index 49ce2fe..eb1791b 100644
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,13 @@
+2015-09-18  Romain Geissler  <[hidden email]>
+
+ * configure.ac: Add --enable-default-relro switch.
+ * options.cc (General_options::finalize): Disable relro if not set
+ explicitly when linking incrementally.
+ * options.h (General_options): Handle ENABLE_DEFAULT_RELRO.
+ * config.in: Regenerate.
+ * configure: Regenerate.
+ * Makefile.in: Regenerate.
+
 2015-09-07  Cary Coutant  <[hidden email]>

  PR gold/18930
diff --git a/gold/Makefile.in b/gold/Makefile.in
index 8a23524..9bc1de7 100644
--- a/gold/Makefile.in
+++ b/gold/Makefile.in
@@ -70,8 +70,8 @@ subdir = .
 DIST_COMMON = NEWS README ChangeLog $(srcdir)/Makefile.in \
  $(srcdir)/Makefile.am $(top_srcdir)/configure \
  $(am__configure_deps) $(srcdir)/config.in \
- $(srcdir)/../mkinstalldirs $(top_srcdir)/po/Make-in mremap.c \
- pread.c ffsll.c ftruncate.c yyscript.h yyscript.c \
+ $(srcdir)/../mkinstalldirs $(top_srcdir)/po/Make-in ffsll.c \
+ ftruncate.c pread.c mremap.c yyscript.h yyscript.c \
  $(srcdir)/../depcomp $(srcdir)/../ylwrap
 ACLOCAL_M4 = $(top_srcdir)/aclocal.m4
 am__aclocal_m4_deps = $(top_srcdir)/../config/depstand.m4 \
diff --git a/gold/config.in b/gold/config.in
index 88e8712..aecc9ac 100644
--- a/gold/config.in
+++ b/gold/config.in
@@ -10,6 +10,9 @@
 /* Define if building universal (internal helper macro) */
 #undef AC_APPLE_UNIVERSAL_BUILD

+/* Define to mark relocations read-only by default. */
+#undef ENABLE_DEFAULT_RELRO
+
 /* Define to 1 if translation of program messages to the user's native
    language is requested. */
 #undef ENABLE_NLS
diff --git a/gold/configure b/gold/configure
index eac9669..51b57b6e 100755
--- a/gold/configure
+++ b/gold/configure
@@ -789,6 +789,7 @@ enable_gold
 enable_threads
 enable_plugins
 enable_targets
+enable_default_relro
 with_lib_path
 enable_dependency_tracking
 enable_nls
@@ -1438,6 +1439,7 @@ Optional Features:
   --enable-threads        multi-threaded linking
   --enable-plugins        linker plugins
   --enable-targets        alternative target configurations
+  --enable-default-relro       mark relocations read-only by default
   --disable-dependency-tracking  speeds up one-time build
   --enable-dependency-tracking   do not reject slow dependency extractors
   --disable-nls           do not use Native Language Support
@@ -3382,6 +3384,16 @@ if test -n "$enable_targets"; then
   done
 fi

+# Check whether --enable-default-relro was given.
+if test "${enable_default_relro+set}" = set; then :
+  enableval=$enable_default_relro;
+
+$as_echo "#define ENABLE_DEFAULT_RELRO  " >>confdefs.h
+
+
+fi
+
+
 # See which specific instantiations we need.
 targetobjs=
 all_targets=
diff --git a/gold/configure.ac b/gold/configure.ac
index 94cae31..913f5d3 100644
--- a/gold/configure.ac
+++ b/gold/configure.ac
@@ -144,6 +144,13 @@ if test -n "$enable_targets"; then
   done
 fi

+AC_ARG_ENABLE(default-relro,
+              [  --enable-default-relro       mark relocations read-only by default],
+[
+  AC_DEFINE([ENABLE_DEFAULT_RELRO], [ ],
+            [Define to mark relocations read-only by default.])
+], [])
+
 # See which specific instantiations we need.
 targetobjs=
 all_targets=
diff --git a/gold/options.cc b/gold/options.cc
index c42623f..2c1994a 100644
--- a/gold/options.cc
+++ b/gold/options.cc
@@ -1279,7 +1279,12 @@ General_options::finalize()
       if (this->has_plugins())
  gold_fatal(_("incremental linking is not compatible with --plugin"));
       if (this->relro())
- gold_fatal(_("incremental linking is not compatible with -z relro"));
+      {
+        if (this->user_set_relro())
+          gold_fatal(_("incremental linking is not compatible with -z relro"));
+        else
+          this->set_relro(false);
+      }
       if (this->gc_sections())
  {
   gold_warning(_("ignoring --gc-sections for an incremental link"));
diff --git a/gold/options.h b/gold/options.h
index 641efee..228c527 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -1327,7 +1327,12 @@ class General_options
   DEFINE_bool(origin, options::DASH_Z, '\0', false,
       N_("Mark DSO to indicate that needs immediate $ORIGIN "
  "processing at runtime"), NULL);
-  DEFINE_bool(relro, options::DASH_Z, '\0', false,
+  DEFINE_bool(relro, options::DASH_Z, '\0',
+#ifdef ENABLE_DEFAULT_RELRO
+              true,
+#else
+              false,
+#endif
       N_("Where possible mark variables read-only after relocation"),
       N_("Don't mark variables read-only after relocation"));
   DEFINE_bool(text, options::DASH_Z, '\0', false,
diff --git a/ld/ChangeLog b/ld/ChangeLog
index e73abeb..b3a9350 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,12 @@
+2015-09-18  Romain Geissler  <[hidden email]>
+
+ * configure.ac: Add --enable-default-relro switch.
+ * emultempl/elf32.em: Handle ENABLE_DEFAULT_RELRO.
+ * testsuite/config/default.exp: Disable RELRO.
+ * testsuite/ld-bootstrap/bootstrap.exp: Disable RELRO.
+ * config.in: Regenerate.
+ * configure: Regenerate.
+
 2015-09-18  Alan Modra  <[hidden email]>

  * ld.texinfo: Document PowerPC64 --{no-,}save-restore-funcs.
diff --git a/ld/config.in b/ld/config.in
index a9a37e0..541d75e 100644
--- a/ld/config.in
+++ b/ld/config.in
@@ -7,6 +7,9 @@
 #endif
 #define __CONFIG_H__ 1

+/* Define to mark relocations read-only by default. */
+#undef ENABLE_DEFAULT_RELRO
+
 /* Define to 1 if translation of program messages to the user's native
    language is requested. */
 #undef ENABLE_NLS
diff --git a/ld/configure b/ld/configure
index 1249852..ab426ca 100755
--- a/ld/configure
+++ b/ld/configure
@@ -788,6 +788,7 @@ enable_64_bit_bfd
 with_sysroot
 enable_gold
 enable_got
+enable_default_relro
 enable_werror
 enable_build_warnings
 enable_nls
@@ -1444,6 +1445,7 @@ Optional Features:
   --enable-gold[=ARG]     build gold [ARG={default,yes,no}]
   --enable-got=<type>     GOT handling scheme (target, single, negative,
                           multigot)
+  --enable-default-relro       mark relocations read-only by default
   --enable-werror         treat compile warnings as errors
   --enable-build-warnings enable build-time compiler warnings
   --disable-nls           do not use Native Language Support
@@ -11713,7 +11715,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11716 "configure"
+#line 11718 "configure"
 #include "confdefs.h"

 #if HAVE_DLFCN_H
@@ -11819,7 +11821,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11822 "configure"
+#line 11824 "configure"
 #include "confdefs.h"

 #if HAVE_DLFCN_H
@@ -15521,6 +15523,16 @@ $as_echo "#define GOT_HANDLING_DEFAULT GOT_HANDLING_MULTIGOT" >>confdefs.h
   *)  as_fn_error "bad value ${got_handling} for --enable-got option" "$LINENO" 5 ;;
 esac

+# Check whether --enable-default-relro was given.
+if test "${enable_default_relro+set}" = set; then :
+  enableval=$enable_default_relro;
+
+$as_echo "#define ENABLE_DEFAULT_RELRO  " >>confdefs.h
+
+
+fi
+
+

 # Set the 'development' global.
 . $srcdir/../bfd/development.sh
diff --git a/ld/configure.ac b/ld/configure.ac
index e1d2c81..4715aea 100644
--- a/ld/configure.ac
+++ b/ld/configure.ac
@@ -143,6 +143,13 @@ case "${got_handling}" in
   *)  AC_MSG_ERROR(bad value ${got_handling} for --enable-got option) ;;
 esac

+AC_ARG_ENABLE(default-relro,
+              [  --enable-default-relro       mark relocations read-only by default],
+[
+  AC_DEFINE([ENABLE_DEFAULT_RELRO], [ ],
+            [Define to mark relocations read-only by default.])
+], [])
+
 AM_BINUTILS_WARNINGS

 AM_LC_MESSAGES
diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index 7fe9089..dbf6a0b 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -102,6 +102,9 @@ gld${EMULATION_NAME}_before_parse (void)
   input_flags.dynamic = ${DYNAMIC_LINK-TRUE};
   config.has_shared = `if test -n "$GENERATE_SHLIB_SCRIPT" ; then echo TRUE ; else echo FALSE ; fi`;
   config.separate_code = `if test "x${SEPARATE_CODE}" = xyes ; then echo TRUE ; else echo FALSE ; fi`;
+#ifdef ENABLE_DEFAULT_RELRO
+  link_info.relro = TRUE;
+#endif
 }

 EOF
diff --git a/ld/testsuite/config/default.exp b/ld/testsuite/config/default.exp
index 5b3e29f..5c56b5b 100644
--- a/ld/testsuite/config/default.exp
+++ b/ld/testsuite/config/default.exp
@@ -22,7 +22,7 @@
 #

 if ![info exists ld] then {
-    set ld [findfile $base_dir/ld-new $base_dir/ld-new [transform ld]]
+    set ld "[findfile $base_dir/ld-new $base_dir/ld-new [transform ld]] -znorelro"
 }

 if ![info exists as] then {
@@ -60,7 +60,7 @@ if {![file isdirectory tmpdir/ld]} then {
     catch "exec ln -s ld tmpdir/ld/collect-ld" status
     catch "exec ln -s ../../../gas/as-new tmpdir/ld/as" status
 }
-set gcc_B_opt "-B[pwd]/tmpdir/ld/"
+set gcc_B_opt "-B[pwd]/tmpdir/ld/ -Wl,-z,norelro"

 # load the linker path
 set ld_L_opt ""
@@ -272,7 +272,7 @@ if ![info exists READELFFLAGS] then {
 }

 if ![info exists LD] then {
-    set LD [findfile $base_dir/ld-new ./ld-new [transform ld]]
+    set LD "[findfile $base_dir/ld-new ./ld-new [transform ld]] -znorelro"
 }

 if ![info exists LDFLAGS] then {
diff --git a/ld/testsuite/ld-bootstrap/bootstrap.exp b/ld/testsuite/ld-bootstrap/bootstrap.exp
index 3b6eb84..2a7ac8f 100644
--- a/ld/testsuite/ld-bootstrap/bootstrap.exp
+++ b/ld/testsuite/ld-bootstrap/bootstrap.exp
@@ -78,7 +78,13 @@ foreach flags $test_flags {

     # This test can only be run if we have the ld build directory,
     # since we need the object files.
-    if {$ld != "$objdir/ld-new"} {
+    set ldexe $ld
+    set ldparm [string first " " $ld]
+    if { $ldparm > 0 } then {
+   set ldparm [expr $ldparm - 1]
+   set ldexe [string range $ld 0 $ldparm]
+    }
+    if {$ldexe != "$objdir/ld-new"} {
  untested $testname
  continue
     }
--
2.3.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] add a configure option for using RELRO by default

Romain Geissler
On Sat, 19 Sep 2015, Romain Geissler wrote:

> Hi,
>
> Daniel Micay originally submitted a patch here
> https://sourceware.org/ml/binutils/2015-01/msg00165.html to allow distro
> maintainers to enable relro by default when building binutils. However
> that patch never made it into the repo, since gold patch was missing. I
> have just finished the work made by him by changing gold as well.
>
> Tested without regression both with and without the --enable-default-relro
> flag on a SLES 11 SP1 x64, for both ld and gold.
>
> Ok for the trunk ?
>
> Cheers,
> Romain

Ping
https://sourceware.org/ml/binutils/2015-09/msg00222.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] add a configure option for using RELRO by default

H.J. Lu-30
In reply to this post by Romain Geissler
On Sat, Sep 19, 2015 at 8:11 AM, Romain Geissler
<[hidden email]> wrote:

> Hi,
>
> Daniel Micay originally submitted a patch here
> https://sourceware.org/ml/binutils/2015-01/msg00165.html to allow distro
> maintainers to enable relro by default when building binutils. However
> that patch never made it into the repo, since gold patch was missing. I
> have just finished the work made by him by changing gold as well.
>
> Tested without regression both with and without the --enable-default-relro
> flag on a SLES 11 SP1 x64, for both ld and gold.
>
> Ok for the trunk ?
>
> Cheers,
> Romain
>
> 2015-09-18  Romain Geissler  <[hidden email]>
>
>         * ld/configure.ac: Add --enable-default-relro switch.
>         * ld/emultempl/elf32.em: Handle ENABLE_DEFAULT_RELRO.
>         * ld/testsuite/config/default.exp: Disable RELRO.
>         * ld/testsuite/ld-bootstrap/bootstrap.exp: Disable RELRO.
>         * ld/config.in: Regenerate.
>         * ld/configure: Regenerate.
>         * gold/configure.ac: Add --enable-default-relro switch.
>         * gold/options.cc (General_options::finalize): Disable relro if not set
>         explicitly when linking incrementally.
>         * gold/options.h (General_options): Handle ENABLE_DEFAULT_RELRO.
>         * gold/config.in: Regenerate.
>         * gold/configure: Regenerate.
>         * gold/Makefile.in: Regenerate.
>
> ---
>  gold/ChangeLog                          | 10 ++++++++++
>  gold/Makefile.in                        |  4 ++--
>  gold/config.in                          |  3 +++
>  gold/configure                          | 12 ++++++++++++
>  gold/configure.ac                       |  7 +++++++
>  gold/options.cc                         |  7 ++++++-
>  gold/options.h                          |  7 ++++++-
>  ld/ChangeLog                            |  9 +++++++++
>  ld/config.in                            |  3 +++
>  ld/configure                            | 16 ++++++++++++++--
>  ld/configure.ac                         |  7 +++++++
>  ld/emultempl/elf32.em                   |  3 +++
>  ld/testsuite/config/default.exp         |  6 +++---
>  ld/testsuite/ld-bootstrap/bootstrap.exp |  8 +++++++-
>  14 files changed, 92 insertions(+), 10 deletions(-)
>


>  EOF
> diff --git a/ld/testsuite/config/default.exp b/ld/testsuite/config/default.exp
> index 5b3e29f..5c56b5b 100644
> --- a/ld/testsuite/config/default.exp
> +++ b/ld/testsuite/config/default.exp
> @@ -22,7 +22,7 @@
>  #
>
>  if ![info exists ld] then {
> -    set ld [findfile $base_dir/ld-new $base_dir/ld-new [transform ld]]
> +    set ld "[findfile $base_dir/ld-new $base_dir/ld-new [transform ld]] -znorelro"
>  }
>
>  if ![info exists as] then {
> @@ -60,7 +60,7 @@ if {![file isdirectory tmpdir/ld]} then {
>      catch "exec ln -s ld tmpdir/ld/collect-ld" status
>      catch "exec ln -s ../../../gas/as-new tmpdir/ld/as" status
>  }
> -set gcc_B_opt "-B[pwd]/tmpdir/ld/"
> +set gcc_B_opt "-B[pwd]/tmpdir/ld/ -Wl,-z,norelro"
>
>  # load the linker path
>  set ld_L_opt ""
> @@ -272,7 +272,7 @@ if ![info exists READELFFLAGS] then {
>  }
>
>  if ![info exists LD] then {
> -    set LD [findfile $base_dir/ld-new ./ld-new [transform ld]]
> +    set LD "[findfile $base_dir/ld-new ./ld-new [transform ld]] -znorelro"
>  }
>
>  if ![info exists LDFLAGS] then {
> diff --git a/ld/testsuite/ld-bootstrap/bootstrap.exp b/ld/testsuite/ld-bootstrap/bootstrap.exp
> index 3b6eb84..2a7ac8f 100644
> --- a/ld/testsuite/ld-bootstrap/bootstrap.exp
> +++ b/ld/testsuite/ld-bootstrap/bootstrap.exp
> @@ -78,7 +78,13 @@ foreach flags $test_flags {
>
>      # This test can only be run if we have the ld build directory,
>      # since we need the object files.
> -    if {$ld != "$objdir/ld-new"} {
> +    set ldexe $ld
> +    set ldparm [string first " " $ld]
> +    if { $ldparm > 0 } then {
> +   set ldparm [expr $ldparm - 1]
> +   set ldexe [string range $ld 0 $ldparm]
> +    }
> +    if {$ldexe != "$objdir/ld-new"} {
>         untested $testname
>         continue
>      }
> --
> 2.3.0
>

You can't do this way since not all linkers allow -z relro.


--
H.J.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] add a configure option for using RELRO by default

Mike Frysinger
In reply to this post by Romain Geissler
On 19 Sep 2015 17:11, Romain Geissler wrote:
> --- a/gold/configure.ac
> +++ b/gold/configure.ac
>
> +AC_ARG_ENABLE(default-relro,
> +              [  --enable-default-relro       mark relocations read-only by default],

please use AS_HELP_STRING instead

> +[
> +  AC_DEFINE([ENABLE_DEFAULT_RELRO], [ ],
> +            [Define to mark relocations read-only by default.])
> +], [])

this is wrong for two reasons:
(1) you must check $enableval and change behavior based on that
(2) you should define it to a value like 1

you should also just omit the 4th arg

> --- a/ld/configure.ac
> +++ b/ld/configure.ac
>
> +AC_ARG_ENABLE(default-relro,
> +              [  --enable-default-relro       mark relocations read-only by default],
> +[
> +  AC_DEFINE([ENABLE_DEFAULT_RELRO], [ ],
> +            [Define to mark relocations read-only by default.])
> +], [])

same feedback here
-mike

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] add a configure option for using RELRO by default

Andreas Schwab
In reply to this post by Romain Geissler
Romain Geissler <[hidden email]> writes:

> diff --git a/gold/options.h b/gold/options.h
> index 641efee..228c527 100644
> --- a/gold/options.h
> +++ b/gold/options.h
> @@ -1327,7 +1327,12 @@ class General_options
>    DEFINE_bool(origin, options::DASH_Z, '\0', false,
>        N_("Mark DSO to indicate that needs immediate $ORIGIN "
>   "processing at runtime"), NULL);
> -  DEFINE_bool(relro, options::DASH_Z, '\0', false,
> +  DEFINE_bool(relro, options::DASH_Z, '\0',
> +#ifdef ENABLE_DEFAULT_RELRO
> +              true,
> +#else
> +              false,
> +#endif

You cannot use preprocessing directives inside macro arguments.

Andreas.

--
Andreas Schwab, SUSE Labs, [hidden email]
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."