From 389d945d7fbee987e10c9ee5b96dbc1848c639ed Mon Sep 17 00:00:00 2001 From: Alex Richardson Date: Tue, 20 Aug 2024 10:53:09 -0700 Subject: [PATCH] [CHERI] Correctly add .cfi_undefined to _start The existing approach of using inline assembly to inject .cfi_undefined into _start was fragile: after upgrading to LLVM 16, the undefined information was placed before the normal stack setup metadata which meant it was being ignored by libunwind and it continues unwinding. For dynamically linked libraries this should not cause any problems since (as of the last commit) RTLD's entry point has the required termination annotation, but for statically linked binaries we keep unwinding and eventually trigger a bounds violation while parsing nonsense data. Make this current annotation less fragile by using a naked function that has the .cfi_undefined annotation and calls the real C entry. --- lib/csu/aarch64c/crt1_c.c | 21 +++++++++++++++++++-- lib/csu/riscv/crt1_c.c | 1 - lib/csu/riscv/crt1_s.S | 1 + lib/csu/riscv64c/crt1_c.c | 21 +++++++++++++++++++-- 4 files changed, 39 insertions(+), 5 deletions(-) diff --git a/lib/csu/aarch64c/crt1_c.c b/lib/csu/aarch64c/crt1_c.c index ca3cf8d39ee3..3fba07a6ae41 100644 --- a/lib/csu/aarch64c/crt1_c.c +++ b/lib/csu/aarch64c/crt1_c.c @@ -49,6 +49,7 @@ struct Struct_Obj_Entry; void _start(void *, void (*)(void), struct Struct_Obj_Entry *) __dead2 __exported; +void __start(void *, void (*)(void), struct Struct_Obj_Entry *) __dead2; #ifdef GCRT /* Profiling support. */ @@ -69,11 +70,10 @@ Elf_Auxinfo *__auxargs; * linker takes care of initialization otherwise. */ void -_start(void *auxv, +__start(void *auxv, void (*cleanup)(void), /* from shared loader */ struct Struct_Obj_Entry *obj) /* from shared loader */ { - __asm__ volatile(".cfi_undefined c30"); int argc = 0; char **argv = NULL; char **env = NULL; @@ -131,3 +131,20 @@ _start(void *auxv, __libc_start1(argc, argv, env, cleanup, main, data_cap, code_cap); } + +/* + * The real entry point _start just sets the unwind info for CRA to undefined + * which tells libunwind to stop unwinding and then calls the real __start. + * This is needed because ".cfi_undefined" inline assembly in a non-naked + * function could be overridden by the default unwinding information. + */ +__attribute__((naked, used)) void +_start(void *auxv, + void (*cleanup)(void), /* from shared loader */ + struct Struct_Obj_Entry *obj) /* from shared loader */ +{ + __asm__( + ".cfi_undefined c30\n" + "bl %0" + :: "i"(__start)); +} diff --git a/lib/csu/riscv/crt1_c.c b/lib/csu/riscv/crt1_c.c index 41ae7cff9f2a..04d6e7b26749 100644 --- a/lib/csu/riscv/crt1_c.c +++ b/lib/csu/riscv/crt1_c.c @@ -57,7 +57,6 @@ void __start(int argc, char **argv, char **env, void (*cleanup)(void)) __dead2; void __start(int argc, char **argv, char **env, void (*cleanup)(void)) { - __asm__ volatile(".cfi_undefined ra"); #ifdef SHOULD_PROCESS_CAP_RELOCS /* * Initialize __cap_relocs for static executables. The run-time linker diff --git a/lib/csu/riscv/crt1_s.S b/lib/csu/riscv/crt1_s.S index 8f77d2358ae7..bf69d1435bb8 100644 --- a/lib/csu/riscv/crt1_s.S +++ b/lib/csu/riscv/crt1_s.S @@ -35,6 +35,7 @@ #include ENTRY(_start) + .cfi_undefined ra # Stop unwinding here mv a3, a2 # cleanup addi a1, a0, 8 # get argv ld a0, 0(a0) # load argc diff --git a/lib/csu/riscv64c/crt1_c.c b/lib/csu/riscv64c/crt1_c.c index 3108ebf562c9..ef4e8a43ab83 100644 --- a/lib/csu/riscv64c/crt1_c.c +++ b/lib/csu/riscv64c/crt1_c.c @@ -49,6 +49,7 @@ struct Struct_Obj_Entry; void _start(void *, void (*)(void), struct Struct_Obj_Entry *) __dead2 __exported; +void __start(void *, void (*)(void), struct Struct_Obj_Entry *) __dead2; #ifdef GCRT /* Profiling support. */ @@ -69,11 +70,10 @@ Elf_Auxinfo *__auxargs; * linker takes care of initialization otherwise. */ void -_start(void *auxv, +__start(void *auxv, void (*cleanup)(void), /* from shared loader */ struct Struct_Obj_Entry *obj) /* from shared loader */ { - __asm__ volatile(".cfi_undefined cra"); int argc = 0; char **argv = NULL; char **env = NULL; @@ -128,3 +128,20 @@ _start(void *auxv, __libc_start1(argc, argv, env, cleanup, main, NULL, NULL); } + +/* + * The real entry point _start just sets the unwind info for CRA to undefined + * which tells libunwind to stop unwinding and then calls the real __start. + * This is needed because ".cfi_undefined" inline assembly in a non-naked + * function could be overridden by the default unwinding information. + */ +__attribute__((naked, used)) void +_start(void *auxv, + void (*cleanup)(void), /* from shared loader */ + struct Struct_Obj_Entry *obj) /* from shared loader */ +{ + __asm__( + ".cfi_undefined cra\n" + "ccall %0" + :: "i"(__start)); +}