Patchwork elf: Delay RELRO after relocation and early initialization

login
register
mail settings
Submitter Florian Weimer
Date Oct. 8, 2019, 5:02 p.m.
Message ID <87blur9nmm.fsf@oldenburg2.str.redhat.com>
Download mbox | patch
Permalink /patch/34879/
State New
Headers show

Comments

Florian Weimer - Oct. 8, 2019, 5:02 p.m.
This runs some user code without RELRO protections in place (namely,
IFUNC resolvers for non-lazy bindings).  However, this affects only
very little code, code that should not have any dependencies on
external inputs, so the loss of security hardening is minimal.

The new libc_initialized flag ensures that we perform initialization
only once.  This is important because only on the first call with
libc.so present, RELRO has not been activated yet.  Subsequent
initialization attempts would crash if they tried to write to RELRO
variables.

In the future, this will allow us to initialze RELRO variables
in libc using a special initialization routine (similar to
_dl_var_init, which is used only on some targets today).

On targets which do use _dl_var_init, the function is called during
rtld startup for the main namespace as well, but this is harmless (and
reflects future plans).

Tested on x86_64-linux-gnu, i686-linux-gnu, aarch64-linux-gnu,
powerpc64le-linux-gnu, s390x-linux-gnu, with no regressions.  Built
using build-many-glibcs.py.

2019-10-02  Florian Weimer  <fweimer@redhat.com>

	elf: Delay RELRO after relocation and early initialization.
	* elf/dl-early-init.h: New file.
	* elf/dl-reloc.c (_dl_relocate_object): Do not apply RELRO
	protection.
	* elf/dl-open.c (dl_open_worker): Call call_libc_early_init
	instead of DL_STATIC_INIT.  Apply RELRO protection after early
	initialization.
	* elf/rtld.c (dl_main): Call call_libc_early_init after relocation
	is complete.  Delay RELRO protection until after that.
	* sysdeps/generic/ldsodefs.h (DL_STATIC_INIT): Default to true.
	(struct rtld_global): Add libc_initialized flag to namespace
	array.
	* sysdeps/unix/sysv/linux/aarch64/dl-static.c
	(_dl_unprotect_relro): Remove function.
	(_dl_static_init): Do not call _dl_unprotect_relro and
	_dl_protect_relro.  Return flag.
	* sysdeps/unix/sysv/linux/aarch64/ldsodefs.h (_dl_static_init):
	Adjust declaration.
	* sysdeps/unix/sysv/linux/ia64/dl-static.c (_dl_static_init):
	Return flag.
	* sysdeps/unix/sysv/linux/ia64/ldsodefs.h (_dl_static_init):
	Adjust declaration.
	* sysdeps/unix/sysv/linux/m68k/dl-static.c (_dl_unprotect_relro):
	Remove function.
	(_dl_static_init): Do not call _dl_unprotect_relro and
	_dl_protect_relro.  Return flag.
	* sysdeps/unix/sysv/linux/m68k/ldsodefs.h (_dl_static_init):
	Adjust declaration.
	* sysdeps/unix/sysv/linux/mips/dl-static.c (_dl_unprotect_relro):
	Remove function.
	(_dl_static_init): Do not call _dl_unprotect_relro and
	_dl_protect_relro.  Return flag.
	* sysdeps/unix/sysv/linux/mips/ldsodefs.h (_dl_static_init):
	Adjust declaration.
	* sysdeps/unix/sysv/linux/powerpc/dl-static.c
	(_dl_unprotect_relro): Remove function.
	(_dl_static_init): Do not call _dl_unprotect_relro and
	_dl_protect_relro.  Return flag.
	* sysdeps/unix/sysv/linux/powerpc/ldsodefs.h (_dl_static_init):
	Adjust declaration.
Florian Weimer - Oct. 10, 2019, 6:08 a.m.
* Florian Weimer:

> 2019-10-02  Florian Weimer  <fweimer@redhat.com>
>
> 	elf: Delay RELRO after relocation and early initialization.

Please do not review this patch.  I do not think it is necessary to go
in this direction.  I am exploring other options.

Thanks,
Florian

Patch

