Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i#2666 dyn cxt: Fix early context free #4722

Merged
merged 1 commit into from
Feb 6, 2021
Merged

i#2666 dyn cxt: Fix early context free #4722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions core/win32/os.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2503,7 +2503,7 @@ os_take_over_thread(dcontext_t *dcontext, HANDLE hthread, thread_id_t tid, bool
bool success = true;
DWORD cxt_flags = CONTEXT_DR_STATE;
size_t bufsz = nt_get_context_size(cxt_flags);
char *buf = (char *)global_heap_alloc(bufsz HEAPACCT(ACCT_THREAD_MGT));
char *buf = (char *)heap_alloc(dcontext, bufsz HEAPACCT(ACCT_THREAD_MGT));
CONTEXT *cxt = nt_initialize_context(buf, bufsz, cxt_flags);
ASSERT(tid == thread_id_from_handle(hthread));
if ((suspended || nt_thread_suspend(hthread, NULL)) &&
Expand Down Expand Up @@ -2531,7 +2531,7 @@ os_take_over_thread(dcontext_t *dcontext, HANDLE hthread, thread_id_t tid, bool
if (is_in_dynamo_dll((app_pc)cxt->CXT_XIP) ||
new_thread_is_waiting_for_dr_init(tid, (app_pc)cxt->CXT_XIP)) {
LOG(GLOBAL, LOG_THREADS, 1, "\tthread " TIDFMT " is already waiting\n", tid);
global_heap_free(buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
heap_free(dcontext, buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
return true; /* it's waiting for us to take it over */
}
/* Avoid double-takeover.
Expand Down Expand Up @@ -2568,7 +2568,7 @@ os_take_over_thread(dcontext_t *dcontext, HANDLE hthread, thread_id_t tid, bool
*/
ASSERT_CURIOSITY(false && "thread takeover context reverted!");
} else {
global_heap_free(buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
heap_free(dcontext, buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
return true;
}
} else {
Expand Down Expand Up @@ -2614,7 +2614,7 @@ os_take_over_thread(dcontext_t *dcontext, HANDLE hthread, thread_id_t tid, bool
LOG(GLOBAL, LOG_THREADS, 1, "\tfailed to suspend/query thread " TIDFMT "\n", tid);
success = false;
}
global_heap_free(buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
heap_free(dcontext, buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
return success;
}

Expand Down Expand Up @@ -5159,14 +5159,14 @@ thread_get_mcontext(thread_record_t *tr, priv_mcontext_t *mc)
{
DWORD cxt_flags = CONTEXT_DR_STATE;
size_t bufsz = nt_get_context_size(cxt_flags);
char *buf = (char *)global_heap_alloc(bufsz HEAPACCT(ACCT_THREAD_MGT));
char *buf = (char *)heap_alloc(tr->dcontext, bufsz HEAPACCT(ACCT_THREAD_MGT));
CONTEXT *cxt = nt_initialize_context(buf, bufsz, cxt_flags);
bool res = false;
if (thread_get_context(tr, cxt)) {
context_to_mcontext(mc, cxt);
res = true;
}
global_heap_free(buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
heap_free(tr->dcontext, buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
return res;
}

Expand All @@ -5175,15 +5175,15 @@ thread_set_mcontext(thread_record_t *tr, priv_mcontext_t *mc)
{
DWORD cxt_flags = CONTEXT_DR_STATE;
size_t bufsz = nt_get_context_size(cxt_flags);
char *buf = (char *)global_heap_alloc(bufsz HEAPACCT(ACCT_THREAD_MGT));
char *buf = (char *)heap_alloc(tr->dcontext, bufsz HEAPACCT(ACCT_THREAD_MGT));
CONTEXT *cxt = nt_initialize_context(buf, bufsz, cxt_flags);
/* i#1033: get the context from the dst thread to make sure
* segments are correctly set.
*/
thread_get_context(tr, cxt);
mcontext_to_context(cxt, mc, false /* !set_cur_seg */);
bool res = thread_set_context(tr, cxt);
global_heap_free(buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
heap_free(tr->dcontext, buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
return res;
}

Expand Down
31 changes: 16 additions & 15 deletions core/win32/syscall.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1722,7 +1722,8 @@ add_dr_env_vars(dcontext_t *dcontext, HANDLE phandle, uint64 env_ptr, bool peb_i
* first thread). Retrieves context from thread handle.
*/
static bool
not_first_thread_in_new_process(HANDLE process_handle, HANDLE thread_handle)
not_first_thread_in_new_process(dcontext_t *dcontext, HANDLE process_handle,
HANDLE thread_handle)
{
#ifndef X64
bool peb_is_32 = is_32bit_process(process_handle);
Expand All @@ -1739,12 +1740,12 @@ not_first_thread_in_new_process(HANDLE process_handle, HANDLE thread_handle)
#endif
DWORD cxt_flags = CONTEXT_DR_STATE;
size_t bufsz = nt_get_context_size(cxt_flags);
char *buf = (char *)global_heap_alloc(bufsz HEAPACCT(ACCT_THREAD_MGT));
char *buf = (char *)heap_alloc(dcontext, bufsz HEAPACCT(ACCT_THREAD_MGT));
CONTEXT *cxt = nt_initialize_context(buf, bufsz, cxt_flags);
bool res = false;
if (NT_SUCCESS(nt_get_context(thread_handle, cxt)))
res = !is_first_thread_in_new_process(process_handle, cxt);
global_heap_free(buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
heap_free(dcontext, buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
return res;
}

Expand Down Expand Up @@ -1853,7 +1854,7 @@ presys_ResumeThread(dcontext_t *dcontext, reg_t *param_base)
"Not injecting so not setting DR env vars in pid=" PIFX "\n", pid);
return;
}
if (not_first_thread_in_new_process(process_handle, thread_handle)) {
if (not_first_thread_in_new_process(dcontext, process_handle, thread_handle)) {
LOG(THREAD, LOG_SYSCALLS, 1,
"Not first thread so not setting DR env vars in pid=" PIFX "\n", pid);
return;
Expand Down Expand Up @@ -2091,7 +2092,7 @@ presys_SetContextThread(dcontext_t *dcontext, reg_t *param_base)
*/
DWORD cxt_flags = CONTEXT_DR_STATE;
size_t bufsz = nt_get_context_size(cxt_flags);
char *buf = (char *)global_heap_alloc(bufsz HEAPACCT(ACCT_THREAD_MGT));
char *buf = (char *)heap_alloc(dcontext, bufsz HEAPACCT(ACCT_THREAD_MGT));
CONTEXT *alt_cxt = nt_initialize_context(buf, bufsz, cxt_flags);
STATS_INC(num_app_setcontext_no_control);
if (thread_get_context(tr, alt_cxt) &&
Expand Down Expand Up @@ -2119,7 +2120,7 @@ presys_SetContextThread(dcontext_t *dcontext, reg_t *param_base)
intercept = false;
ASSERT_NOT_REACHED();
}
global_heap_free(buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
heap_free(dcontext, buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
}
if (intercept) {
/* modify the being-set cxt so that we retain control */
Expand Down Expand Up @@ -3269,7 +3270,7 @@ postsys_CreateUserProcess(dcontext_t *dcontext, reg_t *param_base, bool success)
}
DWORD cxt_flags = CONTEXT_DR_STATE;
size_t bufsz = nt_get_context_size(cxt_flags);
char *buf = (char *)global_heap_alloc(bufsz HEAPACCT(ACCT_THREAD_MGT));
char *buf = (char *)heap_alloc(dcontext, bufsz HEAPACCT(ACCT_THREAD_MGT));
CONTEXT *context;
CONTEXT *cxt = NULL;
int res;
Expand Down Expand Up @@ -3310,10 +3311,9 @@ postsys_CreateUserProcess(dcontext_t *dcontext, reg_t *param_base, bool success)
ASSERT(cxt != NULL || DYNAMO_OPTION(early_inject)); /* Else, exited above. */
/* Do the actual injection. */
if (!maybe_inject_into_process(dcontext, proc_handle, thread_handle, cxt)) {
global_heap_free(buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
heap_free(dcontext, buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
return;
}
global_heap_free(buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
propagate_options_via_env_vars(dcontext, proc_handle, thread_handle);
if (cxt != NULL) {
/* injection routine is assuming doesn't have to install cxt */
Expand All @@ -3328,6 +3328,7 @@ postsys_CreateUserProcess(dcontext_t *dcontext, reg_t *param_base, bool success)
"Failed to set context of child thread");
}
}
heap_free(dcontext, buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
}

/* NtGetContextThread */
Expand All @@ -3350,7 +3351,7 @@ postsys_GetContextThread(dcontext_t *dcontext, reg_t *param_base, bool success)

DWORD cxt_flags = CONTEXT_DR_STATE;
size_t bufsz = nt_get_context_size(cxt_flags);
char *buf = (char *)global_heap_alloc(bufsz HEAPACCT(ACCT_THREAD_MGT));
char *buf = (char *)heap_alloc(dcontext, bufsz HEAPACCT(ACCT_THREAD_MGT));

/* FIXME : we are going to read/write the context argument which is
* potentially unsafe, since success it must have been readable when
Expand Down Expand Up @@ -3413,7 +3414,7 @@ postsys_GetContextThread(dcontext_t *dcontext, reg_t *param_base, bool success)
* we don't translate other than the pc anyway.
*/
d_r_mutex_unlock(&thread_initexit_lock);
global_heap_free(buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
heap_free(dcontext, buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
return;
}
xlate_cxt = alt_cxt;
Expand Down Expand Up @@ -3483,7 +3484,7 @@ postsys_GetContextThread(dcontext_t *dcontext, reg_t *param_base, bool success)
SELF_PROTECT_LOCAL(trec->dcontext, READONLY);
}
d_r_mutex_unlock(&thread_initexit_lock);
global_heap_free(buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
heap_free(dcontext, buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
}

/* NtSuspendThread */
Expand Down Expand Up @@ -3523,7 +3524,7 @@ postsys_SuspendThread(dcontext_t *dcontext, reg_t *param_base, bool success)
if (!mutex_testlock(&all_threads_lock)) {
DWORD cxt_flags = CONTEXT_DR_STATE;
size_t bufsz = nt_get_context_size(cxt_flags);
char *buf = (char *)global_heap_alloc(bufsz HEAPACCT(ACCT_THREAD_MGT));
char *buf = (char *)heap_alloc(dcontext, bufsz HEAPACCT(ACCT_THREAD_MGT));
CONTEXT *cxt = nt_initialize_context(buf, bufsz, cxt_flags);
thread_record_t *tr;
/* know thread isn't holding any of the locks we will need */
Expand All @@ -3547,12 +3548,12 @@ postsys_SuspendThread(dcontext_t *dcontext, reg_t *param_base, bool success)
LOG(THREAD, LOG_SYNCH, 2,
"SuspendThread suspended thread " TIDFMT " at good place\n", tid);
SELF_PROTECT_LOCAL(tr->dcontext, READONLY);
global_heap_free(buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
heap_free(dcontext, buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
return;
}
SELF_PROTECT_LOCAL(tr->dcontext, READONLY);
}
global_heap_free(buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
heap_free(dcontext, buf, bufsz HEAPACCT(ACCT_THREAD_MGT));
} else {
LOG(THREAD, LOG_SYNCH, 2,
"SuspendThread couldn't get all_threads_lock to test if thread " TIDFMT
Expand Down