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

client interactions with cross-arch execve #147

Closed
derekbruening opened this issue Nov 27, 2014 · 17 comments · Fixed by #4324
Closed

client interactions with cross-arch execve #147

derekbruening opened this issue Nov 27, 2014 · 17 comments · Fixed by #4324

Comments

@derekbruening
Copy link
Contributor

From derek.br...@gmail.com on May 23, 2009 10:55:03

With debug build and a 32-bit client, if an app does execve to a 64-bit
app, we'll kill the process with a usage error due to the 32-bit client
mismatch:

<-- parent 16846 forked child 16903 -->
<-- execve /bin/sh -->
<Application sh (16903) DynamoRIO usage error :
Error opening instrumentation library
/work/dr/tot/exports_drmemory/libdrmemory.so:
/work/dr/tot/exports_drmemory/libdrmemory.so: wrong ELF class:
ELFCLASS32>

We should at least provide an option to turn this into a non-fatal error,
to enable using debug build in such situations. In my usage case the execve
of 64-bit sh is followed by execve of 32-bit perlbmk, and I'm fine with my
client not being loaded into sh, but I do want to be loaded into perlbmk.
This is what happens today with a release build, with a non-fatal message:

<Application sh (18109). Unable to load client library:
Error opening instrumentation library
/work/dr/tot/exports_drmemory/libdrmemory.so:
/work/dr/tot/exports_drmemory/libdrmemory.so: wrong ELF class:
ELFCLASS32.>
<Starting application sh (18109)>
<Starting application perlbmk (18110)>

We should consider what model we want to support for clients in such scenarios:
specify separate 32-bit and 64-bit clients?
Xref all the similar Windows issues: PR 240257, PR 254193, issue #49.
Those Windows cases also focus on mixed-mode processes.

Original issue: http://code.google.com/p/dynamorio/issues/detail?id=147

@derekbruening
Copy link
Contributor Author

From derek.br...@gmail.com on May 25, 2009 14:34:25

in r162 :

  • eliminated debug-build assert for 32-vs-64 errors to allow
    debug build to continue across execve chains
  • will leave case open for more substantial changes depending on
    the model we want

Labels: -Type-Defect -Priority-Medium Type-Enhancement Priority-Low

@derekbruening
Copy link
Contributor Author

This was blocking usage in https://groups.google.com/g/dynamorio-users/c/2cAuAhIBR8Y

@derekbruening derekbruening self-assigned this Jun 6, 2020
@derekbruening
Copy link
Contributor Author

derekbruening commented Jun 6, 2020

I think there are two possible solutions and that we should do both?

  • Add a new option to specify the other-arch path to the client(s). -client_lib32, -client_lib64, and shortcuts -c32 and -c64? So you could do bin64/drrun -c bin64/client64.so -c32 bin32/client32.so or bin32/drrun -c bin/client32.so -c64 bin64/client64.so or bin64/drrun -c64 bin64/client64.so -c32 bin32/client32.so.

  • Have the pre-execve code try some standard substitutions for the existing client paths like s/bin32/bin64/;s/bin/bin64/;s/lib32/lib64;s/lib/lib64;.

@johnfxgalea wondering if you've ever hit this? It seems surprising that nobody has really hit this in all these years.

@derekbruening
Copy link
Contributor Author

Note that using drconfig to create named config files for other-arch setup has always been an option, but is not always easy to use if child executable names are not known.

@derekbruening
Copy link
Contributor Author

Hmm for the -c <path> <options> syntax where there's no delimiter to end the options exception the -- for the app, maybe it's better to have -c3264 <path32> <path64> <options-for-both>?

@johnfxgalea
Copy link
Contributor

johnfxgalea commented Jun 6, 2020

Hmm, no, I don't recall ever encountering this issue. At the moment, my experimental setup does not mix archs - I use a 32-bit OS when working on 32-bit apps.

@derekbruening
Copy link
Contributor Author

For drmem, since not using drrun: I guess it just calls
dr_register_client() for the other arch which will create a 1-time config
file.