diff --git a/elf/dl-early-init.h b/elf/dl-early-init.h
new file mode 100644
index 0000000000..ac9fee840a
--- /dev/null
+++ b/elf/dl-early-init.h
@@ -0,0 +1,35 @@ 
+/* Early initialization of libc.so.
+   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/>.  */
+
+#ifndef EARLY_INIT_H
+#define EARLY_INIT_H
+
+#include <ldsodefs.h>
+
+static __attribute__ ((unused)) void
+call_libc_early_init (Lmid_t nsid, struct link_map *new_object)
+{
+  if (!GL(dl_ns)[nsid].libc_initialized)
+    {
+#ifndef SHARED
+      GL(dl_ns)[nsid].libc_initialized = DL_STATIC_INIT (new_object);
+#endif
+    }
+}
+
+#endif /* EARLY_INIT_H */
diff --git a/elf/dl-open.c b/elf/dl-open.c
index a9fd4cbced..7d3abb6c36 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -33,6 +33,7 @@ 
 #include <stap-probe.h>
 #include <atomic.h>
 #include <libc-internal.h>
+#include <dl-early-init.h>
 
 #include <dl-dst.h>
 #include <dl-prop.h>
@@ -502,9 +503,15 @@  TLS generation counter wrapped!  Please report this."));
   if (relocation_in_progress)
     LIBC_PROBE (reloc_complete, 3, args->nsid, r, new);
 
-#ifndef SHARED
-  DL_STATIC_INIT (new);
-#endif
+  /* Initialize libc.so early, before applying RELRO protection and
+     running ELF constructors.  */
+  call_libc_early_init (args->nsid, new);
+
+  /* Apply RELRO protection after early initialization, which may have
+     updated RELRO data.  */
+  for (unsigned int i = nmaps; i-- > 0; )
+    if (maps[i]->l_relro_size != 0)
+      _dl_protect_relro (maps[i]);
 
   /* Run the initializer functions of new objects.  */
   _dl_init (new, args->argc, args->argv, args->env);
diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
index 725a074a79..346d647acf 100644
--- a/elf/dl-reloc.c
+++ b/elf/dl-reloc.c
@@ -295,11 +295,6 @@  _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
 
       textrels = textrels->next;
     }
-
-  /* In case we can protect the data now that the relocations are
-     done, do it.  */
-  if (l->l_relro_size != 0)
-    _dl_protect_relro (l);
 }
 
 
diff --git a/elf/rtld.c b/elf/rtld.c
index 8a6e1a11b2..870a11dae0 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -43,6 +43,7 @@ 
 #include <stap-probe.h>
 #include <stackinfo.h>
 #include <not-cancel.h>
+#include <dl-early-init.h>
 
 #include <assert.h>
 
@@ -2209,8 +2210,6 @@  ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
       for (struct link_map *l = main_map; l != NULL; l = l->l_next)
 	{
 	  l->l_relocated = 1;
-	  if (l->l_relro_size)
-	    _dl_protect_relro (l);
 
 	  /* Add object to slot information data if necessasy.  */
 	  if (l->l_tls_blocksize != 0 && tls_init_tp_called)
@@ -2314,6 +2313,21 @@  ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
       rtld_timer_accum (&relocate_time, start);
     }
 
+  /* Perform early libc.so initialization before applying RELRO
+     protection.  */
+  call_libc_early_init (LM_ID_BASE, main_map);
+
+  /* Apply RELRO protection to everything that requests it.  */
+  {
+    unsigned i = main_map->l_searchlist.r_nlist;
+    while (i-- > 0)
+      {
+	struct link_map *l = main_map->l_initfini[i];
+	if (l->l_relro_size != 0)
+	  _dl_protect_relro (l);
+      }
+  }
+
   /* Do any necessary cleanups for the startup OS interface code.
      We do these now so that no calls are made after rtld re-relocation
      which might be resolved to different functions than we expect.
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index f3ba13ee68..8d3051153e 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -131,9 +131,11 @@  dl_symbol_visibility_binds_local_p (const ElfW(Sym) *sym)
 #endif
 
 /* By default we do not need special support to initialize DSOs loaded
-   by statically linked binaries.  */
+   by statically linked binaries.  A true value indicates that
+   initialization happened (and that GL(dl_ns)[nsid].libc_initialized
+   should be updated).  */
 #ifndef DL_STATIC_INIT
-# define DL_STATIC_INIT(map)
+# define DL_STATIC_INIT(map) true
 #endif
 
 /* Reloc type classes as returned by elf_machine_type_class().
@@ -323,6 +325,11 @@  struct rtld_global
     struct link_map *_ns_loaded;
     /* Number of object in the _dl_loaded list.  */
     unsigned int _ns_nloaded;
