Skip to content

Commit

Permalink
i#2641: handle vvar+vdso in libdynamorio's text-data gap (#2640)
Browse files Browse the repository at this point in the history
Fixes 3 problems related to the kernel placing vvar+vdso in libdynamorio's text-data gap:

1) Ensures old_libdr_base is NULL by not relying on the calling convention
   during early injection.

2) Handles asserts/curiosities inside memquery code by adding
   is_readable_without_exception_query_os_noblock()
   and memquery_from_os_will_block() to avoid deadlock on locks
   used on UNIX for memory queries.

3) Detects something in the text-data gap and reloads libdynamorio during
   early injection as the rest of DR assumes there's nothing there.

Fixes #2641
  • Loading branch information
derekbruening authored Sep 26, 2017
1 parent 2b5cd35 commit 4f1058e
Show file tree
Hide file tree
Showing 11 changed files with 139 additions and 19 deletions.
1 change: 1 addition & 0 deletions core/arch/x86/x86.asm
Original file line number Diff line number Diff line change
Expand Up @@ -1164,6 +1164,7 @@ GLOBAL_LABEL(_start:)
cmp REG_XDI, 0 /* if reloaded, skip for speed + preserve xdi and xsi */
jne reloaded_xfer
CALLC3(GLOBAL_REF(relocate_dynamorio), 0, 0, REG_XSP)
mov REG_XDI, 0 /* xdi should be callee-saved but is not always: i#2641 */

reloaded_xfer:
xor REG_XBP, REG_XBP /* Terminate stack traces at NULL. */
Expand Down
1 change: 1 addition & 0 deletions core/os_shared.h
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,7 @@ bool get_stack_bounds(dcontext_t *dcontext, byte **base, byte **top);

bool is_readable_without_exception(const byte *pc, size_t size);
bool is_readable_without_exception_query_os(byte *pc, size_t size);
bool is_readable_without_exception_query_os_noblock(byte *pc, size_t size);
bool safe_read(const void *base, size_t size, void *out_buf);
bool safe_read_ex(const void *base, size_t size, void *out_buf, size_t *bytes_read);
bool safe_write_ex(void *base, size_t size, const void *in_buf, size_t *bytes_written);
Expand Down
63 changes: 58 additions & 5 deletions core/unix/loader.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@

extern size_t wcslen(const wchar_t *str); /* in string.c */

#if 1//DO NOT CHECK IN
bool vvar_in_gap;
#endif

/* Written during initialization only */
/* FIXME: i#460, the path lookup itself is a complicated process,
* so we just list possible common but in-complete paths for now.
Expand Down Expand Up @@ -1587,6 +1591,34 @@ privload_mem_is_elf_so_header(byte *mem)
return true;
}

static bool
dynamorio_lib_gap_empty(void)
{
/* XXX: get_dynamorio_dll_start() is already calling
* memquery_library_bounds_by_iterator() which is doing this maps walk: can we
* avoid this extra walk by somehow passing info back to us? Have an
* "interrupted" output param or sthg and is_dynamorio_dll_interrupted()?
*/
memquery_iter_t iter;
bool res = true;
if (memquery_iterator_start(&iter, NULL, false/*no heap*/)) {
while (memquery_iterator_next(&iter)) {
if (iter.vm_start >= get_dynamorio_dll_start() &&
iter.vm_end <= get_dynamorio_dll_end() &&
iter.comment[0] != '\0' &&
strstr(iter.comment, DYNAMORIO_LIBRARY_NAME) == NULL) {
/* There's a non-.bss mapping inside: probably vvar and/or vdso. */
res = false;
break;
}
if (iter.vm_start >= get_dynamorio_dll_end())
break;
}
memquery_iterator_stop(&iter);
}
return res;
}

/* 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!
*/
Expand Down Expand Up @@ -1658,7 +1690,6 @@ reload_dynamorio(void **init_sp, app_pc conflict_start, app_pc conflict_end)
ASSERT(dr_map != NULL);
ASSERT(is_elf_so_header(dr_map, 0));

/* Relocate it */
/* Relocate it */
memset(&opd, 0, sizeof(opd));
module_get_os_privmod_data(dr_map, dr_size, false/*!relocated*/, &opd);
Expand Down Expand Up @@ -1727,8 +1758,23 @@ privload_early_inject(void **sp, byte *old_libdr_base, size_t old_libdr_size)
}

/* i#1227: if we reloaded ourselves, unload the old libdynamorio */
if (old_libdr_base != NULL)
os_unmap_file(old_libdr_base, old_libdr_size);
if (old_libdr_base != NULL) {
/* i#2641: we can't blindly unload the whole region as vvar+vdso may be
* in the text-data gap.
*/
if (memquery_iterator_start(&iter, NULL, false/*no heap*/)) {
while (memquery_iterator_next(&iter)) {
if (iter.vm_start >= old_libdr_base &&
iter.vm_end <= old_libdr_base + old_libdr_size &&
strstr(iter.comment, DYNAMORIO_LIBRARY_NAME) != NULL) {
os_unmap_file(iter.vm_start, iter.vm_end - iter.vm_start);
}
if (iter.vm_start >= old_libdr_base + old_libdr_size)
break;
}
memquery_iterator_stop(&iter);
}
}

dynamorio_set_envp(envp);

Expand Down Expand Up @@ -1757,8 +1803,15 @@ privload_early_inject(void **sp, byte *old_libdr_base, size_t old_libdr_size)
exe_map = module_vaddr_from_prog_header((app_pc)exe_ld.phdrs,
exe_ld.ehdr->e_phnum, NULL, &exe_end);
/* i#1227: on a conflict with the app: reload ourselves */
if (get_dynamorio_dll_start() < exe_end &&
get_dynamorio_dll_end() > exe_map) {
if ((get_dynamorio_dll_start() < exe_end &&
get_dynamorio_dll_end() > exe_map) ||
/* i#2641: we can't handle something in the text-data gap.
* Various parts of DR assume there's nothing inside (and we even fill the
* gap with a PROT_NONE mmap later: i#1659), so we reload to avoid it,
* under the assumption that it's rare and we're not paying this cost
* very often.
*/
!dynamorio_lib_gap_empty()) {
reload_dynamorio(sp, exe_map, exe_end);
ASSERT_NOT_REACHED();
}
Expand Down
24 changes: 15 additions & 9 deletions core/unix/memquery.c
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ memquery_library_bounds_by_iterator(const char *name, app_pc *start/*IN/OUT*/,
(iter.comment[0] == '\0' && prev_end != NULL &&
prev_end != iter.vm_start))) {
last_lib_base = iter.vm_start;
/* Include a prior anon mapping if contiguous and a header and this
/* Include a prior anon mapping if interrupted and a header and this
* mapping is not a header. This happens for some page mapping
* schemes (i#2566).
*/
Expand Down Expand Up @@ -200,14 +200,20 @@ memquery_library_bounds_by_iterator(const char *name, app_pc *start/*IN/OUT*/,
* header to know since we can't assume that a subsequent anonymous
* region is .bss. */
if (image_size != 0 && cur_end - mod_start < image_size) {
/* Found a .bss section. Check current mapping (note might only be
* part of the mapping (due to os region merging? FIXME investigate). */
ASSERT_CURIOSITY(iter.vm_start == cur_end /* no gaps, FIXME might there be
* a gap if the file has large
* alignment and no data section?
* curiosity for now*/);
ASSERT_CURIOSITY(iter.inode == 0); /* .bss is anonymous */
ASSERT_CURIOSITY(iter.vm_end - mod_start >= image_size);/* should be big enough */
if (iter.comment[0] != '\0') {
/* There's something else in the text-data gap: xref i#2641. */
} else {
/* Found a .bss section. Check current mapping (note might only be
* part of the mapping (due to os region merging? FIXME investigate).
*/
ASSERT_CURIOSITY(iter.vm_start == cur_end /* no gaps, FIXME might there be
* a gap if the file has large
* alignment and no data section?
* curiosity for now*/);
ASSERT_CURIOSITY(iter.inode == 0); /* .bss is anonymous */
/* should be big enough */
ASSERT_CURIOSITY(iter.vm_end - mod_start >= image_size);
}
count++;
cur_end = mod_start + image_size;
} else {
Expand Down
12 changes: 10 additions & 2 deletions core/unix/memquery.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* *******************************************************************************
* Copyright (c) 2013-2015 Google, Inc. All rights reserved.
* Copyright (c) 2013-2017 Google, Inc. All rights reserved.
* *******************************************************************************/

/*
Expand Down Expand Up @@ -124,8 +124,16 @@ int
find_vm_areas_via_probe(void);
#endif

/* This routine does not acquire locks */
/* This routine might acquire locks. is_readable_without_exception_query_os_noblock()
* can be used to avoid blocking.
*/
bool
memquery_from_os(const byte *pc, OUT dr_mem_info_t *info, OUT bool *have_type);

/* The result can change if another thread grabs the lock, but this will identify
* whether the current thread holds the lock, avoiding a hang.
*/
bool
memquery_from_os_will_block(void);

#endif /* _MEMQUERY_H_ */
6 changes: 6 additions & 0 deletions core/unix/memquery_emulate.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ memquery_exit(void)
{
}

bool
memquery_from_os_will_block(bool may_alloc)
{
return false;
}

bool
memquery_iterator_start(memquery_iter_t *iter, app_pc start, bool may_alloc)
{
Expand Down
16 changes: 16 additions & 0 deletions core/unix/memquery_linux.c
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,22 @@ memquery_exit(void)
DELETE_LOCK(maps_iter_buf_lock);
}

bool
memquery_from_os_will_block(void)
{
#ifdef DEADLOCK_AVOIDANCE
return memory_info_buf_lock.owner != INVALID_THREAD_ID;
#else
/* "may_alloc" is false for memquery_from_os() */
bool res = true;
if (mutex_trylock(&memory_info_buf_lock)) {
res = false;
mutex_unlock(&memory_info_buf_lock);
}
return res;
#endif
}

bool
memquery_iterator_start(memquery_iter_t *iter, app_pc start, bool may_alloc)
{
Expand Down
18 changes: 17 additions & 1 deletion core/unix/memquery_macos.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* *******************************************************************************
* Copyright (c) 2013-2014 Google, Inc. All rights reserved.
* Copyright (c) 2013-2017 Google, Inc. All rights reserved.
* *******************************************************************************/

/*
Expand Down Expand Up @@ -93,6 +93,22 @@ memquery_exit(void)
DELETE_LOCK(memquery_backing_lock);
}

bool
memquery_from_os_will_block(void)
{
#ifdef DEADLOCK_AVOIDANCE
return memquery_backing_lock.owner != INVALID_THREAD_ID;
#else
/* "may_alloc" is false for memquery_from_os() */
bool res = true;
if (mutex_trylock(&memquery_backing_lock)) {
res = false;
mutex_unlock(&memquery_backing_lock);
}
return res;
#endif
}

static bool
memquery_file_backing(struct proc_regionwithpathinfo *info, app_pc addr)
{
Expand Down
8 changes: 8 additions & 0 deletions core/unix/os.c
Original file line number Diff line number Diff line change
Expand Up @@ -4681,6 +4681,14 @@ is_readable_without_exception_query_os(byte *pc, size_t size)
return is_readable_without_exception_internal(pc, size, true);
}

bool
is_readable_without_exception_query_os_noblock(byte *pc, size_t size)
{
if (memquery_from_os_will_block())
return false;
return is_readable_without_exception_internal(pc, size, true);
}

bool
is_user_address(byte *pc)
{
Expand Down
3 changes: 1 addition & 2 deletions core/utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -2242,13 +2242,12 @@ report_dynamorio_problem(dcontext_t *dcontext, uint dumpcore_flag,
}
for (num = 0, pc = (ptr_uint_t *) report_ebp;
num < REPORT_NUM_STACK && pc != NULL &&
is_readable_without_exception_query_os((app_pc) pc, 2*sizeof(reg_t));
is_readable_without_exception_query_os_noblock((app_pc) pc, 2*sizeof(reg_t));
num++, pc = (ptr_uint_t *) *pc) {
len = snprintf(curbuf, REPORT_LEN_STACK_EACH, PFX" "PFX"\n",
pc, *(pc+1));
curbuf += (len == -1 ? REPORT_LEN_STACK_EACH : (len < 0 ? 0 : len));
}

#ifdef CLIENT_INTERFACE
/* Only walk the module list if we think the data structs are safe */
if (!TEST(DUMPCORE_INTERNAL_EXCEPTION, dumpcore_flag)) {
Expand Down
6 changes: 6 additions & 0 deletions core/win32/os.c
Original file line number Diff line number Diff line change
Expand Up @@ -5674,6 +5674,12 @@ is_readable_without_exception_query_os(byte *pc, size_t size)
return is_readable_without_exception(pc, size);
}

bool
is_readable_without_exception_query_os_noblock(byte *pc, size_t size)
{
return is_readable_without_exception_query_os(pc, size);
}

/* Reads size bytes starting at base and puts them in out_buf, this is safe
* to call even if the memory at base is unreadable, returns true if the
* read succeeded */
Expand Down

0 comments on commit 4f1058e

Please sign in to comment.