Skip to content

Commit

Permalink
i#2113: avoid racy crashes in ELF header queries
Browse files Browse the repository at this point in the history
Changes is_elf_so_header_common() to always use a safe_read() once the fast
version is available during init.  This avoids crashes due to protection
changing in a racy manner.

Fixes #2113

Review-URL: https://codereview.appspot.com/319000043
  • Loading branch information
derekbruening committed Dec 20, 2016
1 parent 03ce33b commit 71427ac
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 2 deletions.
15 changes: 13 additions & 2 deletions core/unix/module_elf.c
Expand Up @@ -86,6 +86,12 @@ safe_read(const void *base, size_t size, void *out_buf)
return true;
}

bool
safe_read_if_fast(const void *base, size_t size, void *out_buf)
{
return safe_read(base, size, out_buf);
}

#else /* !NOT_DYNAMORIO_CORE_PROPER */

#ifdef CLIENT_INTERFACE
Expand Down Expand Up @@ -174,9 +180,14 @@ is_elf_so_header_common(app_pc base, size_t size, bool memory)
return false;
}

/* read the header */
/* Read the header. We used to directly deref if size >= sizeof(ELF_HEADER_TYPE)
* but given that we now have safe_read_fast() it's best to always use it and
* avoid races (like i#2113). However, the non-fast version hits deadlock on
* memquery during client init, so we use a special routine safe_read_if_fast().
*/
if (size >= sizeof(ELF_HEADER_TYPE)) {
elf_header = *(ELF_HEADER_TYPE *)base;
if (!safe_read_if_fast(base, sizeof(ELF_HEADER_TYPE), &elf_header))
return false;
} else if (size == 0) {
if (!safe_read(base, sizeof(ELF_HEADER_TYPE), &elf_header))
return false;
Expand Down
11 changes: 11 additions & 0 deletions core/unix/os.c
Expand Up @@ -4514,6 +4514,17 @@ safe_read_ex(const void *base, size_t size, void *out_buf, size_t *bytes_read)
}
}

bool
safe_read_if_fast(const void *base, size_t size, void *out_buf)
{
if (!fault_handling_initialized) {
memcpy(out_buf, base, size);
return true;
} else {
return safe_read_ex(base, size, out_buf, NULL);
}
}

/* FIXME - fold this together with safe_read_ex() (is a lot of places to update) */
bool
safe_read(const void *base, size_t size, void *out_buf)
Expand Down
3 changes: 3 additions & 0 deletions core/unix/os_private.h
Expand Up @@ -210,6 +210,9 @@ set_executable_path(const char *);
uint
memprot_to_osprot(uint prot);

bool
safe_read_if_fast(const void *base, size_t size, void *out_buf);

bool
mmap_syscall_succeeded(byte *retval);

Expand Down

0 comments on commit 71427ac

Please sign in to comment.