+
+    /* Non-zero if libc has been found in the namespace and has been
+       initialized.  */
+    bool libc_initialized;
+
     /* Direct pointer to the searchlist of the main object.  */
     struct r_scope_elem *_ns_main_searchlist;
     /* This is zero at program start to signal that the global scope map is
diff --git a/sysdeps/unix/sysv/linux/aarch64/dl-static.c b/sysdeps/unix/sysv/linux/aarch64/dl-static.c
index 992440b504..be2b30d0af 100644
--- a/sysdeps/unix/sysv/linux/aarch64/dl-static.c
+++ b/sysdeps/unix/sysv/linux/aarch64/dl-static.c
@@ -45,46 +45,23 @@  static void *variables[] =
   &GLRO(dl_pagesize)
 };
 
-static void
-_dl_unprotect_relro (struct link_map *l)
-{
-  ElfW(Addr) start = ((l->l_addr + l->l_relro_addr)
-		      & ~(GLRO(dl_pagesize) - 1));
-  ElfW(Addr) end = ((l->l_addr + l->l_relro_addr + l->l_relro_size)
-		    & ~(GLRO(dl_pagesize) - 1));
-
-  if (start != end)
-    __mprotect ((void *) start, end - start, PROT_READ | PROT_WRITE);
-}
-
-void
+bool
 _dl_static_init (struct link_map *l)
 {
-  struct link_map *rtld_map = l;
-  struct r_scope_elem **scope;
   const ElfW(Sym) *ref = NULL;
   lookup_t loadbase;
   void (*f) (void *[]);
-  size_t i;
 
   loadbase = _dl_lookup_symbol_x ("_dl_var_init", l, &ref, l->l_local_scope,
 				  NULL, 0, 1, NULL);
-
-  for (scope = l->l_local_scope; *scope != NULL; scope++)
-    for (i = 0; i < (*scope)->r_nlist; i++)
-      if ((*scope)->r_list[i] == loadbase)
-	{
-	  rtld_map = (*scope)->r_list[i];
-	  break;
-	}
-
   if (ref != NULL)
     {
       f = (void (*) (void *[])) DL_SYMBOL_ADDRESS (loadbase, ref);
-      _dl_unprotect_relro (rtld_map);
       f (variables);
-      _dl_protect_relro (rtld_map);
+      return true;
     }
+  else
+    return false;
 }
 
 #endif
diff --git a/sysdeps/unix/sysv/linux/aarch64/ldsodefs.h b/sysdeps/unix/sysv/linux/aarch64/ldsodefs.h
index 42c4960549..a047f9fab8 100644
--- a/sysdeps/unix/sysv/linux/aarch64/ldsodefs.h
+++ b/sysdeps/unix/sysv/linux/aarch64/ldsodefs.h
@@ -26,7 +26,7 @@ 
 
 /* We need special support to initialize DSO loaded for statically linked
    binaries.  */
-extern void _dl_static_init (struct link_map *map);
+extern bool _dl_static_init (struct link_map *map);
 #undef DL_STATIC_INIT
 #define DL_STATIC_INIT(map) _dl_static_init (map)
 
diff --git a/sysdeps/unix/sysv/linux/ia64/dl-static.c b/sysdeps/unix/sysv/linux/ia64/dl-static.c
index 3a5fb3a958..68f6357c7f 100644
--- a/sysdeps/unix/sysv/linux/ia64/dl-static.c
+++ b/sysdeps/unix/sysv/linux/ia64/dl-static.c
@@ -50,7 +50,7 @@  static void *variables[] =
   &GLRO(dl_clktck)
 };
 
-void
+bool
 _dl_static_init (struct link_map *map)
 {
   const ElfW(Sym) *ref = NULL;
@@ -63,7 +63,10 @@  _dl_static_init (struct link_map *map)
     {
       f = (void (*) (void *[])) DL_SYMBOL_ADDRESS (loadbase, ref);
       f (variables);
+      return true;
     }
+  else
+    return false;
 }
 
 #endif
diff --git a/sysdeps/unix/sysv/linux/ia64/ldsodefs.h b/sysdeps/unix/sysv/linux/ia64/ldsodefs.h
index d8e87e63ca..4ce34bdec7 100644
--- a/sysdeps/unix/sysv/linux/ia64/ldsodefs.h
+++ b/sysdeps/unix/sysv/linux/ia64/ldsodefs.h
@@ -25,7 +25,7 @@ 
 
 /* We need special support to initialize DSO loaded for statically linked
    binaries.  */
