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.

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#725: add attach for windows #5075

Merged
merged 218 commits into from
Sep 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
218 commits
Select commit Hold shift + click to select a range
33813aa
i#725: add attach for windows
Sep 2, 2021
a14f62d
style fixes
Sep 7, 2021
0e6c122
fix formatting
Sep 7, 2021
aa91d10
attach documentation
Sep 8, 2021
b17a7ce
use late injection implicitly when attaching
Sep 8, 2021
190a64a
add limitation note
Sep 8, 2021
b5831a1
add attach test
Sep 10, 2021
e1395fe
fix formatting
Sep 10, 2021
1c122be
run test target native
Sep 10, 2021
ed9e6fe
clean test target dr command
Sep 10, 2021
c17cacb
move all dr params to nudge
Sep 10, 2021
7f3ecff
fix nudge_arg
Sep 10, 2021
48ae3a1
test fix
Sep 10, 2021
4bd99cf
test fix2
Sep 10, 2021
6f71095
change target to win32.attachee
Sep 13, 2021
810d158
i#725: add attach for windows
Sep 2, 2021
0a92b19
style fixes
Sep 7, 2021
b68ac76
fix formatting
Sep 7, 2021
54004d7
attach documentation
Sep 8, 2021
297623c
use late injection implicitly when attaching
Sep 8, 2021
65aa1d3
add limitation note
Sep 8, 2021
9c88f8a
add attach test
Sep 10, 2021
b240bb5
fix formatting
Sep 10, 2021
5e3918b
run test target native
Sep 10, 2021
bf46cb3
clean test target dr command
Sep 10, 2021
35b4d3a
move all dr params to nudge
Sep 10, 2021
9627368
fix nudge_arg
Sep 10, 2021
9b5ea65
test fix
Sep 10, 2021
6123e34
test fix2
Sep 10, 2021
587eddc
change target to win32.attachee
Sep 13, 2021
e278ba0
merge branch 'add-windows-attach' of https://github.com/OrBenPorath/d…
Sep 13, 2021
d63c3dc
fix formatting
Sep 13, 2021
41eb20d
fix formatting
Sep 13, 2021
f6491fb
fix formatting
Sep 13, 2021
2d3f73c
fix formatting
Sep 13, 2021
ea7075d
fix name extraction
Sep 13, 2021
803b9b6
fix expected test output
Sep 13, 2021
ed79c08
create thread for injection
Sep 14, 2021
d19ac58
fix conversion warning
Sep 14, 2021
c370933
fix format
Sep 14, 2021
dd6787e
longer sleep in attachee
Sep 14, 2021
8aa5174
comment styling
Sep 14, 2021
9f96719
remove unused var
Sep 14, 2021
59e2b2f
use infloop instead of attachee
Sep 14, 2021
a5be58e
delete attachee
Sep 14, 2021
ad08207
merge master
Sep 14, 2021
a864eed
treat client in attach test
Sep 14, 2021
4059d69
time sync test
Sep 14, 2021
120af3a
run attach in background
Sep 15, 2021
b3c40ce
add comment
Sep 15, 2021
4d653dd
add debug prints
Sep 15, 2021
8c64ddc
add verbose debug prints
Sep 15, 2021
dffe96e
test print output on TO
Sep 15, 2021
e773e2b
test nudge output debug print
Sep 15, 2021
4b8638f
debug direct nudge output to same out file
Sep 15, 2021
a59f3bd
test sleep before attach
Sep 15, 2021
b456527
runall debug prints
Sep 15, 2021
26120cf
debug prints
Sep 15, 2021
36d86f5
debug prints
Sep 15, 2021
ba70942
debug prints
Sep 15, 2021
329a48d
format fix
Sep 15, 2021
accfb2b
remove prints
Sep 15, 2021
aa2c4de
remove prints
Sep 15, 2021
29b6849
restore done message
Sep 16, 2021
afd6fa5
Merge branch 'master' into add-windows-attach
Sep 16, 2021
5d17e95
debug prints
Sep 16, 2021
1d28f79
debug prints
Sep 16, 2021
61a6806
debug prints
Sep 16, 2021
1d9e5cf
debug prints
Sep 16, 2021
d73b411
debug remove workflow
Sep 16, 2021
9d9f158
debug remove workflow
Sep 16, 2021
4c80ef6
debug remove workflow
Sep 16, 2021
2a52ee7
restore workflows
Sep 16, 2021
f49c55b
Merge branch 'master' into add-windows-attach
Sep 16, 2021
dfd7aa2
debug prints
Sep 16, 2021
59a0bca
merge master
Sep 16, 2021
87bc182
debug prints
Sep 16, 2021
40b2e3c
fix format
Sep 16, 2021
84f0d94
fix format
Sep 16, 2021
c5fb1cb
debug prints
Sep 16, 2021
d9d8397
debug prints
Sep 16, 2021
5a80d47
debug prints
Sep 16, 2021
7d3d7d3
find sleep address dynamically
Sep 16, 2021
db27625
fix format
Sep 16, 2021
ece642a
make find_remote_dll_base not static
Sep 16, 2021
1e2d514
Merge branch 'master' into add-windows-attach
Sep 16, 2021
a277d9b
make find_remote_dll_base case insensitive
Sep 17, 2021
5f94a61
fix format
Sep 17, 2021
60e922a
debug print
Sep 17, 2021
7221081
tmate
Sep 17, 2021
91d8871
tmate
Sep 17, 2021
f8d0d26
tmate
Sep 17, 2021
3a0f85c
tmate
Sep 17, 2021
a7c06ce
tmate
Sep 17, 2021
372ac42
debug try sleep 0
Sep 17, 2021
fccef32
fix format
Sep 17, 2021
a6ddcbf
fix CreateRemoteThread call
Sep 17, 2021
3fc4382
fix CreateRemoteThread call
Sep 17, 2021
7a61fc1
Merge branch 'master' into add-windows-attach
Sep 17, 2021
3ba05bd
fix format
Sep 17, 2021
38c36c2
cleanup
Sep 17, 2021
b255d54
tmate
Sep 17, 2021
26a60cc
run test
Sep 18, 2021
1178999
register exit event before writing to output
Sep 18, 2021
64e285f
remove tmate
Sep 18, 2021
144e343
add deployment note for linux
Sep 18, 2021
80a66f4
add deployment note for linux
Sep 18, 2021
8d2e6ee
test
Sep 18, 2021
64eff60
test
Sep 18, 2021
bd075d2
debug print
Sep 18, 2021
53e2c0d
tmate
Sep 18, 2021
ef1fe09
test
Sep 18, 2021
fafe09f
test
Sep 18, 2021
f21c03d
test
Sep 18, 2021
9d65948
allow multi-thread exit
Sep 18, 2021
814bb84
wait for newline after attach
Sep 18, 2021
69e28f9
allow multi-thread exit
Sep 18, 2021
bd12962
Merge branch 'master' into add-windows-attach
Sep 18, 2021
5dfff56
test
Sep 18, 2021
6309d9c
Merge branch 'master' into add-windows-attach
Sep 18, 2021
ce1ba11
tmate
Sep 18, 2021
c057c8f
test
Sep 18, 2021
a094f15
test
Sep 18, 2021
f8e192c
test
Sep 18, 2021
34efad1
debug: add thread prints
Sep 19, 2021
a6a483f
debug: add thread prints
Sep 19, 2021
7339837
wait for thread takeover
Sep 19, 2021
b3e922d
wait for thread takeover
Sep 19, 2021
ce2567e
report takeover once
Sep 19, 2021
c621abb
test
Sep 19, 2021
35616d0
remove tmate
Sep 19, 2021
d8fe564
test
Sep 19, 2021
e7a3aad
cleanup
Sep 19, 2021
e46101d
add multi_thread_exit
Sep 19, 2021
7a085af
wait for modules load
Sep 19, 2021
d011ed6
wait for modules load
Sep 19, 2021
f08ccf5
wait for modules load
Sep 19, 2021
fe71092
remove modules wait
Sep 19, 2021
aa7076f
tmate
Sep 19, 2021
16fb008
wait for both threads
Sep 20, 2021
c49d24d
tmate
Sep 20, 2021
414d98f
test
Sep 20, 2021
7fe6225
debug prints
Sep 20, 2021
0d328f8
debug prints
Sep 20, 2021
a624caf
debug prints
Sep 20, 2021
dace614
test
Sep 20, 2021
b104f8d
test
Sep 20, 2021
6acf667
debug prints
Sep 21, 2021
7ef77d4
debug print
Sep 21, 2021
e01450a
Merge branch 'master' into add-windows-attach
Sep 21, 2021
e37d6a1
debug print
Sep 21, 2021
1a03662
sleep between takeovers
Sep 21, 2021
2e9610b
test
Sep 21, 2021
30f0b44
debug print
Sep 21, 2021
90f92c8
debug print
Sep 21, 2021
9950805
debug - add sleep
Sep 21, 2021
0f72795
debug - add sleep
Sep 21, 2021
70580b1
Merge branch 'master' into add-windows-attach
Sep 21, 2021
6b2b253
test
Sep 21, 2021
e989487
debug - remove workflows
Sep 21, 2021
54c9a2d
test
Sep 21, 2021
ee83360
debug print
Sep 21, 2021
54e67ea
debug print
Sep 21, 2021
963a780
test
Sep 21, 2021
ec949d8
debug - remove other tests
Sep 21, 2021
f4ffe77
debug - remove other tests
Sep 21, 2021
749d26e
debug - restore other tests
Sep 21, 2021
df2f5b5
Merge branch 'master' into add-windows-attach
Sep 22, 2021
18523f0
debug print
Sep 22, 2021
6bff993
debug - add sleep
Sep 22, 2021
d163d41
debug - add sleep
Sep 22, 2021
7b6e4c9
test
Sep 22, 2021
72f3c0a
debug - add sleep
Sep 22, 2021
6ea5c52
debug - add sleep
Sep 22, 2021
028b220
test
Sep 22, 2021
f36e351
debug print
Sep 22, 2021
836f664
debug print
Sep 22, 2021
9acba5b
debug print
Sep 22, 2021
17f4769
test
Sep 22, 2021
58aa93e
add FIXME for find_remote_dll_base
Sep 22, 2021
5d387e4
change sleep
Sep 22, 2021
955d1c9
fix format
Sep 22, 2021
1b4f4c1
restore workflows
Sep 22, 2021
fd0b2c5
test
Sep 22, 2021
9695c9f
Merge branch 'master' into add-windows-attach
Sep 22, 2021
cba58d2
remove tmate
Sep 22, 2021
972eeca
use attachee
Sep 22, 2021
6760cf7
fix format
Sep 22, 2021
19d8d1a
fix attach_test.template
Sep 22, 2021
28b7ce2
test
Sep 22, 2021
d481517
wait for pidfile content
Sep 22, 2021
203cc5b
fix infinite loop in find_remote_dll_base
Sep 22, 2021
6cbb4a8
add remark for attachee instead of infloop
Sep 22, 2021
2595ffc
waith for thread init
Sep 22, 2021
b961748
waith for thread init
Sep 22, 2021
12a61c1
test
Sep 23, 2021
eb5dada
test
Sep 23, 2021
cc9171b
test
Sep 23, 2021
7d910c6
test
Sep 23, 2021
916110c
test
Sep 23, 2021
57b10dd
test
Sep 23, 2021
384edf6
test
Sep 23, 2021
b5917a1
test
Sep 23, 2021
4d17238
Merge branch 'master' into add-windows-attach
Sep 23, 2021
23d5fbd
print on exception
Sep 23, 2021
7f34d6d
print on exception
Sep 23, 2021
0bc9fbd
print on exception
Sep 23, 2021
c07fc17
print on exception
Sep 23, 2021
c91494b
print on exception
Sep 23, 2021
be0de15
fix format
Sep 23, 2021
8698c55
print on exception
Sep 23, 2021
bbf4720
test
Sep 23, 2021
84b4f03
test
Sep 23, 2021
06cf8c7
test
Sep 23, 2021
5128890
test
Sep 23, 2021
cb3de86
test
Sep 23, 2021
3e889dc
test
Sep 23, 2021
96304dd
mark test as flaky
Sep 23, 2021
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
13 changes: 13 additions & 0 deletions api/docs/deployment.dox
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,12 @@ bbsize sample client using the following configure-and-run command:
bin32/drrun.exe -c samples/bin32/bbsize.dll -- notepad
\endcode

