c5d972
commit ad43cac44a6860eaefcadadfb2acb349921e96bf
c5d972
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
c5d972
Date:   Fri Jun 15 16:14:58 2018 +0100
c5d972
c5d972
    rtld: Use generic argv adjustment in ld.so [BZ #23293]
c5d972
    
c5d972
    When an executable is invoked as
c5d972
    
c5d972
      ./ld.so [ld.so-args] ./exe [exe-args]
c5d972
    
c5d972
    then the argv is adujusted in ld.so before calling the entry point of
c5d972
    the executable so ld.so args are not visible to it.  On most targets
c5d972
    this requires moving argv, env and auxv on the stack to ensure correct
c5d972
    stack alignment at the entry point.  This had several issues:
c5d972
    
c5d972
    - The code for this adjustment on the stack is written in asm as part
c5d972
      of the target specific ld.so _start code which is hard to maintain.
c5d972
    
c5d972
    - The adjustment is done after _dl_start returns, where it's too late
c5d972
      to update GLRO(dl_auxv), as it is already readonly, so it points to
c5d972
      memory that was clobbered by the adjustment. This is bug 23293.
c5d972
    
c5d972
    - _environ is also wrong in ld.so after the adjustment, but it is
c5d972
      likely not used after _dl_start returns so this is not user visible.
c5d972
    
c5d972
    - _dl_argv was updated, but for this it was moved out of relro, which
c5d972
      changes security properties across targets unnecessarily.
c5d972
    
c5d972
    This patch introduces a generic _dl_start_args_adjust function that
c5d972
    handles the argument adjustments after ld.so processed its own args
c5d972
    and before relro protection is applied.
c5d972
    
c5d972
    The same algorithm is used on all targets, _dl_skip_args is now 0, so
c5d972
    existing target specific adjustment code is no longer used.  The bug
c5d972
    affects aarch64, alpha, arc, arm, csky, ia64, nios2, s390-32 and sparc,
c5d972
    other targets don't need the change in principle, only for consistency.
c5d972
    
c5d972
    The GNU Hurd start code relied on _dl_skip_args after dl_main returned,
c5d972
    now it checks directly if args were adjusted and fixes the Hurd startup
c5d972
    data accordingly.
c5d972
    
c5d972
    Follow up patches can remove _dl_skip_args and DL_ARGV_NOT_RELRO.
c5d972
    
c5d972
    Tested on aarch64-linux-gnu and cross tested on i686-gnu.
c5d972
    
c5d972
    Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
c5d972
c5d972
diff --git a/elf/rtld.c b/elf/rtld.c
c5d972
index aee5ca357f66121e..22cceeab40319582 100644
c5d972
--- a/elf/rtld.c
c5d972
+++ b/elf/rtld.c
c5d972
@@ -1127,6 +1127,62 @@ rtld_chain_load (struct link_map *main_map, char *argv0)
c5d972
 		   rtld_soname, pathname, errcode);
c5d972
 }
c5d972
 
c5d972
+/* Adjusts the contents of the stack and related globals for the user
c5d972
+   entry point.  The ld.so processed skip_args arguments and bumped
c5d972
+   _dl_argv and _dl_argc accordingly.  Those arguments are removed from
c5d972
+   argv here.  */
c5d972
+static void
c5d972
+_dl_start_args_adjust (int skip_args)
c5d972
+{
c5d972
+  void **sp = (void **) (_dl_argv - skip_args - 1);
c5d972
+  void **p = sp + skip_args;
c5d972
+
c5d972
+  if (skip_args == 0)
c5d972
+    return;
c5d972
+
c5d972
+  /* Sanity check.  */
c5d972
+  intptr_t argc = (intptr_t) sp[0] - skip_args;
c5d972
+  assert (argc == _dl_argc);
c5d972
+
c5d972
+  /* Adjust argc on stack.  */
c5d972
+  sp[0] = (void *) (intptr_t) _dl_argc;
c5d972
+
c5d972
+  /* Update globals in rtld.  */
c5d972
+  _dl_argv -= skip_args;
c5d972
+  _environ -= skip_args;
c5d972
+
c5d972
+  /* Shuffle argv down.  */
c5d972
+  do
c5d972
+    *++sp = *++p;
c5d972
+  while (*p != NULL);
c5d972
+
c5d972
+  assert (_environ == (char **) (sp + 1));
c5d972
+
c5d972
+  /* Shuffle envp down.  */
c5d972
+  do
c5d972
+    *++sp = *++p;
c5d972
+  while (*p != NULL);
c5d972
+
c5d972
+#ifdef HAVE_AUX_VECTOR
c5d972
+  void **auxv = (void **) GLRO(dl_auxv) - skip_args;
c5d972
+  GLRO(dl_auxv) = (ElfW(auxv_t) *) auxv; /* Aliasing violation.  */
c5d972
+  assert (auxv == sp + 1);
c5d972
+
c5d972
+  /* Shuffle auxv down. */
c5d972
+  ElfW(auxv_t) ax;
c5d972
+  char *oldp = (char *) (p + 1);
c5d972
+  char *newp = (char *) (sp + 1);
c5d972
+  do
c5d972
+    {
c5d972
+      memcpy (&ax, oldp, sizeof (ax));
c5d972
+      memcpy (newp, &ax, sizeof (ax));
c5d972
+      oldp += sizeof (ax);
c5d972
+      newp += sizeof (ax);
c5d972
+    }
c5d972
+  while (ax.a_type != AT_NULL);
c5d972
+#endif
c5d972
+}
c5d972
+
c5d972
 static void
c5d972
 dl_main (const ElfW(Phdr) *phdr,
c5d972
 	 ElfW(Word) phnum,
c5d972
@@ -1185,6 +1241,7 @@ dl_main (const ElfW(Phdr) *phdr,
c5d972
       rtld_is_main = true;
c5d972
 
c5d972
       char *argv0 = NULL;
c5d972
+      char **orig_argv = _dl_argv;
c5d972
 
c5d972
       /* Note the place where the dynamic linker actually came from.  */
c5d972
       GL(dl_rtld_map).l_name = rtld_progname;
c5d972
@@ -1199,7 +1256,6 @@ dl_main (const ElfW(Phdr) *phdr,
c5d972
 		GLRO(dl_lazy) = -1;
c5d972
 	      }
c5d972
 
c5d972
-	    ++_dl_skip_args;
c5d972
 	    --_dl_argc;
c5d972
 	    ++_dl_argv;
c5d972
 	  }
c5d972
@@ -1208,14 +1264,12 @@ dl_main (const ElfW(Phdr) *phdr,
c5d972
 	    if (state.mode != rtld_mode_help)
c5d972
 	      state.mode = rtld_mode_verify;
c5d972
 
c5d972
-	    ++_dl_skip_args;
c5d972
 	    --_dl_argc;
c5d972
 	    ++_dl_argv;
c5d972
 	  }
c5d972
 	else if (! strcmp (_dl_argv[1], "--inhibit-cache"))
c5d972
 	  {
c5d972
 	    GLRO(dl_inhibit_cache) = 1;
c5d972
-	    ++_dl_skip_args;
c5d972
 	    --_dl_argc;
c5d972
 	    ++_dl_argv;
c5d972
 	  }
c5d972
@@ -1225,7 +1279,6 @@ dl_main (const ElfW(Phdr) *phdr,
c5d972
 	    state.library_path = _dl_argv[2];
c5d972
 	    state.library_path_source = "--library-path";
c5d972
 
c5d972
-	    _dl_skip_args += 2;
c5d972
 	    _dl_argc -= 2;
c5d972
 	    _dl_argv += 2;
c5d972
 	  }
c5d972
@@ -1234,7 +1287,6 @@ dl_main (const ElfW(Phdr) *phdr,
c5d972
 	  {
c5d972
 	    GLRO(dl_inhibit_rpath) = _dl_argv[2];
c5d972
 
c5d972
-	    _dl_skip_args += 2;
c5d972
 	    _dl_argc -= 2;
c5d972
 	    _dl_argv += 2;
c5d972
 	  }
c5d972
@@ -1242,14 +1294,12 @@ dl_main (const ElfW(Phdr) *phdr,
c5d972
 	  {
c5d972
 	    audit_list_add_string (&state.audit_list, _dl_argv[2]);
c5d972
 
c5d972
-	    _dl_skip_args += 2;
c5d972
 	    _dl_argc -= 2;
c5d972
 	    _dl_argv += 2;
c5d972
 	  }
c5d972
 	else if (! strcmp (_dl_argv[1], "--preload") && _dl_argc > 2)
c5d972
 	  {
c5d972
 	    state.preloadarg = _dl_argv[2];
c5d972
-	    _dl_skip_args += 2;
c5d972
 	    _dl_argc -= 2;
c5d972
 	    _dl_argv += 2;
c5d972
 	  }
c5d972
@@ -1257,7 +1307,6 @@ dl_main (const ElfW(Phdr) *phdr,
c5d972
 	  {
c5d972
 	    argv0 = _dl_argv[2];
c5d972
 
c5d972
-	    _dl_skip_args += 2;
c5d972
 	    _dl_argc -= 2;
c5d972
 	    _dl_argv += 2;
c5d972
 	  }
c5d972
@@ -1265,7 +1314,6 @@ dl_main (const ElfW(Phdr) *phdr,
c5d972
 		 && _dl_argc > 2)
c5d972
 	  {
c5d972
 	    state.glibc_hwcaps_prepend = _dl_argv[2];
c5d972
-	    _dl_skip_args += 2;
c5d972
 	    _dl_argc -= 2;
c5d972
 	    _dl_argv += 2;
c5d972
 	  }
c5d972
@@ -1273,7 +1321,6 @@ dl_main (const ElfW(Phdr) *phdr,
c5d972
 		 && _dl_argc > 2)
c5d972
 	  {
c5d972
 	    state.glibc_hwcaps_mask = _dl_argv[2];
c5d972
-	    _dl_skip_args += 2;
c5d972
 	    _dl_argc -= 2;
c5d972
 	    _dl_argv += 2;
c5d972
 	  }
c5d972
@@ -1282,7 +1329,6 @@ dl_main (const ElfW(Phdr) *phdr,
c5d972
 	  {
c5d972
 	    state.mode = rtld_mode_list_tunables;
c5d972
 
c5d972
-	    ++_dl_skip_args;
c5d972
 	    --_dl_argc;
c5d972
 	    ++_dl_argv;
c5d972
 	  }
c5d972
@@ -1291,7 +1337,6 @@ dl_main (const ElfW(Phdr) *phdr,
c5d972
 	  {
c5d972
 	    state.mode = rtld_mode_list_diagnostics;
c5d972
 
c5d972
-	    ++_dl_skip_args;
c5d972
 	    --_dl_argc;
c5d972
 	    ++_dl_argv;
c5d972
 	  }
c5d972
@@ -1337,7 +1382,6 @@ dl_main (const ElfW(Phdr) *phdr,
c5d972
 	    _dl_usage (ld_so_name, NULL);
c5d972
 	}
c5d972
 
c5d972
-      ++_dl_skip_args;
c5d972
       --_dl_argc;
c5d972
       ++_dl_argv;
c5d972
 
c5d972
@@ -1433,6 +1477,9 @@ dl_main (const ElfW(Phdr) *phdr,
c5d972
       /* Set the argv[0] string now that we've processed the executable.  */
c5d972
       if (argv0 != NULL)
c5d972
         _dl_argv[0] = argv0;
c5d972
+
c5d972
+      /* Adjust arguments for the application entry point.  */
c5d972
+      _dl_start_args_adjust (_dl_argv - orig_argv);
c5d972
     }
c5d972
   else
c5d972
     {
c5d972
diff --git a/sysdeps/mach/hurd/dl-sysdep.c b/sysdeps/mach/hurd/dl-sysdep.c
c5d972
index 7bd1d70c96c229e0..8aab46bf6396c8d4 100644
c5d972
--- a/sysdeps/mach/hurd/dl-sysdep.c
c5d972
+++ b/sysdeps/mach/hurd/dl-sysdep.c
c5d972
@@ -107,6 +107,7 @@ _dl_sysdep_start (void **start_argptr,
c5d972
 {
c5d972
   void go (intptr_t *argdata)
c5d972
     {
c5d972
+      char *orig_argv0;
c5d972
       char **p;
c5d972
 
c5d972
       /* Cache the information in various global variables.  */
c5d972
@@ -115,6 +116,8 @@ _dl_sysdep_start (void **start_argptr,
c5d972
       _environ = &_dl_argv[_dl_argc + 1];
c5d972
       for (p = _environ; *p++;); /* Skip environ pointers and terminator.  */
c5d972
 
c5d972
+      orig_argv0 = _dl_argv[0];
c5d972
+
c5d972
       if ((void *) p == _dl_argv[0])
c5d972
 	{
c5d972
 	  static struct hurd_startup_data nodata;
c5d972
@@ -189,30 +192,23 @@ unfmh();			/* XXX */
c5d972
 
c5d972
       /* The call above might screw a few things up.
c5d972
 
c5d972
-	 First of all, if _dl_skip_args is nonzero, we are ignoring
c5d972
-	 the first few arguments.  However, if we have no Hurd startup
c5d972
-	 data, it is the magical convention that ARGV[0] == P.  The
c5d972
+	 P is the location after the terminating NULL of the list of
c5d972
+	 environment variables.  It has to point to the Hurd startup
c5d972
+	 data or if that's missing then P == ARGV[0] must hold. The
c5d972
 	 startup code in init-first.c will get confused if this is not
c5d972
 	 the case, so we must rearrange things to make it so.  We'll
c5d972
-	 overwrite the origional ARGV[0] at P with ARGV[_dl_skip_args].
c5d972
+	 recompute P and move the Hurd data or the new ARGV[0] there.
c5d972
 
c5d972
-	 Secondly, if we need to be secure, it removes some dangerous
c5d972
-	 environment variables.  If we have no Hurd startup date this
c5d972
-	 changes P (since that's the location after the terminating
c5d972
-	 NULL in the list of environment variables).  We do the same
c5d972
-	 thing as in the first case but make sure we recalculate P.
c5d972
-	 If we do have Hurd startup data, we have to move the data
c5d972
-	 such that it starts just after the terminating NULL in the
c5d972
-	 environment list.
c5d972
+	 Note: directly invoked ld.so can move arguments and env vars.
c5d972
 
c5d972
 	 We use memmove, since the locations might overlap.  */
c5d972
-      if (__libc_enable_secure || _dl_skip_args)
c5d972
-	{
c5d972
-	  char **newp;
c5d972
 
c5d972
-	  for (newp = _environ; *newp++;);
c5d972
+      char **newp;
c5d972
+      for (newp = _environ; *newp++;);
c5d972
 
c5d972
-	  if (_dl_argv[-_dl_skip_args] == (char *) p)
c5d972
+      if (newp != p || _dl_argv[0] != orig_argv0)
c5d972
+	{
c5d972
+	  if (orig_argv0 == (char *) p)
c5d972
 	    {
c5d972
 	      if ((char *) newp != _dl_argv[0])
c5d972
 		{