[2/3] aarch64: Use generic argv adjustment in ld.so [BZ #23293]
Checks
| Context |
Check |
Description |
| dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
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(-)
Comments
* 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
* 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
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)
@@ -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
{
@@ -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
@@ -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]
@@ -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