Alternatively, you can first run \c notepad.exe, and then use \c drrun
to attach to it:
\code
bin32/drrun.exe -attach <notepad_pid> -c samples/bin32/bbsize.dll
\endcode
derekbruening marked this conversation as resolved.
Show resolved Hide resolved

derekbruening marked this conversation as resolved.
Show resolved Hide resolved
To use system-wide injection, allowing for an application to be run
under DynamoRIO regardless of how it is invoked, configure the application
first (-syswide_on requires administrative privileges):
Expand Down Expand Up @@ -307,6 +313,13 @@ under DynamoRIO with the bbsize sample client:
\code
% bin32/drrun -c samples/bin32/libbbsize.so -- ls
\endcode

Alternatively, you can first run the target, and then use \c drrun
to attach to it:
\code
% bin32/drrun -attach <target_pid> -c samples/bin32/libbbsize.so
\endcode

Run \c drrun with no options to get a list of the options and
environment variable shortcuts it supports. To disable following across
child execve calls, use the \ref op_children "-no_follow_children" runtime
Expand Down
6 changes: 6 additions & 0 deletions api/docs/release.dox
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ Further non-compatibility-affecting changes include:
- Added \p drstatecmp Extension which provides mechanisms to enable systematic
and exhaustive machine state comparisons across instrumentation sequences.
- Added drmodtrack_lookup_segment().
- Added a new drrun option \p -attach for attaching to a running process.

