Implement attach for both windows and linux #3328

summershrimp · 2018-12-29T03:39:58Z

For linux, we use ptrace() to inject dynamorio to running process.
For windows, we get target process handle using DebugActiveProcess()
and inject with original dr_inject_process_inject().

For linux, we use ptrace() to inject dynamorio to running process. For windows, we get target process handle using DebugActiveProcess() and inject with original dr_inject_process_inject().

summershrimp · 2018-12-29T03:41:13Z

I think it's only working on x86/x64 since inject_ptrace is not available for arm

derekbruening · 2019-01-02T04:26:07Z

Thank you for the patch. Just letting you know that I will look at it soon: been on vacation.

derekbruening

Thank you for working on this -- it is exciting to finally finish off an attach feature!

Sorry for the delay in reviewing it. I do have a bunch of suggestions to improve the patch before it goes in.

derekbruening · 2019-01-09T16:46:13Z

core/CMakeLists.txt

@@ -278,9 +278,9 @@ endif (WIN32)

 if (WIN32)
  if (DEBUG)
-    set(WIN32_C_LIB libcmtd)
+    set(WIN32_C_LIB libcmtd Psapi)


This psapi dependence should not be added here since this is linked into the core DR library. We do not want the core DR library to have extra dependences: it will fail to function with early injection and will hit transparency issues in general. Please see the transparency slides in the DR tutorial and http://dynamorio.org/docs/transparency.html#sec_trans_resource. Please only link this with non-core targets like drinjectlib.

core/lib/dr_inject.h

derekbruening · 2019-01-09T21:24:45Z

core/win32/injector.c

+            return ERROR_INVALID_PARAMETER;
+        }
+
+        strcpy(info->image_name, pExeName + 1);


strcpy is not allowed: use strncpy and NULL_TERMINATE_BUFFER

derekbruening · 2019-01-09T21:25:45Z

core/win32/injector.c

