Implement file download from the remote machine in GDB protocol#6576
Implement file download from the remote machine in GDB protocol#6576moste00 wants to merge 3 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files
... and 62 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
notxvilka
left a comment
There was a problem hiding this comment.
Ideally, would be great to test it with our mock gdbserver Python script like we do already for some other stuff.
|
@notxvilka Hey, added a test and did some neat-ing up of the code. One slighlty hack-ish thing I had to do is adding a If it's a deal breaker then if we remove it, we can't have the prompt-user-to-download feature, I personally like this feature though, it saves an extra |
|
I know I need to document this in the rizin book and I already located the section where I need to edit, I'm just waiting for this PR to be approved so that the feature behaviour is fixed and then I can document it. |
|
@wargio @notxvilka ping. |
Your checklist for this pull request
RZ_APIfunction and struct this PR changes.RZ_API).Detailed description
There is an issue to download binary files from a remote machine during a GDB remote session #3975, this file is a simple implementation of that addresses the simplest case.
Implementation as follows:
(1) An existing
gdbr_read_filethat implemented a bounded read of a remote file into a caller-provided buffer was refactored, it now takes an additional argumentstart_offset, and starts reading from the remote file at this offset. The more general logic is namedgdbr_pread_file, and the behaviour ofgdbr_read_fileis recovered by calling it with astart_offsetof 0.(2) Using
gdbr_pread_fileas a generic primitive, file downloading is then implemented as an outer loop repeatedly calling it with increasing offsets until a zero-length or shorter-than-expected read (indicating the remote file is complete).(3) A sub-command
download_filewas added where the user controls both the remote source and the local destination(4) A convenience was added to the main function so that if the user runs
rizin -D gdb gdb://..., it offers to download the remote executable being deubugged if any of the following is true (a) the remote host is NOT localhost (b) the file doesn't exist by its full path locally, even if the remote host is localhost.Note: the localhost check is intentionally conservative, it checks for a literal "gdb://localhost:" in the prefix of the URI, so (e.g.) it rejects
127.0.0.1and considers it "not local". This is because parsing IP addresses doesn't have any well-established utilities in Rizin AFAICT, and I don't want to implement a half-baked buggy and slow implementation of half of an IP parser in the main function. Note that127.0.0.1is not unique as a loopback address anyway, and sometimes it can be behind container/namespace routing that make its filesystem not identical to the local filesystem Rizin is running on.So for now,
localhostis treated as a magic escape when the user knows the file doesn't need to be downloaded, and any other remote address is treated as remote.Test plan
There are no automated tests, sorry, it would be awkward to test network code like this.
Here's a concrete UX difference:
1- First, run a gdb server with a binary that is inaccessible to Rizin, this can be done with VMs or SSH, but a much simpler way to do it with bwrap is:
sudo bwrap --ro-bind / / --dev-bind /dev /dev --proc /proc --tmpfs /tmp --bind "$tmp/remote-sleep" /tmp/remote-sleep gdbserver :2345 /tmp/remote-sleep 1000(bwrap is only a convience wrapper over namespaces here, so this is doable on any modern linux without bwrap as well.)
2- Run Rizin without this PR (or with this PR but select no when the prompt for downloading the remote file appears)
3- run
is, the reasonable expectation is: symbols should appear. But the errorERROR: No binary object currently selected.appears instead.4- Even after running a full
aaaaanalysis pass, a simple debug command likedcu maindoesn't actually stop at main. The errorWARNING: Breakpoint won't be placed since it's not in a valid map. You can bypass this check by setting dbg.bpinmaps to false.appears instead, and the sleep command appears to run to completion before control is handed back to Rizin.With this PR, choosing
yat the prompt will make (3) and (4) work as expected.Closing issues
Closes #3975 but might need extra features.
Future/RFC
The following concerns are reasonably too big/unknown to handle in this PR, but arguably should:
1- If the binary being debugged in the remote machine loads
.soobjects, does the current prompt in main naturally extend to download them as well ? should it ? what if loading is lazy and happens mid-execution? Do we need a new prompt for every one ?2- If the file is not downloaded by the prompt or manually by the user, should other triggers (e.g. the
iscommand) re-prompt the user or should we treat user refusal as final, given that the manual command is always available in case user changes their mind ?3- Right now, for simplicity, the convenience prompt generates a new temp file on every download and deletes it on exit, should we make any attempt to build a hash-keyed persistent local database ?
4- Instead of downloading to the filesystem for simplicity, maybe it's better to download to memory instead.
Almost all of those problems/nitpicks are around the triggers and UX of downloading, downloading itself once the decision is made is straightforward enough and uses existing code for the most part.