**************************************************
<hr>
Expand Down Expand Up @@ -1754,6 +1755,11 @@ I/O and nudges.
through the Windows-On-Windows or WOW64 emulator so system call and
indirect call processing clients must be aware of
#instr_is_wow64_syscall().
- On all versions of Windows, attaching DynamoRIO to an already-running
process can result in loss of control if the attach point is in the
middle of an operating system event callback. From the callback return
point until the next system call hook, no instructions will be observed
by a client.
\anchor limits_64bit
- This release of DynamoRIO supports running
64-bit Windows applications, using the 64-bit DynamoRIO build, on
Expand Down
7 changes: 4 additions & 3 deletions core/dynamo.c
Original file line number Diff line number Diff line change
Expand Up @@ -2880,8 +2880,6 @@ dynamo_thread_not_under_dynamo(dcontext_t *dcontext)
#endif
}

#define MAX_TAKE_OVER_ATTEMPTS 8

/* Mark this thread as under DR, and take over other threads in the current process.
*/
void
Expand All @@ -2892,6 +2890,7 @@ dynamorio_take_over_threads(dcontext_t *dcontext)
*/
bool found_threads;
uint attempts = 0;
uint max_takeover_attempts = DYNAMO_OPTION(takeover_attempts);

os_process_under_dynamorio_initiate(dcontext);
/* We can start this thread now that we've set up process-wide actions such
Expand All @@ -2912,7 +2911,9 @@ dynamorio_take_over_threads(dcontext_t *dcontext)
attempts++;
if (found_threads && !bb_lock_start)
bb_lock_start = true;
} while (found_threads && attempts < MAX_TAKE_OVER_ATTEMPTS);
if (DYNAMO_OPTION(sleep_between_takeovers))
os_thread_sleep(1);
} while (found_threads && attempts < max_takeover_attempts);
os_process_under_dynamorio_complete(dcontext);
/* End the barrier to new threads. */
signal_event(dr_attach_finished);
Expand Down
18 changes: 18 additions & 0 deletions core/lib/dr_inject.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,24 @@ DR_EXPORT
int
dr_inject_process_create(const char *app_name, const char **app_cmdline, void **data);

