diff mbox series

[v2,06/12] elf: Enable relro for static build

Message ID 20191212181614.31782-6-adhemerval.zanella@linaro.org
State New
Headers show
Series [01/12] linux: Fix vDSO macros build with time64 interfaces | expand

Commit Message

Adhemerval Zanella Dec. 12, 2019, 6:16 p.m. UTC 
Changes from previous version:

  - Added tests for partial and full relro.

--

The code is similar to the one at elf/dl-reloc.c, where it checks for
the l_relro_size from the link_map (obtained from PT_GNU_RELRO header
from program headers) and calls_dl_protected_relro.

Checked on x86_64-linux-gnu, i686-linux-gnu, powerpc64le-linux-gnu,
aarch64-linux-gnu, s390x-linux-gnu, and sparc64-linux-gnu.  I also
check with --enable-static pie on x86_64-linux-gnu, i686-linux-gnu,
and aarch64-linux-gnu which seems the only architectures where
static PIE is actually working (as per 9d7a3741c9e, on
arm-linux-gnueabihf, powerpc64{le}-linux-gnu, and s390x-linux-gnu
I am seeing runtime issues not related to my patch).
---
 elf/Makefile                     | 11 +++++++--
 elf/dl-support.c                 | 18 +++++++++++---
 elf/tst-data-relro-lazy-static.c |  1 +
 elf/tst-data-relro-lazy.c        |  1 +
 elf/tst-data-relro-now-static.c  |  1 +
 elf/tst-data-relro-now.c         |  1 +
 elf/tst-data-relro.c             | 42 ++++++++++++++++++++++++++++++++
 7 files changed, 69 insertions(+), 6 deletions(-)
 create mode 100644 elf/tst-data-relro-lazy-static.c
 create mode 100644 elf/tst-data-relro-lazy.c
 create mode 100644 elf/tst-data-relro-now-static.c
 create mode 100644 elf/tst-data-relro-now.c
 create mode 100644 elf/tst-data-relro.c

-- 
2.17.1

Comments

Florian Weimer Dec. 13, 2019, 12:08 p.m. UTC | #1 
* Adhemerval Zanella:

> +static volatile int val __attribute__ ((section (".data.rel.ro")));


I would prefer something that needs an actual run-time relocation
because I doubt that all targets use .data.rel.ro.

> +int do_test (void)

> +{

> +  struct support_capture_subprocess result

> +    = support_capture_subprocess (callback, NULL);

> +  support_capture_subprocess_check (&result, "tst-relro", -SIGSEGV,

> +				    sc_allow_stdout);

> +  return 0;

> +}


My test <https://sourceware.org/ml/libc-alpha/2019-10/msg00130.html>
used a new support/ helper for that.

Thanks,
Florian
Adhemerval Zanella Dec. 13, 2019, 12:14 p.m. UTC | #2 
On 13/12/2019 09:08, Florian Weimer wrote:
> * Adhemerval Zanella:

> 

>> +static volatile int val __attribute__ ((section (".data.rel.ro")));

> 

> I would prefer something that needs an actual run-time relocation

> because I doubt that all targets use .data.rel.ro.

> 

>> +int do_test (void)

>> +{

>> +  struct support_capture_subprocess result

>> +    = support_capture_subprocess (callback, NULL);

>> +  support_capture_subprocess_check (&result, "tst-relro", -SIGSEGV,

>> +				    sc_allow_stdout);

>> +  return 0;

>> +}

> 

> My test <https://sourceware.org/ml/libc-alpha/2019-10/msg00130.html>

> used a new support/ helper for that.


Alright, we can use these tests instead. I can update my patch to remove
them.
diff mbox series

Patch

diff --git a/elf/Makefile b/elf/Makefile
index b2b3be203f..45b5ad4ea6 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -154,7 +154,8 @@  endif
 tests-static-normal := tst-leaks1-static tst-array1-static tst-array5-static \
 	       tst-dl-iter-static \
 	       tst-tlsalign-static tst-tlsalign-extern-static \
-	       tst-linkall-static tst-env-setuid tst-env-setuid-tunables
+	       tst-linkall-static tst-env-setuid tst-env-setuid-tunables \
+	       tst-data-relro-lazy-static tst-data-relro-now-static
 tests-static-internal := tst-tls1-static tst-tls2-static \
 	       tst-ptrguard1-static tst-stackguard1-static \
 	       tst-tls1-static-non-pie tst-libc_dlvsym-static
@@ -205,7 +206,8 @@  tests-internal += loadtest unload unload2 circleload1 \
 	 neededtest neededtest2 neededtest3 neededtest4 \
 	 tst-tls3 tst-tls6 tst-tls7 tst-tls8 tst-dlmopen2 \
 	 tst-ptrguard1 tst-stackguard1 tst-libc_dlvsym \
-	 tst-create_format1
+	 tst-create_format1 \
+	 tst-data-relro-now tst-data-relro-lazy
 tests-container += tst-pldd tst-dlopen-tlsmodid-container \
   tst-dlopen-self-container
 test-srcs = tst-pathopt
@@ -1627,3 +1629,8 @@  $(objpfx)tst-dlopenfailmod1.so: \
   $(shared-thread-library) $(objpfx)tst-dlopenfaillinkmod.so
 LDFLAGS-tst-dlopenfaillinkmod.so = -Wl,-soname,tst-dlopenfail-missingmod.so
 $(objpfx)tst-dlopenfailmod2.so: $(shared-thread-library)
+
+LDFLAGS-tst-data-relro-lazy += -Wl,-z,relro -Wl,-z,lazy
+LDFLAGS-tst-data-relro-lazy-static += -Wl,-z,relro -Wl,-z,lazy
+LDFLAGS-tst-data-relro-now += -Wl,-z,relro -Wl,-z,now
+LDFLAGS-tst-data-relro-now-static += -Wl,-z,relro -Wl,-z,now
diff --git a/elf/dl-support.c b/elf/dl-support.c
index 5526d5ee6e..b2b1b12f6f 100644
--- a/elf/dl-support.c
+++ b/elf/dl-support.c
@@ -367,14 +367,24 @@  _dl_non_dynamic_init (void)
   if (_dl_platform != NULL)
     _dl_platformlen = strlen (_dl_platform);
 
-  /* Scan for a program header telling us the stack is nonexecutable.  */
   if (_dl_phdr != NULL)
-    for (uint_fast16_t i = 0; i < _dl_phnum; ++i)
-      if (_dl_phdr[i].p_type == PT_GNU_STACK)
+    for (const ElfW(Phdr) *ph = _dl_phdr; ph < &_dl_phdr[_dl_phnum]; ++ph)
+      switch (ph->p_type)
 	{
-	  _dl_stack_flags = _dl_phdr[i].p_flags;
+	/* Check if the stack is nonexecutable.  */
+	case PT_GNU_STACK:
+	  _dl_stack_flags = ph->p_flags;
+	  break;
+
+	case PT_GNU_RELRO:
+	  _dl_main_map.l_relro_addr = ph->p_vaddr;
+	  _dl_main_map.l_relro_size = ph->p_memsz;
 	  break;
 	}
+
+  /* Setup relro on the binary itself.  */
+  if (_dl_main_map.l_relro_size != 0)
+    _dl_protect_relro (&_dl_main_map);
 }
 
 #ifdef DL_SYSINFO_IMPLEMENTATION
diff --git a/elf/tst-data-relro-lazy-static.c b/elf/tst-data-relro-lazy-static.c
new file mode 100644
index 0000000000..364a206506
--- /dev/null
+++ b/elf/tst-data-relro-lazy-static.c
@@ -0,0 +1 @@ 
+#include <elf/tst-data-relro.c>
diff --git a/elf/tst-data-relro-lazy.c b/elf/tst-data-relro-lazy.c
new file mode 100644
index 0000000000..364a206506
--- /dev/null
+++ b/elf/tst-data-relro-lazy.c
@@ -0,0 +1 @@ 
+#include <elf/tst-data-relro.c>
diff --git a/elf/tst-data-relro-now-static.c b/elf/tst-data-relro-now-static.c
new file mode 100644
index 0000000000..364a206506
--- /dev/null
+++ b/elf/tst-data-relro-now-static.c
@@ -0,0 +1 @@ 
+#include <elf/tst-data-relro.c>
diff --git a/elf/tst-data-relro-now.c b/elf/tst-data-relro-now.c
new file mode 100644
index 0000000000..364a206506
--- /dev/null
+++ b/elf/tst-data-relro-now.c
@@ -0,0 +1 @@ 
+#include <elf/tst-data-relro.c>
diff --git a/elf/tst-data-relro.c b/elf/tst-data-relro.c
new file mode 100644
index 0000000000..bd63b24b3f
--- /dev/null
+++ b/elf/tst-data-relro.c
@@ -0,0 +1,42 @@ 
+/* Test if variables places on relro section are not writable.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <signal.h>
+
+#include <support/capture_subprocess.h>
+#include <support/check.h>
+
+static volatile int val __attribute__ ((section (".data.rel.ro")));
+
+static void
+callback (void *closure)
+{
+  /* It should trigger an invalid write.  */
+  val = 1;
+}
+
+int do_test (void)
+{
+  struct support_capture_subprocess result
+    = support_capture_subprocess (callback, NULL);
+  support_capture_subprocess_check (&result, "tst-relro", -SIGSEGV,
+				    sc_allow_stdout);
+  return 0;
+}
+
+#include <support/test-driver.c>