i#1865: relocate DR prior to running any C code

Moves the relocation of DR during early injection to the _start assembly routines, to make the C code more robust and less prone to mistakes such as referencing global variables. Fixes #1865 Review-URL: https://codereview.appspot.com/282450043
DynamoRIO · Feb 2, 2016 · 362b7a9 · 362b7a9
1 parent d2ebf56
commit 362b7a9
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 17 deletions.
diff --git a/core/arch/arm/arm.asm b/core/arch/arm/arm.asm
@@ -43,6 +43,8 @@ DECL_EXTERN(dynamorio_app_take_over_helper)
 DECL_EXTERN(master_signal_handler_C)
 DECL_EXTERN(dr_setjmp_sigmask)
 #endif
+DECL_EXTERN(relocate_dynamorio)
+DECL_EXTERN(privload_early_inject)
 
 DECL_EXTERN(exiting_thread_count)
 DECL_EXTERN(initstack)
@@ -85,18 +87,24 @@ DECL_EXTERN(initstack_mutex)
 # if !defined(STANDALONE_UNIT_TEST) && !defined(STATIC_LIBRARY)
         DECLARE_FUNC(_start)
 GLOBAL_LABEL(_start:)
+        /* i#1676, i#1708: relocate dynamorio if it is not loaded to preferred address.
+         * We call this here to ensure it's safe to access globals once in C code
+         * (xref i#1865).
+         */
+        CALLC2(GLOBAL_REF(relocate_dynamorio), #0, #0)
+
         /* Clear 2nd & 3rd args to distinguish from xfer_to_new_libdr */
         eor      ARG2, ARG2
         eor      ARG3, ARG3
-        /* Entry from xfer_to_new_libdr is here: it assumes ARM! */
+
+        /* Entry from xfer_to_new_libdr is here.  It has set up 2nd & 3rd args already. */
+.L_start_invoke_C:
         eor      r11, r11  /* clear frame ptr for stack trace bottom */
-        mov      r0, sp    /* arg to privload_early_inject */
-        bl       GLOBAL_REF(privload_early_inject)
+        mov      r0, sp    /* 1st arg to privload_early_inject */
+        blx      GLOBAL_REF(privload_early_inject)
         /* shouldn't return */
         bl       GLOBAL_REF(unexpected_return)
         END_FUNC(_start)
-# endif /* !STANDALONE_UNIT_TEST && !STATIC_LIBRARY */
-
 
 /* i#1227: on a conflict with the app we reload ourselves.
  * xfer_to_new_libdr(entry, init_sp, cur_dr_map, cur_dr_size)
@@ -107,15 +115,22 @@ GLOBAL_LABEL(_start:)
         DECLARE_FUNC(xfer_to_new_libdr)
 GLOBAL_LABEL(xfer_to_new_libdr:)
         mov     r5, ARG1
-        /* Skip 1st 2 instrs of _start (assumes ARM) */
-        add     r5, #(2*4)
         /* Restore sp */
         mov     sp, ARG2
+        /* Skip prologue that calls relocate_dynamorio() and clears args 2+3 by
+         * adjusting the _start in the reloaded DR by the same distance as in
+         * the current DR, but w/o clobbering ARG3 or ARG4.
+         */
+        adr     r0, .L_start_invoke_C
+        adr     r1, _start
+        sub     r0, r0, r1
+        add     r5, r5, r0
         /* _start expects these as 2nd & 3rd args */
         mov     ARG2, ARG3
         mov     ARG3, ARG4
         bx      r5
         END_FUNC(xfer_to_new_libdr)
+# endif /* !STANDALONE_UNIT_TEST && !STATIC_LIBRARY */
 #endif /* UNIX */
 
 /* all of the CPUID registers are only accessible in privileged modes */

diff --git a/core/arch/x86/x86.asm b/core/arch/x86/x86.asm
@@ -1,5 +1,5 @@
 /* **********************************************************
- * Copyright (c) 2011-2015 Google, Inc.  All rights reserved.
+ * Copyright (c) 2011-2016 Google, Inc.  All rights reserved.
  * Copyright (c) 2001-2010 VMware, Inc.  All rights reserved.
  * ********************************************************** */
 
@@ -185,6 +185,7 @@ DECL_EXTERN(fixup_rtframe_pointers)
 #ifdef UNIX
 DECL_EXTERN(dr_setjmp_sigmask)
 DECL_EXTERN(privload_early_inject)
+DECL_EXTERN(relocate_dynamorio)
 DECL_EXTERN(dynamorio_dl_fixup)
 #endif
 #ifdef WINDOWS
@@ -1155,6 +1156,15 @@ GLOBAL_LABEL(client_int_syscall:)
  */
         DECLARE_FUNC(_start)
 GLOBAL_LABEL(_start:)
+        /* i#1676, i#1708: relocate dynamorio if it is not loaded to preferred address.
+         * We call this here to ensure it's safe to access globals once in C code
+         * (xref i#1865).
+         */
+        cmp     REG_XDI, 0 /* if reloaded, skip for speed + preserve xdi and xsi */
+        jne     reloaded_xfer
+        CALLC2(GLOBAL_REF(relocate_dynamorio), 0, 0)
+
+reloaded_xfer:
         xor     REG_XBP, REG_XBP  /* Terminate stack traces at NULL. */
 # ifdef X64
         /* Reverse order to avoid clobbering */

diff --git a/core/unix/loader.c b/core/unix/loader.c
@@ -941,6 +941,7 @@ get_private_library_bounds(IN app_pc modbase, OUT byte **start, OUT byte **end)
 }
 
 #ifdef LINUX
+# if !defined(STANDALONE_UNIT_TEST) && !defined(STATIC_LIBRARY)
 /* XXX: This routine is called before dynamorio relocation when we are in a
  * fragile state and thus no globals access or use of ASSERT/LOG/STATS!
  */
@@ -1053,6 +1054,7 @@ privload_early_relocate_os_privmod_data(os_privmod_data_t *opd, byte *mod_base)
         }
     }
 }
+# endif /* !defined(STANDALONE_UNIT_TEST) && !defined(STATIC_LIBRARY) */
 
 /*  This routine is duplicated at privload_early_relocate_os_privmod_data. */
 static void
@@ -1080,7 +1082,7 @@ privload_relocate_os_privmod_data(os_privmod_data_t *opd, byte *mod_base)
         }
     }
 }
-#endif
+#endif /* LINUX */
 
 static void
 privload_relocate_mod(privmod_t *mod)
@@ -1348,6 +1350,7 @@ privload_redirect_sym(ptr_uint_t *r_addr, const char *name)
  */
 
 #ifdef LINUX
+# if !defined(STANDALONE_UNIT_TEST) && !defined(STATIC_LIBRARY)
 /* Find the auxiliary vector and adjust it to look as if the kernel had set up
  * the stack for the ELF mapped at map.  The auxiliary vector starts after the
  * terminating NULL pointer in the envp array.
@@ -1551,7 +1554,7 @@ privload_mem_is_elf_so_header(byte *mem)
 /* XXX: This routine is called before dynamorio relocation when we are in a
  * fragile state and thus no globals access or use of ASSERT/LOG/STATS!
  */
-static void
+void
 relocate_dynamorio(byte *dr_map, size_t dr_size)
 {
     os_privmod_data_t opd = { {0}};
@@ -1645,6 +1648,9 @@ reload_dynamorio(void **init_sp, app_pc conflict_start, app_pc conflict_end)
  * kernel set up for us, and it points to the usual argc, argv, envp, and auxv
  * that the kernel puts on the stack.  The 2nd & 3rd args must be 0 in
  * the initial call.
+ *
+ * We assume that _start has already called relocate_dynamorio() for us and
+ * that it is now safe to access globals.
  */
 void
 privload_early_inject(void **sp, byte *old_libdr_base, size_t old_libdr_size)
@@ -1662,11 +1668,8 @@ privload_early_inject(void **sp, byte *old_libdr_base, size_t old_libdr_size)
     bool success;
     memquery_iter_t iter;
     app_pc interp_map;
-    kernel_init_sp = (void *)sp;
 
-    /* i#1676, i#1708: relocate dynamorio if it is not loaded to preferred address */
-    if (old_libdr_base == NULL)
-        relocate_dynamorio(NULL, 0);
+    kernel_init_sp = (void *)sp;
 
     /* XXX i#47: for Linux, we can't easily have this option on by default as
      * code like get_application_short_name() called from drpreload before
@@ -1815,21 +1818,22 @@ privload_early_inject(void **sp, byte *old_libdr_base, size_t old_libdr_size)
          * if the app has been mapped correctly without involving DR's code
          * cache.
          */
-#ifdef X86
+#  ifdef X86
         asm ("mov %0, %%"ASM_XSP"\n\t"
              "jmp *%1\n\t"
              : : "r"(sp), "r"(entry));
-#elif defined(ARM)
+#  elif defined(ARM)
         /* FIXME i#1551: NYI on ARM */
         ASSERT_NOT_REACHED();
-#endif
+#  endif
     }
 
     memset(&mc, 0, sizeof(mc));
     mc.xsp = (reg_t) sp;
     mc.pc = entry;
     dynamo_start(&mc);
 }
+# endif /* !defined(STANDALONE_UNIT_TEST) && !defined(STATIC_LIBRARY) */
 #else
 /* XXX i#1285: implement MacOS private loader */
 #endif