#ifdef WINDOWS
DR_EXPORT
/**
* Attach to an existing process.
*
* \param[in] pid PID for process to attach.
*
* \param[out] data An opaque pointer that should be passed to
* subsequent dr_inject_* routines to refer to
* this process.
* \param[out] app_name Pointer to the name of the target process.
* Only valid until dr_inject_process_exit.
* \return Returns 0 on success. On failure, returns a system error code.`
*/
int
dr_inject_process_attach(process_id_t pid, void **data, char **app_name);
#endif

#ifdef UNIX

DR_EXPORT
Expand Down
10 changes: 10 additions & 0 deletions core/optionsx.h
Original file line number Diff line number Diff line change
Expand Up @@ -2633,6 +2633,16 @@ OPTION_COMMAND(bool, native_exec_opt, false, "native_exec_opt",
"optimize control flow transition between native and non-native modules",
STATIC, OP_PCACHE_GLOBAL)

#ifdef WINDOWS
OPTION_DEFAULT(bool, skip_terminating_threads, false,
"do not takeover threads that are terminating")
#endif

OPTION_DEFAULT(bool, sleep_between_takeovers, false,
"sleep between takeover attempts to allow threads to exit syscalls")

OPTION_DEFAULT(uint, takeover_attempts, 8, "number of takeover attempts")

/* vestiges from our previous life as a dynamic optimizer */
OPTION_DEFAULT_INTERNAL(bool, inline_calls, true, "inline calls in traces")

Expand Down
39 changes: 2 additions & 37 deletions core/win32/inject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1139,41 +1139,6 @@ generate_switch_mode_jmp_to_hook(HANDLE phandle, byte *local_code_buf,
}
#endif

static uint64
find_remote_ntdll_base(HANDLE phandle, bool find64bit)
{
MEMORY_BASIC_INFORMATION64 mbi;
uint64 got;
NTSTATUS res;
uint64 addr = 0;
char name[MAXIMUM_PATH];
do {
res = remote_query_virtual_memory_maybe64(phandle, addr, &mbi, sizeof(mbi), &got);
if (got != sizeof(mbi) || !NT_SUCCESS(res))
break;
#if VERBOSE
print_file(STDERR, "0x%I64x-0x%I64x type=0x%x state=0x%x\n", mbi.BaseAddress,
mbi.BaseAddress + mbi.RegionSize, mbi.Type, mbi.State);
#endif
if (mbi.Type == MEM_IMAGE && mbi.BaseAddress == mbi.AllocationBase) {
bool is_64;
if (get_remote_dll_short_name(phandle, mbi.BaseAddress, name,
BUFFER_SIZE_ELEMENTS(name), &is_64)) {
#if VERBOSE
print_file(STDERR, "found |%s| @ 0x%I64x 64=%d\n", name, mbi.BaseAddress,
is_64);
#endif
if (strcmp(name, "ntdll.dll") == 0 && BOOLS_MATCH(find64bit, is_64))
return mbi.BaseAddress;
}
}
if (addr + mbi.RegionSize < addr)
break;
addr += mbi.RegionSize;
} while (true);
return 0;
}

static uint64
inject_gencode_mapped_helper(HANDLE phandle, char *dynamo_path, uint64 hook_location,
byte hook_buf[EARLY_INJECT_HOOK_SIZE], byte *map,
Expand Down Expand Up @@ -1235,7 +1200,7 @@ inject_gencode_mapped_helper(HANDLE phandle, char *dynamo_path, uint64 hook_loca

/* see below on why it's easier to point at args in memory */
args.dr_base = (uint64)map;
args.ntdll_base = find_remote_ntdll_base(phandle, target_64);
args.ntdll_base = find_remote_dll_base(phandle, target_64, "ntdll.dll");
if (args.ntdll_base == 0)
goto error;
args.tofree_base = remote_code_buf;
Expand Down Expand Up @@ -1572,7 +1537,7 @@ inject_into_new_process(HANDLE phandle, HANDLE thandle, char *dynamo_path, bool
}
if (hook_location == 0) {
bool target_64 = !x86_code IF_X64(|| DYNAMO_OPTION(inject_x64));
uint64 ntdll_base = find_remote_ntdll_base(phandle, target_64);
uint64 ntdll_base = find_remote_dll_base(phandle, target_64, "ntdll.dll");
uint64 thread_start =
get_remote_proc_address(phandle, ntdll_base, "RtlUserThreadStart");
if (thread_start != 0)
Expand Down
115 changes: 115 additions & 0 deletions core/win32/injector.c
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ typedef struct _dr_inject_info_t {
PROCESS_INFORMATION pi;
bool using_debugger_injection;
bool using_thread_injection;
bool attached;
TCHAR wimage_name[MAXIMUM_PATH];
/* We need something to point at for dr_inject_get_image_name so we just
* keep a utf8 buffer as well.
Expand Down Expand Up @@ -843,6 +844,116 @@ dr_inject_process_create(const char *app_name, const char **argv, void **data OU
return errcode;
}

static int
create_attach_thread(HANDLE process_handle IN, PHANDLE thread_handle OUT, PDWORD tid OUT)
{
uint64 kernel32;
uint64 sleep_address;
bool target_is_32;

*thread_handle = NULL;
*tid = 0;

target_is_32 = is_32bit_process(process_handle);
kernel32 = find_remote_dll_base(process_handle, !target_is_32, "kernel32.dll");
if (kernel32 == 0) {
return ERROR_INVALID_PARAMETER;
}

sleep_address = get_remote_proc_address(process_handle, kernel32, "SleepEx");
if (sleep_address == 0) {
return ERROR_INVALID_PARAMETER;
}

*thread_handle = CreateRemoteThread(process_handle, NULL, 0,
(LPTHREAD_START_ROUTINE)(SIZE_T)sleep_address,
(LPVOID)(SIZE_T)INFINITE, CREATE_SUSPENDED, tid);
if (*thread_handle == NULL) {
return GetLastError();
}

return ERROR_SUCCESS;
}

DYNAMORIO_EXPORT
int
dr_inject_process_attach(process_id_t pid, void **data OUT, char **app_name OUT)
{
dr_inject_info_t *info = HeapAlloc(GetProcessHeap(), 0, sizeof(*info));
memset(info, 0, sizeof(*info));
bool received_initial_debug_event = false;
DEBUG_EVENT dbgevt = { 0 };
wchar_t exe_path[MAX_PATH];
DWORD exe_path_size = MAX_PATH;
wchar_t *exe_name;
HANDLE process_handle;
int res;

*data = info;

if (DebugActiveProcess((DWORD)pid) == false) {
return GetLastError();
}

DebugSetProcessKillOnExit(false);

info->using_debugger_injection = false;
info->attached = true;

do {
dbgevt.dwProcessId = (DWORD)pid;
WaitForDebugEvent(&dbgevt, INFINITE);
ContinueDebugEvent(dbgevt.dwProcessId, dbgevt.dwThreadId, DBG_CONTINUE);

if (dbgevt.dwDebugEventCode == CREATE_PROCESS_DEBUG_EVENT) {
received_initial_debug_event = true;
}
} while (received_initial_debug_event == false);

info->pi.dwProcessId = dbgevt.dwProcessId;

DuplicateHandle(GetCurrentProcess(), dbgevt.u.CreateProcessInfo.hProcess,
GetCurrentProcess(), &info->pi.hProcess, 0, FALSE,
DUPLICATE_SAME_ACCESS);

process_handle = info->pi.hProcess;

/* XXX i#725: Attach does not begin as long as the injected thread is blocking.
* To overcome it, We create a new thread in the target process that will live
* as long as the target lives, and inject into it.
* For better transparency we should exit the thread immediately after injection.
* Would require changing termination assumptions in win32/syscall.c.
*/
res = create_attach_thread(process_handle, &info->pi.hThread, &info->pi.dwThreadId);
if (res != ERROR_SUCCESS) {
return res;
}

BOOL(__stdcall * query_full_process_image_name_w)
(HANDLE, DWORD, LPWSTR, PDWORD) = (BOOL(__stdcall *)(HANDLE, DWORD, LPWSTR, PDWORD))(
GetProcAddress(GetModuleHandle(TEXT("Kernel32")), "QueryFullProcessImageNameW"));

if (query_full_process_image_name_w(process_handle, 0, exe_path, &exe_path_size) ==
0) {
return GetLastError();
}

exe_name = wcsrchr(exe_path, '\\');
if (exe_name == NULL) {
return ERROR_INVALID_PARAMETER;
}

wchar_to_char(info->image_name, BUFFER_SIZE_ELEMENTS(info->image_name), exe_name,
wcslen(exe_name) * sizeof(wchar_t));

char_to_tchar(info->image_name, info->wimage_name,
BUFFER_SIZE_ELEMENTS(info->image_name));

*app_name = info->image_name;

return ERROR_SUCCESS;
}

DYNAMORIO_EXPORT
bool
dr_inject_use_late_injection(void *data)
Expand Down Expand Up @@ -954,6 +1065,10 @@ bool
dr_inject_process_run(void *data)
{
dr_inject_info_t *info = (dr_inject_info_t *)data;
if (info->attached == true) {
/* detach the debugger */
DebugActiveProcessStop(info->pi.dwProcessId);
}
/* resume the suspended app process so its main thread can run */
ResumeThread(info->pi.hThread);
close_handle(info->pi.hThread);
Expand Down
42 changes: 42 additions & 0 deletions core/win32/module_shared.c
Original file line number Diff line number Diff line change
Expand Up @@ -1326,6 +1326,48 @@ read_remote_maybe64(HANDLE process, uint64 addr, size_t bufsz, void *buf)
num_read == bufsz;
}

uint64
find_remote_dll_base(HANDLE phandle, bool find64bit, char *dll_name)
{
MEMORY_BASIC_INFORMATION64 mbi;
uint64 got;
NTSTATUS res;
uint64 addr = 0;
char name[MAXIMUM_PATH];
do {
res = remote_query_virtual_memory_maybe64(phandle, addr, &mbi, sizeof(mbi), &got);
if (got != sizeof(mbi) || !NT_SUCCESS(res))
break;
# if VERBOSE
print_file(STDERR, "0x%I64x-0x%I64x type=0x%x state=0x%x\n", mbi.BaseAddress,
mbi.BaseAddress + mbi.RegionSize, mbi.Type, mbi.State);
# endif
if (mbi.Type == MEM_IMAGE && mbi.BaseAddress == mbi.AllocationBase) {
bool is_64;
if (get_remote_dll_short_name(phandle, mbi.BaseAddress, name,
BUFFER_SIZE_ELEMENTS(name), &is_64)) {
# if VERBOSE
print_file(STDERR, "found |%s| @ 0x%I64x 64=%d\n", name, mbi.BaseAddress,
is_64);
# endif
if (_strcmpi(name, dll_name) == 0 && BOOLS_MATCH(find64bit, is_64))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It also compares with _strcmpi because kernel32 was loaded upper case.
When I used strcmp and did not find anything, find_remote_dll_base would go into an infinite loop - I did not check the root cause of it.

You mean the outer loop never terminated? I'm not seeing how that could happen -- mbi.RegionSize being incorrect?? But we have many other loops like this.

It does not cause any issues now, because it is only used with ntdll and kernel32 which are always loaded. But now that it accepts general DLLs, I think it should be fixed.

No kernel32 is not always loaded: early on in the life of a process there's only ntdll, and DR's earliest injection takes over well before kernel32 is loaded, so you could imagine this function being called then by DR code. (And I think it's possible to create a process that never uses kernel32 but a pathological case for sure.)

But maybe not worth effort at this point

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean the outer loop never terminated? I'm not seeing how that could happen -- mbi.RegionSize being incorrect?? But we have many other loops like this.

Output was something like this:

...
found |wow64.dll| @ 0x7ffca0d40000 64=1
0x7ffca0d41000-0x7ffca0d7a000 type=0x1000000 state=0x1000
0x7ffca0d7a000-0x7ffca0d8f000 type=0x1000000 state=0x1000
0x7ffca0d8f000-0x7ffca0d91000 type=0x1000000 state=0x1000
0x7ffca0d91000-0x7ffca0d99000 type=0x1000000 state=0x1000
0x7ffca0d99000-0x7ffca1ad0000 type=0x0 state=0x10000
0x7ffca1ad0000-0x7ffca1ad1000 type=0x1000000 state=0x1000
found |ntdll.dll| @ 0x7ffca1ad0000 64=1
0x7ffca1ad1000-0x7ffca1bec000 type=0x1000000 state=0x1000
0x7ffca1bec000-0x7ffca1c34000 type=0x1000000 state=0x1000
0x7ffca1c34000-0x7ffca1c35000 type=0x1000000 state=0x1000
0x7ffca1c35000-0x7ffca1c37000 type=0x1000000 state=0x1000
0x7ffca1c37000-0x7ffca1c40000 type=0x1000000 state=0x1000
0x7ffca1c40000-0x7ffca1cc5000 type=0x1000000 state=0x1000
0x7ffca1cc5000-0x7fffffff0000 type=0x0 state=0x10000
0x7ffca1cc5000-0x7fffffff0000 type=0x0 state=0x10000
0x7ffca1cc5000-0x7fffffff0000 type=0x0 state=0x10000
0x7ffca1cc5000-0x7fffffff0000 type=0x0 state=0x10000
0x7ffca1cc5000-0x7fffffff0000 type=0x0 state=0x10000
0x7ffca1cc5000-0x7fffffff0000 type=0x0 state=0x10000
0x7ffca1cc5000-0x7fffffff0000 type=0x0 state=0x10000
0x7ffca1cc5000-0x7fffffff0000 type=0x0 state=0x10000
0x7ffca1cc5000-0x7fffffff0000 type=0x0 state=0x10000
0x7ffca1cc5000-0x7fffffff0000 type=0x0 state=0x10000
...
...
...

No kernel32 is not always loaded: early on in the life of a process there's only ntdll, and DR's earliest injection takes over well before kernel32 is loaded, so you could imagine this function being called then by DR code. (And I think it's possible to create a process that never uses kernel32 but a pathological case for sure.)

But maybe not worth effort at this point

I see. But for now we only use kernel32 for attach, so maybe it is ok to assume it is loaded? (though clearly not the best)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add a XXX or TODO or even FIXME comment to that loop explaining the infinite loop you saw.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do that.

In the meanwhile, I added a print of the address:

...
found |ntdll.dll| @ 0x7ffca1ad0000 64=1
calling remote_query_virtual_memory_maybe64, addr=0x7ffca1ad1000
0x7ffca1ad1000-0x7ffca1bec000 size=0x11b000 type=0x1000000 state=0x1000
calling remote_query_virtual_memory_maybe64, addr=0x7ffca1bec000
0x7ffca1bec000-0x7ffca1c34000 size=0x48000 type=0x1000000 state=0x1000
calling remote_query_virtual_memory_maybe64, addr=0x7ffca1c34000
0x7ffca1c34000-0x7ffca1c35000 size=0x1000 type=0x1000000 state=0x1000
calling remote_query_virtual_memory_maybe64, addr=0x7ffca1c35000
0x7ffca1c35000-0x7ffca1c37000 size=0x2000 type=0x1000000 state=0x1000
calling remote_query_virtual_memory_maybe64, addr=0x7ffca1c37000
0x7ffca1c37000-0x7ffca1c40000 size=0x9000 type=0x1000000 state=0x1000
calling remote_query_virtual_memory_maybe64, addr=0x7ffca1c40000
0x7ffca1c40000-0x7ffca1cc5000 size=0x85000 type=0x1000000 state=0x1000
calling remote_query_virtual_memory_maybe64, addr=0x7ffca1cc5000
0x7ffca1cc5000-0x7fffffff0000 size=0x35e32b000 type=0x0 state=0x10000
calling remote_query_virtual_memory_maybe64, addr=0x7fffffff0000
0x7ffca1cc5000-0x7fffffff0000 size=0x35e32b000 type=0x0 state=0x10000
calling remote_query_virtual_memory_maybe64, addr=0x80035e31b000
0x7ffca1cc5000-0x7fffffff0000 size=0x35e32b000 type=0x0 state=0x10000
calling remote_query_virtual_memory_maybe64, addr=0x8006bc646000
0x7ffca1cc5000-0x7fffffff0000 size=0x35e32b000 type=0x0 state=0x10000
calling remote_query_virtual_memory_maybe64, addr=0x800a1a971000
0x7ffca1cc5000-0x7fffffff0000 size=0x35e32b000 type=0x0 state=0x10000
...

So something weird happens when the address passes 0x800000000000.
Maybe just add

if (mbi.BaseAddress + mbi.RegionSize < addr)
    break;

WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this infinite loop only happening for 32-bit (WOW64)? The drrun and the target are the same bitwidth?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I only tested on a 64 bit machine: it only happens for 32-bit (both the drrun and infloop).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do remember some weird Windows kernel behavior with memory query walks but not quite this one.
The one I'm thinking of is the AllocationBase: DynamoRIO/drmemory#2328 (comment)

return mbi.BaseAddress;
}
}
if (addr + mbi.RegionSize < addr)
break;
/* XXX - this check is needed because otherwise, for 32-bit targets on a 64
* bit machine, this loop doesn't return if the dll is not loaded.
* When addr passes 0x800000000000 remote_query_virtual_memory_maybe64
* returns the previous mbi (ending at 0x7FFFFFFF0000).
* For now just return if the addr is not inside the mbi region. */
if (mbi.BaseAddress + mbi.RegionSize < addr)
break;
addr += mbi.RegionSize;
} while (true);
return 0;
}

/* Handles 32-bit or 64-bit remote processes.
* Ignores forwarders and ordinals.
*/
Expand Down
15 changes: 15 additions & 0 deletions core/win32/ntdll.c
Original file line number Diff line number Diff line change
Expand Up @@ -2383,6 +2383,21 @@ nt_set_context(HANDLE hthread, CONTEXT *cxt)
return NT_SYSCALL(SetContextThread, hthread, cxt);
}

bool
nt_is_thread_terminating(HANDLE hthread)
{
ULONG previous_suspend_count;
NTSTATUS res;
GET_RAW_SYSCALL(SuspendThread, IN HANDLE ThreadHandle,
OUT PULONG PreviousSuspendCount OPTIONAL);
res = NT_SYSCALL(SuspendThread, hthread, &previous_suspend_count);
if (NT_SUCCESS(res)) {
nt_thread_resume(hthread, (int *)&previous_suspend_count);
}

return res == STATUS_THREAD_IS_TERMINATING;
}

bool
nt_thread_suspend(HANDLE hthread, int *previous_suspend_count)
{
Expand Down
Loading