-extern void _dl_static_init (struct link_map *map);
+extern bool _dl_static_init (struct link_map *map);
 #undef DL_STATIC_INIT
 #define DL_STATIC_INIT(map) _dl_static_init (map)
 
diff --git a/sysdeps/unix/sysv/linux/m68k/dl-static.c b/sysdeps/unix/sysv/linux/m68k/dl-static.c
index 8ab3abda68..e851934834 100644
--- a/sysdeps/unix/sysv/linux/m68k/dl-static.c
+++ b/sysdeps/unix/sysv/linux/m68k/dl-static.c
@@ -45,46 +45,23 @@  static void *variables[] =
   &GLRO(dl_pagesize)
 };
 
-static void
-_dl_unprotect_relro (struct link_map *l)
-{
-  ElfW(Addr) start = ((l->l_addr + l->l_relro_addr)
-		      & ~(GLRO(dl_pagesize) - 1));
-  ElfW(Addr) end = ((l->l_addr + l->l_relro_addr + l->l_relro_size)
-		    & ~(GLRO(dl_pagesize) - 1));
-
-  if (start != end)
-    __mprotect ((void *) start, end - start, PROT_READ | PROT_WRITE);
-}
-
-void
+bool
 _dl_static_init (struct link_map *l)
 {
-  struct link_map *rtld_map = l;
-  struct r_scope_elem **scope;
   const ElfW(Sym) *ref = NULL;
   lookup_t loadbase;
   void (*f) (void *[]);
-  size_t i;
 
   loadbase = _dl_lookup_symbol_x ("_dl_var_init", l, &ref, l->l_local_scope,
 				  NULL, 0, 1, NULL);
-
-  for (scope = l->l_local_scope; *scope != NULL; scope++)
-    for (i = 0; i < (*scope)->r_nlist; i++)
-      if ((*scope)->r_list[i] == loadbase)
-	{
-	  rtld_map = (*scope)->r_list[i];
-	  break;
-	}
-
   if (ref != NULL)
     {
       f = (void (*) (void *[])) DL_SYMBOL_ADDRESS (loadbase, ref);
-      _dl_unprotect_relro (rtld_map);
       f (variables);
-      _dl_protect_relro (rtld_map);
+      return true;
     }
+  else
+    return false;
 }
 
 #endif
diff --git a/sysdeps/unix/sysv/linux/m68k/ldsodefs.h b/sysdeps/unix/sysv/linux/m68k/ldsodefs.h
index bf0d53386e..bc4e06c2f4 100644
--- a/sysdeps/unix/sysv/linux/m68k/ldsodefs.h
+++ b/sysdeps/unix/sysv/linux/m68k/ldsodefs.h
@@ -25,7 +25,7 @@ 
 
 /* We need special support to initialize DSO loaded for statically linked
    binaries.  */
-extern void _dl_static_init (struct link_map *map);
+extern bool _dl_static_init (struct link_map *map);
 #undef DL_STATIC_INIT
 #define DL_STATIC_INIT(map) _dl_static_init (map)
 
diff --git a/sysdeps/unix/sysv/linux/mips/dl-static.c b/sysdeps/unix/sysv/linux/mips/dl-static.c
index 4a606e42a2..db98986069 100644
--- a/sysdeps/unix/sysv/linux/mips/dl-static.c
+++ b/sysdeps/unix/sysv/linux/mips/dl-static.c
@@ -45,46 +45,23 @@  static void *variables[] =
   &GLRO(dl_pagesize)
 };
 
-static void
-_dl_unprotect_relro (struct link_map *l)
-{
-  ElfW(Addr) start = ((l->l_addr + l->l_relro_addr)
-		      & ~(GLRO(dl_pagesize) - 1));
-  ElfW(Addr) end = ((l->l_addr + l->l_relro_addr + l->l_relro_size)
-		    & ~(GLRO(dl_pagesize) - 1));
-
-  if (start != end)
-    __mprotect ((void *) start, end - start, PROT_READ | PROT_WRITE);
-}
-
-void
+bool
 _dl_static_init (struct link_map *l)
 {
-  struct link_map *rtld_map = l;
-  struct r_scope_elem **scope;
   const ElfW(Sym) *ref = NULL;
   lookup_t loadbase;
   void (*f) (void *[]);
-  size_t i;
 
   loadbase = _dl_lookup_symbol_x ("_dl_var_init", l, &ref, l->l_local_scope,
 				  NULL, 0, 1, NULL);
-
-  for (scope = l->l_local_scope; *scope != NULL; scope++)
-    for (i = 0; i < (*scope)->r_nlist; i++)
-      if ((*scope)->r_list[i] == loadbase)
-	{
-	  rtld_map = (*scope)->r_list[i];
-	  break;
-	}
-
   if (ref != NULL)
     {
       f = (void (*) (void *[])) DL_SYMBOL_ADDRESS (loadbase, ref);
-      _dl_unprotect_relro (rtld_map);
       f (variables);
-      _dl_protect_relro (rtld_map);
+      return true;
     }
+  else
+    return false;
 }
 
 #endif
diff --git a/sysdeps/unix/sysv/linux/mips/ldsodefs.h b/sysdeps/unix/sysv/linux/mips/ldsodefs.h
index 2d560d17cb..6d0130f69e 100644
--- a/sysdeps/unix/sysv/linux/mips/ldsodefs.h
+++ b/sysdeps/unix/sysv/linux/mips/ldsodefs.h
@@ -25,7 +25,7 @@ 
 
 /* We need special support to initialize DSO loaded for statically linked
    binaries.  */
-extern void _dl_static_init (struct link_map *map);
+extern bool _dl_static_init (struct link_map *map);
 #undef DL_STATIC_INIT
 #define DL_STATIC_INIT(map) _dl_static_init (map)
 
diff --git a/sysdeps/unix/sysv/linux/powerpc/dl-static.c b/sysdeps/unix/sysv/linux/powerpc/dl-static.c
index ff0cdf46fa..70319ee32f 100644
--- a/sysdeps/unix/sysv/linux/powerpc/dl-static.c
+++ b/sysdeps/unix/sysv/linux/powerpc/dl-static.c
@@ -45,46 +45,23 @@  static void *variables[] =
   &GLRO(dl_pagesize)
 };
 
-static void
-_dl_unprotect_relro (struct link_map *l)
-{
-  ElfW(Addr) start = ((l->l_addr + l->l_relro_addr)
-		      & ~(GLRO(dl_pagesize) - 1));
-  ElfW(Addr) end = ((l->l_addr + l->l_relro_addr + l->l_relro_size)
-		    & ~(GLRO(dl_pagesize) - 1));
-
-  if (start != end)
-    __mprotect ((void *) start, end - start, PROT_READ | PROT_WRITE);
-}
-
-void
+bool
 _dl_static_init (struct link_map *l)
 {
-  struct link_map *rtld_map = l;
-  struct r_scope_elem **scope;
   const ElfW(Sym) *ref = NULL;
   lookup_t loadbase;
   void (*f) (void *[]);
-  size_t i;
 
   loadbase = _dl_lookup_symbol_x ("_dl_var_init", l, &ref, l->l_local_scope,
 				  NULL, 0, 1, NULL);
-
-  for (scope = l->l_local_scope; *scope != NULL; scope++)
-    for (i = 0; i < (*scope)->r_nlist; i++)
-      if ((*scope)->r_list[i] == loadbase)
-	{
-	  rtld_map = (*scope)->r_list[i];
-	  break;
-	}
-
   if (ref != NULL)
     {
       f = (void (*) (void *[])) DL_SYMBOL_ADDRESS (loadbase, ref);
-      _dl_unprotect_relro (rtld_map);
       f (variables);
-      _dl_protect_relro (rtld_map);
+      return true;
     }
+  else
+    return false;
 }
 
 #endif
diff --git a/sysdeps/unix/sysv/linux/powerpc/ldsodefs.h b/sysdeps/unix/sysv/linux/powerpc/ldsodefs.h
index c9783fdf8b..4a809f160e 100644
--- a/sysdeps/unix/sysv/linux/powerpc/ldsodefs.h
+++ b/sysdeps/unix/sysv/linux/powerpc/ldsodefs.h
@@ -26,7 +26,7 @@ 
 
 /* We need special support to initialize DSO loaded for statically linked
    binaries.  */
-extern void _dl_static_init (struct link_map *map);
+extern bool _dl_static_init (struct link_map *map);
 #undef DL_STATIC_INIT
 #define DL_STATIC_INIT(map) _dl_static_init (map)