Skip to content

Commit

Permalink
i#5463: Avoid globals pre-relocation
Browse files Browse the repository at this point in the history
Fixes errors that can lead to crashes in relocate_dynamorio, where its
access to a function pointer and to the page size can both return
garbage pre-relocation.  We solve this with assembly to get the
current PC, and just using a 4K minimum page size for the backward
walk.

Tested with the PR #5462 branch where the new client.attach_blocking
test crashes without this fix on AArch64.
Manually tested on arm as well.

Fixes #5463
  • Loading branch information
derekbruening committed Apr 14, 2022
1 parent 19af000 commit d6a5486
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 3 deletions.
14 changes: 14 additions & 0 deletions core/arch/arch_exports.h
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,19 @@ get_stack_ptr(void);
__asm__ __volatile__("rdtsc" : "=a"(low), "=d"(high)); \
(llval) = ((uint64)high << 32) | low; \
} while (0)
# define GET_CUR_PC(var) \
__asm__ __volatile__("call 1f; 1: pop %%rax; mov %%rax, %0" \
: "=m"(var) \
: \
: "rax", "memory")
# else
/* We define RDTSC_LL differently on 32-bit for better performance */
# define RDTSC_LL(llval) __asm__ __volatile__("rdtsc" : "=A"(llval))
# define GET_CUR_PC(var) \
__asm__ __volatile__("call 1f; 1: pop %%rax; mov %%eax, %0" \
: "=m"(var) \
: \
: "eax", "memory")
# endif /* 64/32 */
# define GET_FRAME_PTR(var) \
asm("mov %%" IF_X64_ELSE("rbp", "ebp") ", %0" : "=m"(var))
Expand All @@ -318,12 +328,16 @@ get_stack_ptr(void);

# define GET_FRAME_PTR(var) __asm__ __volatile__("mov %0, x29" : "=r"(var))
# define GET_STACK_PTR(var) __asm__ __volatile__("mov %0, sp" : "=r"(var))
# define GET_CUR_PC(var) \
__asm__ __volatile__("bl 1f; 1: str x30, %0" : "=m"(var) : : "x30")
# elif defined(DR_HOST_ARM)
# define RDTSC_LL(llval) (llval) = proc_get_timestamp()
/* FIXME i#1551: frame pointer is r7 in thumb mode */
# define GET_FRAME_PTR(var) \
__asm__ __volatile__("str " IF_X64_ELSE("x29", "r11") ", %0" : "=m"(var))
# define GET_STACK_PTR(var) __asm__ __volatile__("str sp, %0" : "=m"(var))
# define GET_CUR_PC(var) \
__asm__ __volatile__("bl 1f; 1: str lr, %0" : "=m"(var) : : "lr")
# endif /* X86/ARM */
#endif /* UNIX */

Expand Down
13 changes: 10 additions & 3 deletions core/unix/loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -1819,13 +1819,18 @@ relocate_dynamorio(byte *dr_map, size_t dr_size, byte *sp)
const char **env = (const char **)sp + argc + 2;
os_privmod_data_t opd = { { 0 } };

os_page_size_init(env, true);
/* We can't use PAGE_SIZE as that may require relocations to access. */
const int min_page_size = 4096;

if (dr_map == NULL) {
/* We can't start with the address of relocate_dynamorio or something as that
* may require relocations to access!
*/
GET_CUR_PC(dr_map);
/* we do not know where dynamorio is, so check backward page by page */
dr_map = (app_pc)ALIGN_BACKWARD((ptr_uint_t)relocate_dynamorio, PAGE_SIZE);
dr_map = (app_pc)ALIGN_BACKWARD(dr_map, min_page_size);
while (dr_map != NULL && !privload_mem_is_elf_so_header(dr_map)) {
dr_map -= PAGE_SIZE;
dr_map -= min_page_size;
}
}
if (dr_map == NULL)
Expand All @@ -1834,6 +1839,8 @@ relocate_dynamorio(byte *dr_map, size_t dr_size, byte *sp)
/* Relocate it */
if (privload_get_os_privmod_data(dr_map, &opd))
privload_early_relocate_os_privmod_data(&opd, dr_map);

os_page_size_init(env, true);
}

/* i#1227: on a conflict with the app we reload ourselves.
Expand Down

0 comments on commit d6a5486

Please sign in to comment.