And for a -t tool file like drcov with no dedicated front-end: can have
CLIENT{32,64}{_REL}?

@derekbruening
Copy link
Contributor Author

derekbruening commented Jun 6, 2020

For drmem, since not using drrun: I guess it just calls
dr_register_client() for the other arch which will create a 1-time config
file.

No that would only help if the top-level process did execve to the
other arch: won't help w/ children who will have different pids.

It has to set the -client_libXX option right? No current top-level
solution via drconfiglib: has to set the option string? Should we
add an other-arch-client-path to dr_register_client()?

@derekbruening
Copy link
Contributor Author

I'm adding an other-arch-client-path to drconfiglib via dr_register_client_ex() which takes in a new struct (so we can add more fields more easily in the future), plus new routines dr_get_client_info_ex() and dr_client_iterator_next_ex() to support querying other-bitwidth client registration via the same struct.

I decided not to add the substitution attempts above: it seems risky. If the user did not specify both client paths, DR will just fail to load the wrong-bitwidth client in the child which is a fatal error. Seem ok?

@hunterbr72
Copy link

I would vote for a -c32 -c64 solution. Who knows if it is necessary for certain clients to have dedicated options in certain cases.

@hunterbr72
Copy link

I mean separate options for the 32 and the 64 bit world.

@derekbruening
Copy link
Contributor Author

derekbruening commented Jun 9, 2020

I would vote for a -c32 -c64 solution. Who knows if it is necessary for certain clients to have dedicated options in certain cases.

So my current prototype branch for this does have the underlying capability for separate options, if you use the -client_lib32 and -client_lib64 raw DR options, or if you use the drinjectlib interface. The reason I moved toward the -c3264 is for easy option parsing. If you had separate -c32 and -c64 what you want the syntax to look like? Have a -- in between as well as separating the app args?

$ bin64/drrun -c32 /my/lib32.so -op1 for_32 -op2 for_32 -- \
  -c64 /my/lib64.so -op3 for_64 -op4 -- \
  /my/app app_arg1

@johnfxgalea
Copy link
Contributor

I actually prefer the separation, but if its implementation is an overkill I don't mind -c3264.

@derekbruening
Copy link
Contributor Author

You will always be able to do the raw -client_lib* options though they are awkward:

$ ../exports/bin64/drrun \
  -client_lib32 ~/dr/releases/DynamoRIO-Linux-8.0.0-1/samples/bin32/libbbcount.so\;0\; \
  -client_lib64 ~/dr/releases/DynamoRIO-Linux-8.0.0-1/samples/bin64/libbbcount.so\;0\; \
  -- ~/dr/test/execve64 ~/dr/test/hello32

Maybe we should just do both: -c3264 and separate -c32 and -c64, with the extra --.

@hunterbr72
Copy link

yup, agree, both would be nice, but as John said, depends on the effort.

@derekbruening
Copy link
Contributor Author

Note that none of the drrun proposals here allow setting separate DR options: only separate client options. You can set separate DR options with drinjectlib, or the tool file interface.

derekbruening added a commit that referenced this issue Jun 14, 2020
Adds new options and interfaces to specify alternate-bitwidth client
libraries for use when the application creates a child process of the
other bitwidth.

For DR, adds -client_lib32 and -client_lib64 options.  Switches main
usage to use the appropriate option, with its contents then copied
into -client_lib (to avoid the pain of removing that options).

For drconfiglib, adds dr_register_client_ex() with
dr_client_iterator_next_ex() to support querying other-bitwidth client
registrations.

Adds a new libutil.drconfig_test for drconfiglib.  Fixes a bug found
by the test: existing client queries were cutting off the last
character of the path and options.

For drrun and drconfig, adds "-c32" and "-c64" options, with an
additional "--" separating the client options between them.

