diff mbox series

[v4,1/4] rtld: Use generic argv adjustment in ld.so [BZ #23293]

Message ID d6afb31c729a2f9e98bbb9243e60bcc6f582714e.1651643916.git.szabolcs.nagy@arm.com
State Superseded
Headers
Series Args adjustment with ./ld.so exe [BZ #23293] |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Szabolcs Nagy May 4, 2022, 6:26 a.m. UTC 
  When an executable is invoked as

  ./ld.so [ld.so-args] ./exe [exe-args]

then the argv is adujusted in ld.so before calling the entry point of
the executable so ld.so args are not visible to it.  On most targets
this requires moving argv, env and auxv on the stack to ensure correct
stack alignment at the entry point.  This had several issues:

- The code for this adjustment on the stack is written in asm as part
  of the target specific ld.so _start code which is hard to maintain.

- The adjustment is done after _dl_start returns, where it's too late
  to update GLRO(dl_auxv), as it is already readonly, so it points to
  memory that was clobbered by the adjustment. This is bug 23293.

- _environ is also wrong in ld.so after the adjustment, but it is
  likely not used after _dl_start returns so this is not user visible.

- _dl_argv was updated, but for this it was moved out of relro, which
  changes security properties across targets unnecessarily.

This patch introduces a generic _dl_start_args_adjust function that
handles the argument adjustments after ld.so processed its own args
and before relro protection is applied.  The initial sp at ld.so entry
is passed down to dl_main so it can do the adjustment.

The same algorithm is used on all targets, _dl_skip_args is now 0, so
existing target specific adjustment code is no longer used.  The bug
affects aarch64, alpha, arc, arm, csky, ia64, nios2, s390-32 and sparc,
other targets don't need the change in principle, but it does not hurt
and makes the behaviour more consistent.

Follow up patches can remove _dl_skip_args and DL_ARGV_NOT_RELRO.

---
v4:
- New code is unconditionally used on all targets.
- Hide auxv adjustments behind HAVE_AUX_VECTOR.
- DL_NEED_START_ARGS_ADJUST macro is removed.
- _dl_skip_args is no longer unused.
- start_argptr is passed down to dl_main instead of using a global.
- moved aarch64 DL_ARGV_NOT_RELRO removal to separate patch.
v2:
- use p != NULL, and a_type != AT_NULL
- remove the confusing paragraph from the commit message.
---
 elf/rtld.c                          | 83 +++++++++++++++++++++++------
 sysdeps/generic/ldsodefs.h          |  3 +-
 sysdeps/unix/sysv/linux/dl-sysdep.c |  5 +-
 3 files changed, 73 insertions(+), 18 deletions(-)

Comments

Florian Weimer May 4, 2022, 9:28 a.m. UTC | #1 
* Szabolcs Nagy via Libc-alpha:

> When an executable is invoked as
>
>   ./ld.so [ld.so-args] ./exe [exe-args]
>
> then the argv is adujusted in ld.so before calling the entry point of
> the executable so ld.so args are not visible to it.  On most targets
> this requires moving argv, env and auxv on the stack to ensure correct
> stack alignment at the entry point.  This had several issues:
>
> - The code for this adjustment on the stack is written in asm as part
>   of the target specific ld.so _start code which is hard to maintain.
>
> - The adjustment is done after _dl_start returns, where it's too late
>   to update GLRO(dl_auxv), as it is already readonly, so it points to
>   memory that was clobbered by the adjustment. This is bug 23293.
>
> - _environ is also wrong in ld.so after the adjustment, but it is
>   likely not used after _dl_start returns so this is not user visible.
>
> - _dl_argv was updated, but for this it was moved out of relro, which
>   changes security properties across targets unnecessarily.
>
> This patch introduces a generic _dl_start_args_adjust function that
> handles the argument adjustments after ld.so processed its own args
> and before relro protection is applied.  The initial sp at ld.so entry
> is passed down to dl_main so it can do the adjustment.
>
> The same algorithm is used on all targets, _dl_skip_args is now 0, so
> existing target specific adjustment code is no longer used.  The bug
> affects aarch64, alpha, arc, arm, csky, ia64, nios2, s390-32 and sparc,
> other targets don't need the change in principle, but it does not hurt
> and makes the behaviour more consistent.
>
> Follow up patches can remove _dl_skip_args and DL_ARGV_NOT_RELRO.

I believe this causes a Hurd build failure:

In file included from ../sysdeps/mach/hurd/x86/dl-sysdep.c:21:
../sysdeps/mach/hurd/dl-sysdep.c:72:1: error: conflicting types for ‘_dl_sysdep_start’; have ‘Elf32_Addr(void **, void (*)(const Elf32_Phdr *, Elf32_Word,  Elf32_Addr *, Elf32_auxv_t *))’ {aka ‘unsigned int(void **, void (*)(const Elf32_Phdr *, unsigned int,  unsigned int *, Elf32_auxv_t *))’}
   72 | _dl_sysdep_start (void **start_argptr,
      | ^~~~~~~~~~~~~~~~
In file included from ../sysdeps/x86/ldsodefs.h:65,
                 from ../sysdeps/gnu/ldsodefs.h:46,
                 from ../sysdeps/x86/dl-procinfo.h:21,
                 from /home/bmg/build/glibcs/i686-gnu/glibc/dl-tunable-list.h:6,
                 from ../elf/dl-tunables.h:52,
                 from ../sysdeps/x86/cpu-tunables.c:24,
                 from ../sysdeps/mach/hurd/x86/dl-sysdep.c:20:
../sysdeps/generic/ldsodefs.h:1156:19: note: previous declaration of ‘_dl_sysdep_start’ with type ‘Elf32_Addr(void **, void (*)(const Elf32_Phdr *, Elf32_Word,  Elf32_Addr *, Elf32_auxv_t *, void **))’ {aka ‘unsigned int(void **, void (*)(const Elf32_Phdr *, unsigned int,  unsigned int *, Elf32_auxv_t *, void **))’}
 1156 | extern ElfW(Addr) _dl_sysdep_start (void **start_argptr,
      |                   ^~~~~~~~~~~~~~~~


Rest looks okay.

Thanks,
Florian
diff mbox series

Patch

diff --git a/elf/rtld.c b/elf/rtld.c
index 3b2e05bf4c..d959dab0d0 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -397,7 +397,8 @@  extern struct rtld_global_ro _rtld_local_ro
 
 
 static void dl_main (const ElfW(Phdr) *phdr, ElfW(Word) phnum,
-		     ElfW(Addr) *user_entry, ElfW(auxv_t) *auxv);
+		     ElfW(Addr) *user_entry, ElfW(auxv_t) *auxv,
+		     void **start_argptr);
 
 /* These two variables cannot be moved into .data.rel.ro.  */
 static struct libname_list _dl_rtld_libname;
@@ -1306,11 +1307,60 @@  rtld_setup_main_map (struct link_map *main_map)
   return has_interp;
 }
 
+static void
+_dl_start_args_adjust (void **start_argptr, int skip_args)
+{
+  void **sp = start_argptr;
+  void **p;
+  long argc;
+  char **argv;
+
+  if (skip_args == 0)
+    return;
+
+  /* Adjust argc on stack.  */
+  argc = (long) sp[0] - skip_args;
+  sp[0] = (void *) argc;
+
+  argv = (char **) (sp + 1); /* Necessary aliasing violation.  */
+  p = sp + skip_args;
+  /* Shuffle argv down.  */
+  do
+    *++sp = *++p;
+  while (*p != NULL);
+
+  /* Shuffle envp down.  */
+  do
+    *++sp = *++p;
+  while (*p != NULL);
+
+  /* Update globals in rtld.  */
+  _dl_argv = argv;
+  _environ = argv + argc + 1;
+#ifdef HAVE_AUX_VECTOR
+  GLRO(dl_auxv) = (ElfW(auxv_t) *) (sp + 1); /* Aliasing violation.  */
+
+  /* Shuffle auxv down. */
+  void *a, *b; /* Use a pair of pointers for an auxv entry.  */
+  unsigned long a_type;
+  do
+    {
+      a_type = ((ElfW(auxv_t) *) (p + 1))->a_type;
+      a = *++p;
+      b = *++p;
+      *++sp = a;
+      *++sp = b;
+    }
+  while (a_type != AT_NULL);
+#endif
+}
+
 static void
 dl_main (const ElfW(Phdr) *phdr,
 	 ElfW(Word) phnum,
 	 ElfW(Addr) *user_entry,
-	 ElfW(auxv_t) *auxv)
+	 ElfW(auxv_t) *auxv,
+	 void **start_argptr)
 {
   struct link_map *main_map;
   size_t file_size;
@@ -1359,6 +1409,7 @@  dl_main (const ElfW(Phdr) *phdr,
       rtld_is_main = true;
 
       char *argv0 = NULL;
+      int skip_args = 0;
 
       /* Note the place where the dynamic linker actually came from.  */
       GL(dl_rtld_map).l_name = rtld_progname;
@@ -1373,7 +1424,7 @@  dl_main (const ElfW(Phdr) *phdr,
 		GLRO(dl_lazy) = -1;
 	      }
 
-	    ++_dl_skip_args;
+	    ++skip_args;
 	    --_dl_argc;
 	    ++_dl_argv;
 	  }
@@ -1382,14 +1433,14 @@  dl_main (const ElfW(Phdr) *phdr,
 	    if (state.mode != rtld_mode_help)
 	      state.mode = rtld_mode_verify;
 
-	    ++_dl_skip_args;
+	    ++skip_args;
 	    --_dl_argc;
 	    ++_dl_argv;
 	  }
 	else if (! strcmp (_dl_argv[1], "--inhibit-cache"))
 	  {
 	    GLRO(dl_inhibit_cache) = 1;
-	    ++_dl_skip_args;
+	    ++skip_args;
 	    --_dl_argc;
 	    ++_dl_argv;
 	  }
@@ -1399,7 +1450,7 @@  dl_main (const ElfW(Phdr) *phdr,
 	    state.library_path = _dl_argv[2];
 	    state.library_path_source = "--library-path";
 
-	    _dl_skip_args += 2;
+	    skip_args += 2;
 	    _dl_argc -= 2;
 	    _dl_argv += 2;
 	  }
@@ -1408,7 +1459,7 @@  dl_main (const ElfW(Phdr) *phdr,
 	  {
 	    GLRO(dl_inhibit_rpath) = _dl_argv[2];
 
-	    _dl_skip_args += 2;
+	    skip_args += 2;
 	    _dl_argc -= 2;
 	    _dl_argv += 2;
 	  }
@@ -1416,14 +1467,14 @@  dl_main (const ElfW(Phdr) *phdr,
 	  {
 	    audit_list_add_string (&state.audit_list, _dl_argv[2]);
 
-	    _dl_skip_args += 2;
+	    skip_args += 2;
 	    _dl_argc -= 2;
 	    _dl_argv += 2;
 	  }
 	else if (! strcmp (_dl_argv[1], "--preload") && _dl_argc > 2)
 	  {
 	    state.preloadarg = _dl_argv[2];
-	    _dl_skip_args += 2;
+	    skip_args += 2;
 	    _dl_argc -= 2;
 	    _dl_argv += 2;
 	  }
@@ -1431,7 +1482,7 @@  dl_main (const ElfW(Phdr) *phdr,
 	  {
 	    argv0 = _dl_argv[2];
 
-	    _dl_skip_args += 2;
+	    skip_args += 2;
 	    _dl_argc -= 2;
 	    _dl_argv += 2;
 	  }
@@ -1439,7 +1490,7 @@  dl_main (const ElfW(Phdr) *phdr,
 		 && _dl_argc > 2)
 	  {
 	    state.glibc_hwcaps_prepend = _dl_argv[2];
-	    _dl_skip_args += 2;
+	    skip_args += 2;
 	    _dl_argc -= 2;
 	    _dl_argv += 2;
 	  }
@@ -1447,7 +1498,7 @@  dl_main (const ElfW(Phdr) *phdr,
 		 && _dl_argc > 2)
 	  {
 	    state.glibc_hwcaps_mask = _dl_argv[2];
-	    _dl_skip_args += 2;
+	    skip_args += 2;
 	    _dl_argc -= 2;
 	    _dl_argv += 2;
 	  }
@@ -1456,7 +1507,7 @@  dl_main (const ElfW(Phdr) *phdr,
 	  {
 	    state.mode = rtld_mode_list_tunables;
 
-	    ++_dl_skip_args;
+	    ++skip_args;
 	    --_dl_argc;
 	    ++_dl_argv;
 	  }
@@ -1465,7 +1516,7 @@  dl_main (const ElfW(Phdr) *phdr,
 	  {
 	    state.mode = rtld_mode_list_diagnostics;
 
-	    ++_dl_skip_args;
+	    ++skip_args;
 	    --_dl_argc;
 	    ++_dl_argv;
 	  }
@@ -1511,7 +1562,7 @@  dl_main (const ElfW(Phdr) *phdr,
 	    _dl_usage (ld_so_name, NULL);
 	}
 
-      ++_dl_skip_args;
+      ++skip_args;
       --_dl_argc;
       ++_dl_argv;
 
@@ -1610,6 +1661,8 @@  dl_main (const ElfW(Phdr) *phdr,
       /* Set the argv[0] string now that we've processed the executable.  */
       if (argv0 != NULL)
         _dl_argv[0] = argv0;
+      /* Adjust arguments for the application entry point.  */
+      _dl_start_args_adjust (start_argptr, skip_args);
     }
   else
     {
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index 4a5e698db2..31de149f23 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -1165,7 +1165,8 @@  extern ElfW(Addr) _dl_sysdep_start (void **start_argptr,
 				    void (*dl_main) (const ElfW(Phdr) *phdr,
 						     ElfW(Word) phnum,
 						     ElfW(Addr) *user_entry,
-						     ElfW(auxv_t) *auxv))
+						     ElfW(auxv_t) *auxv,
+						     void **start_argptr))
      attribute_hidden;
 
 extern void _dl_sysdep_start_cleanup (void) attribute_hidden;
diff --git a/sysdeps/unix/sysv/linux/dl-sysdep.c b/sysdeps/unix/sysv/linux/dl-sysdep.c
index a67c454673..8c4a7e72b9 100644
--- a/sysdeps/unix/sysv/linux/dl-sysdep.c
+++ b/sysdeps/unix/sysv/linux/dl-sysdep.c
@@ -98,7 +98,8 @@  _dl_sysdep_parse_arguments (void **start_argptr,
 ElfW(Addr)
 _dl_sysdep_start (void **start_argptr,
 		  void (*dl_main) (const ElfW(Phdr) *phdr, ElfW(Word) phnum,
-				   ElfW(Addr) *user_entry, ElfW(auxv_t) *auxv))
+				   ElfW(Addr) *user_entry, ElfW(auxv_t) *auxv,
+				   void **start_argptr))
 {
   __libc_stack_end = DL_STACK_END (start_argptr);
 
@@ -138,7 +139,7 @@  _dl_sysdep_start (void **start_argptr,
     __libc_check_standard_fds ();
 
   (*dl_main) (dl_main_args.phdr, dl_main_args.phnum,
-              &dl_main_args.user_entry, GLRO(dl_auxv));
+	      &dl_main_args.user_entry, GLRO(dl_auxv), start_argptr);
   return dl_main_args.user_entry;
 }