public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Args adjustment with ./ld.so exe [BZ #23293]
@ 2022-04-12 12:55 Szabolcs Nagy
  2022-04-12 12:55 ` [PATCH 1/3] Remove _dl_skip_args_internal declaration Szabolcs Nagy
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Szabolcs Nagy @ 2022-04-12 12:55 UTC (permalink / raw)
  To: libc-alpha

Trying to make the ld.so start code more generic and less error prone.

The introduced DL_NEED_START_ARGS_ADJUST may not be the nicest
solution, but it avoids affecting code on other targets for now.

Szabolcs Nagy (3):
  Remove _dl_skip_args_internal declaration
  aarch64: Use generic argv adjustment in ld.so [BZ #23293]
  aarch64: Move ld.so _start to separate file

 elf/rtld.c                          | 56 +++++++++++++++++++++
 sysdeps/aarch64/Makefile            |  1 +
 sysdeps/aarch64/dl-machine.h        | 77 +----------------------------
 sysdeps/aarch64/dl-start.S          | 53 ++++++++++++++++++++
 sysdeps/aarch64/dl-sysdep.h         |  2 +-
 sysdeps/generic/ldsodefs.h          |  6 +--
 sysdeps/unix/sysv/linux/dl-sysdep.c | 10 ++++
 7 files changed, 125 insertions(+), 80 deletions(-)
 create mode 100644 sysdeps/aarch64/dl-start.S

-- 
2.25.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] Remove _dl_skip_args_internal declaration
  2022-04-12 12:55 [PATCH 0/3] Args adjustment with ./ld.so exe [BZ #23293] Szabolcs Nagy
@ 2022-04-12 12:55 ` Szabolcs Nagy
  2022-04-12 13:40   ` Florian Weimer
  2022-04-12 13:51   ` Andreas Schwab
  2022-04-12 12:55 ` [PATCH 2/3] aarch64: Use generic argv adjustment in ld.so [BZ #23293] Szabolcs Nagy
  2022-04-12 12:55 ` [PATCH 3/3] aarch64: Move ld.so _start to separate file Szabolcs Nagy
  2 siblings, 2 replies; 10+ messages in thread
From: Szabolcs Nagy @ 2022-04-12 12:55 UTC (permalink / raw)
  To: libc-alpha

It does not seem to be used.
---
 sysdeps/generic/ldsodefs.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index 44750461a9..29f005499b 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -781,11 +781,6 @@ extern char **_dl_argv
 rtld_hidden_proto (_dl_argv)
 #if IS_IN (rtld)
 extern unsigned int _dl_skip_args attribute_hidden
-# ifndef DL_ARGV_NOT_RELRO
-     attribute_relro
-# endif
-     ;
-extern unsigned int _dl_skip_args_internal attribute_hidden
 # ifndef DL_ARGV_NOT_RELRO
      attribute_relro
 # endif
-- 
2.25.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/3] aarch64: Use generic argv adjustment in ld.so [BZ #23293]
  2022-04-12 12:55 [PATCH 0/3] Args adjustment with ./ld.so exe [BZ #23293] Szabolcs Nagy
  2022-04-12 12:55 ` [PATCH 1/3] Remove _dl_skip_args_internal declaration Szabolcs Nagy
@ 2022-04-12 12:55 ` Szabolcs Nagy
  2022-04-12 14:12   ` Florian Weimer
  2022-04-12 12:55 ` [PATCH 3/3] aarch64: Move ld.so _start to separate file Szabolcs Nagy
  2 siblings, 1 reply; 10+ messages in thread
From: Szabolcs Nagy @ 2022-04-12 12:55 UTC (permalink / raw)
  To: 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.  It sets _dl_skip_args to 0 so
the existing adjustment in asm is not invoked.  Each target has to
opt-in to use this new adjustment since some targets don't need it.
Once all targets are updated, _dl_argv declaration can be simplified.

A new _dl_start_argptr was introduced because the original sp is not
passed to dl_main which now has to do the adjustments.

A seemingly simpler approach is to deal with unaligned sp in crt1.o,
i.e. align sp in the entry point of the exe before __libc_start_main
and pass unaligned sp from ld.so after updating argc (like it is done
on x86), however this is not a backward compatible solution, new ld.so
would not work with old exe on targets where old crt1 does not align.
---
 elf/rtld.c                          | 56 +++++++++++++++++++++++++++++
 sysdeps/aarch64/dl-sysdep.h         |  2 +-
 sysdeps/generic/ldsodefs.h          |  3 ++
 sysdeps/unix/sysv/linux/dl-sysdep.c | 10 ++++++
 4 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/elf/rtld.c b/elf/rtld.c
index 19e328f89e..c08f7ed9e2 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -1311,6 +1311,58 @@ rtld_setup_main_map (struct link_map *main_map)
   return has_interp;
 }
 
+#ifdef DL_NEED_START_ARGS_ADJUST
+static void
+_dl_start_args_adjust (void)
+{
+  void **sp;
+  void **p;
+  long argc;
+  char **argv;
+  ElfW(auxv_t) *auxv;
+
+  if (_dl_skip_args == 0)
+    return;
+
+  sp = _dl_start_argptr;
+
+  /* Adjust argc on stack.  */
+  argc = (long) sp[0] - _dl_skip_args;
+  sp[0] = (void *) argc;
+
+  argv = (char **) (sp + 1); /* Necessary aliasing violation.  */
+  p = sp + _dl_skip_args;
+  /* Shuffle argv down.  */
+  do
+    *++sp = *++p;
+  while (*p);
+
+  /* Shuffle envp down.  */
+  do
+    *++sp = *++p;
+  while (*p);
+
+  auxv = (ElfW(auxv_t) *) (sp + 1); /* Necessary aliasing violation.  */
+  /* Shuffle auxv down. */
+  void *a, *b; /* Use a pair of pointers for an auxv entry.  */
+  do
+    {
+      a = *++p;
+      b = *++p;
+      *++sp = a;
+      *++sp = b;
+    }
+  while (a);
+
+  /* Update globals in rtld.  */
+  _dl_argv = argv;
+  _environ = argv + argc + 1;
+  GLRO(dl_auxv) = auxv;
+  /* No longer need to skip args.  */
+  _dl_skip_args = 0;
+}
+#endif
+
 static void
 dl_main (const ElfW(Phdr) *phdr,
 	 ElfW(Word) phnum,
@@ -1615,6 +1667,10 @@ 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;
+#ifdef DL_NEED_START_ARGS_ADJUST
+      /* Adjust arguments for the application entry point.  */
+      _dl_start_args_adjust ();
+#endif
     }
   else
     {
diff --git a/sysdeps/aarch64/dl-sysdep.h b/sysdeps/aarch64/dl-sysdep.h
index 667786671c..1df4c2c528 100644
--- a/sysdeps/aarch64/dl-sysdep.h
+++ b/sysdeps/aarch64/dl-sysdep.h
@@ -20,6 +20,6 @@
 
 /* _dl_argv cannot be attribute_relro, because _dl_start_user
    might write into it after _dl_start returns.  */
-#define DL_ARGV_NOT_RELRO 1
+#define DL_NEED_START_ARGS_ADJUST 1
 
 #define DL_EXTERN_PROTECTED_DATA
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index 29f005499b..f322d36570 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -785,6 +785,9 @@ extern unsigned int _dl_skip_args attribute_hidden
      attribute_relro
 # endif
      ;
+# ifdef DL_NEED_START_ARGS_ADJUST
+extern void **_dl_start_argptr attribute_hidden attribute_relro;
+# endif
 #endif
 #define rtld_progname _dl_argv[0]
 
diff --git a/sysdeps/unix/sysv/linux/dl-sysdep.c b/sysdeps/unix/sysv/linux/dl-sysdep.c
index c90f109b11..66f003e2a3 100644
--- a/sysdeps/unix/sysv/linux/dl-sysdep.c
+++ b/sysdeps/unix/sysv/linux/dl-sysdep.c
@@ -58,6 +58,12 @@ void *__libc_stack_end attribute_relro = NULL;
 rtld_hidden_data_def(__libc_stack_end)
 void *_dl_random attribute_relro = NULL;
 
+#ifdef DL_NEED_START_ARGS_ADJUST
+/* Original sp at ELF entry, used when rtld is executed explicitly
+   and needs to adjust arg components for the actual application.  */
+void **_dl_start_argptr attribute_hidden attribute_relro = NULL;
+#endif
+
 #ifndef DL_STACK_END
 # define DL_STACK_END(cookie) ((void *) (cookie))
 #endif
@@ -114,6 +120,10 @@ _dl_sysdep_start (void **start_argptr,
 
   __brk (0);			/* Initialize the break.  */
 
+#ifdef DL_NEED_START_ARGS_ADJUST
+  _dl_start_argptr = start_argptr;
+#endif
+
 #ifdef DL_PLATFORM_INIT
   DL_PLATFORM_INIT;
 #endif
-- 
2.25.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/3] aarch64: Move ld.so _start to separate file
  2022-04-12 12:55 [PATCH 0/3] Args adjustment with ./ld.so exe [BZ #23293] Szabolcs Nagy
  2022-04-12 12:55 ` [PATCH 1/3] Remove _dl_skip_args_internal declaration Szabolcs Nagy
  2022-04-12 12:55 ` [PATCH 2/3] aarch64: Use generic argv adjustment in ld.so [BZ #23293] Szabolcs Nagy
@ 2022-04-12 12:55 ` Szabolcs Nagy
  2022-04-12 14:11   ` Florian Weimer
  2 siblings, 1 reply; 10+ messages in thread
From: Szabolcs Nagy @ 2022-04-12 12:55 UTC (permalink / raw)
  To: libc-alpha

A separate asm file is easier to maintain than a macro that expands to
inline asm.

The RTLD_START macro is only needed now becase _dl_start is local in
rtld.c, but _start has to call it, if _dl_start was made hidden then
it could be empty.
---
 sysdeps/aarch64/Makefile     |  1 +
 sysdeps/aarch64/dl-machine.h | 77 +-----------------------------------
 sysdeps/aarch64/dl-start.S   | 53 +++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 75 deletions(-)
 create mode 100644 sysdeps/aarch64/dl-start.S

diff --git a/sysdeps/aarch64/Makefile b/sysdeps/aarch64/Makefile
index 7183895d04..17fb1c5b72 100644
--- a/sysdeps/aarch64/Makefile
+++ b/sysdeps/aarch64/Makefile
@@ -33,6 +33,7 @@ tst-audit27-ENV = LD_AUDIT=$(objpfx)tst-auditmod27.so
 endif
 
 ifeq ($(subdir),elf)
+sysdep-rtld-routines += dl-start
 sysdep-dl-routines += tlsdesc dl-tlsdesc
 gen-as-const-headers += dl-link.sym
 
diff --git a/sysdeps/aarch64/dl-machine.h b/sysdeps/aarch64/dl-machine.h
index b40050a981..fe120bb507 100644
--- a/sysdeps/aarch64/dl-machine.h
+++ b/sysdeps/aarch64/dl-machine.h
@@ -105,81 +105,8 @@ elf_machine_runtime_setup (struct link_map *l, struct r_scope_elem *scope[],
   return lazy;
 }
 
-/* Initial entry point for the dynamic linker. The C function
-   _dl_start is the real entry point, its return value is the user
-   program's entry point */
-#ifdef __LP64__
-# define RTLD_START RTLD_START_1 ("x", "3", "sp")
-#else
-# define RTLD_START RTLD_START_1 ("w", "2", "wsp")
-#endif
-
-
-#define RTLD_START_1(PTR, PTR_SIZE_LOG, PTR_SP) asm ("\
-.text									\n\
-.globl _start								\n\
-.type _start, %function							\n\
-.globl _dl_start_user							\n\
-.type _dl_start_user, %function						\n\
-_start:									\n\
-	// bti c							\n\
-	hint	34							\n\
-	mov	" PTR "0, " PTR_SP "					\n\
-	bl	_dl_start						\n\
-	// returns user entry point in x0				\n\
-	mov	x21, x0							\n\
-_dl_start_user:								\n\
-	// get the original arg count					\n\
-	ldr	" PTR "1, [sp]						\n\
-	// get the argv address						\n\
-	add	" PTR "2, " PTR_SP ", #(1<<"  PTR_SIZE_LOG ")		\n\
-	// get _dl_skip_args to see if we were				\n\
-	// invoked as an executable					\n\
-	adrp	x4, _dl_skip_args					\n\
-        ldr	w4, [x4, #:lo12:_dl_skip_args]				\n\
-	// do we need to adjust argc/argv				\n\
-        cmp	w4, 0							\n\
-	beq	.L_done_stack_adjust					\n\
-	// subtract _dl_skip_args from original arg count		\n\
-	sub	" PTR "1, " PTR "1, " PTR "4				\n\
-	// store adjusted argc back to stack				\n\
-	str	" PTR "1, [sp]						\n\
-	// find the first unskipped argument				\n\
-	mov	" PTR "3, " PTR "2					\n\
-	add	" PTR "4, " PTR "2, " PTR "4, lsl #" PTR_SIZE_LOG "	\n\
-	// shuffle argv down						\n\
-1:	ldr	" PTR "5, [x4], #(1<<"  PTR_SIZE_LOG ")			\n\
-	str	" PTR "5, [x3], #(1<<"  PTR_SIZE_LOG ")			\n\
-	cmp	" PTR "5, #0						\n\
-	bne	1b							\n\
-	// shuffle envp down						\n\
-1:	ldr	" PTR "5, [x4], #(1<<"  PTR_SIZE_LOG ")			\n\
-	str	" PTR "5, [x3], #(1<<"  PTR_SIZE_LOG ")			\n\
-	cmp	" PTR "5, #0						\n\
-	bne	1b							\n\
-	// shuffle auxv down						\n\
-1:	ldp	" PTR "0, " PTR "5, [x4, #(2<<"  PTR_SIZE_LOG ")]!	\n\
-	stp	" PTR "0, " PTR "5, [x3], #(2<<"  PTR_SIZE_LOG ")	\n\
-	cmp	" PTR "0, #0						\n\
-	bne	1b							\n\
-	// Update _dl_argv						\n\
-	adrp	x3, __GI__dl_argv					\n\
-	str	" PTR "2, [x3, #:lo12:__GI__dl_argv]			\n\
-.L_done_stack_adjust:							\n\
-	// compute envp							\n\
-	add	" PTR "3, " PTR "2, " PTR "1, lsl #" PTR_SIZE_LOG "	\n\
-	add	" PTR "3, " PTR "3, #(1<<"  PTR_SIZE_LOG ")		\n\
-	adrp	x16, _rtld_local					\n\
-        add	" PTR "16, " PTR "16, #:lo12:_rtld_local		\n\
-        ldr	" PTR "0, [x16]						\n\
-	bl	_dl_init						\n\
-	// load the finalizer function					\n\
-	adrp	x0, _dl_fini						\n\
-	add	" PTR "0, " PTR "0, #:lo12:_dl_fini			\n\
-	// jump to the user_s entry point				\n\
-	mov     x16, x21						\n\
-	br      x16							\n\
-");
+/* In elf/rtld.c _dl_start should be global so dl-start.S can reference it.  */
+#define RTLD_START asm (".globl _dl_start");
 
 #define elf_machine_type_class(type)					\
   ((((type) == AARCH64_R(JUMP_SLOT)					\
diff --git a/sysdeps/aarch64/dl-start.S b/sysdeps/aarch64/dl-start.S
new file mode 100644
index 0000000000..a3a57bd5a1
--- /dev/null
+++ b/sysdeps/aarch64/dl-start.S
@@ -0,0 +1,53 @@
+/* ld.so _start code.
+   Copyright (C) 2022 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 <sysdep.h>
+
+ENTRY (_start)
+	/* Create an initial frame with 0 LR and FP */
+	cfi_undefined (x30)
+	mov	x29, #0
+	mov	x30, #0
+
+	mov	x0, sp
+	PTR_ARG (0)
+	bl	_dl_start
+	/* Returns user entry point in x0.  */
+	mov	PTR_REG (21), PTR_REG (0)
+.globl _dl_start_user
+.type _dl_start_user, %function
+_dl_start_user:
+	/* Get argc.  */
+	ldr	PTR_REG (1), [sp]
+	/* Get argv.  */
+	add	x2, sp, PTR_SIZE
+	/* Compute envp.  */
+	add	PTR_REG (3), PTR_REG (2), PTR_REG (1), lsl PTR_LOG_SIZE
+	add	PTR_REG (3), PTR_REG (3), PTR_SIZE
+	adrp	x16, _rtld_local
+	add	PTR_REG (16), PTR_REG (16), :lo12:_rtld_local
+	ldr	PTR_REG (0), [x16]
+	bl	_dl_init
+	/* Load the finalizer function.  */
+	adrp	x0, _dl_fini
+	add	PTR_REG (0), PTR_REG (0), :lo12:_dl_fini
+	/* Jump to the user's entry point.  */
+	mov     x16, x21
+	br      x16
+END (_start)
-- 
2.25.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] Remove _dl_skip_args_internal declaration
  2022-04-12 12:55 ` [PATCH 1/3] Remove _dl_skip_args_internal declaration Szabolcs Nagy
@ 2022-04-12 13:40   ` Florian Weimer
  2022-04-12 13:51   ` Andreas Schwab
  1 sibling, 0 replies; 10+ messages in thread
From: Florian Weimer @ 2022-04-12 13:40 UTC (permalink / raw)
  To: Szabolcs Nagy via Libc-alpha; +Cc: Szabolcs Nagy

* Szabolcs Nagy via Libc-alpha:

> It does not seem to be used.
> ---
>  sysdeps/generic/ldsodefs.h | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index 44750461a9..29f005499b 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -781,11 +781,6 @@ extern char **_dl_argv
>  rtld_hidden_proto (_dl_argv)
>  #if IS_IN (rtld)
>  extern unsigned int _dl_skip_args attribute_hidden
> -# ifndef DL_ARGV_NOT_RELRO
> -     attribute_relro
> -# endif
> -     ;
> -extern unsigned int _dl_skip_args_internal attribute_hidden
>  # ifndef DL_ARGV_NOT_RELRO
>       attribute_relro
>  # endif

Looks okay.

Reviewed-by: Florian Weimer <fweimer@redhat.com>

Thanks,
Florian


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] Remove _dl_skip_args_internal declaration
  2022-04-12 12:55 ` [PATCH 1/3] Remove _dl_skip_args_internal declaration Szabolcs Nagy
  2022-04-12 13:40   ` Florian Weimer
@ 2022-04-12 13:51   ` Andreas Schwab
  1 sibling, 0 replies; 10+ messages in thread
From: Andreas Schwab @ 2022-04-12 13:51 UTC (permalink / raw)
  To: Szabolcs Nagy via Libc-alpha; +Cc: Szabolcs Nagy

On Apr 12 2022, Szabolcs Nagy via Libc-alpha wrote:

> It does not seem to be used.

Ok.  Looks like it has never been defined even when it was added.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] aarch64: Move ld.so _start to separate file
  2022-04-12 12:55 ` [PATCH 3/3] aarch64: Move ld.so _start to separate file Szabolcs Nagy
@ 2022-04-12 14:11   ` Florian Weimer
  0 siblings, 0 replies; 10+ messages in thread
From: Florian Weimer @ 2022-04-12 14:11 UTC (permalink / raw)
  To: Szabolcs Nagy via Libc-alpha; +Cc: Szabolcs Nagy

* Szabolcs Nagy via Libc-alpha:

> A separate asm file is easier to maintain than a macro that expands to
> inline asm.
>
> The RTLD_START macro is only needed now becase _dl_start is local in

typo: beca[u]se

> rtld.c, but _start has to call it, if _dl_start was made hidden then
> it could be empty.

The commit message should mention the _dl_skip_args removal.  Or that
could be done in the previous patch.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] aarch64: Use generic argv adjustment in ld.so [BZ #23293]
  2022-04-12 12:55 ` [PATCH 2/3] aarch64: Use generic argv adjustment in ld.so [BZ #23293] Szabolcs Nagy
@ 2022-04-12 14:12   ` Florian Weimer
  2022-04-12 14:25     ` Florian Weimer
  2022-04-13  8:09     ` Szabolcs Nagy
  0 siblings, 2 replies; 10+ messages in thread
From: Florian Weimer @ 2022-04-12 14:12 UTC (permalink / raw)
  To: Szabolcs Nagy via Libc-alpha; +Cc: Szabolcs Nagy

* Szabolcs Nagy via Libc-alpha:

> A seemingly simpler approach is to deal with unaligned sp in crt1.o,
> i.e. align sp in the entry point of the exe before __libc_start_main
> and pass unaligned sp from ld.so after updating argc (like it is done
> on x86), however this is not a backward compatible solution, new ld.so
> would not work with old exe on targets where old crt1 does not align.

I do not understand this comment.  Main executable crt1 runs after this
code in ld.so.  ld.so has a custom crt1 equivalent in a <dl-machine.h>
assembler fragment.

> diff --git a/elf/rtld.c b/elf/rtld.c
> index 19e328f89e..c08f7ed9e2 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -1311,6 +1311,58 @@ rtld_setup_main_map (struct link_map *main_map)
>    return has_interp;
>  }
>  
> +#ifdef DL_NEED_START_ARGS_ADJUST
> +static void
> +_dl_start_args_adjust (void)
> +{
> +  void **sp;
> +  void **p;
> +  long argc;
> +  char **argv;
> +  ElfW(auxv_t) *auxv;
> +
> +  if (_dl_skip_args == 0)
> +    return;
> +
> +  sp = _dl_start_argptr;
> +
> +  /* Adjust argc on stack.  */
> +  argc = (long) sp[0] - _dl_skip_args;
> +  sp[0] = (void *) argc;
> +
> +  argv = (char **) (sp + 1); /* Necessary aliasing violation.  */
> +  p = sp + _dl_skip_args;
> +  /* Shuffle argv down.  */
> +  do
> +    *++sp = *++p;
> +  while (*p);

*p != NULL?

This looks like a memmove.  Maybe this will need
-fno-tree-loop-distribute-patterns in the future?

> +  /* Shuffle envp down.  */
> +  do
> +    *++sp = *++p;
> +  while (*p);

Likewise.

> +  auxv = (ElfW(auxv_t) *) (sp + 1); /* Necessary aliasing violation.  */
> +  /* Shuffle auxv down. */
> +  void *a, *b; /* Use a pair of pointers for an auxv entry.  */
> +  do
> +    {
> +      a = *++p;
> +      b = *++p;
> +      *++sp = a;
> +      *++sp = b;
> +    }
> +  while (a);

Likewise.

> +
> +  /* Update globals in rtld.  */
> +  _dl_argv = argv;
> +  _environ = argv + argc + 1;
> +  GLRO(dl_auxv) = auxv;
> +  /* No longer need to skip args.  */
> +  _dl_skip_args = 0;
> +}
> +#endif

Maybe we can remove _dl_skip_args completely?

Thanks,
Florian


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] aarch64: Use generic argv adjustment in ld.so [BZ #23293]
  2022-04-12 14:12   ` Florian Weimer
@ 2022-04-12 14:25     ` Florian Weimer
  2022-04-13  8:09     ` Szabolcs Nagy
  1 sibling, 0 replies; 10+ messages in thread
From: Florian Weimer @ 2022-04-12 14:25 UTC (permalink / raw)
  To: Szabolcs Nagy via Libc-alpha; +Cc: Szabolcs Nagy

* Florian Weimer:

> This looks like a memmove.  Maybe this will need
> -fno-tree-loop-distribute-patterns in the future?

Or maybe not because we disable multi-arch memmove in ld.so.

(The style nit regarding the NULL check still stands.)

Thanks,
Florian


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] aarch64: Use generic argv adjustment in ld.so [BZ #23293]
  2022-04-12 14:12   ` Florian Weimer
  2022-04-12 14:25     ` Florian Weimer
@ 2022-04-13  8:09     ` Szabolcs Nagy
  1 sibling, 0 replies; 10+ messages in thread
From: Szabolcs Nagy @ 2022-04-13  8:09 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Szabolcs Nagy via Libc-alpha

The 04/12/2022 16:12, Florian Weimer wrote:
> * Szabolcs Nagy via Libc-alpha:
> 
> > A seemingly simpler approach is to deal with unaligned sp in crt1.o,
> > i.e. align sp in the entry point of the exe before __libc_start_main
> > and pass unaligned sp from ld.so after updating argc (like it is done
> > on x86), however this is not a backward compatible solution, new ld.so
> > would not work with old exe on targets where old crt1 does not align.
> 
> I do not understand this comment.  Main executable crt1 runs after this
> code in ld.so.  ld.so has a custom crt1 equivalent in a <dl-machine.h>
> assembler fragment.


i mean now the entire set of argv, env, auxv is moved in memory
so we can keep sp as is.

instead we could bump sp such that the first few argv is dropped.
the only thing needed for that to work is to handle the unaligned
sp in the _start code of the exe (sp can be immediately aligned
down after saving it into some other register).

however aligned sp at entry is now abi. (the elf entry does not
have to follow the c call abi so i think it could have been
otherwise).

> > +  argv = (char **) (sp + 1); /* Necessary aliasing violation.  */
> > +  p = sp + _dl_skip_args;
> > +  /* Shuffle argv down.  */
> > +  do
> > +    *++sp = *++p;
> > +  while (*p);
> 
> *p != NULL?
> 

will fix these.

> This looks like a memmove.  Maybe this will need
> -fno-tree-loop-distribute-patterns in the future?
> 
> > +  /* Shuffle envp down.  */
> > +  do
> > +    *++sp = *++p;
> > +  while (*p);
> 
> Likewise.
> 
> > +  auxv = (ElfW(auxv_t) *) (sp + 1); /* Necessary aliasing violation.  */
> > +  /* Shuffle auxv down. */
> > +  void *a, *b; /* Use a pair of pointers for an auxv entry.  */
> > +  do
> > +    {
> > +      a = *++p;
> > +      b = *++p;
> > +      *++sp = a;
> > +      *++sp = b;
> > +    }
> > +  while (a);
> 
> Likewise.

note: technically it should be (auxp->a_type != 0)
but i wanted to avoid dealing with elfxx_auxv_t
while moving auxv.

> > +
> > +  /* Update globals in rtld.  */
> > +  _dl_argv = argv;
> > +  _environ = argv + argc + 1;
> > +  GLRO(dl_auxv) = auxv;
> > +  /* No longer need to skip args.  */
> > +  _dl_skip_args = 0;
> > +}
> > +#endif
> 
> Maybe we can remove _dl_skip_args completely?

it can be removed if all targets use this adjustment code.
but e.g. x86_64 ld.so _start just bumps sp up by _dl_skip_args
and then aligns sp down in the _start of the exe.

i'm not against changing this (x86_64 would work too with arg
adjustment and generic code is better than custom solutions)

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-04-13  8:09 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-12 12:55 [PATCH 0/3] Args adjustment with ./ld.so exe [BZ #23293] Szabolcs Nagy
2022-04-12 12:55 ` [PATCH 1/3] Remove _dl_skip_args_internal declaration Szabolcs Nagy
2022-04-12 13:40   ` Florian Weimer
2022-04-12 13:51   ` Andreas Schwab
2022-04-12 12:55 ` [PATCH 2/3] aarch64: Use generic argv adjustment in ld.so [BZ #23293] Szabolcs Nagy
2022-04-12 14:12   ` Florian Weimer
2022-04-12 14:25     ` Florian Weimer
2022-04-13  8:09     ` Szabolcs Nagy
2022-04-12 12:55 ` [PATCH 3/3] aarch64: Move ld.so _start to separate file Szabolcs Nagy
2022-04-12 14:11   ` Florian Weimer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).