diff --git a/libexec/rtld-cheri-elf/mips/cheri_plt.cpp b/libexec/rtld-cheri-elf/mips/cheri_plt.cpp index 6059aa9bb653..61f1e6619ef6 100644 --- a/libexec/rtld-cheri-elf/mips/cheri_plt.cpp +++ b/libexec/rtld-cheri-elf/mips/cheri_plt.cpp @@ -244,7 +244,10 @@ _mips_rtld_bind(void* _plt_stub) lock_release(rtld_bind_lock, &lockstate); // Setup the target $cgp so that we can actually call the function // TODO: return two values instead (will a 2 cap struct use $c3/$c4?) - __asm__ volatile("cmove $cgp, %0"::"C"(target_cgp)); + // FIXME: memory clobber should ensure that this is moved after the + // restore of $cgp (due to the call to lock_release) but it would be + // much nicer if we could just return (target, $cgp) in $c3,$c4 + __asm__ volatile("cmove $cgp, %0"::"C"(target_cgp): "$c26", "memory"); return target; } diff --git a/libexec/rtld-cheri-elf/mips/rtld_machdep.h b/libexec/rtld-cheri-elf/mips/rtld_machdep.h index 7cd3403c29c5..dfa2f70f871b 100644 --- a/libexec/rtld-cheri-elf/mips/rtld_machdep.h +++ b/libexec/rtld-cheri-elf/mips/rtld_machdep.h @@ -41,6 +41,7 @@ #else #include #define dbg_assert(cond) assert(cond) +#define dbg_cheri(...) #define dbg(...) #endif @@ -241,10 +242,26 @@ _call_init_fini_array_pointer(const struct Struct_Obj_Entry *obj, vaddr_t target /* Set the target object $cgp when calling the pointer: * Note: we use target_cgp_for_func() to support per-function captable */ const void *init_fini_cgp = target_cgp_for_func(obj, (dlfunc_t)func); - __asm__ volatile("cmove $cgp, %0" :: "C"(init_fini_cgp)); -#endif - /* FIXME: should probably do this from assembly to ensure that $cgp is correct */ + dbg_cheri("Setting init function $cgp to %#p for call to %#p. " + "Current $cgp: %#p\n", init_fini_cgp, (void*)func, cheri_getidc()); + /* + * Invoke the function from assembly to ensure that the $cgp value is + * correct and the compiler can't reorder things. + * When I was calling the function from C, it broke at -O2. + * Note: we need the memory clobber here to ensure that the setting of + * $cgp is not ignored due to reordering of instructions (e.g. by adding + * a $cgp restore after external function calls). + */ + __asm__ volatile("cmove $cgp, %[cgp_val]\n\t" + :/*out*/ + :/*in*/[cgp_val]"C"(init_fini_cgp) + :/*clobber*/"$c26", "memory"); + func(argc, argv, env); + /* Ensure that the function call is not reordered before/after asm */ + __compiler_membar(); +#else func(argc, argv, env); +#endif } #define call_init_array_pointer(obj, target) \ diff --git a/libexec/rtld-elf/rtld.c b/libexec/rtld-elf/rtld.c index b7ea958206e2..c65e072eeeb3 100644 --- a/libexec/rtld-elf/rtld.c +++ b/libexec/rtld-elf/rtld.c @@ -887,8 +887,12 @@ _rtld(Elf_Addr *sp, func_ptr_type *exit_proc, Obj_Entry **objp) // and should have the same effect. const void *entry_cgp = target_cgp_for_func(obj_main, (dlfunc_t)obj_main->entry); dbg_cheri("Setting initial $cgp for %s to %-#p", obj_main->path, entry_cgp); - __asm__ volatile("cmove $cgp, %0" :: "C"(entry_cgp)); assert(cheri_getperm(obj_main->entry) & CHERI_PERM_EXECUTE); + /* Add memory clobber to ensure that it is done last thing before return + * + * TODO: would be nice if we could return pairs in $c3/$c4 + */ + __asm__ volatile("cmove $cgp, %0" :: "C"(entry_cgp): "$c26", "memory"); #endif return (func_ptr_type) obj_main->entry; }