For tool files, adds CLIENT{32,64}_{REL,ABS}.  Updates drcov,
drcpusim, and drcachesim to use the new syntax and drcachesim's
launcher to process it.  Tested these manually:
    ===========================================================================
    $ ninja install
    $ rm *.log
    $ ../exports/bin64/drrun -t drcov -- ~/dr/test/execve64 ~/dr/test/hello32
    $ l -t *.log
    20K drcov.execve64.109583.0000.proc.log  20K drcov.execve64.109585.0000.proc.log
    20K drcov.hello32.109585.0000.proc.log
    $ rm -rf drm*.dir
    $ ../exports/bin64/drrun -verbose -t drcachesim -verbose 1 -offline -- ~/dr/test/execve64 ~/dr/test/hello32
    $ l -td *.dir
    4.0K drmemtrace.hello32.117714.7095.dir/  4.0K drmemtrace.execve64.117714.6260.dir/
    4.0K drmemtrace.execve64.117713.9314.dir/
    ===========================================================================

Adds tests of -c32/-c64 to the existing cross-arch linux.execve{32,64}
tests (Windows won't work until #803 is addressed).
The tests look like this:
    ===========================================================================
    $ cmake . && ctest -V -R 'linux.execve(32|64)'

    8: Running test command: "/home/bruening/dr/git/build_x64_dbg_tests/bin64/drrun" "-32" "-dr_home" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/32bit" "-stderr_mask" "0xC" "-dumpcore_mask" "0" "-c32" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/32bit/suite/tests/bin/libclient.large_options.dll.so" "-paramA" "foo" "-paramB" "bar" "--" "-c64" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/bin/libclient.large_options.dll.so" "-paramA" "foo" "-paramB" "bar" "--" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/32bit/suite/tests/bin/linux.execve32" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/bin/linux.execve-sub64"
    8: large_options passed: -paramA foo -paramB bar
    8: parent is running under DynamoRIO
    8: parent waiting for child
    8: child is running under DynamoRIO
    8: large_options passed: -paramA foo -paramB bar
    8: it_worked
    8: running under DynamoRIO
    8: large_options exiting
    8: child has exited
    8: large_options exiting
    8:
    1/2 Test #8: linux.execve32 ...................   Passed    3.93 sec

    9: Test command: /home/bruening/dr/git/build_x64_dbg_tests/bin64/drrun "-stderr_mask" "0xC" "-dumpcore_mask" "0" "-c32" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/32bit/suite/tests/bin/libclient.large_options.dll.so" "-paramA" "foo" "-paramB" "bar" "--" "-c64" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/bin/libclient.large_options.dll.so" "-paramA" "foo" "-paramB" "bar" "--" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/bin/linux.execve64" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/32bit/suite/tests/bin/linux.execve-sub32"
    9: Test timeout computed to be: 1500
    9: large_options passed: -paramA foo -paramB bar
    9: parent is running under DynamoRIO
    9: parent waiting for child
    9: child is running under DynamoRIO
    9: large_options passed: -paramA foo -paramB bar
    9: it_worked
    9: running under DynamoRIO
    9: large_options exiting
    9: child has exited
    9: large_options exiting
    2/2 Test #9: linux.execve64 ...................   Passed    0.86 sec
    ===========================================================================

Issue: #147, #803
Fixes: #147
@derekbruening
Copy link
Contributor Author

#4324 has my proposal to solve this. It uses separate -c32 -c64 for drrun, -client_lib{32,64} for core DR, new interfaces for drconfiblib, and CLIENT{32,64}_{REL,ABS} for tool files.

derekbruening added a commit that referenced this issue Jun 15, 2020
Adds new options and interfaces to specify alternate-bitwidth client
libraries for use when the application creates a child process of the
other bitwidth.

For DR, adds -client_lib32 and -client_lib64 options.  Switches main
usage to use the appropriate option, with its contents then copied
into -client_lib (to avoid the pain of removing that options).

For drconfiglib, adds dr_register_client_ex() with
dr_client_iterator_next_ex() to support querying other-bitwidth client
registrations.

Adds a new libutil.drconfig_test for drconfiglib.  Fixes a UNIX bug
found by the test: existing client queries were cutting off the last
character of the path and options due to differing snprintf semantics.
Also fixes a Windows drconfig handle leak found by the test that
was preventing unregistration from deleting config files.

For drrun and drconfig, adds "-c32" and "-c64" options, with an
additional "--" separating the client options between them.

For tool files, adds CLIENT{32,64}_{REL,ABS}.  Updates drcov,
drcpusim, and drcachesim to use the new syntax and drcachesim's
launcher to process it.  Tested these manually:
    ===========================================================================
    $ ninja install
    $ rm *.log
    $ ../exports/bin64/drrun -t drcov -- ~/dr/test/execve64 ~/dr/test/hello32
    $ l -t *.log
    20K drcov.execve64.109583.0000.proc.log  20K drcov.execve64.109585.0000.proc.log
    20K drcov.hello32.109585.0000.proc.log
    $ rm -rf drm*.dir
    $ ../exports/bin64/drrun -verbose -t drcachesim -verbose 1 -offline -- ~/dr/test/execve64 ~/dr/test/hello32
    $ l -td *.dir
    4.0K drmemtrace.hello32.117714.7095.dir/  4.0K drmemtrace.execve64.117714.6260.dir/
    4.0K drmemtrace.execve64.117713.9314.dir/
    ===========================================================================

Adds tests of -c32/-c64 to the existing cross-arch linux.execve{32,64}
tests (Windows won't work until #803 is addressed).
The tests look like this:
    ===========================================================================
    $ cmake . && ctest -V -R 'linux.execve(32|64)'

    8: Running test command: "/home/bruening/dr/git/build_x64_dbg_tests/bin64/drrun" "-32" "-dr_home" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/32bit" "-stderr_mask" "0xC" "-dumpcore_mask" "0" "-c32" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/32bit/suite/tests/bin/libclient.large_options.dll.so" "-paramA" "foo" "-paramB" "bar" "--" "-c64" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/bin/libclient.large_options.dll.so" "-paramA" "foo" "-paramB" "bar" "--" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/32bit/suite/tests/bin/linux.execve32" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/bin/linux.execve-sub64"
    8: large_options passed: -paramA foo -paramB bar
    8: parent is running under DynamoRIO
    8: parent waiting for child
    8: child is running under DynamoRIO
    8: large_options passed: -paramA foo -paramB bar
    8: it_worked
    8: running under DynamoRIO
    8: large_options exiting
    8: child has exited
    8: large_options exiting
    8:
    1/2 Test #8: linux.execve32 ...................   Passed    3.93 sec

    9: Test command: /home/bruening/dr/git/build_x64_dbg_tests/bin64/drrun "-stderr_mask" "0xC" "-dumpcore_mask" "0" "-c32" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/32bit/suite/tests/bin/libclient.large_options.dll.so" "-paramA" "foo" "-paramB" "bar" "--" "-c64" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/bin/libclient.large_options.dll.so" "-paramA" "foo" "-paramB" "bar" "--" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/bin/linux.execve64" "/home/bruening/dr/git/build_x64_dbg_tests/suite/tests/32bit/suite/tests/bin/linux.execve-sub32"
    9: Test timeout computed to be: 1500
    9: large_options passed: -paramA foo -paramB bar
    9: parent is running under DynamoRIO
    9: parent waiting for child
    9: child is running under DynamoRIO
    9: large_options passed: -paramA foo -paramB bar
    9: it_worked
    9: running under DynamoRIO
    9: large_options exiting
    9: child has exited
    9: large_options exiting
    2/2 Test #9: linux.execve64 ...................   Passed    0.86 sec
    ===========================================================================

Issue: #147, #803
Fixes: #147
derekbruening added a commit that referenced this issue Jan 4, 2021
Adds a long-missing feature: following into a Windows child process of
a different bitwidth.

Switches injection from DR and from drinjectlib (including drrun and
drinject) to use -early_inject_map.  This was most easily done by
turning on -early_inject by default as well.  However, the
-early_inject_location default is INJECT_LOCATION_ImageEntry, which is
the same late takeover point as with thread injection.  Switching all
injection over to map-from-the-parent simplifies cross-arch following,
as well as making it easier to shift the takeover point to an earlier
spot in the future.  This is a step toward #607 by switching
drinjectlib to use map injection; the takeover point, as mentioned, is
still the image entry.

Adds an -inject_x64 option to inject a 64-bit DR lib into a 32-bit
child from a 64-bit parent, but this option is only sketched out and
is not fully supported yet: #49 covers adding tests and official
support.

Adds library swapping code to find the other-bitwidth library, which
assumes a parallel directory structure.  Add a new fatal error if the
library for a child is not found.

To support generating code for all 3 child-parent cases (same-same,
32-64, and 64-32), and in particular for 32-64, switches the small
gencode sequence for -early_inject_map from using IR to using raw
bytes.  A multi-arch encoder (#1684) would help but we would need
cross-bitwidth support there, which is not on the horizon.  Fixes what
look like bugs in the original gencode generation along the way
(s/pc/cur_local_pos/ and s/local_code_buf/remote_code_buf/): it's not
clear how it worked before.

Adds support for several system calls from a 32-bit parent to a 64-bit
child where the desired NtWow64* system call does not exist.  We use
switch_modes_and_call() for NtProtectVirtualMemory and
NtQueryVirtualMemory.

Changes all types in the injection code to handle 64-bit addresses in
32-bit code.  Adds UNICODE_STRING_32 and
RTL_USER_PROCESS_PARAMETERS_32 for handling 32-bit structures from
64-bit parents.  Similarly, adds RTL_USER_PROCESS_PARAMETERS_64 and
PROCESS_BASIC_INFORMATION64.

Adds get_process_imgname_cmdline() capability for 64-bit remote from 32-bit.

Adds get_remote_proc_address() and uses it to look up
dynamorio_earliest_init_takeover() in a child DR.

Finds the remote ntdll base via a remote query memory walk plus remote
image header parsing.  This requires adding a switch_modes_and_call()
version of NtQueryVirtualMemory (also mentioned above), which needs
64-bit args: so we refactor switch_modes_and_call() to take in a
struct of all 64-bit fields for the args.

Fixes a few bugs in other routines to properly get the image name and
image entry for 32-bit children of 64-bit parents.

Updates environment variable propagation code to handle a 32-bit
parent and a 64-bit child.  Updates a 64-bit parent and 32-bit child
to insert the variables into the 32-bit PEB (64-bit does no good),
which requires finding the 32-bit PEB.  This is done via the 32-bit
TEB, using a hack due to what seems like a kernel bug where it has the
TebBaseAddress 0x2000 too low.

Makes environment variable propagation failures fatal and visible,
unlike previously where errors would just result in silently letting
the child run natively.  Turns some other prior soft errors into fatal
errors on child takeover.

Moves environment variable propagation to post-CreateUserProcess
instead of waiting for ResumeThread, which avoids having to get the
thread context (for which we have no other-bitwidth support) to figure
out whether it's the first thread in the process or not.  We bail on
propagation for pre-Vista where we'd have to wait for ResumeThred.

Generalizes the other-bitwidth Visual Studio toolchain environment
variable setting for use in a new build-and-test other-bitwidth test
which builds dynamorio and the large_options client (to ensure options
are propagated to children; and it has convenient init and exit time
prints) for the other bitwidth, arranges parallel lib dirs, and runs
the other client

Issue: #803, #147, #607, #49
Fixes #803
derekbruening added a commit that referenced this issue Jan 5, 2021
Adds a long-missing feature: following into a Windows child process of
a different bitwidth.

Switches injection from DR and from drinjectlib (including drrun and
drinject) to use -early_inject_map.  This was most easily done by turning
on -early_inject by default as well.  However, the -early_inject_location
default is INJECT_LOCATION_ThreadStart, a new "early" injection location
which is the same late takeover point as with thread injection (we could
also use _ImageEntry, which is only very slightly later, but that fails for
.NET and other applications).  Switching all injection over to
map-from-the-parent simplifies cross-arch following, as well as making it
easier to shift the takeover point to an earlier spot in the future.  This
is a step toward #607 by switching drinjectlib to use map injection; the
takeover point, as mentioned, is still the thread start.

Placing a hook at the thread start causes some stability issues, so instead
of the usual hook for -early_inject_map, for INJECT_LOCATION_ThreadStart we
set the thread context, like thread injection does.  The gencode still
restores the hook as a nop, for simplicity.  For parent64 child32, we can't
easily locate the thread start, so we assume it's
ntdll32!RtlUserThreadStart (which is also a fallback if anything fails in
other cases; the final fallback is a hook at the image entry, which works
nearly everywhere but not for .NET where the image entry is not reached).

Adds an -inject_x64 option to inject a 64-bit DR lib into a 32-bit
child from a 64-bit parent, but this option is only sketched out and
is not fully supported yet: #49 covers adding tests and official
support.

Adds library swapping code to find the other-bitwidth library, which
assumes a parallel directory structure.  Add a new fatal error if the
library for a child is not found.

To support generating code for all 3 child-parent cases (same-same,
32-64, and 64-32), and in particular for 32-64, switches the small
gencode sequence for -early_inject_map from using IR to using raw
bytes.  A multi-arch encoder (#1684) would help but we would need
cross-bitwidth support there, which is not on the horizon.  Fixes what
look like bugs in the original gencode generation along the way
(s/pc/cur_local_pos/ and s/local_code_buf/remote_code_buf/): it's not
clear how it worked before.

Adds support for several system calls from a 32-bit parent to a 64-bit
child where the desired NtWow64* system call does not exist.  We use
switch_modes_and_call() for NtProtectVirtualMemory and
NtQueryVirtualMemory.

Changes all types in the injection code to handle 64-bit addresses in
32-bit code.  Adds UNICODE_STRING_32 and
RTL_USER_PROCESS_PARAMETERS_32 for handling 32-bit structures from
64-bit parents.  Similarly, adds RTL_USER_PROCESS_PARAMETERS_64 and
PROCESS_BASIC_INFORMATION64.

Adds get_process_imgname_cmdline() capability for 64-bit remote from 32-bit.

Adds get_remote_proc_address() and uses it to look up
dynamorio_earliest_init_takeover() in a child DR.

Finds the remote ntdll base via a remote query memory walk plus remote
image header parsing.  This requires adding a switch_modes_and_call()
version of NtQueryVirtualMemory (also mentioned above), which needs
64-bit args: so we refactor switch_modes_and_call() to take in a
struct of all 64-bit fields for the args.

Fixes a few bugs in other routines to properly get the image name and
image entry for 32-bit children of 64-bit parents.

Updates environment variable propagation code to handle a 32-bit
parent and a 64-bit child.  Updates a 64-bit parent and 32-bit child
to insert the variables into the 32-bit PEB (64-bit does no good),
which requires finding the 32-bit PEB.  This is done via the 32-bit
TEB, using a hack due to what seems like a kernel bug where it has the
TebBaseAddress 0x2000 too low.

Makes environment variable propagation failures fatal and visible,
unlike previously where errors would just result in silently letting
the child run natively.  Turns some other prior soft errors into fatal
errors on child takeover.

Moves environment variable propagation to post-CreateUserProcess
instead of waiting for ResumeThread, which avoids having to get the
thread context (for which we have no other-bitwidth support) to figure
out whether it's the first thread in the process or not.  We bail on
propagation for pre-Vista where we'd have to wait for ResumeThred.

Generalizes the other-bitwidth Visual Studio toolchain environment
variable setting for use in a new build-and-test other-bitwidth test
which builds dynamorio and the large_options client (to ensure options
are propagated to children; and it has convenient init and exit time
prints) for the other bitwidth, arranges parallel lib dirs, and runs
the other client.

Issue: #803, #147, #607, #49
Fixes #803
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants