i#725: Add attach for windows (#5075)

Adds an attach feature for Windows, marked as experimental.

Builds on the unsubmitted PR #3328.
One difference from the original PR, is not taking over threads that are terminating (otherwise attach always fails).
Added the possibility to sleep 1 millisecond between takeover attempts, and controlling the number of attempts.

Attach fails when the main thread (to which we are injecting) is blocking.  To solve this, we create a new suspended thread that sleeps indefinitely and we inject into it.  As part of pointing this thread at a Sleep function, generalizes  find_remote_ntdll_base() to find_remote_dll_base(), and solves an infinite loop there coming from strange kernel behavior.

Adds attach documentation to the section on how to deploy an application under DR.

Adds a new test client.attach.  It uses a new target rather than the existing infloop due to differences in staging the output checking steps.  Unfortunately there are still some flaky failures, so the test is added to the ignore list for now in order to make progress and get this key feature into DR.

Co-authored-by: Yibai Zhang <xm1994@gmail.com>
Co-authored-by: orbp <obporathl@gmail.com>

Issue: #725

api/docs/deployment.dox

@@ -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
+
 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):
@@ -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

api/docs/release.dox

@@ -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>
@@ -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

core/dynamo.c

@@ -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
@@ -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
@@ -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);

core/lib/dr_inject.h

@@ -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

core/optionsx.h

@@ -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")
 

core/win32/inject.c

@@ -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,
@@ -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;
@@ -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)

core/win32/injector.c

@@ -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.
@@ -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)
@@ -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);

core/win32/module_shared.c

@@ -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))
+                    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.
  */

core/win32/ntdll.c

@@ -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)
 {