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

Adding support for mmap over qemu user #579

Closed
wants to merge 1 commit into from

Conversation

nnewram
Copy link

@nnewram nnewram commented Nov 17, 2020

Adding support for mmap over qemu user

Description/Motivation/Screenshots

This patch allows vmmap to work on a local qemu user session. Previously pid was taken using get_pid() but because qemu gives 1 as a pid and the actual system pid as a tid, gef could not find the correct /proc/$TID/maps. This is fixed by using gdb.selected_thread().ptid[1] rather than get_pid()

https://imgur.com/V4rLF85

How Has This Been Tested?

Architecture Yes/No Comments
x86-32 ✖️
x86-64 ✔️
ARM ✖️
AARCH64 ✖️
MIPS ✖️
POWERPC ✖️
SPARC ✖️
RISC-V ✖️
make tests ✔️ https://imgur.com/a/S1PQCfA

Checklist

  • My PR was done against the dev branch, not master.
  • My code follows the code style of this project.
  • [] My change includes a change to the documentation, if required.
  • [?] My change adds tests as appropriate.
  • I have read and agree to the CONTRIBUTING document.

Comment on adding tests

I do not know if a test is suited here. I guess it would be possible to launch a qemu process and check against it, but then the user would have to have qemu-user installed.

@Grazfather
Copy link
Collaborator

Cool, we've been meaning to get better support, but this is too hacked in.

Could you maybe abstract this out? Maybe we can have a global current_remote, which is None for local, and then a true local, or a qemu local, and then move the functionality you need into methods, so e.g. get_pid() would use the current_remote?

Maybe a Remote isn't the right abstraction. We can think about this.

@nnewram
Copy link
Author

nnewram commented Nov 17, 2020

Yes I agree, get_pid could check if it is using qemu and return the tid,
if you want more abstraction "ENABLE=" in gdb.execute("maintenance packet Qqemu.sstepbits", to_string=True, from_tty=False) should be moved to a function, is_qemu(), maybe we could move gef_qemu_mode there too, that would simplify some things. Also checking if it is using qemu that way is probably way more reliable than trusting user for flag.

I'm currently talking with people on qemu to see if maybe the gdbstub could be expanded a bit, in particular it would be nice to have file transfer. That would allow us to directly get /proc/pid/maps, among other things. Copying libs aswell.

@hugsy
Copy link
Owner

hugsy commented Dec 9, 2020

I too like the patch a lot but very hack-ish (for good reason, mostly qemu limitation I admit).

I'm currently talking with people on qemu to see if maybe the gdbstub could be expanded a bit, in particular it would be nice to have file transfer. That would allow us to directly get /proc/pid/maps, among other things. Copying libs aswell.

That would be perfect, do you have a follow-up on that @nnewram ? Or maybe a link we could use for tracking ?

Thanks

@nnewram
Copy link
Author

nnewram commented Dec 9, 2020

I too like the patch a lot but very hack-ish (for good reason, mostly qemu limitation I admit).

I'm currently talking with people on qemu to see if maybe the gdbstub could be expanded a bit, in particular it would be nice to have file transfer. That would allow us to directly get /proc/pid/maps, among other things. Copying libs aswell.

That would be perfect, do you have a follow-up on that @nnewram ? Or maybe a link we could use for tracking ?

Thanks

I spoke with people from both gdb and qemu about this. Both gdb and qemu shifts blame on the other party,
[15:18] <stsquad> nnewram: not currently - your options are 1) write a custom gdb command (ala qemu.sstepbits and friends) 2) get gdb maintainers to specify a gdbserver packet and stop info proc being brain dead (...)

As stsquad suggests, it might already be possible to do, could be worth looking in to.

@hugsy hugsy added this to the [Stable Release] 2021.04 milestone Mar 25, 2021
@hugsy hugsy self-assigned this Mar 25, 2021
hugsy pushed a commit that referenced this pull request Apr 9, 2021
…- which would not work on non 64b archs, we completely assumes that the debugged binary is emulated by qemu and therefore show the qemu mem maping. This allows for a clean and effective way to have the correct page permissions set in gef (and might help oneself who tries to escape the qemu process :) )
@hugsy hugsy mentioned this pull request Apr 9, 2021
5 tasks
@hugsy
Copy link
Owner

hugsy commented Apr 11, 2021

See #638

@hugsy hugsy closed this Apr 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants