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

Fix searching when connected to qemu-system instance #906

Merged
merged 7 commits into from Nov 23, 2022

Conversation

clubby789
Copy link
Contributor

@clubby789 clubby789 commented Oct 22, 2022

Description/Motivation/Screenshots

Fixes #905
Detects when we're connected to a QEMU kernel instance, and uses monitor info mem to retrieve the real kernel memory mappings for searching within.

Against which architecture was this tested ?

"Tested" indicates that the PR works and the unit test (see docs/testing.md) run passes without issue.

  • x86-32
  • x86-64
  • ARM
  • AARCH64
  • MIPS
  • POWERPC
  • SPARC
  • RISC-V

Checklist

  • My PR was done against the dev branch, not main.
  • My code follows the code style of this project.
  • My change includes a change to the documentation, if required.
  • If my change adds new code, adequate tests have been added.
  • I have read and agree to the CONTRIBUTING document.

I will add tests shortly, just leaving this initial draft.

@clubby789
Copy link
Contributor Author

I noticed that the code for detecting if we're in a qemu/qemu-system context already exists, so I've removed my duplicated code. The code added is mainly just invoking and parsing monitor info mem, and using those memory mappings when needed. As there is currently no testing harness for qemu-system, I haven't added tests.

@clubby789
Copy link
Contributor Author

Because of a typo (QOffsets -> qOffsets), qemu sessions would always be identified as kernel-mode. All tests look good locally now

@clubby789 clubby789 marked this pull request as ready for review October 22, 2022 16:27
@clubby789
Copy link
Contributor Author

As a side note, it looks like the search command buffers the output til completion. Having it print as items are discovered (or at least having that as an option) would help when the search takes a long time (i.e. traversing the large memory space of the kernel)

@Grazfather
Copy link
Collaborator

Looks good, a few comments, and I'll have to figure out how to test it.

As a side note, it looks like the search command buffers the output til completion. Having it print as items are discovered (or at least having that as an option) would help when the search takes a long time (i.e. traversing the large memory space of the kernel)

Yes, good call. Go ahead and file a ticket if you don't mind.

Copy link
Collaborator

@Grazfather Grazfather left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to submit my review.

gef.py Outdated Show resolved Hide resolved
gef.py Outdated Show resolved Hide resolved
gef.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@Grazfather Grazfather left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just more nitpicking.

gef.py Outdated Show resolved Hide resolved
gef.py Show resolved Hide resolved
gef.py Outdated Show resolved Hide resolved
Copy link
Owner

@hugsy hugsy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another great PR @clubby789 , again some small stuff to fix.

gef.py Outdated Show resolved Hide resolved
gef.py Outdated Show resolved Hide resolved
gef.py Outdated Show resolved Hide resolved
gef.py Show resolved Hide resolved
@hugsy
Copy link
Owner

hugsy commented Nov 17, 2022

BTW you should be able to merge the latest changes from dev, the CI tests should run again successfully on your next push.

Cheers!

@clubby789
Copy link
Contributor Author

Tests look okay locally and on Ubuntu 20, probably random failure?

@hugsy
Copy link
Owner

hugsy commented Nov 17, 2022

Tests look okay locally and on Ubuntu 20, probably random failure?

It can happen, I just relaunched the tests.
If still fails, I'll have a look tonight but it should be minor.

@hugsy
Copy link
Owner

hugsy commented Nov 18, 2022

@clubby789
Your changes look good, go ahead and merge the latest changes from dev. I believe all tests should pass now, then I'll go ahead and merge your PR.

@hugsy hugsy merged commit d1833d3 into hugsy:dev Nov 23, 2022
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.

[Bug] search-pattern doesn't work in gef-remote when connected to qemu-system.
3 participants