+        }
+        char szExePath[MAX_PATH];
+        char *pExeName = NULL;
+        GetModuleFileNameExA(dbgevt.u.CreateProcessInfo.hProcess, NULL, szExePath,


We should use the ExW version to properly handle non-ascii chars

derekbruening · 2019-01-09T21:28:44Z

tools/drdeploy.c

    "       -logdir <dir>      Logfiles will be stored in this directory.\n"
 #    endif
+    "       -attach <pid>      Attach to the process with the given pid.  Pass 0\n"


The big problem with attach on Windows is that if we attach when a thread is inside a Windows callback we will lose control when it goes back into the kernel, since there is no way to know where it will resume. We have some syscall hooks to try and regain control but there will at least be a window where that thread is native. We should document this: maybe label Windows attach support as "experimental" or describe this issue.

derekbruening · 2019-01-09T21:30:21Z

tools/drdeploy.c

    "       -logdir <dir>      Logfiles will be stored in this directory.\n"
 #    endif
+    "       -attach <pid>      Attach to the process with the given pid.  Pass 0\n"


Maybe the main documentation ("page_deploy" in intro.dox) should talk about attaching as a supported feature?

It should be listed in the changelog in release.dox for sure as a major feature.

derekbruening · 2019-01-09T21:33:31Z

tools/drdeploy.c

+    if (attach_pid != 0) {
+        errcode = dr_inject_process_attach(attach_pid, &inject_data);
+    } else
+#    endif /* WINDOWS */


Would it be possible to write some simple tests of these new attach features? It will take a little more work than adding a regular test because you'll need to launch something in the background. See client.nudge_test and how it has a .runall file and uses run_in_bg to run the infloop app in the background.

If you'd prefer to make adding a test a separate CL that would be fine.

AssadHashmi · 2019-01-30T13:02:16Z

run arm tests

AssadHashmi · 2019-01-30T14:57:33Z

run arm tests

derekbruening · 2019-04-04T13:23:12Z

@summershrimp are you planning to resume this PR?

summershrimp · 2019-04-08T01:09:52Z

Yes, but a little busy these days. I'll do it asap

derekbruening · 2019-05-08T21:51:24Z

This should reference the attach issues: #38, #725

derekbruening · 2020-12-10T18:35:36Z

Pasting from #4601 (comment)

Hello,

I'm trying to finish this PR 3328 because I need to be able to attach DynamoRIO to an executed process (i use it for fuzzing with winAFL).
I fixed everything and I can compile now.
I just need some help to test it because I don't really get which process I have to run, but since Summershrimp added the -attach to DRCONFIG it's probably with drconfig.exe?

Thank you and have a nice day!

Thank you for contributing!

That drdploy.c file turns into drconfg.exe, drrun.exe, and drinject.exe. The -attach would apply only to drrun.exe and drinject.exe. drinject.exe is rarely used so it would be drrun.exe for a manual test. See https://dynamorio.org/dynamorio_docs/page_deploy.html#win_deploy

As requested above (#3328 (comment)) we would want an automated test added to the suite as well.

summershrimp · 2020-12-11T02:24:30Z

I would restart working on this PR soon

raefko · 2020-12-14T11:42:06Z

Hello,
Thank you @derekbruening fo your answer, I'm new to DynamoRIO and I'm trying to better understand how it works.
I took a look at the code and the issues/PR you mentioned before and I'm starting to get how it works.
I think that the -attach doesn't really do what I thought. Is it an option we add before the " -- app and args to run " (seems like yes). Or the option is here to replace the running of a process.
For example :
I have notpad.exe that is running, I get the pid which is XYZ, and I just run
./drrun.exe [options] -attach XYZ
I mean, what is the main use of the -attach option?

Thank you and have a nice day!

derekbruening · 2020-12-14T19:22:41Z

I think that the -attach doesn't really do what I thought. Is it an option we add before the " -- app and args to run " (seems like yes). Or the option is here to replace the running of a process.

You're asking why this PR addes an -attach flag but doesn't update the usage message which says to always pass "-- app"? Agreed, it should update the usage message to say that an application should not be passed, and should complain about it (I think the PR code would just silently ignore the passed app now).

raefko · 2020-12-14T23:34:27Z

I think that the -attach doesn't really do what I thought. Is it an option we add before the " -- app and args to run " (seems like yes). Or the option is here to replace the running of a process.

You're asking why this PR addes an -attach flag but doesn't update the usage message which says to always pass "-- app"? Agreed, it should update the usage message to say that an application should not be passed, and should complain about it (I think the PR code would just silently ignore the passed app now).

Okey i will fix that thanks!

raefko · 2020-12-23T10:36:52Z

I would restart working on this PR soon

Hello,
Can i have your e-mail please? I think that the one in your github profile isn't the current one.

ajohnston9 · 2021-02-19T17:42:18Z

Any progress here? This is a critical feature for me.

derekbruening · 2021-02-22T16:00:40Z

Any progress here? This is a critical feature for me.

I would say go ahead and submit a PR yourself finishing this off since it is not clear what the status is of the others who have started on it.

derekbruening · 2021-08-16T16:50:46Z

#5019 which includes the Linux portion of the changes here is merged -- but the Windows portion of this PR is not yet merged.

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

derekbruening · 2021-09-23T21:34:47Z

With PR #5075 adding Windows attach by building on this PR I think everything here is now merged so we can close this. Thank you for this initial PR: even though it didn't get merged it seeded the subsequent PR's.

Implement attach for both windows and linux

2141465

For linux, we use ptrace() to inject dynamorio to running process. For windows, we get target process handle using DebugActiveProcess() and inject with original dr_inject_process_inject().

Fix formatting.

c36cac9

derekbruening reviewed Jan 9, 2019

View reviewed changes

derekbruening mentioned this pull request May 8, 2019

attaching to running process not yet implemented #3597

Closed

raefko mentioned this pull request Dec 10, 2020

Implement attach for windows #4601

Closed

raefko mentioned this pull request Dec 16, 2020

Implement attach for Windows #4626

Closed

M3m3M4n mentioned this pull request Jul 20, 2021

i#38: Attach injection on Linux #5019

Merged

OrBenPorath mentioned this pull request Sep 2, 2021

i#725: add attach for windows #5075

Merged

derekbruening closed this Sep 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement attach for both windows and linux #3328

Implement attach for both windows and linux #3328

summershrimp commented Dec 29, 2018

summershrimp commented Dec 29, 2018

derekbruening commented Jan 2, 2019

derekbruening left a comment

derekbruening Jan 9, 2019

derekbruening Jan 9, 2019

derekbruening Jan 9, 2019

derekbruening Jan 9, 2019

derekbruening Jan 9, 2019

derekbruening Jan 9, 2019

AssadHashmi commented Jan 30, 2019

AssadHashmi commented Jan 30, 2019

derekbruening commented Apr 4, 2019

summershrimp commented Apr 8, 2019

derekbruening commented May 8, 2019

derekbruening commented Dec 10, 2020

summershrimp commented Dec 11, 2020

raefko commented Dec 14, 2020 •

edited

Loading

derekbruening commented Dec 14, 2020

raefko commented Dec 14, 2020

raefko commented Dec 23, 2020

ajohnston9 commented Feb 19, 2021

derekbruening commented Feb 22, 2021

derekbruening commented Aug 16, 2021

derekbruening commented Sep 23, 2021

Implement attach for both windows and linux #3328

Implement attach for both windows and linux #3328

Conversation

summershrimp commented Dec 29, 2018

summershrimp commented Dec 29, 2018

derekbruening commented Jan 2, 2019

derekbruening left a comment

Choose a reason for hiding this comment

derekbruening Jan 9, 2019

Choose a reason for hiding this comment

derekbruening Jan 9, 2019

Choose a reason for hiding this comment

derekbruening Jan 9, 2019

Choose a reason for hiding this comment

derekbruening Jan 9, 2019

Choose a reason for hiding this comment

derekbruening Jan 9, 2019

Choose a reason for hiding this comment

derekbruening Jan 9, 2019

Choose a reason for hiding this comment

AssadHashmi commented Jan 30, 2019

AssadHashmi commented Jan 30, 2019

derekbruening commented Apr 4, 2019

summershrimp commented Apr 8, 2019

derekbruening commented May 8, 2019

derekbruening commented Dec 10, 2020

summershrimp commented Dec 11, 2020

raefko commented Dec 14, 2020 • edited Loading

derekbruening commented Dec 14, 2020

raefko commented Dec 14, 2020

raefko commented Dec 23, 2020

ajohnston9 commented Feb 19, 2021

derekbruening commented Feb 22, 2021

derekbruening commented Aug 16, 2021

derekbruening commented Sep 23, 2021

raefko commented Dec 14, 2020 •

edited

Loading